David Marchand, May 20, 2026 at 11:56:
> On Mon, 18 May 2026 at 15:27, Robin Jarry <[email protected]> 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 <[email protected]>
>> Signed-off-by: Robin Jarry <[email protected]>
>
> 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.
