Re: [PATCH] net: support VLAN stacking packet type parsing

[fengchengwen <fengchengwen@huawei.com>]

On 2025/7/4 18:18, Morten Brørup wrote:
>> From: Dengdui Huang [mailto:huangdengdui@huawei.com]
>> Sent: Thursday, 3 July 2025 11.30
>>
>> The current rte_net_get_ptype() only supports parsing packets with
>> one 0x8100 VLAN tag or two 0x88a8 VLAN tags. This patch extends it
>> to support parsing packets with two 0x8100 VLAN tags.
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>>  lib/net/rte_net.c | 34 ++++++++++++++++++++++------------
>>  lib/net/rte_net.h |  2 ++
>>  2 files changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/net/rte_net.c b/lib/net/rte_net.c
>> index 44fb6c0f51..fa8d023ca7 100644
>> --- a/lib/net/rte_net.c
>> +++ b/lib/net/rte_net.c
>> @@ -352,14 +352,19 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf
>> *m,
>>  	if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN)) {
>>  		const struct rte_vlan_hdr *vh;
>>  		struct rte_vlan_hdr vh_copy;
>> +		uint8_t vlan_num = 1;
>>
>>  		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;
>> +		while (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) &&
>> +		       vlan_num <= MAX_VLAN_STACKING_TAGS) {
>> +			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;
>> +			vlan_num++;
>> +		}
> 
> It's not that simple.
> 
> VLAN tags are not like MPLS labels.
> With the MPLS packet type, we expect a stack of labels.
> But with VLAN tagged packets, we expect exactly one VLAN tag at each outer/inner layer of headers. (Or no VLAN tag at the inner layer.)
> 
> This function already supports packets with 2 VLAN tags:
> A stack of 2 VLAN tags is currently detected as 1 outer VLAN tag, adjusting hdr_lens->l2_len accordingly, and 1 inner VLAN tag, adjusting hdr_lens->inner_l2_len accordingly.
> This behavior should not be changed.

I notice current the hdr_lens->inner_l2_len was added one comment: /* Outer_L4_len + ... + inner L2_len for tunneling pkt. */
It means the inner_l2_len was used in tunneling packet.

But one IPv4 packet carry two 8100 VLANs, it will parsed with hdr_lens->inner_l2_len was set by rte_net_get_ptype, it is unreasonable I think.

I notice Linux kernel vlan_get_protocol() [1], it will parse max 8 vlan (whether the TPID is 8100 or 88A8, [2])

I think we should parse like Linux kernel. If worried about breaking the ABI/API, consider adding a new function.

[1] https://elixir.bootlin.com/linux/v6.15.6/source/include/linux/if_vlan.h#L645
[2] https://elixir.bootlin.com/linux/v6.15.6/source/include/linux/if_vlan.h#L310

> 
> I don't have a firm idea about how to better represent packets with a stack of 3+ VLAN tags than what we do today, so suggestions are welcome.
> 
> Which use case is this patch addressing?
> Maybe information about the use case could guide us in some direction.
> 
>>  	} else if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_QINQ)) {
>>  		const struct rte_vlan_hdr *vh;
>>  		struct rte_vlan_hdr vh_copy;
>> @@ -504,15 +509,20 @@ uint32_t rte_net_get_ptype(const struct rte_mbuf
>> *m,
>>  	if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN)) {
>>  		const struct rte_vlan_hdr *vh;
>>  		struct rte_vlan_hdr vh_copy;
>> +		uint8_t vlan_num = 1;
>>
>>  		pkt_type &= ~RTE_PTYPE_INNER_L2_MASK;
>>  		pkt_type |= RTE_PTYPE_INNER_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->inner_l2_len += sizeof(*vh);
>> -		proto = vh->eth_proto;
>> +		while (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_VLAN) &&
>> +		       vlan_num <= MAX_VLAN_STACKING_TAGS) {
>> +			vh = rte_pktmbuf_read(m, off, sizeof(*vh), &vh_copy);
>> +			if (unlikely(vh == NULL))
>> +				return pkt_type;
>> +			off += sizeof(*vh);
>> +			hdr_lens->inner_l2_len += sizeof(*vh);
>> +			proto = vh->eth_proto;
>> +			vlan_num++;
>> +		}
>>  	} else if (proto == rte_cpu_to_be_16(RTE_ETHER_TYPE_QINQ)) {
>>  		const struct rte_vlan_hdr *vh;
>>  		struct rte_vlan_hdr vh_copy;
>> diff --git a/lib/net/rte_net.h b/lib/net/rte_net.h
>> index 65d724b84b..fdd55d57c8 100644
>> --- a/lib/net/rte_net.h
>> +++ b/lib/net/rte_net.h
>> @@ -13,6 +13,8 @@
>>  extern "C" {
>>  #endif
>>
>> +#define MAX_VLAN_STACKING_TAGS	2
>> +
>>  /**
>>   * Structure containing header lengths associated to a packet, filled
>>   * by rte_net_get_ptype().
>> --
>> 2.33.0
> 
>