Re: [PATCH] app/testpmd: fix L4 protocol retrieval from L3 header

[huangdengdui <huangdengdui@huawei.com>]

On 2025/9/16 0:39, Stephen Hemminger wrote:
> On Sat, 30 Aug 2025 09:29:13 +0800
> Dengdui Huang <huangdengdui@huawei.com> wrote:
> 
>> Currently, when retrieving the L4 protocol from the L3 header,
>> the case of IPv6 with extension headers is not handled correctly.
>> This patch fixes it.
>>
>> Fixes: 76730c7b9b5a ("app/testpmd: use packet type parsing API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>>  app/test-pmd/csumonly.c | 39 ++++++++++++++++-----------------------
>>  1 file changed, 16 insertions(+), 23 deletions(-)
>>
>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
>> index d355dbd8c0..a6e872e5a4 100644
>> --- a/app/test-pmd/csumonly.c
>> +++ b/app/test-pmd/csumonly.c
>> @@ -525,20 +525,8 @@ get_tunnel_ol_flags_by_ptype(uint32_t ptype)
>>  	}
>>  }
>>  
>> -static void
>> -parse_inner_l4_proto(void *outer_l3_hdr,
>> -			struct testpmd_offload_info *info)
>> -{
>> -	struct rte_ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>> -	struct rte_ipv6_hdr *ipv6_hdr = outer_l3_hdr;
>> -	if (info->ethertype == _htons(RTE_ETHER_TYPE_IPV4))
>> -		info->l4_proto = ipv4_hdr->next_proto_id;
>> -	else
>> -		info->l4_proto = ipv6_hdr->proto;
>> -}
>> -
>>  static uint8_t
>> -parse_l4_proto(const struct rte_mbuf *m, uint32_t off, uint32_t ptype)
>> +parse_l4_proto(const struct rte_mbuf *m, uint32_t off, uint32_t ptype, bool parse_inner)
>>  {
>>  	int frag = 0, ret;
>>  
>> @@ -557,16 +545,19 @@ parse_l4_proto(const struct rte_mbuf *m, uint32_t off, uint32_t ptype)
>>  		if (unlikely(ip6h == NULL))
>>  			return 0;
>>  
>> -		if ((ptype & RTE_PTYPE_INNER_L3_MASK) ==
>> -				RTE_PTYPE_INNER_L3_IPV6_EXT) {
>> -			ret = rte_net_skip_ip6_ext(ip6h->proto, m, &off, &frag);
>> -			if (ret < 0)
>> -				return 0;
>> -			return ret;
>> -		}
>> +		if (!parse_inner && (ptype & RTE_PTYPE_L3_MASK) != RTE_PTYPE_L3_IPV6_EXT)
>> +			return ip6h->proto;
>> +
>> +		if (parse_inner && (ptype & RTE_PTYPE_INNER_L3_MASK) != RTE_PTYPE_INNER_L3_IPV6_EXT)
>> +			return ip6h->proto;
>>  
>> -		return ip6h->proto;
>> +		off += sizeof(struct rte_ipv6_hdr);
>> +		ret = rte_net_skip_ip6_ext(ip6h->proto, m, &off, &frag);
>> +		if (ret < 0)
>> +			return 0;
>> +		return ret;
>>  	}
>> +
>>  	return 0;
>>  }
>>  
>> @@ -705,7 +696,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>  		info.l4_len = hdr_lens.l4_len;
>>  		info.ethertype = get_ethertype_by_ptype(eth_hdr,
>>  					ptype & RTE_PTYPE_L3_MASK);
>> -		info.l4_proto = parse_l4_proto(m, info.l2_len, ptype);
>> +		info.l4_proto = parse_l4_proto(m, info.l2_len, ptype, false);
> 
> 
> Setting parse_inner to false would cause l4_proto to be set to the extension
> header type (rather than the real L4 protocol). Was this change intentional?
> Looks like it would cause checksum to be computed incorrectly for simple
> case of IPV6 with extension header and TCP.
> 

Setting `parse_inner` to `false` does not result in `l4_proto` being assigned as an extended header type.
The purpose of `parse_inner` is solely to indicate whether the `parse_l4_proto` function is handling the outer packet header or the inner packet header.
Perhaps renaming the variable to `is_inner` would be more appropriate.