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..).
> + 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;
--
David Marchand