[dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration

Hemant Agrawal hemant.agrawal at nxp.com
Fri Sep 1 09:41:19 CEST 2017


On 9/1/2017 8:06 AM, David Harton wrote:
> Some devices may not support or fail setting VLAN offload
> configuration based on dynamic circurmstances so the
> vlan_offload_set_t vector is modified to return an int so
> the caller can determine success or not.
>
> rte_eth_dev_set_vlan_offload is updated to return the
> value provided by the vector when called along with restoring
> the original offload configs on failure.
>
> Existing vlan_offload_set_t vectors are modified to return
> an int.  Majority of cases return 0 but a few that actually
> can fail now return their failure codes.
>
> Finally, a vlan_offload_set_t vector is added to virtio
> to facilitate dynamically turning VLAN strip on or off.
>
> Signed-off-by: David Harton <dharton at cisco.com>
> ---
>
> v4
> * Modified commit message heading
> * Moved rel_note comments from ABI to API section
> * Renamed locals of rte_eth_dev_set_vlan_offload from 'org*' to 'orig*'
>
> v3
> * Fixed a format error.
> * Apologies...need to figure out why checkpatches.pl keeps saying
>   valid patch when I've got soft tabs.
>
> v2
> * Fixed a missed format error.
> * Removed vlan offload vector call casts and replaced with checks
>   for return values.
>
> v1
> * This is an ABI breakage that has been previously negotiated
>   with Thomas and the proposed rel note change is included as well.
>
>  doc/guides/rel_notes/release_17_11.rst |  8 +++++++-
>  drivers/net/avp/avp_ethdev.c           | 12 +++++++++---
>  drivers/net/bnxt/bnxt_ethdev.c         |  9 ++++++---
>  drivers/net/dpaa2/dpaa2_ethdev.c       | 13 ++++++++++---
>  drivers/net/e1000/em_ethdev.c          | 12 +++++++++---
>  drivers/net/e1000/igb_ethdev.c         | 12 +++++++++---
>  drivers/net/enic/enic_ethdev.c         |  8 +++++---
>  drivers/net/fm10k/fm10k_ethdev.c       |  3 ++-
>  drivers/net/i40e/i40e_ethdev.c         | 11 ++++++++---
>  drivers/net/i40e/i40e_ethdev_vf.c      |  9 ++++++---
>  drivers/net/ixgbe/ixgbe_ethdev.c       | 25 ++++++++++++++++++-------
>  drivers/net/mlx5/mlx5.h                |  2 +-
>  drivers/net/mlx5/mlx5_vlan.c           |  3 ++-
>  drivers/net/nfp/nfp_net.c              | 13 +++++++------
>  drivers/net/qede/qede_ethdev.c         |  9 +++++++--
>  drivers/net/virtio/virtio_ethdev.c     | 21 +++++++++++++++++++++
>  drivers/net/vmxnet3/vmxnet3_ethdev.c   | 10 +++++++---
>  lib/librte_ether/rte_ethdev.c          | 28 ++++++++++++++++++++--------
>  lib/librte_ether/rte_ethdev.h          |  2 +-
>  19 files changed, 155 insertions(+), 55 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst
> index 170f4f9..22df4fd 100644
> --- a/doc/guides/rel_notes/release_17_11.rst
> +++ b/doc/guides/rel_notes/release_17_11.rst
> @@ -110,6 +110,13 @@ API Changes
>     Also, make sure to start the actual text at the margin.
>     =========================================================
>
> +* **Modified the vlan_offload_set_t function prototype in the ethdev library.**
> +
> +  Changed the function prototype of ``vlan_offload_set_t``.  The return value
> +  has been changed from ``void`` to ``int`` so the caller to knows whether
> +  the backing device supports the operation or if the operation was
> +  successfully performed.
> +
>
>  ABI Changes
>  -----------
> @@ -125,7 +132,6 @@ ABI Changes
>     =========================================================
>
>
> -
>  Shared Library Versions
>  -----------------------
>
> diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
> index c746a0e..4011dfa 100644
> --- a/drivers/net/avp/avp_ethdev.c
> +++ b/drivers/net/avp/avp_ethdev.c
> @@ -70,7 +70,7 @@ static int avp_dev_create(struct rte_pci_device *pci_dev,
>  static void avp_dev_close(struct rte_eth_dev *dev);
>  static void avp_dev_info_get(struct rte_eth_dev *dev,
>  			     struct rte_eth_dev_info *dev_info);
> -static void avp_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int avp_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static int avp_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete);
>  static void avp_dev_promiscuous_enable(struct rte_eth_dev *dev);
>  static void avp_dev_promiscuous_disable(struct rte_eth_dev *dev);
> @@ -2031,7 +2031,12 @@ struct avp_queue {
>  	mask = (ETH_VLAN_STRIP_MASK |
>  		ETH_VLAN_FILTER_MASK |
>  		ETH_VLAN_EXTEND_MASK);
> -	avp_vlan_offload_set(eth_dev, mask);
> +	ret = avp_vlan_offload_set(eth_dev, mask);
> +	if (ret < 0) {
> +		PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=%d\n",
> +			    ret);
> +		goto unlock;
> +	}
>
>  	/* update device config */
>  	memset(&config, 0, sizeof(config));
> @@ -2214,7 +2219,7 @@ struct avp_queue {
>  	}
>  }
>
> -static void
> +static int
>  avp_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>  {
>  	struct avp_dev *avp = AVP_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
> @@ -2239,6 +2244,7 @@ struct avp_queue {
>  		if (eth_dev->data->dev_conf.rxmode.hw_vlan_extend)
>  			PMD_DRV_LOG(ERR, "VLAN extend offload not supported\n");
>  	}
> +	return 0;
>  }
>
>  static void
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index c9d1122..547bd55 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -144,7 +144,7 @@
>  	ETH_RSS_NONFRAG_IPV6_TCP |	\
>  	ETH_RSS_NONFRAG_IPV6_UDP)
>
> -static void bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
> +static int bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask);
>
>  /***********************/
>
> @@ -522,7 +522,9 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev)
>  		vlan_mask |= ETH_VLAN_FILTER_MASK;
>  	if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip)
>  		vlan_mask |= ETH_VLAN_STRIP_MASK;
> -	bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
> +	rc = bnxt_vlan_offload_set_op(eth_dev, vlan_mask);
> +	if (rc)
> +		goto error;
>
>  	return 0;
>
> @@ -1275,7 +1277,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev,
>  		return bnxt_del_vlan_filter(bp, vlan_id);
>  }
>
> -static void
> +static int
>  bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
>  {
>  	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> @@ -1307,6 +1309,7 @@ static int bnxt_vlan_filter_set_op(struct rte_eth_dev *eth_dev,
>
>  	if (mask & ETH_VLAN_EXTEND_MASK)
>  		RTE_LOG(ERR, PMD, "Extend VLAN Not supported\n");
> +	return 0;
>  }
>
>  static void
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
> index 429b3a0..3390cb3 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -138,7 +138,7 @@
>  	return ret;
>  }
>
> -static void
> +static int
>  dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	struct dpaa2_dev_priv *priv = dev->data->dev_private;
> @@ -158,6 +158,7 @@
>  			RTE_LOG(ERR, PMD, "Unable to set vlan filter = %d\n",
>  				ret);
>  	}
> +	return 0;
>  }
>
>  static int
> @@ -643,8 +644,14 @@
>  		return ret;
>  	}
>  	/* VLAN Offload Settings */
> -	if (priv->max_vlan_filters)
> -		dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK);
> +	if (priv->max_vlan_filters) {
> +		ret = dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK);
> +		if (ret) {
> +			PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:"
> +				     "code = %d\n", ret);
> +			return ret;
> +		}
> +	}
>
>  	return 0;
>  }
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 3d4ab93..51f49d8 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -99,7 +99,7 @@ static int eth_em_interrupt_action(struct rte_eth_dev *dev,
>
>  static int eth_em_vlan_filter_set(struct rte_eth_dev *dev,
>  		uint16_t vlan_id, int on);
> -static void eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static void em_vlan_hw_filter_enable(struct rte_eth_dev *dev);
>  static void em_vlan_hw_filter_disable(struct rte_eth_dev *dev);
>  static void em_vlan_hw_strip_enable(struct rte_eth_dev *dev);
> @@ -668,7 +668,12 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev)
>
>  	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \
>  			ETH_VLAN_EXTEND_MASK;
> -	eth_em_vlan_offload_set(dev, mask);
> +	ret = eth_em_vlan_offload_set(dev, mask);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Unable to update vlan offload");
> +		em_dev_clear_queues(dev);
> +		return ret;
> +	}
>
>  	/* Set Interrupt Throttling Rate to maximum allowed value. */
>  	E1000_WRITE_REG(hw, E1000_ITR, UINT16_MAX);
> @@ -1447,7 +1452,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev)
>  	E1000_WRITE_REG(hw, E1000_CTRL, reg);
>  }
>
> -static void
> +static int
>  eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	if(mask & ETH_VLAN_STRIP_MASK){
> @@ -1463,6 +1468,7 @@ static int eth_em_pci_remove(struct rte_pci_device *pci_dev)
>  		else
>  			em_vlan_hw_filter_disable(dev);
>  	}
> +	return 0;
>  }
>
>  /*
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index e4f7a9f..fa15ee9 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -157,7 +157,7 @@ static int eth_igb_vlan_filter_set(struct rte_eth_dev *dev,
>  static int eth_igb_vlan_tpid_set(struct rte_eth_dev *dev,
>  				 enum rte_vlan_type vlan_type,
>  				 uint16_t tpid_id);
> -static void eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>
>  static void igb_vlan_hw_filter_enable(struct rte_eth_dev *dev);
>  static void igb_vlan_hw_filter_disable(struct rte_eth_dev *dev);
> @@ -1400,7 +1400,12 @@ static int eth_igbvf_pci_remove(struct rte_pci_device *pci_dev)
>  	 */
>  	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \
>  			ETH_VLAN_EXTEND_MASK;
> -	eth_igb_vlan_offload_set(dev, mask);
> +	ret = eth_igb_vlan_offload_set(dev, mask);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Unable to set vlan offload");
> +		igb_dev_clear_queues(dev);
> +		return ret;
> +	}
>
>  	if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) {
>  		/* Enable VLAN filter since VMDq always use VLAN filter */
> @@ -2715,7 +2720,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  						2 * VLAN_TAG_SIZE);
>  }
>
> -static void
> +static int
>  eth_igb_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	if(mask & ETH_VLAN_STRIP_MASK){
> @@ -2738,6 +2743,7 @@ static int eth_igbvf_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  		else
>  			igb_vlan_hw_extend_disable(dev);
>  	}
> +	return 0;
>  }
>
>
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index da8fec2..fc1eac2 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -347,7 +347,7 @@ static int enicpmd_vlan_filter_set(struct rte_eth_dev *eth_dev,
>  	return err;
>  }
>
> -static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
> +static int enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>  {
>  	struct enic *enic = pmd_priv(eth_dev);
>
> @@ -371,6 +371,8 @@ static void enicpmd_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>  		dev_warning(enic,
>  			"Configuration of extended VLAN is not supported\n");
>  	}
> +
> +	return 0;
>  }
>
>  static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev)
> @@ -392,9 +394,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev)
>  			eth_dev->data->dev_conf.rxmode.split_hdr_size);
>  	}
>
> -	enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK);
> +	ret = enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK);
>  	enic->hw_ip_checksum = eth_dev->data->dev_conf.rxmode.hw_ip_checksum;
> -	return 0;
> +	return ret;
>  }
>
>  /* Start the device.
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index e60d3a3..f4626f7 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -1590,7 +1590,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  	return 0;
>  }
>
> -static void
> +static int
>  fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	if (mask & ETH_VLAN_STRIP_MASK) {
> @@ -1609,6 +1609,7 @@ static int fm10k_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  		if (!dev->data->dev_conf.rxmode.hw_vlan_filter)
>  			PMD_INIT_LOG(ERR, "VLAN filter is always on in fm10k");
>  	}
> +	return 0;
>  }
>
>  /* Add/Remove a MAC address, and update filters to main VSI */
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 00b6082..d03a44b 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -278,7 +278,7 @@ static int i40e_vlan_filter_set(struct rte_eth_dev *dev,
>  static int i40e_vlan_tpid_set(struct rte_eth_dev *dev,
>  			      enum rte_vlan_type vlan_type,
>  			      uint16_t tpid);
> -static void i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static void i40e_vlan_strip_queue_set(struct rte_eth_dev *dev,
>  				      uint16_t queue,
>  				      int on);
> @@ -3130,7 +3130,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  	return ret;
>  }
>
> -static void
> +static int
>  i40e_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> @@ -3163,6 +3163,7 @@ static int i40e_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  		else
>  			i40e_vsi_config_double_vlan(vsi, FALSE);
>  	}
> +	return 0;
>  }
>
>  static void
> @@ -5216,7 +5217,11 @@ struct i40e_vsi *
>
>  	/* Apply vlan offload setting */
>  	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK;
> -	i40e_vlan_offload_set(dev, mask);
> +	ret = i40e_vlan_offload_set(dev, mask);
> +	if (ret) {
> +		PMD_DRV_LOG(INFO, "Failed to update vlan offload");
> +		return ret;
> +	}
>
>  	/* Apply double-vlan setting, not implemented yet */
>
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index f6d8293..f7fffc2 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -118,7 +118,7 @@ static int i40evf_dev_xstats_get_names(struct rte_eth_dev *dev,
>  static void i40evf_dev_xstats_reset(struct rte_eth_dev *dev);
>  static int i40evf_vlan_filter_set(struct rte_eth_dev *dev,
>  				  uint16_t vlan_id, int on);
> -static void i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static int i40evf_vlan_pvid_set(struct rte_eth_dev *dev, uint16_t pvid,
>  				int on);
>  static void i40evf_dev_close(struct rte_eth_dev *dev);
> @@ -1634,7 +1634,9 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
>  	int ret;
>
>  	/* Apply vlan offload setting */
> -	i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
> +	ret = i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK);
> +	if (ret)
> +		return ret;
>
>  	/* Apply pvid setting */
>  	ret = i40evf_vlan_pvid_set(dev, data->dev_conf.txmode.pvid,
> @@ -1642,7 +1644,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
>  	return ret;
>  }
>
> -static void
> +static int
>  i40evf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	struct rte_eth_conf *dev_conf = &dev->data->dev_conf;
> @@ -1655,6 +1657,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
>  		else
>  			i40evf_disable_vlan_strip(dev);
>  	}
> +	return 0;
>  }
>
>  static int
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 22171d8..1ec5aaf 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -218,7 +218,7 @@ static void ixgbe_vlan_hw_strip_bitmap_set(struct rte_eth_dev *dev,
>  		uint16_t queue, bool on);
>  static void ixgbe_vlan_strip_queue_set(struct rte_eth_dev *dev, uint16_t queue,
>  		int on);
> -static void ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static void ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue);
>  static void ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue);
>  static void ixgbe_vlan_hw_extend_enable(struct rte_eth_dev *dev);
> @@ -274,7 +274,7 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_dev *dev,
>  		uint16_t vlan_id, int on);
>  static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev,
>  		uint16_t queue, int on);
> -static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on);
>  static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev,
>  					    uint16_t queue_id);
> @@ -2125,7 +2125,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)
>  	 */
>  }
>
> -static void
> +static int
>  ixgbe_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	if (mask & ETH_VLAN_STRIP_MASK) {
> @@ -2148,6 +2148,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)
>  		else
>  			ixgbe_vlan_hw_extend_disable(dev);
>  	}
> +	return 0;
>  }
>
>  static void
> @@ -2568,9 +2569,13 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)
>  		goto error;
>  	}
>
> -    mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
> +	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
>  		ETH_VLAN_EXTEND_MASK;
> -	ixgbe_vlan_offload_set(dev, mask);
> +	err = ixgbe_vlan_offload_set(dev, mask);
> +	if (err) {
> +		PMD_INIT_LOG(ERR, "Unable to set VLAN offload");
> +		goto error;
> +	}
>
>  	if (dev->data->dev_conf.rxmode.mq_mode == ETH_MQ_RX_VMDQ_ONLY) {
>  		/* Enable vlan filtering for VMDq */
> @@ -4993,7 +4998,12 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  	/* Set HW strip */
>  	mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK |
>  		ETH_VLAN_EXTEND_MASK;
> -	ixgbevf_vlan_offload_set(dev, mask);
> +	err = ixgbevf_vlan_offload_set(dev, mask);
> +	if (err) {
> +		PMD_INIT_LOG(ERR, "Unable to set VLAN offload (%d)", err);
> +		ixgbe_dev_clear_queues(dev);
> +		return err;
> +	}
>
>  	ixgbevf_dev_rxtx_start(dev);
>
> @@ -5153,7 +5163,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
>  	ixgbe_vlan_hw_strip_bitmap_set(dev, queue, on);
>  }
>
> -static void
> +static int
>  ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	struct ixgbe_hw *hw =
> @@ -5168,6 +5178,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
>  		for (i = 0; i < hw->mac.max_rx_queues; i++)
>  			ixgbevf_vlan_strip_queue_set(dev, i, on);
>  	}
> +	return 0;
>  }
>
>  int
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index 43c5384..93e71be 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -283,7 +283,7 @@ int mlx5_xstats_get_names(struct rte_eth_dev *,
>  /* mlx5_vlan.c */
>
>  int mlx5_vlan_filter_set(struct rte_eth_dev *, uint16_t, int);
> -void mlx5_vlan_offload_set(struct rte_eth_dev *, int);
> +int mlx5_vlan_offload_set(struct rte_eth_dev *, int);
>  void mlx5_vlan_strip_queue_set(struct rte_eth_dev *, uint16_t, int);
>
>  /* mlx5_trigger.c */
> diff --git a/drivers/net/mlx5/mlx5_vlan.c b/drivers/net/mlx5/mlx5_vlan.c
> index 1b0fa40..7215909 100644
> --- a/drivers/net/mlx5/mlx5_vlan.c
> +++ b/drivers/net/mlx5/mlx5_vlan.c
> @@ -210,7 +210,7 @@
>   * @param mask
>   *   VLAN offload bit mask.
>   */
> -void
> +int
>  mlx5_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	struct priv *priv = dev->data->dev_private;
> @@ -230,4 +230,5 @@
>  			priv_vlan_strip_queue_set(priv, i, hw_vlan_strip);
>  		priv_unlock(priv);
>  	}
> +	return 0;
>  }
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index 92b03c4..6473edc 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -2149,11 +2149,12 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq)
>  	return i;
>  }
>
> -static void
> +static int
>  nfp_net_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	uint32_t new_ctrl, update;
>  	struct nfp_net_hw *hw;
> +	int ret;
>
>  	hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	new_ctrl = 0;
> @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq *txq)
>  		new_ctrl = hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN;
>
>  	if (new_ctrl == 0)
> -		return;
> +		return 0;
>
>  	update = NFP_NET_CFG_UPDATE_GEN;
>
> -	if (nfp_net_reconfig(hw, new_ctrl, update) < 0)
> -		return;
> -
> -	hw->ctrl = new_ctrl;
> +	ret = nfp_net_reconfig(hw, new_ctrl, update);
> +	if (!ret)
> +		hw->ctrl = new_ctrl;
> +	return ret;
>  }
>
>  /* Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device */
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index 0e05989..644f69d 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -975,7 +975,7 @@ static int qede_vlan_filter_set(struct rte_eth_dev *eth_dev,
>  	return rc;
>  }
>
> -static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
> +static int qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>  {
>  	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
>  	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
> @@ -1013,6 +1013,8 @@ static void qede_vlan_offload_set(struct rte_eth_dev *eth_dev, int mask)
>
>  	DP_INFO(edev, "vlan offload mask %d vlan-strip %d vlan-filter %d\n",
>  		mask, rxmode->hw_vlan_strip, rxmode->hw_vlan_filter);
> +
> +	return 0;
>  }
>
>  static void qede_prandom_bytes(uint32_t *buff)
> @@ -1157,6 +1159,7 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
>  	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
>  	struct ecore_dev *edev = QEDE_INIT_EDEV(qdev);
>  	struct rte_eth_rxmode *rxmode = &eth_dev->data->dev_conf.rxmode;
> +	int ret;
>
>  	PMD_INIT_FUNC_TRACE(edev);
>
> @@ -1237,9 +1240,11 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev)
>  	qdev->enable_lro = rxmode->enable_lro;
>
>  	/* Enable VLAN offloads by default */
> -	qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK  |
> +	ret = qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK  |
>  			ETH_VLAN_FILTER_MASK |
>  			ETH_VLAN_EXTEND_MASK);
> +	if (ret)
> +		return ret;
>
>  	DP_INFO(edev, "Device configured with RSS=%d TSS=%d\n",
>  			QEDE_RSS_COUNT(qdev), QEDE_TSS_COUNT(qdev));
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index e320811..72b4248 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -72,6 +72,7 @@ static void virtio_dev_info_get(struct rte_eth_dev *dev,
>  				struct rte_eth_dev_info *dev_info);
>  static int virtio_dev_link_update(struct rte_eth_dev *dev,
>  	int wait_to_complete);
> +static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>
>  static void virtio_set_hwaddr(struct virtio_hw *hw);
>  static void virtio_get_hwaddr(struct virtio_hw *hw);
> @@ -779,6 +780,7 @@ struct rte_virtio_xstats_name_off {
>  	.stats_reset             = virtio_dev_stats_reset,
>  	.xstats_reset            = virtio_dev_stats_reset,
>  	.link_update             = virtio_dev_link_update,
> +	.vlan_offload_set        = virtio_dev_vlan_offload_set,
>  	.rx_queue_setup          = virtio_dev_rx_queue_setup,
>  	.rx_queue_intr_enable    = virtio_dev_rx_queue_intr_enable,
>  	.rx_queue_intr_disable   = virtio_dev_rx_queue_intr_disable,
> @@ -1875,6 +1877,25 @@ static void virtio_dev_free_mbufs(struct rte_eth_dev *dev)
>  	return (old.link_status == link.link_status) ? -1 : 0;
>  }
>
> +static int
> +virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
> +{
> +	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
> +	struct virtio_hw *hw = dev->data->dev_private;
> +
> +	if (rxmode->hw_vlan_filter &&
> +	    !vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VLAN)) {
> +		PMD_DRV_LOG(NOTICE,
> +			    "vlan filtering not available on this host");
> +		return -ENOTSUP;
> +	}
> +
> +	if (mask & ETH_VLAN_STRIP_MASK)
> +		hw->vlan_strip = rxmode->hw_vlan_strip;
> +
> +	return 0;
> +}
> +
>  static void
>  virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  {
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index 3910991..06735dd 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -100,7 +100,7 @@ static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
>  vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev);
>  static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
>  				       uint16_t vid, int on);
> -static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> +static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
>  				 struct ether_addr *mac_addr);
>  static void vmxnet3_interrupt_handler(void *param);
> @@ -730,8 +730,10 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev)
>  		devRead->rssConfDesc.confPA  = hw->rss_confPA;
>  	}
>
> -	vmxnet3_dev_vlan_offload_set(dev,
> +	ret = vmxnet3_dev_vlan_offload_set(dev,
>  				     ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK);
> +	if (ret)
> +		return ret;
>
>  	vmxnet3_write_mac(hw, dev->data->mac_addrs->addr_bytes);
>
> @@ -1275,7 +1277,7 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev)
>  	return 0;
>  }
>
> -static void
> +static int
>  vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
>  {
>  	struct vmxnet3_hw *hw = dev->data->dev_private;
> @@ -1301,6 +1303,8 @@ static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev)
>  		VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>  				       VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>  	}
> +
> +	return 0;
>  }
>
>  static void
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 0597641..05e52b8 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -2048,29 +2048,35 @@ struct rte_eth_dev *
>  	struct rte_eth_dev *dev;
>  	int ret = 0;
>  	int mask = 0;
> -	int cur, org = 0;
> +	int cur, orig = 0;
> +	uint8_t orig_strip, orig_filter, orig_extend;
>
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>  	dev = &rte_eth_devices[port_id];
>
> +	/* save original values in case of failure */
> +	orig_strip = dev->data->dev_conf.rxmode.hw_vlan_strip;
> +	orig_filter = dev->data->dev_conf.rxmode.hw_vlan_filter;
> +	orig_extend = dev->data->dev_conf.rxmode.hw_vlan_extend;
> +
>  	/*check which option changed by application*/
>  	cur = !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD);
> -	org = !!(dev->data->dev_conf.rxmode.hw_vlan_strip);
> -	if (cur != org) {
> +	orig = !!(dev->data->dev_conf.rxmode.hw_vlan_strip);
> +	if (cur != orig) {
>  		dev->data->dev_conf.rxmode.hw_vlan_strip = (uint8_t)cur;
>  		mask |= ETH_VLAN_STRIP_MASK;
>  	}
>
>  	cur = !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD);
> -	org = !!(dev->data->dev_conf.rxmode.hw_vlan_filter);
> -	if (cur != org) {
> +	orig = !!(dev->data->dev_conf.rxmode.hw_vlan_filter);
> +	if (cur != orig) {
>  		dev->data->dev_conf.rxmode.hw_vlan_filter = (uint8_t)cur;
>  		mask |= ETH_VLAN_FILTER_MASK;
>  	}
>
>  	cur = !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD);
> -	org = !!(dev->data->dev_conf.rxmode.hw_vlan_extend);
> -	if (cur != org) {
> +	orig = !!(dev->data->dev_conf.rxmode.hw_vlan_extend);
> +	if (cur != orig) {
>  		dev->data->dev_conf.rxmode.hw_vlan_extend = (uint8_t)cur;
>  		mask |= ETH_VLAN_EXTEND_MASK;
>  	}
> @@ -2080,7 +2086,13 @@ struct rte_eth_dev *
>  		return ret;
>
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP);
> -	(*dev->dev_ops->vlan_offload_set)(dev, mask);
> +	ret = (*dev->dev_ops->vlan_offload_set)(dev, mask);
> +	if (ret) {
> +		/* hit an error restore  original values */
> +		dev->data->dev_conf.rxmode.hw_vlan_strip = orig_strip;
> +		dev->data->dev_conf.rxmode.hw_vlan_filter = orig_filter;
> +		dev->data->dev_conf.rxmode.hw_vlan_extend = orig_extend;
> +	}
>
Currently vlan offload is combining three functions:
strip, filter and extend.

Not all PMDs in DPDK support all of three.
Should not the error we extended to reflect, which of the VLAN offload 
is not supported?

>  	return ret;
>  }
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 0adf327..7254fd0 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1245,7 +1245,7 @@ typedef int (*vlan_tpid_set_t)(struct rte_eth_dev *dev,
>  			       enum rte_vlan_type type, uint16_t tpid);
>  /**< @internal set the outer/inner VLAN-TPID by an Ethernet device. */
>
> -typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask);
> +typedef int (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask);
>  /**< @internal set VLAN offload function by an Ethernet device. */
>
>  typedef int (*vlan_pvid_set_t)(struct rte_eth_dev *dev,
>



More information about the dev mailing list