* [PATCH] net/ice: fix assumption about tag placement order
@ 2025-07-16 17:39 Bruce Richardson
2025-07-17 8:19 ` David Marchand
2025-07-18 15:43 ` [PATCH v2] net/intel: " Bruce Richardson
0 siblings, 2 replies; 7+ messages in thread
From: Bruce Richardson @ 2025-07-16 17:39 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson, stable
The specific placement of outer/inner VLAN tags in E810 and related NICs
is configurable. Therefore, remove the assumption that if the L2Tag2
field is filled in, that the L2Tag1 must also be. Instead, check the
existing mbuf VLAN flags, and move tags and set flags as appropriate.
This fixes an issue where, with QinQ packets with different Tag ethtypes
(0x88a8 vs 0x8100), we get an mbuf reporting two valid tags, but only
having had one tag stripped.
Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops")
Cc: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/net/intel/ice/ice_rxtx.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/intel/ice/ice_rxtx.c b/drivers/net/intel/ice/ice_rxtx.c
index da508592aa..f965aab6ee 100644
--- a/drivers/net/intel/ice/ice_rxtx.c
+++ b/drivers/net/intel/ice/ice_rxtx.c
@@ -1835,9 +1835,13 @@ ice_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union ci_rx_flex_desc *rxdp)
#ifndef RTE_NET_INTEL_USE_16BYTE_DESC
if (rte_le_to_cpu_16(rxdp->wb.status_error1) &
(1 << ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) {
- mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | RTE_MBUF_F_RX_QINQ |
- RTE_MBUF_F_RX_VLAN_STRIPPED | RTE_MBUF_F_RX_VLAN;
- mb->vlan_tci_outer = mb->vlan_tci;
+ if ((mb->ol_flags & RTE_MBUF_F_RX_VLAN_STRIPPED) == 0) {
+ mb->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED;
+ } else {
+ /* if two tags, move Tag1 to outer tag field */
+ mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | RTE_MBUF_F_RX_QINQ;
+ mb->vlan_tci_outer = mb->vlan_tci;
+ }
mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.l2tag2_2nd);
PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u",
rte_le_to_cpu_16(rxdp->wb.l2tag2_1st),
--
2.48.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] net/ice: fix assumption about tag placement order 2025-07-16 17:39 [PATCH] net/ice: fix assumption about tag placement order Bruce Richardson @ 2025-07-17 8:19 ` David Marchand 2025-07-17 8:32 ` Bruce Richardson 2025-07-18 15:43 ` [PATCH v2] net/intel: " Bruce Richardson 1 sibling, 1 reply; 7+ messages in thread From: David Marchand @ 2025-07-17 8:19 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, stable On Wed, Jul 16, 2025 at 7:39 PM Bruce Richardson <bruce.richardson@intel.com> wrote: > > The specific placement of outer/inner VLAN tags in E810 and related NICs > is configurable. Therefore, remove the assumption that if the L2Tag2 > field is filled in, that the L2Tag1 must also be. Instead, check the > existing mbuf VLAN flags, and move tags and set flags as appropriate. > This fixes an issue where, with QinQ packets with different Tag ethtypes > (0x88a8 vs 0x8100), we get an mbuf reporting two valid tags, but only > having had one tag stripped. > > Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops") > Cc: stable@dpdk.org > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> The ice code seems copy/pasted from i40e. So I guess i40e is affected as well. -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/ice: fix assumption about tag placement order 2025-07-17 8:19 ` David Marchand @ 2025-07-17 8:32 ` Bruce Richardson 2025-07-17 8:36 ` David Marchand 0 siblings, 1 reply; 7+ messages in thread From: Bruce Richardson @ 2025-07-17 8:32 UTC (permalink / raw) To: David Marchand; +Cc: dev, stable On Thu, Jul 17, 2025 at 10:19:24AM +0200, David Marchand wrote: > On Wed, Jul 16, 2025 at 7:39 PM Bruce Richardson > <bruce.richardson@intel.com> wrote: > > > > The specific placement of outer/inner VLAN tags in E810 and related NICs > > is configurable. Therefore, remove the assumption that if the L2Tag2 > > field is filled in, that the L2Tag1 must also be. Instead, check the > > existing mbuf VLAN flags, and move tags and set flags as appropriate. > > This fixes an issue where, with QinQ packets with different Tag ethtypes > > (0x88a8 vs 0x8100), we get an mbuf reporting two valid tags, but only > > having had one tag stripped. > > > > Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops") > > Cc: stable@dpdk.org > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > The ice code seems copy/pasted from i40e. > So I guess i40e is affected as well. > I'll investigate more. These fixes I intend for 25.11 anyway, since I'd rather not risk them late in the 25.07 release cycle. /Bruce ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] net/ice: fix assumption about tag placement order 2025-07-17 8:32 ` Bruce Richardson @ 2025-07-17 8:36 ` David Marchand 0 siblings, 0 replies; 7+ messages in thread From: David Marchand @ 2025-07-17 8:36 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, stable On Thu, Jul 17, 2025 at 10:33 AM Bruce Richardson <bruce.richardson@intel.com> wrote: > > On Thu, Jul 17, 2025 at 10:19:24AM +0200, David Marchand wrote: > > On Wed, Jul 16, 2025 at 7:39 PM Bruce Richardson > > <bruce.richardson@intel.com> wrote: > > > > > > The specific placement of outer/inner VLAN tags in E810 and related NICs > > > is configurable. Therefore, remove the assumption that if the L2Tag2 > > > field is filled in, that the L2Tag1 must also be. Instead, check the > > > existing mbuf VLAN flags, and move tags and set flags as appropriate. > > > This fixes an issue where, with QinQ packets with different Tag ethtypes > > > (0x88a8 vs 0x8100), we get an mbuf reporting two valid tags, but only > > > having had one tag stripped. > > > > > > Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > > > The ice code seems copy/pasted from i40e. > > So I guess i40e is affected as well. > > > > I'll investigate more. These fixes I intend for 25.11 anyway, since I'd iavf has a similar helper. > rather not risk them late in the 25.07 release cycle. Yes, too late for the current release. -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] net/intel: fix assumption about tag placement order 2025-07-16 17:39 [PATCH] net/ice: fix assumption about tag placement order Bruce Richardson 2025-07-17 8:19 ` David Marchand @ 2025-07-18 15:43 ` Bruce Richardson 2025-08-07 14:31 ` Loftus, Ciara 1 sibling, 1 reply; 7+ messages in thread From: Bruce Richardson @ 2025-07-18 15:43 UTC (permalink / raw) To: dev; +Cc: Bruce Richardson, stable The specific placement of outer/inner VLAN tags in NIC descriptors is configurable. Therefore, remove the assumption that if the L2Tag2 field is filled in, that the L2Tag1 must also be. Instead, check the existing mbuf VLAN flags, and move tags and set flags as appropriate. This fixes an issue where, with QinQ packets with different Tag ethtypes (0x88a8 vs 0x8100), we get an mbuf reporting two valid tags, but only having had one tag stripped. Fixes: cc9d0456b870 ("i40e: support double vlan stripping and insertion") Fixes: 1e728b01120c ("net/iavf: rework Tx path") Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops") Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/intel/i40e/i40e_rxtx.c | 10 +++++++--- drivers/net/intel/iavf/iavf_rxtx.c | 12 +++++++----- drivers/net/intel/ice/ice_rxtx.c | 10 +++++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/drivers/net/intel/i40e/i40e_rxtx.c b/drivers/net/intel/i40e/i40e_rxtx.c index aba3c11ee5..a1fc320d05 100644 --- a/drivers/net/intel/i40e/i40e_rxtx.c +++ b/drivers/net/intel/i40e/i40e_rxtx.c @@ -128,9 +128,13 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union ci_rx_desc *rxdp) #ifndef RTE_NET_INTEL_USE_16BYTE_DESC if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) & (1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) { - mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | RTE_MBUF_F_RX_QINQ | - RTE_MBUF_F_RX_VLAN_STRIPPED | RTE_MBUF_F_RX_VLAN; - mb->vlan_tci_outer = mb->vlan_tci; + if ((mb->ol_flags & RTE_MBUF_F_RX_VLAN_STRIPPED) == 0) { + mb->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED; + } else { + /* if two tags, move Tag1 to outer tag field */ + mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | RTE_MBUF_F_RX_QINQ; + mb->vlan_tci_outer = mb->vlan_tci; + } mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_2); PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u", rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_1), diff --git a/drivers/net/intel/iavf/iavf_rxtx.c b/drivers/net/intel/iavf/iavf_rxtx.c index 7033a74610..887dcd1b2f 100644 --- a/drivers/net/intel/iavf/iavf_rxtx.c +++ b/drivers/net/intel/iavf/iavf_rxtx.c @@ -1169,11 +1169,13 @@ iavf_flex_rxd_to_vlan_tci(struct rte_mbuf *mb, if (rte_le_to_cpu_16(rxdp->wb.status_error1) & (1 << IAVF_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) { - mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | - RTE_MBUF_F_RX_QINQ | - RTE_MBUF_F_RX_VLAN_STRIPPED | - RTE_MBUF_F_RX_VLAN; - mb->vlan_tci_outer = mb->vlan_tci; + if ((mb->ol_flags & RTE_MBUF_F_RX_VLAN_STRIPPED) == 0) { + mb->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED; + } else { + /* if two tags, move Tag1 to outer tag field */ + mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | RTE_MBUF_F_RX_QINQ; + mb->vlan_tci_outer = mb->vlan_tci; + } mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.l2tag2_2nd); PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u", rte_le_to_cpu_16(rxdp->wb.l2tag2_1st), diff --git a/drivers/net/intel/ice/ice_rxtx.c b/drivers/net/intel/ice/ice_rxtx.c index da508592aa..f965aab6ee 100644 --- a/drivers/net/intel/ice/ice_rxtx.c +++ b/drivers/net/intel/ice/ice_rxtx.c @@ -1835,9 +1835,13 @@ ice_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union ci_rx_flex_desc *rxdp) #ifndef RTE_NET_INTEL_USE_16BYTE_DESC if (rte_le_to_cpu_16(rxdp->wb.status_error1) & (1 << ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) { - mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | RTE_MBUF_F_RX_QINQ | - RTE_MBUF_F_RX_VLAN_STRIPPED | RTE_MBUF_F_RX_VLAN; - mb->vlan_tci_outer = mb->vlan_tci; + if ((mb->ol_flags & RTE_MBUF_F_RX_VLAN_STRIPPED) == 0) { + mb->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED; + } else { + /* if two tags, move Tag1 to outer tag field */ + mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | RTE_MBUF_F_RX_QINQ; + mb->vlan_tci_outer = mb->vlan_tci; + } mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.l2tag2_2nd); PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: %u", rte_le_to_cpu_16(rxdp->wb.l2tag2_1st), -- 2.48.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2] net/intel: fix assumption about tag placement order 2025-07-18 15:43 ` [PATCH v2] net/intel: " Bruce Richardson @ 2025-08-07 14:31 ` Loftus, Ciara 2025-08-07 18:59 ` Bruce Richardson 0 siblings, 1 reply; 7+ messages in thread From: Loftus, Ciara @ 2025-08-07 14:31 UTC (permalink / raw) To: Richardson, Bruce, dev; +Cc: Richardson, Bruce, stable > > The specific placement of outer/inner VLAN tags in NIC descriptors > is configurable. Therefore, remove the assumption that if the L2Tag2 > field is filled in, that the L2Tag1 must also be. Instead, check the > existing mbuf VLAN flags, and move tags and set flags as appropriate. > This fixes an issue where, with QinQ packets with different Tag ethtypes > (0x88a8 vs 0x8100), we get an mbuf reporting two valid tags, but only > having had one tag stripped. > > Fixes: cc9d0456b870 ("i40e: support double vlan stripping and insertion") > Fixes: 1e728b01120c ("net/iavf: rework Tx path") > Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops") > Cc: stable@dpdk.org > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/net/intel/i40e/i40e_rxtx.c | 10 +++++++--- > drivers/net/intel/iavf/iavf_rxtx.c | 12 +++++++----- > drivers/net/intel/ice/ice_rxtx.c | 10 +++++++--- > 3 files changed, 21 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/intel/i40e/i40e_rxtx.c > b/drivers/net/intel/i40e/i40e_rxtx.c > index aba3c11ee5..a1fc320d05 100644 > --- a/drivers/net/intel/i40e/i40e_rxtx.c > +++ b/drivers/net/intel/i40e/i40e_rxtx.c > @@ -128,9 +128,13 @@ i40e_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile > union ci_rx_desc *rxdp) > #ifndef RTE_NET_INTEL_USE_16BYTE_DESC > if (rte_le_to_cpu_16(rxdp->wb.qword2.ext_status) & > (1 << I40E_RX_DESC_EXT_STATUS_L2TAG2P_SHIFT)) { > - mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | > RTE_MBUF_F_RX_QINQ | > - RTE_MBUF_F_RX_VLAN_STRIPPED | > RTE_MBUF_F_RX_VLAN; > - mb->vlan_tci_outer = mb->vlan_tci; > + if ((mb->ol_flags & RTE_MBUF_F_RX_VLAN_STRIPPED) == 0) { > + mb->ol_flags |= RTE_MBUF_F_RX_VLAN | > RTE_MBUF_F_RX_VLAN_STRIPPED; > + } else { > + /* if two tags, move Tag1 to outer tag field */ > + mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | > RTE_MBUF_F_RX_QINQ; > + mb->vlan_tci_outer = mb->vlan_tci; > + } > mb->vlan_tci = rte_le_to_cpu_16(rxdp- > >wb.qword2.l2tag2_2); > PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: > %u", > rte_le_to_cpu_16(rxdp->wb.qword2.l2tag2_1), > diff --git a/drivers/net/intel/iavf/iavf_rxtx.c > b/drivers/net/intel/iavf/iavf_rxtx.c > index 7033a74610..887dcd1b2f 100644 > --- a/drivers/net/intel/iavf/iavf_rxtx.c > +++ b/drivers/net/intel/iavf/iavf_rxtx.c > @@ -1169,11 +1169,13 @@ iavf_flex_rxd_to_vlan_tci(struct rte_mbuf *mb, > > if (rte_le_to_cpu_16(rxdp->wb.status_error1) & > (1 << IAVF_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) { > - mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | > - RTE_MBUF_F_RX_QINQ | > - RTE_MBUF_F_RX_VLAN_STRIPPED | > - RTE_MBUF_F_RX_VLAN; > - mb->vlan_tci_outer = mb->vlan_tci; > + if ((mb->ol_flags & RTE_MBUF_F_RX_VLAN_STRIPPED) == 0) { > + mb->ol_flags |= RTE_MBUF_F_RX_VLAN | > RTE_MBUF_F_RX_VLAN_STRIPPED; > + } else { > + /* if two tags, move Tag1 to outer tag field */ > + mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | > RTE_MBUF_F_RX_QINQ; > + mb->vlan_tci_outer = mb->vlan_tci; > + } > mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.l2tag2_2nd); > PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: > %u", > rte_le_to_cpu_16(rxdp->wb.l2tag2_1st), > diff --git a/drivers/net/intel/ice/ice_rxtx.c b/drivers/net/intel/ice/ice_rxtx.c > index da508592aa..f965aab6ee 100644 > --- a/drivers/net/intel/ice/ice_rxtx.c > +++ b/drivers/net/intel/ice/ice_rxtx.c > @@ -1835,9 +1835,13 @@ ice_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile > union ci_rx_flex_desc *rxdp) > #ifndef RTE_NET_INTEL_USE_16BYTE_DESC > if (rte_le_to_cpu_16(rxdp->wb.status_error1) & > (1 << ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) { > - mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | > RTE_MBUF_F_RX_QINQ | > - RTE_MBUF_F_RX_VLAN_STRIPPED | > RTE_MBUF_F_RX_VLAN; > - mb->vlan_tci_outer = mb->vlan_tci; > + if ((mb->ol_flags & RTE_MBUF_F_RX_VLAN_STRIPPED) == 0) { > + mb->ol_flags |= RTE_MBUF_F_RX_VLAN | > RTE_MBUF_F_RX_VLAN_STRIPPED; > + } else { > + /* if two tags, move Tag1 to outer tag field */ > + mb->ol_flags |= RTE_MBUF_F_RX_QINQ_STRIPPED | > RTE_MBUF_F_RX_QINQ; > + mb->vlan_tci_outer = mb->vlan_tci; > + } > mb->vlan_tci = rte_le_to_cpu_16(rxdp->wb.l2tag2_2nd); > PMD_RX_LOG(DEBUG, "Descriptor l2tag2_1: %u, l2tag2_2: > %u", > rte_le_to_cpu_16(rxdp->wb.l2tag2_1st), > -- > 2.48.1 Acked-by: Ciara Loftus <ciara.loftus@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] net/intel: fix assumption about tag placement order 2025-08-07 14:31 ` Loftus, Ciara @ 2025-08-07 18:59 ` Bruce Richardson 0 siblings, 0 replies; 7+ messages in thread From: Bruce Richardson @ 2025-08-07 18:59 UTC (permalink / raw) To: Loftus, Ciara; +Cc: dev, stable On Thu, Aug 07, 2025 at 03:31:11PM +0100, Loftus, Ciara wrote: > > > > The specific placement of outer/inner VLAN tags in NIC descriptors > > is configurable. Therefore, remove the assumption that if the L2Tag2 > > field is filled in, that the L2Tag1 must also be. Instead, check the > > existing mbuf VLAN flags, and move tags and set flags as appropriate. > > This fixes an issue where, with QinQ packets with different Tag ethtypes > > (0x88a8 vs 0x8100), we get an mbuf reporting two valid tags, but only > > having had one tag stripped. > > > > Fixes: cc9d0456b870 ("i40e: support double vlan stripping and insertion") > > Fixes: 1e728b01120c ("net/iavf: rework Tx path") > > Fixes: e0dcf94a0d7f ("net/ice: support VLAN ops") > > Cc: stable@dpdk.org > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> > > --- > > drivers/net/intel/i40e/i40e_rxtx.c | 10 +++++++--- > > drivers/net/intel/iavf/iavf_rxtx.c | 12 +++++++----- > > drivers/net/intel/ice/ice_rxtx.c | 10 +++++++--- > > 3 files changed, 21 insertions(+), 11 deletions(-) > > Acked-by: Ciara Loftus <ciara.loftus@intel.com> > Applied to dpdk-next-net-intel. /Bruce ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-08-07 18:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-07-16 17:39 [PATCH] net/ice: fix assumption about tag placement order Bruce Richardson 2025-07-17 8:19 ` David Marchand 2025-07-17 8:32 ` Bruce Richardson 2025-07-17 8:36 ` David Marchand 2025-07-18 15:43 ` [PATCH v2] net/intel: " Bruce Richardson 2025-08-07 14:31 ` Loftus, Ciara 2025-08-07 18:59 ` Bruce Richardson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).