Re: [PATCH] app/test-pmd: fix L4 checksum with padding data

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 11/16/2023 10:58 PM, Stephen Hemminger wrote:
> On Thu, 2 Nov 2023 19:20:07 +0000
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> 
>> On 8/4/2023 9:28 AM, Kaiwen Deng wrote:
>>> IEEE 802 packets may have a minimum size limit. The data fields
>>> should be padded when necessary. In some cases, the padding data
>>> is not zero. Testpmd does not trim these IP packets to the true
>>> length of the frame, so errors will occur when calculating TCP
>>> or UDP checksum.
>>>   
>>
>> Hi Kaiwen,
>>
>> I am trying to understand the problem, what is the testcase that has
>> checksum error?
>>
>> Are the received mbuf data_len & pkt_len wrong? Instead of trying to fix
>> the mbuf during forwarding, can we fix where packet generated?
> 
> The root cause is that get_udptcp_cksum_mbuf is using m->pkt_len
> which maybe larger than the actual data. The real issue is there and
> in rte_ip.h checksum code. The correct fix would be to use l3_len instead.
> 

I see, you are right.

In 'rte_ipv4_udptcp_cksum_mbuf()',
as payload length "mbuf->pkt_len - l4_off" is used, which includes
padding and if padding is not zero it will end up producing wrong checksum.


I agree using 'l3_len' instead is correct fix.

But this requires ABI/API change,
plus do we have any reason to keep the padding, discarding it as this
patch does is also simpler alternative.


Other alternative can be to zero the padding bytes. I guess standard
doesn't enforce them to be zero, but we can do this to remove its impact
on checksum calculation.


@Kaiwen, did you able to test this with HW offload, what is the behavior
of the HW, does is remove padding bytes?


> It also looks like test-pmd is not validating the IP header.
> Both parse_ipv4() and parse_ipv6() should check if packet was truncated.
> Same for both UDP and TCP lengths.
>