Re: [PATCH dpdk v5 2/5] net: support multiple stacked VLAN tags

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

David Marchand, May 20, 2026 at 11:56:
> On Mon, 18 May 2026 at 15:27, Robin Jarry <rjarry@redhat.com> wrote:
>>
>> The VLAN and QinQ code paths in rte_net_get_ptype handle at most two
>> tags with duplicated logic. Replace them with a single loop that
>> consumes all consecutive VLAN/QinQ headers regardless of depth.
>>
>> Bugzilla ID: 1941
>> Suggested-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Robin Jarry <rjarry@redhat.com>
>
> For the record, I am still skeptical about the usecase behind this change.
>
>
>> ---
>>  lib/net/rte_net.c | 35 ++++++++++++++++-------------------
>>  1 file changed, 16 insertions(+), 19 deletions(-)
>>
>> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
>> index c70b57fdc0f8..d3cded961fb5 100644
>> --- a/lib/net/rte_net.c
>> +++ b/lib/net/rte_net.c
>> @@ -349,29 +349,26 @@ 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;
>>
>> -               pkt_type = RTE_PTYPE_L2_ETHER_VLAN;
>> -               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;
>> +               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;
>
> Kevin noted that it is weird to report back some packet type when the
> packet is malformed.
> Maybe return RTE_PTYPE_UNKNOWN here so that the application is forced
> to validate the packet? (it should already be doing it, in any
> case..).

If we do this, we need to fix it in the entire function. There are
several other places where the "current" value of pkt_type is returned
on error.

>
>
>> +                       off += sizeof(*vh);
>> +                       hdr_lens->l2_len += sizeof(*vh);
>> +                       proto = vh->eth_proto;
>> +               } 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)) ||
>>                 (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLSM))) {
>>                 unsigned int i;


-- 
Robin

> Avoid contact with skin.