[v3,2/2] ethdev: fix VLAN offloads set if no relative capabilities
diff mbox series

Message ID 1592794047-51529-3-git-send-email-xavier.huwei@huawei.com
State Superseded, archived
Delegated to: Ferruh Yigit
Headers show
Series
  • ethdev: minor bugfixes
Related show

Checks

Context Check Description
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Wei Hu (Xavier) June 22, 2020, 2:47 a.m. UTC
Currently, there is a potential problem that calling the API function
rte_eth_dev_set_vlan_offload to start VLAN hardware offloads which the
driver does not support. If the PMD driver does not support certain VLAN
hardware offloads and does not check for it, the hardware setting will
not change, but the VLAN offloads in dev->data->dev_conf.rxmode.offloads
will be turned on.

It is supposed to check the hardware capabilities to decide whether the
relative callback needs to be called just like the behavior in the API
function named rte_eth_dev_configure. And it is also needed to cleanup
duplicated checks which are done in some PMDs. Also, note that it is
behaviour change for some PMDs which simply ignore (with error/warning log
message) unsupported VLAN offloads, but now it will fail.

Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v2 -> v3: Add __rte_unused to avoid unused parameter 'dev' and 'mask'
	  warning.
v1 -> v2: cleanup duplicated checks which are done in some PMDs.
---
 drivers/net/dpaa2/dpaa2_ethdev.c           |  5 -----
 drivers/net/enic/enic_ethdev.c             | 12 ------------
 drivers/net/fm10k/fm10k_ethdev.c           | 23 ++---------------------
 drivers/net/hinic/hinic_pmd_ethdev.c       |  6 ------
 drivers/net/i40e/i40e_ethdev.c             |  5 -----
 drivers/net/nfp/nfp_net.c                  |  5 -----
 drivers/net/octeontx/octeontx_ethdev_ops.c | 10 ----------
 drivers/net/octeontx2/otx2_vlan.c          |  5 -----
 drivers/net/qede/qede_ethdev.c             |  3 ---
 lib/librte_ethdev/rte_ethdev.c             | 21 +++++++++++++++++++++
 10 files changed, 23 insertions(+), 72 deletions(-)

Comments

