From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hemant Agrawal Subject: Re: [PATCH v4] ethdev: allow returning error on VLAN offload configuration Date: Fri, 1 Sep 2017 13:11:19 +0530 Message-ID: References: <20170825134717.18376-1-dharton@cisco.com> <20170901023628.21308-1-dharton@cisco.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: To: David Harton , , , , , , , , , , , , , , , , , Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0076.outbound.protection.outlook.com [104.47.34.76]) by dpdk.org (Postfix) with ESMTP id 59788108F for ; Fri, 1 Sep 2017 09:41:31 +0200 (CEST) In-Reply-To: <20170901023628.21308-1-dharton@cisco.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > > 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 = ð_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, >