Re: [PATCH dpdk v4] net: fix VLAN packet type

["Robin Jarry" <rjarry@redhat.com>]

David Marchand, Apr 25, 2026 at 10:40:
> On Thu, 23 Apr 2026 at 13:25, Robin Jarry <rjarry@redhat.com> 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?

That looks correct. But you don't impose a limit in the number of VLANs?

I don't see any good reason to support more than 2 stacked tags.