Hyong Youb Kim (hyonkim) June 22, 2020, 3:05 a.m. UTC | #1
> -----Original Message-----
> From: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Sent: Monday, June 22, 2020 11:47 AM
> To: thomas@monjalon.net; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; anatoly.burakov@intel.com;
> hemant.agrawal@nxp.com; sachin.saxena@nxp.com; John Daley (johndale)
> <johndale@cisco.com>; Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>;
> qi.z.zhang@intel.com; xiao.w.wang@intel.com; xuanziyang2@huawei.com;
> cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
> beilei.xing@intel.com; jia.guo@intel.com; heinrich.kuhn@netronome.com;
> hkalra@marvell.com; jerinj@marvell.com; ndabilpuram@marvell.com;
> kirankumark@marvell.com; rmody@marvell.com; shshaikh@marvell.com
> Cc: dev@dpdk.org; xavier.huwei@huawei.com
> Subject: [PATCH v3 2/2] ethdev: fix VLAN offloads set if no relative
> capabilities
> 
> Currently, there is a potential problem that calling the API function
> rte_eth_dev_set_vlan_offload to start VLAN hardware offloads which the
> driver does not support. If the PMD driver does not support certain VLAN
> hardware offloads and does not check for it, the hardware setting will
> not change, but the VLAN offloads in dev->data->dev_conf.rxmode.offloads
> will be turned on.
> 
> It is supposed to check the hardware capabilities to decide whether the
> relative callback needs to be called just like the behavior in the API
> function named rte_eth_dev_configure. And it is also needed to cleanup
> duplicated checks which are done in some PMDs. Also, note that it is
> behaviour change for some PMDs which simply ignore (with error/warning
> log
> message) unsupported VLAN offloads, but now it will fail.
> 
> Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> v2 -> v3: Add __rte_unused to avoid unused parameter 'dev' and 'mask'
> 	  warning.
> v1 -> v2: cleanup duplicated checks which are done in some PMDs.
> ---
>  drivers/net/dpaa2/dpaa2_ethdev.c           |  5 -----
>  drivers/net/enic/enic_ethdev.c             | 12 ------------
>  drivers/net/fm10k/fm10k_ethdev.c           | 23 ++---------------------
>  drivers/net/hinic/hinic_pmd_ethdev.c       |  6 ------
>  drivers/net/i40e/i40e_ethdev.c             |  5 -----
>  drivers/net/nfp/nfp_net.c                  |  5 -----
>  drivers/net/octeontx/octeontx_ethdev_ops.c | 10 ----------
>  drivers/net/octeontx2/otx2_vlan.c          |  5 -----
>  drivers/net/qede/qede_ethdev.c             |  3 ---
>  lib/librte_ethdev/rte_ethdev.c             | 21 +++++++++++++++++++++
>  10 files changed, 23 insertions(+), 72 deletions(-)
> 
[...]
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index 32d5397..ef8900d 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -374,18 +374,6 @@ static int enicpmd_vlan_offload_set(struct
> rte_eth_dev *eth_dev, int mask)
>  			enic->ig_vlan_strip_en = 0;
>  	}
> 
> -	if ((mask & ETH_VLAN_FILTER_MASK) &&
> -	    (offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
> -		dev_warning(enic,
> -			"Configuration of VLAN filter is not supported\n");
> -	}
> -
> -	if ((mask & ETH_VLAN_EXTEND_MASK) &&
> -	    (offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)) {
> -		dev_warning(enic,
> -			"Configuration of extended VLAN is not
> supported\n");
> -	}
> -
>  	return enic_set_vlan_strip(enic);
>  }
> 

For enic,

Acked-by: Hyong Youb Kim <hyonkim@cisco.com>

Thanks.
-Hyong
Sachin Saxena (OSS) June 22, 2020, 5:10 a.m. UTC | #2
On 22-Jun-20 8:17 AM, Wei Hu (Xavier) wrote:
> Currently, there is a potential problem that calling the API function
> rte_eth_dev_set_vlan_offload to start VLAN hardware offloads which the
> driver does not support. If the PMD driver does not support certain VLAN
> hardware offloads and does not check for it, the hardware setting will
> not change, but the VLAN offloads in dev->data->dev_conf.rxmode.offloads
> will be turned on.
>
> It is supposed to check the hardware capabilities to decide whether the
> relative callback needs to be called just like the behavior in the API
> function named rte_eth_dev_configure. And it is also needed to cleanup
> duplicated checks which are done in some PMDs. Also, note that it is
> behaviour change for some PMDs which simply ignore (with error/warning log
> message) unsupported VLAN offloads, but now it will fail.
>
> Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API")
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> v2 -> v3: Add __rte_unused to avoid unused parameter 'dev' and 'mask'
> 	  warning.
> v1 -> v2: cleanup duplicated checks which are done in some PMDs.
> ---
>   drivers/net/dpaa2/dpaa2_ethdev.c           |  5 -----
>   drivers/net/enic/enic_ethdev.c             | 12 ------------
>   drivers/net/fm10k/fm10k_ethdev.c           | 23 ++---------------------
>   drivers/net/hinic/hinic_pmd_ethdev.c       |  6 ------
>   drivers/net/i40e/i40e_ethdev.c             |  5 -----
>   drivers/net/nfp/nfp_net.c                  |  5 -----
>   drivers/net/octeontx/octeontx_ethdev_ops.c | 10 ----------
>   drivers/net/octeontx2/otx2_vlan.c          |  5 -----
>   drivers/net/qede/qede_ethdev.c             |  3 ---
>   lib/librte_ethdev/rte_ethdev.c             | 21 +++++++++++++++++++++
>   10 files changed, 23 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
> index 2f031ec..da5018f 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -169,11 +169,6 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>   			DPAA2_PMD_INFO("Unable to set vlan filter = %d", ret);
>   	}
>   next_mask:

The changes looks good but the label "next_mask" and its usage above 
should also be deleted/replaced.


