> From: Ferruh Yigit [mailto:ferruh.yi...@amd.com] > Sent: Friday, 17 November 2023 10.30 > > On 11/17/2023 3:28 AM, Stephen Hemminger wrote: > > On Fri, 17 Nov 2023 00:50:16 +0000 > > Ferruh Yigit <ferruh.yi...@amd.com> wrote: > > > >>>> 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. > > > > > > Possibly an API version to change the args would work to fix. > > > > rte_ipv4_udptcp_cksum_mbuf() and rte_ipv6_udptcp_cksum_mbuf() are > inline > functions, unfortunately we can't version them. > > But those functions already gets IP header as parameter, can't we use > IP > header to get the payload size? If so this can be fixed without > updating > API.
If rte_ipv4_udptcp_cksum_mbuf() - or any other function in the DPDK Network Headers library - includes Ethernet padding (which may be non-zero) when calculating the TCP/UDP checksum of an IPv4 packet, it is a bug, and must be fixed there. Our test cases should use random padding to catch bugs like this. And I just realized that Ethernet padding may be added to any IP packet, so don't assume that this bug only applies to small packets.
