On 4/25/26 10:40 AM, David Marchand wrote: > On Thu, 23 Apr 2026 at 13:25, Robin Jarry <[email protected]> wrote: >> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c >> index 458b4814a9c9..a871318b21c2 100644 >> --- a/lib/net/rte_net.c >> +++ b/lib/net/rte_net.c >> @@ -357,12 +357,14 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf *m, >> const struct rte_vlan_hdr *vh; >> struct rte_vlan_hdr vh_copy; >> >> + if (vlan_depth == 0) { >> + pkt_type = >> + proto == >> rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) ? >> + RTE_PTYPE_L2_ETHER_VLAN : >> + RTE_PTYPE_L2_ETHER_QINQ; >> + } > > This code is becoming too complex. > The original usecase with more than 2 stacked vlan is a bit strange, > but the max depth limit seems just arbitrary (why 8?). > We have clear boundaries, with the size of the packet (see below, > check on vh == NULL). > > The offending commit also allows stacking mpls on top of vlan, without > mentioning it. > I think we want this behavior, but still was it intended? > > In the end, reverting the previous fix then just advancing off and > breaking once proto is not a vlan/qinq type gives a much simpler fix > when compared to v25.11. > > (ignoring indent changes with -w) > > $ git diff -w v25.11 lib/net/rte_net.c > diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c > index c70b57fdc0..f58d699c83 100644 > --- a/lib/net/rte_net.c > +++ b/lib/net/rte_net.c > @@ -349,30 +349,28 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf *m, > if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_IPV4)) > goto l3; /* fast path if packet is IPv4 */ > > - if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN)) { > + if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) || > + proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_QINQ)) { > const struct rte_vlan_hdr *vh; > struct rte_vlan_hdr vh_copy; > > + if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN)) > pkt_type = RTE_PTYPE_L2_ETHER_VLAN; > + else > + pkt_type = RTE_PTYPE_L2_ETHER_QINQ; > + > + do { > vh = rte_pktmbuf_read(m, off, sizeof(*vh), &vh_copy); > if (unlikely(vh == NULL)) > return pkt_type; > off += sizeof(*vh); > hdr_lens->l2_len += sizeof(*vh); > proto = vh->eth_proto; > - } else if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_QINQ)) { > - const struct rte_vlan_hdr *vh; > - struct rte_vlan_hdr vh_copy; > + } while (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) || > + proto == > rte_cpu_to_be_16(RTE_ETHER_TYPE_QINQ)); > + } > > - pkt_type = RTE_PTYPE_L2_ETHER_QINQ; > - vh = rte_pktmbuf_read(m, off + sizeof(*vh), sizeof(*vh), > - &vh_copy); > - if (unlikely(vh == NULL)) > - return pkt_type; > - off += 2 * sizeof(*vh); > - hdr_lens->l2_len += 2 * sizeof(*vh); > - proto = vh->eth_proto; > - } else if ((proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) || > + if ((proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) || > (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLSM))) { > unsigned int i; > const struct rte_mpls_hdr *mh; > > > This is untested, but what do you think? > >
This version LGTM. Maybe we should consider returning NULL on invalid packets