> -	if (mask & ETH_VLAN_EXTEND_MASK) {
> -		if (dev->data->dev_conf.rxmode.offloads &
> -			DEV_RX_OFFLOAD_VLAN_EXTEND)
> -			DPAA2_PMD_INFO("VLAN extend offload not supported");
> -	}
>   
>   	return 0;
>   }
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index 32d5397..ef8900d 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -374,18 +374,6 @@ static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>   			enic->ig_vlan_strip_en = 0;
>   	}
>   
> -	if ((mask & ETH_VLAN_FILTER_MASK) &&
> -	    (offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
> -		dev_warning(enic,
> -			"Configuration of VLAN filter is not supported\n");
> -	}
> -
> -	if ((mask & ETH_VLAN_EXTEND_MASK) &&
> -	    (offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)) {
> -		dev_warning(enic,
> -			"Configuration of extended VLAN is not supported\n");
> -	}
> -
>   	return enic_set_vlan_strip(enic);
>   }
>   
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index f537ab2..f5b854e 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -1575,28 +1575,9 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
>   }
>   
>   static int
> -fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask)
> +fm10k_vlan_offload_set(struct rte_eth_dev *dev __rte_unused,
> +		       int mask __rte_unused)
>   {
> -	if (mask & ETH_VLAN_STRIP_MASK) {
> -		if (!(dev->data->dev_conf.rxmode.offloads &
> -			DEV_RX_OFFLOAD_VLAN_STRIP))
> -			PMD_INIT_LOG(ERR, "VLAN stripping is "
> -					"always on in fm10k");
> -	}
> -
> -	if (mask & ETH_VLAN_EXTEND_MASK) {
> -		if (dev->data->dev_conf.rxmode.offloads &
> -			DEV_RX_OFFLOAD_VLAN_EXTEND)
> -			PMD_INIT_LOG(ERR, "VLAN QinQ is not "
> -					"supported in fm10k");
> -	}
> -
> -	if (mask & ETH_VLAN_FILTER_MASK) {
> -		if (!(dev->data->dev_conf.rxmode.offloads &
> -			DEV_RX_OFFLOAD_VLAN_FILTER))
> -			PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
> -	}
> -
>   	return 0;
>   }
>   
> diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c b/drivers/net/hinic/hinic_pmd_ethdev.c
> index 0c3e1c0..0009a61 100644
> --- a/drivers/net/hinic/hinic_pmd_ethdev.c
> +++ b/drivers/net/hinic/hinic_pmd_ethdev.c
> @@ -1701,12 +1701,6 @@ static int hinic_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>   			  nic_dev->proc_dev_name, dev->data->port_id);
>   	}
>   
> -	if (mask & ETH_VLAN_EXTEND_MASK) {
> -		PMD_DRV_LOG(ERR, "Don't support vlan qinq, device: %s, port_id: %d",
> -			  nic_dev->proc_dev_name, dev->data->port_id);
> -		return -ENOTSUP;
> -	}
> -
>   	return 0;
>   }
>   
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 970a31c..9211cf5 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -3877,11 +3877,6 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>   	struct i40e_vsi *vsi = pf->main_vsi;
>   	struct rte_eth_rxmode *rxmode;
>   
> -	if (mask & ETH_QINQ_STRIP_MASK) {
> -		PMD_DRV_LOG(ERR, "Strip qinq is not supported.");
> -		return -ENOTSUP;
> -	}
> -
>   	rxmode = &dev->data->dev_conf.rxmode;
>   	if (mask & ETH_VLAN_FILTER_MASK) {
>   		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index 2460ee1..b7a91ae 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -2353,11 +2353,6 @@ nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>   	hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>   	new_ctrl = 0;
>   
> -	if ((mask & ETH_VLAN_FILTER_OFFLOAD) ||
> -	    (mask & ETH_VLAN_EXTEND_OFFLOAD))
> -		PMD_DRV_LOG(INFO, "No support for ETH_VLAN_FILTER_OFFLOAD or"
> -			" ETH_VLAN_EXTEND_OFFLOAD");
> -
>   	/* Enable vlan strip if it is not configured yet */
>   	if ((mask & ETH_VLAN_STRIP_OFFLOAD) &&
>   	    !(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
> diff --git a/drivers/net/octeontx/octeontx_ethdev_ops.c b/drivers/net/octeontx/octeontx_ethdev_ops.c
> index ff627a6..dbe13ce 100644
> --- a/drivers/net/octeontx/octeontx_ethdev_ops.c
> +++ b/drivers/net/octeontx/octeontx_ethdev_ops.c
> @@ -43,16 +43,6 @@ octeontx_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>   
>   	rxmode = &dev->data->dev_conf.rxmode;
>   
> -	if (mask & ETH_VLAN_EXTEND_MASK) {
> -		octeontx_log_err("Extend offload not supported");
> -		return -ENOTSUP;
> -	}
> -
> -	if (mask & ETH_VLAN_STRIP_MASK) {
> -		octeontx_log_err("VLAN strip offload not supported");
> -		return -ENOTSUP;
> -	}
> -
>   	if (mask & ETH_VLAN_FILTER_MASK) {
>   		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER) {
>   			rc = octeontx_vlan_hw_filter(nic, true);
> diff --git a/drivers/net/octeontx2/otx2_vlan.c b/drivers/net/octeontx2/otx2_vlan.c
> index 322a565..7357b06 100644
> --- a/drivers/net/octeontx2/otx2_vlan.c
> +++ b/drivers/net/octeontx2/otx2_vlan.c
> @@ -717,11 +717,6 @@ otx2_nix_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>   
>   	rxmode = &eth_dev->data->dev_conf.rxmode;
>   
> -	if (mask & ETH_VLAN_EXTEND_MASK) {
> -		otx2_err("Extend offload not supported");
> -		return -ENOTSUP;
> -	}
> -
>   	if (mask & ETH_VLAN_STRIP_MASK) {
>   		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) {
>   			offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index c4f8f12..04d17c7 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -1036,9 +1036,6 @@ static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>   		}
>   	}
>   
> -	if (mask & ETH_VLAN_EXTEND_MASK)
> -		DP_ERR(edev, "Extend VLAN not supported\n");
> -
>   	qdev->vlan_offload_mask = mask;
>   
>   	DP_INFO(edev, "VLAN offload mask %d\n", mask);
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index b0b0474..bf79749 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -3260,12 +3260,14 @@ rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
>   int
>   rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
>   {
> +	struct rte_eth_dev_info dev_info;
>   	struct rte_eth_dev *dev;
>   	int ret = 0;
>   	int mask = 0;
>   	int cur, org = 0;
>   	uint64_t orig_offloads;
>   	uint64_t dev_offloads;
> +	uint64_t new_offloads;
>   
>   	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>   	dev = &rte_eth_devices[port_id];
> @@ -3319,6 +3321,25 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
>   	if (mask == 0)
>   		return ret;
>   
> +	ret = rte_eth_dev_info_get(port_id, &dev_info);
> +	if (ret != 0)
> +		return ret;
> +
> +	/*
> +	 * New added Rx VLAN offloading which are not enabled in
> +	 * rte_eth_dev_configure() must be within its device capabilities
> +	 */
> +	if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) {
> +		new_offloads = dev_offloads & ~orig_offloads;
> +		RTE_ETHDEV_LOG(ERR,
> +			"Ethdev port_id=%u requested new added VLAN offloads "
> +			"0x%" PRIx64 " must be within Rx offloads capabilities "
> +			"0x%" PRIx64 " in %s()\n",
> +			port_id, new_offloads, dev_info.rx_offload_capa,
> +			__func__);
> +		return -EINVAL;
> +	}
> +
>   	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP);
>   	dev->data->dev_conf.rxmode.offloads = dev_offloads;
>   	ret = (*dev->dev_ops->vlan_offload_set)(dev, mask);
Wei Hu (Xavier) June 22, 2020, 7:06 a.m. UTC | #3
Hi, Sachin Saxena


On 2020/6/22 13:10, Sachin Saxena (OSS) wrote:
>
> On 22-Jun-20 8:17 AM, Wei Hu (Xavier) wrote:
>> Currently, there is a potential problem that calling the API function
>> rte_eth_dev_set_vlan_offload to start VLAN hardware offloads which the
>> driver does not support. If the PMD driver does not support certain VLAN
>> hardware offloads and does not check for it, the hardware setting will
>> not change, but the VLAN offloads in dev->data->dev_conf.rxmode.offloads
>> will be turned on.
>>
>> It is supposed to check the hardware capabilities to decide whether the
>> relative callback needs to be called just like the behavior in the API
>> function named rte_eth_dev_configure. And it is also needed to cleanup
>> duplicated checks which are done in some PMDs. Also, note that it is
>> behaviour change for some PMDs which simply ignore (with 
>> error/warning log
>> message) unsupported VLAN offloads, but now it will fail.
>>
>> Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> v2 -> v3: Add __rte_unused to avoid unused parameter 'dev' and 'mask'
>>       warning.
>> v1 -> v2: cleanup duplicated checks which are done in some PMDs.
>> ---
>>   drivers/net/dpaa2/dpaa2_ethdev.c           |  5 -----
>>   drivers/net/enic/enic_ethdev.c             | 12 ------------
>>   drivers/net/fm10k/fm10k_ethdev.c           | 23 
>> ++---------------------
>>   drivers/net/hinic/hinic_pmd_ethdev.c       |  6 ------
>>   drivers/net/i40e/i40e_ethdev.c             |  5 -----
>>   drivers/net/nfp/nfp_net.c                  |  5 -----
>>   drivers/net/octeontx/octeontx_ethdev_ops.c | 10 ----------
>>   drivers/net/octeontx2/otx2_vlan.c          |  5 -----
>>   drivers/net/qede/qede_ethdev.c             |  3 ---
>>   lib/librte_ethdev/rte_ethdev.c             | 21 +++++++++++++++++++++
>>   10 files changed, 23 insertions(+), 72 deletions(-)
>>
>> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c 
>> b/drivers/net/dpaa2/dpaa2_ethdev.c
>> index 2f031ec..da5018f 100644
>> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
>> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
>> @@ -169,11 +169,6 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev, 
>> int mask)
>>               DPAA2_PMD_INFO("Unable to set vlan filter = %d", ret);
>>       }
>>   next_mask:
>
> The changes looks good but the label "next_mask" and its usage above 
> should also be deleted/replaced.
OK, I will delete "next_mask" and modify the function that when the offload
is not supported the function fail as below,
what do you think of it? Looking forward to your comment.

