From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from alln-iport-3.cisco.com (alln-iport-3.cisco.com [173.37.142.90]) by dpdk.org (Postfix) with ESMTP id 6176E5A98 for ; Fri, 1 Sep 2017 14:54:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=33599; q=dns/txt; s=iport; t=1504270492; x=1505480092; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=iw/e0I/R8AYCr9Hru+v/sSrGUJO02qV9keZsEBZCwsQ=; b=l/IQhRUQsvoYqxTCsMM8wBxpSBoKuvzDRn2dnMfXsGEy+Mb4G1+ZZLH3 UsEdp43ifnjSBrFsf21QBi4L+wC9SnPVRVIof3LLSpUvdPrcoHnebveox NMR9sw8+GR4tzQVDZUlVKxmNr1EReyWvFDmOW1TrX+urxT7cedaiL/wkd Y=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0CuAACmV6lZ/4kNJK1cGQEBAQEBAQEBA?= =?us-ascii?q?QEBBwEBAQEBg1pkgRUHWIc4hgCQHoFxligOggQkhSMChBQ/GAECAQEBAQEBAWs?= =?us-ascii?q?ohRgBAQEBAgEaAQwTPwwEAgEIEQQBAQEeCQcyFAkIAgQBDQUIDIoVCLBtOoQUA?= =?us-ascii?q?QGHSgEBAQEBAQEBAQEBAQEBAQEBAQEBAR2DKoICgU6BY4MohF2GDAWJfgeJCY1?= =?us-ascii?q?kAodZhzGFPIIcGz+QAYl5jE0BHziBDXcVhWEcgSwBOnYBiXKBDwEBAQ?= X-IronPort-AV: E=Sophos;i="5.41,458,1498521600"; d="scan'208";a="479413729" Received: from alln-core-4.cisco.com ([173.36.13.137]) by alln-iport-3.cisco.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 01 Sep 2017 12:54:50 +0000 Received: from XCH-RCD-010.cisco.com (xch-rcd-010.cisco.com [173.37.102.20]) by alln-core-4.cisco.com (8.14.5/8.14.5) with ESMTP id v81CsoP2024564 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 1 Sep 2017 12:54:50 GMT Received: from xch-rcd-016.cisco.com (173.37.102.26) by XCH-RCD-010.cisco.com (173.37.102.20) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Fri, 1 Sep 2017 07:54:49 -0500 Received: from xch-rcd-016.cisco.com ([173.37.102.26]) by XCH-RCD-016.cisco.com ([173.37.102.26]) with mapi id 15.00.1263.000; Fri, 1 Sep 2017 07:54:49 -0500 From: "David Harton (dharton)" To: Hemant Agrawal , "thomas@monjalon.net" , "ferruh.yigit@intel.com" , "ajit.khaparde@broadcom.com" , "John Daley (johndale)" , "konstantin.ananyev@intel.com" , "jingjing.wu@intel.com" , "beilei.xing@intel.com" , "jing.d.chen@intel.com" , "adrien.mazarguil@6wind.com" , "nelio.laranjeiro@6wind.com" , "alejandro.lucero@netronome.com" , "rasesh.mody@cavium.com" , "harish.patil@cavium.com" , "skhare@vmware.com" , "yliu@fridaylinux.org" , "maxime.coquelin@redhat.com" , "allain.legacy@windriver.com" CC: "dev@dpdk.org" Thread-Topic: [PATCH v4] ethdev: allow returning error on VLAN offload configuration Thread-Index: AQHTIssf+K6PEnUXC0G/S0Koh/YxtaKf+aGAgAACh2A= Date: Fri, 1 Sep 2017 12:54:49 +0000 Message-ID: References: <20170825134717.18376-1-dharton@cisco.com> <20170901023628.21308-1-dharton@cisco.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.82.235.18] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4] ethdev: allow returning error on VLAN offload configuration X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Sep 2017 12:54:53 -0000 > -----Original Message----- > From: Hemant Agrawal [mailto:hemant.agrawal@nxp.com] > Sent: Friday, September 01, 2017 3:41 AM > To: David Harton (dharton) ; thomas@monjalon.net; > ferruh.yigit@intel.com; ajit.khaparde@broadcom.com; John Daley (johndale) > ; konstantin.ananyev@intel.com; jingjing.wu@intel.com= ; > beilei.xing@intel.com; jing.d.chen@intel.com; adrien.mazarguil@6wind.com; > nelio.laranjeiro@6wind.com; alejandro.lucero@netronome.com; > rasesh.mody@cavium.com; harish.patil@cavium.com; skhare@vmware.com; > yliu@fridaylinux.org; maxime.coquelin@redhat.com; > allain.legacy@windriver.com > Cc: dev@dpdk.org > Subject: Re: [PATCH v4] ethdev: allow returning error on VLAN offload > configuration >=20 > 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. > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > +* **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 > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > > > > > - > > 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 =3D (ETH_VLAN_STRIP_MASK | > > ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK); > > - avp_vlan_offload_set(eth_dev, mask); > > + ret =3D avp_vlan_offload_set(eth_dev, mask); > > + if (ret < 0) { > > + PMD_DRV_LOG(ERR, "VLAN offload set failed by host, ret=3D%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 =3D > > 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 |=3D ETH_VLAN_FILTER_MASK; > > if (eth_dev->data->dev_conf.rxmode.hw_vlan_strip) > > vlan_mask |=3D ETH_VLAN_STRIP_MASK; > > - bnxt_vlan_offload_set_op(eth_dev, vlan_mask); > > + rc =3D 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 =3D (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 =3D dev->data->dev_private; @@ -158,6 > > +158,7 @@ > > RTE_LOG(ERR, PMD, "Unable to set vlan filter =3D %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 =3D dpaa2_vlan_offload_set(dev, ETH_VLAN_FILTER_MASK); > > + if (ret) { > > + PMD_INIT_LOG(ERR, "Error to dpaa2_vlan_offload_set:" > > + "code =3D %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 =3D ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ > > ETH_VLAN_EXTEND_MASK; > > - eth_em_vlan_offload_set(dev, mask); > > + ret =3D 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_devic= e > *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 =3D ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | \ > > ETH_VLAN_EXTEND_MASK; > > - eth_igb_vlan_offload_set(dev, mask); > > + ret =3D 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 =3D=3D 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_unuse= d > 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 =3D 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 =3D enicpmd_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK); > > enic->hw_ip_checksum =3D 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 =3D 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 =3D ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK; > > - i40e_vlan_offload_set(dev, mask); > > + ret =3D 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_de= v > *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 =3D i40evf_vlan_offload_set(dev, ETH_VLAN_STRIP_MASK); > > + if (ret) > > + return ret; > > > > /* Apply pvid setting */ > > ret =3D 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 =3D &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 =3D ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > > + mask =3D ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK; > > - ixgbe_vlan_offload_set(dev, mask); > > + err =3D 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 =3D=3D 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 =3D ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | > > ETH_VLAN_EXTEND_MASK; > > - ixgbevf_vlan_offload_set(dev, mask); > > + err =3D 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 =3D > > @@ -5168,6 +5178,7 @@ static void ixgbevf_set_vfta_all(struct > rte_eth_dev *dev, bool on) > > for (i =3D 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 =3D 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 =3D NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > new_ctrl =3D 0; > > @@ -2174,14 +2175,14 @@ uint32_t nfp_net_txq_full(struct nfp_net_txq > *txq) > > new_ctrl =3D hw->ctrl & ~NFP_NET_CFG_CTRL_RXVLAN; > > > > if (new_ctrl =3D=3D 0) > > - return; > > + return 0; > > > > update =3D NFP_NET_CFG_UPDATE_GEN; > > > > - if (nfp_net_reconfig(hw, new_ctrl, update) < 0) > > - return; > > - > > - hw->ctrl =3D new_ctrl; > > + ret =3D nfp_net_reconfig(hw, new_ctrl, update); > > + if (!ret) > > + hw->ctrl =3D 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 =3D QEDE_INIT_QDEV(eth_dev); > > struct ecore_dev *edev =3D 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 =3D QEDE_INIT_QDEV(eth_dev); > > struct ecore_dev *edev =3D QEDE_INIT_EDEV(qdev); > > struct rte_eth_rxmode *rxmode =3D ð_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 =3D rxmode->enable_lro; > > > > /* Enable VLAN offloads by default */ > > - qede_vlan_offload_set(eth_dev, ETH_VLAN_STRIP_MASK | > > + ret =3D 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=3D%d TSS=3D%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 =3D virtio_dev_stats_reset, > > .xstats_reset =3D virtio_dev_stats_reset, > > .link_update =3D virtio_dev_link_update, > > + .vlan_offload_set =3D virtio_dev_vlan_offload_set, > > .rx_queue_setup =3D virtio_dev_rx_queue_setup, > > .rx_queue_intr_enable =3D virtio_dev_rx_queue_intr_enable, > > .rx_queue_intr_disable =3D 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 =3D=3D 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 =3D &dev->data->dev_conf.rxmode; > > + struct virtio_hw *hw =3D 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 =3D 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 =3D hw->rss_confPA; > > } > > > > - vmxnet3_dev_vlan_offload_set(dev, > > + ret =3D 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 =3D 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 =3D 0; > > int mask =3D 0; > > - int cur, org =3D 0; > > + int cur, orig =3D 0; > > + uint8_t orig_strip, orig_filter, orig_extend; > > > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > > dev =3D &rte_eth_devices[port_id]; > > > > + /* save original values in case of failure */ > > + orig_strip =3D dev->data->dev_conf.rxmode.hw_vlan_strip; > > + orig_filter =3D dev->data->dev_conf.rxmode.hw_vlan_filter; > > + orig_extend =3D dev->data->dev_conf.rxmode.hw_vlan_extend; > > + > > /*check which option changed by application*/ > > cur =3D !!(offload_mask & ETH_VLAN_STRIP_OFFLOAD); > > - org =3D !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > > - if (cur !=3D org) { > > + orig =3D !!(dev->data->dev_conf.rxmode.hw_vlan_strip); > > + if (cur !=3D orig) { > > dev->data->dev_conf.rxmode.hw_vlan_strip =3D (uint8_t)cur; > > mask |=3D ETH_VLAN_STRIP_MASK; > > } > > > > cur =3D !!(offload_mask & ETH_VLAN_FILTER_OFFLOAD); > > - org =3D !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > > - if (cur !=3D org) { > > + orig =3D !!(dev->data->dev_conf.rxmode.hw_vlan_filter); > > + if (cur !=3D orig) { > > dev->data->dev_conf.rxmode.hw_vlan_filter =3D (uint8_t)cur; > > mask |=3D ETH_VLAN_FILTER_MASK; > > } > > > > cur =3D !!(offload_mask & ETH_VLAN_EXTEND_OFFLOAD); > > - org =3D !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > > - if (cur !=3D org) { > > + orig =3D !!(dev->data->dev_conf.rxmode.hw_vlan_extend); > > + if (cur !=3D orig) { > > dev->data->dev_conf.rxmode.hw_vlan_extend =3D (uint8_t)cur; > > mask |=3D 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 =3D (*dev->dev_ops->vlan_offload_set)(dev, mask); > > + if (ret) { > > + /* hit an error restore original values */ > > + dev->data->dev_conf.rxmode.hw_vlan_strip =3D orig_strip; > > + dev->data->dev_conf.rxmode.hw_vlan_filter =3D orig_filter; > > + dev->data->dev_conf.rxmode.hw_vlan_extend =3D orig_extend; > > + } > > > Currently vlan offload is combining three functions: > strip, filter and extend. >=20 > 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? There are many examples of APIs that not all PMDs support. There are also other APIs that manipulate many attributes but do not communicate which attribute fails on set. Solving these issues I believe it outside the scope of this change and it requires a larger community discussion. This proposed change at least adheres to the existing paradigm. >=20 > > 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, > >