Re: [PATCH] net/rte_net: fix inner L2 length for tunneled Ethernet packets

[huangdengdui <huangdengdui@huawei.com>]

On 2025/8/1 21:23, Ivan Malov wrote:
> Hi,
> 
> On Fri, 1 Aug 2025, Khadem Ullah wrote:
> 
>> Hi Andrew,
>>
>> Thanks for your feedback.
>>
>> Please check mbuf packet types and the following test case:
>> https://doc.dpdk.org/dts-20.02/test_plans/uni_pkt_test_plan.html#test-case-vxlan-tunnel-packet-type-detect
>> sendp([Ether()/IP()/UDP()/Vxlan()/Ether()/IP(frag=5)/Raw('\0'*40)],
>> iface=txItf)
>>
>> (outer) L2 type: ETHER
>> (outer) L3 type: IPV4_EXT_UNKNOWN
>> (outer) L4 type: Unknown
>> Tunnel type: GRENAT
>> Inner L2 type: ETHER
>> Inner L3 type: IPV4_EXT_UNKNOWN
>> Inner L4 type: L4_FRAG
>>
>>
>> union {
>>        uint32_t packet_type; /**< L2/L3/L4 and tunnel information. */
>>        __extension__
>>        struct {
>>          uint8_t l2_type:4;   /**< (Outer) L2 type. */
>>          uint8_t l3_type:4;   /**< (Outer) L3 type. */
>>          uint8_t l4_type:4;   /**< (Outer) L4 type. */
>>          uint8_t tun_type:4;  /**< Tunnel type. */
>>          union {
>>            uint8_t inner_esp_next_proto;
>>            /**< ESP next protocol type, valid if
>>             * RTE_PTYPE_TUNNEL_ESP tunnel type is set
>>             * on both Tx and Rx.
>>             */
>>            __extension__
>>            struct {
>>              uint8_t inner_l2_type:4;
>>              /**< Inner L2 type. */
>>              uint8_t inner_l3_type:4;
>>              /**< Inner L3 type. */
>>            };
>>          };
>>          uint8_t inner_l4_type:4; /**< Inner L4 type. */
>>        };
>>      };
>>
>>
>> Based on the above, it seems that inner_l2_len have to the length of Ether.
>> Ther might need to be some correspondent between both fields to potray the same information.
>> Or, the inner_l2_type and inner_l2_len are completly different ?
> 
> On the one hand, there is mbuf structure, which has got no 'tunnel_len' field.
> It has 'l2_len' field [1] with a comment saying that for a tunnel packet, it
> includes some extra terms apart from just 'inner L2 header size'. This use of
> the 'l2_len' mbuf field is absolutely legitimate, and PMDs confirm this stance.
> 
> On the other hand, there is 'rte_net_hdr_lens' structure [2], which does have a
> separate 'tunnel_len' field and, in general, has got slightly different naming.
> And the 'inner_l2_len' field has a comment that looks almost like a copy-paste
> from the mbuf structure. So does 'inner_l2_len' really need to include extra
> terms, given the presence of a dedicated 'tunnel_len' field? Is it at all
> correct or could it have been overlooked? One should take a closer look.

When processing a tunnel packet, it is preferable for the inner_l2_len in rte_net_hdr_lens and the l2_len in the mbuf to have consistent meanings, as these two fields are typically used together.

Additionally, I think it would be better to add the tunnel_len field to tx_offload in the mbuf[1].
Moreover, inner_l2_len should only include the length of the Ethernet header.

[1] https://elixir.bootlin.com/dpdk/v25.07/source/lib/mbuf/rte_mbuf_core.h#L656


> 
> [1] https://github.com/DPDK/dpdk/blob/b222395561638f89562e4ef42e1eabf2d6db43dd/lib/mbuf/rte_mbuf_core.h#L628
> [2] https://github.com/DPDK/dpdk/blob/b222395561638f89562e4ef42e1eabf2d6db43dd/lib/net/rte_net.h#L22
> 
> Thank you.
> 
>>
>> Best Regards,
>> Khadem
>>