static int
dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
{
     struct dpaa2_dev_priv *priv = dev->data->dev_private;
     struct fsl_mc_io *dpni = dev->process_private;
     int ret = 0;

     PMD_INIT_FUNC_TRACE();

     if (mask & ETH_VLAN_FILTER_MASK) {
         /* VLAN Filter not avaialble */
         if (!priv->max_vlan_filters) {
             DPAA2_PMD_INFO("VLAN filter not available");
             return -ENOTSUP;
         }

         if (dev->data->dev_conf.rxmode.offloads &
             DEV_RX_OFFLOAD_VLAN_FILTER)
             ret = dpni_enable_vlan_filter(dpni, CMD_PRI_LOW,
                               priv->token, true);
         else
             ret = dpni_enable_vlan_filter(dpni, CMD_PRI_LOW,
                               priv->token, false);
         if (ret < 0)
             DPAA2_PMD_INFO("Unable to set vlan filter = %d", ret);
     }

     return ret;
}

Thanks, Xavier
>
>
>> -    if (mask & ETH_VLAN_EXTEND_MASK) {
>> -        if (dev->data->dev_conf.rxmode.offloads &
>> -            DEV_RX_OFFLOAD_VLAN_EXTEND)
>> -            DPAA2_PMD_INFO("VLAN extend offload not supported");
>> -    }
>>         return 0;
>>   }
>> diff --git a/drivers/net/enic/enic_ethdev.c 
>> b/drivers/net/enic/enic_ethdev.c
>> index 32d5397..ef8900d 100644
>> --- a/drivers/net/enic/enic_ethdev.c
>> +++ b/drivers/net/enic/enic_ethdev.c
>> @@ -374,18 +374,6 @@ static int enicpmd_vlan_offload_set(struct 
>> rte_eth_dev *eth_dev, int mask)
>>               enic->ig_vlan_strip_en = 0;
>>       }
>>   -    if ((mask & ETH_VLAN_FILTER_MASK) &&
>> -        (offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
>> -        dev_warning(enic,
>> -            "Configuration of VLAN filter is not supported\n");
>> -    }
>> -
>> -    if ((mask & ETH_VLAN_EXTEND_MASK) &&
>> -        (offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)) {
>> -        dev_warning(enic,
>> -            "Configuration of extended VLAN is not supported\n");
>> -    }
>> -
>>       return enic_set_vlan_strip(enic);
>>   }
>>   diff --git a/drivers/net/fm10k/fm10k_ethdev.c 
>> b/drivers/net/fm10k/fm10k_ethdev.c
>> index f537ab2..f5b854e 100644
>> --- a/drivers/net/fm10k/fm10k_ethdev.c
>> +++ b/drivers/net/fm10k/fm10k_ethdev.c
>> @@ -1575,28 +1575,9 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev, 
>> uint16_t vlan_id, int on)
>>   }
>>     static int
>> -fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>> +fm10k_vlan_offload_set(struct rte_eth_dev *dev __rte_unused,
>> +               int mask __rte_unused)
>>   {
>> -    if (mask & ETH_VLAN_STRIP_MASK) {
>> -        if (!(dev->data->dev_conf.rxmode.offloads &
>> -            DEV_RX_OFFLOAD_VLAN_STRIP))
>> -            PMD_INIT_LOG(ERR, "VLAN stripping is "
>> -                    "always on in fm10k");
>> -    }
>> -
>> -    if (mask & ETH_VLAN_EXTEND_MASK) {
>> -        if (dev->data->dev_conf.rxmode.offloads &
>> -            DEV_RX_OFFLOAD_VLAN_EXTEND)
>> -            PMD_INIT_LOG(ERR, "VLAN QinQ is not "
>> -                    "supported in fm10k");
>> -    }
>> -
>> -    if (mask & ETH_VLAN_FILTER_MASK) {
>> -        if (!(dev->data->dev_conf.rxmode.offloads &
>> -            DEV_RX_OFFLOAD_VLAN_FILTER))
>> -            PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
>> -    }
>> -
>>       return 0;
>>   }
>>   diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c 
>> b/drivers/net/hinic/hinic_pmd_ethdev.c
>> index 0c3e1c0..0009a61 100644
>> --- a/drivers/net/hinic/hinic_pmd_ethdev.c
>> +++ b/drivers/net/hinic/hinic_pmd_ethdev.c
>> @@ -1701,12 +1701,6 @@ static int hinic_vlan_offload_set(struct 
>> rte_eth_dev *dev, int mask)
>>                 nic_dev->proc_dev_name, dev->data->port_id);
>>       }
>>   -    if (mask & ETH_VLAN_EXTEND_MASK) {
>> -        PMD_DRV_LOG(ERR, "Don't support vlan qinq, device: %s, 
>> port_id: %d",
>> -              nic_dev->proc_dev_name, dev->data->port_id);
>> -        return -ENOTSUP;
>> -    }
>> -
>>       return 0;
>>   }
>>   diff --git a/drivers/net/i40e/i40e_ethdev.c 
>> b/drivers/net/i40e/i40e_ethdev.c
>> index 970a31c..9211cf5 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -3877,11 +3877,6 @@ i40e_vlan_offload_set(struct rte_eth_dev *dev, 
>> int mask)
>>       struct i40e_vsi *vsi = pf->main_vsi;
>>       struct rte_eth_rxmode *rxmode;
>>   -    if (mask & ETH_QINQ_STRIP_MASK) {
>> -        PMD_DRV_LOG(ERR, "Strip qinq is not supported.");
>> -        return -ENOTSUP;
>> -    }
>> -
>>       rxmode = &dev->data->dev_conf.rxmode;
>>       if (mask & ETH_VLAN_FILTER_MASK) {
>>           if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
>> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
>> index 2460ee1..b7a91ae 100644
>> --- a/drivers/net/nfp/nfp_net.c
>> +++ b/drivers/net/nfp/nfp_net.c
>> @@ -2353,11 +2353,6 @@ nfp_net_vlan_offload_set(struct rte_eth_dev 
>> *dev, int mask)
>>       hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>       new_ctrl = 0;
>>   -    if ((mask & ETH_VLAN_FILTER_OFFLOAD) ||
>> -        (mask & ETH_VLAN_EXTEND_OFFLOAD))
>> -        PMD_DRV_LOG(INFO, "No support for ETH_VLAN_FILTER_OFFLOAD or"
>> -            " ETH_VLAN_EXTEND_OFFLOAD");
>> -
>>       /* Enable vlan strip if it is not configured yet */
>>       if ((mask & ETH_VLAN_STRIP_OFFLOAD) &&
>>           !(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
>> diff --git a/drivers/net/octeontx/octeontx_ethdev_ops.c 
>> b/drivers/net/octeontx/octeontx_ethdev_ops.c
>> index ff627a6..dbe13ce 100644
>> --- a/drivers/net/octeontx/octeontx_ethdev_ops.c
>> +++ b/drivers/net/octeontx/octeontx_ethdev_ops.c
>> @@ -43,16 +43,6 @@ octeontx_dev_vlan_offload_set(struct rte_eth_dev 
>> *dev, int mask)
>>         rxmode = &dev->data->dev_conf.rxmode;
>>   -    if (mask & ETH_VLAN_EXTEND_MASK) {
>> -        octeontx_log_err("Extend offload not supported");
>> -        return -ENOTSUP;
>> -    }
>> -
>> -    if (mask & ETH_VLAN_STRIP_MASK) {
>> -        octeontx_log_err("VLAN strip offload not supported");
>> -        return -ENOTSUP;
>> -    }
>> -
>>       if (mask & ETH_VLAN_FILTER_MASK) {
>>           if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER) {
>>               rc = octeontx_vlan_hw_filter(nic, true);
>> diff --git a/drivers/net/octeontx2/otx2_vlan.c 
>> b/drivers/net/octeontx2/otx2_vlan.c
>> index 322a565..7357b06 100644
>> --- a/drivers/net/octeontx2/otx2_vlan.c
>> +++ b/drivers/net/octeontx2/otx2_vlan.c
>> @@ -717,11 +717,6 @@ otx2_nix_vlan_offload_set(struct rte_eth_dev 
>> *eth_dev, int mask)
>>         rxmode = &eth_dev->data->dev_conf.rxmode;
>>   -    if (mask & ETH_VLAN_EXTEND_MASK) {
>> -        otx2_err("Extend offload not supported");
>> -        return -ENOTSUP;
>> -    }
>> -
>>       if (mask & ETH_VLAN_STRIP_MASK) {
>>           if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) {
>>               offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
>> diff --git a/drivers/net/qede/qede_ethdev.c 
>> b/drivers/net/qede/qede_ethdev.c
>> index c4f8f12..04d17c7 100644
>> --- a/drivers/net/qede/qede_ethdev.c
>> +++ b/drivers/net/qede/qede_ethdev.c
>> @@ -1036,9 +1036,6 @@ static int qede_vlan_offload_set(struct 
>> rte_eth_dev *eth_dev, int mask)
>>           }
>>       }
>>   -    if (mask & ETH_VLAN_EXTEND_MASK)
>> -        DP_ERR(edev, "Extend VLAN not supported\n");
>> -
>>       qdev->vlan_offload_mask = mask;
>>         DP_INFO(edev, "VLAN offload mask %d\n", mask);
>> diff --git a/lib/librte_ethdev/rte_ethdev.c 
>> b/lib/librte_ethdev/rte_ethdev.c
>> index b0b0474..bf79749 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -3260,12 +3260,14 @@ rte_eth_dev_set_vlan_ether_type(uint16_t 
>> port_id,
>>   int
>>   rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
>>   {
>> +    struct rte_eth_dev_info dev_info;
>>       struct rte_eth_dev *dev;
>>       int ret = 0;
>>       int mask = 0;
>>       int cur, org = 0;
>>       uint64_t orig_offloads;
>>       uint64_t dev_offloads;
>> +    uint64_t new_offloads;
>>         RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>       dev = &rte_eth_devices[port_id];
>> @@ -3319,6 +3321,25 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, 
>> int offload_mask)
>>       if (mask == 0)
>>           return ret;
>>   +    ret = rte_eth_dev_info_get(port_id, &dev_info);
>> +    if (ret != 0)
>> +        return ret;
>> +
>> +    /*
>> +     * New added Rx VLAN offloading which are not enabled in
>> +     * rte_eth_dev_configure() must be within its device capabilities
>> +     */
>> +    if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) {
>> +        new_offloads = dev_offloads & ~orig_offloads;
>> +        RTE_ETHDEV_LOG(ERR,
>> +            "Ethdev port_id=%u requested new added VLAN offloads "
>> +            "0x%" PRIx64 " must be within Rx offloads capabilities "
>> +            "0x%" PRIx64 " in %s()\n",
>> +            port_id, new_offloads, dev_info.rx_offload_capa,
>> +            __func__);
>> +        return -EINVAL;
>> +    }
>> +
>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP);
>>       dev->data->dev_conf.rxmode.offloads = dev_offloads;
>>       ret = (*dev->dev_ops->vlan_offload_set)(dev, mask);
>
> .
>

Patch
diff mbox series

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 2f031ec..da5018f 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -169,11 +169,6 @@  dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 			DPAA2_PMD_INFO("Unable to set vlan filter = %d", ret);
 	}
 next_mask:
-	if (mask & ETH_VLAN_EXTEND_MASK) {
-		if (dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_VLAN_EXTEND)
-			DPAA2_PMD_INFO("VLAN extend offload not supported");
-	}
 
 	return 0;
 }
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 32d5397..ef8900d 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -374,18 +374,6 @@  static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
 			enic->ig_vlan_strip_en = 0;
 	}
 
-	if ((mask & ETH_VLAN_FILTER_MASK) &&
-	    (offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
-		dev_warning(enic,
-			"Configuration of VLAN filter is not supported\n");
-	}
-
-	if ((mask & ETH_VLAN_EXTEND_MASK) &&
-	    (offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)) {
-		dev_warning(enic,
-			"Configuration of extended VLAN is not supported\n");
-	}
-
 	return enic_set_vlan_strip(enic);
 }
 
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index f537ab2..f5b854e 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1575,28 +1575,9 @@  fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 }
 
 static int
-fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+fm10k_vlan_offload_set(struct rte_eth_dev *dev __rte_unused,
+		       int mask __rte_unused)
 {
-	if (mask & ETH_VLAN_STRIP_MASK) {
-		if (!(dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_VLAN_STRIP))
-			PMD_INIT_LOG(ERR, "VLAN stripping is "
-					"always on in fm10k");
-	}
-
-	if (mask & ETH_VLAN_EXTEND_MASK) {
-		if (dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_VLAN_EXTEND)
-			PMD_INIT_LOG(ERR, "VLAN QinQ is not "
-					"supported in fm10k");
-	}
-
-	if (mask & ETH_VLAN_FILTER_MASK) {
-		if (!(dev->data->dev_conf.rxmode.offloads &
-			DEV_RX_OFFLOAD_VLAN_FILTER))
-			PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
-	}
-
 	return 0;
 }
 
diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c b/drivers/net/hinic/hinic_pmd_ethdev.c
index 0c3e1c0..0009a61 100644
--- a/drivers/net/hinic/hinic_pmd_ethdev.c
+++ b/drivers/net/hinic/hinic_pmd_ethdev.c
@@ -1701,12 +1701,6 @@  static int hinic_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 			  nic_dev->proc_dev_name, dev->data->port_id);
 	}
 
-	if (mask & ETH_VLAN_EXTEND_MASK) {
-		PMD_DRV_LOG(ERR, "Don't support vlan qinq, device: %s, port_id: %d",
-			  nic_dev->proc_dev_name, dev->data->port_id);
-		return -ENOTSUP;
-	}
-
 	return 0;
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 970a31c..9211cf5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3877,11 +3877,6 @@  i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	struct i40e_vsi *vsi = pf->main_vsi;
 	struct rte_eth_rxmode *rxmode;
 
-	if (mask & ETH_QINQ_STRIP_MASK) {
-		PMD_DRV_LOG(ERR, "Strip qinq is not supported.");
-		return -ENOTSUP;
-	}
-
 	rxmode = &dev->data->dev_conf.rxmode;
 	if (mask & ETH_VLAN_FILTER_MASK) {
 		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 2460ee1..b7a91ae 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2353,11 +2353,6 @@  nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	new_ctrl = 0;
 
-	if ((mask & ETH_VLAN_FILTER_OFFLOAD) ||
-	    (mask & ETH_VLAN_EXTEND_OFFLOAD))
-		PMD_DRV_LOG(INFO, "No support for ETH_VLAN_FILTER_OFFLOAD or"
-			" ETH_VLAN_EXTEND_OFFLOAD");
-
 	/* Enable vlan strip if it is not configured yet */
 	if ((mask & ETH_VLAN_STRIP_OFFLOAD) &&
 	    !(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN))
diff --git a/drivers/net/octeontx/octeontx_ethdev_ops.c b/drivers/net/octeontx/octeontx_ethdev_ops.c
index ff627a6..dbe13ce 100644
--- a/drivers/net/octeontx/octeontx_ethdev_ops.c
+++ b/drivers/net/octeontx/octeontx_ethdev_ops.c
@@ -43,16 +43,6 @@  octeontx_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 
 	rxmode = &dev->data->dev_conf.rxmode;
 
-	if (mask & ETH_VLAN_EXTEND_MASK) {
-		octeontx_log_err("Extend offload not supported");
-		return -ENOTSUP;
-	}
-
-	if (mask & ETH_VLAN_STRIP_MASK) {
-		octeontx_log_err("VLAN strip offload not supported");
-		return -ENOTSUP;
-	}
-
 	if (mask & ETH_VLAN_FILTER_MASK) {
 		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER) {
 			rc = octeontx_vlan_hw_filter(nic, true);
diff --git a/drivers/net/octeontx2/otx2_vlan.c b/drivers/net/octeontx2/otx2_vlan.c
index 322a565..7357b06 100644
--- a/drivers/net/octeontx2/otx2_vlan.c
+++ b/drivers/net/octeontx2/otx2_vlan.c
@@ -717,11 +717,6 @@  otx2_nix_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
 
 	rxmode = &eth_dev->data->dev_conf.rxmode;
 
-	if (mask & ETH_VLAN_EXTEND_MASK) {
-		otx2_err("Extend offload not supported");
-		return -ENOTSUP;
-	}
-
 	if (mask & ETH_VLAN_STRIP_MASK) {
 		if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) {
 			offloads |= DEV_RX_OFFLOAD_VLAN_STRIP;
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index c4f8f12..04d17c7 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1036,9 +1036,6 @@  static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
 		}
 	}
 
-	if (mask & ETH_VLAN_EXTEND_MASK)
-		DP_ERR(edev, "Extend VLAN not supported\n");
-
 	qdev->vlan_offload_mask = mask;
 
 	DP_INFO(edev, "VLAN offload mask %d\n", mask);
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index b0b0474..bf79749 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3260,12 +3260,14 @@  rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
 int
 rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
 {
+	struct rte_eth_dev_info dev_info;
 	struct rte_eth_dev *dev;
 	int ret = 0;
 	int mask = 0;
 	int cur, org = 0;
 	uint64_t orig_offloads;
 	uint64_t dev_offloads;
+	uint64_t new_offloads;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
@@ -3319,6 +3321,25 @@  rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
 	if (mask == 0)
 		return ret;
 
+	ret = rte_eth_dev_info_get(port_id, &dev_info);
+	if (ret != 0)
+		return ret;
+
+	/*
+	 * New added Rx VLAN offloading which are not enabled in
+	 * rte_eth_dev_configure() must be within its device capabilities
+	 */
+	if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) {
+		new_offloads = dev_offloads & ~orig_offloads;
+		RTE_ETHDEV_LOG(ERR,
+			"Ethdev port_id=%u requested new added VLAN offloads "
+			"0x%" PRIx64 " must be within Rx offloads capabilities "
+			"0x%" PRIx64 " in %s()\n",
+			port_id, new_offloads, dev_info.rx_offload_capa,
+			__func__);
+		return -EINVAL;
+	}
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP);
 	dev->data->dev_conf.rxmode.offloads = dev_offloads;
 	ret = (*dev->dev_ops->vlan_offload_set)(dev, mask);