| Message ID | 20240821142409.958668-1-krzysztof.galazka@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [iwl-net] selftests/net: Fix csum test for short packets | expand |
Krzysztof Galazka wrote: > For IPv4 and IPv6 packets shorter than minimum Ethernet > frame payload, recvmsg returns lenght including padding. nit: length > Use length from header for checksum verification to avoid > csum test failing on correct packets. > > Fixes: 1d0dc857b5d8 (selftests: drv-net: add checksum tests) > Signed-off-by: Krzysztof Galazka <krzysztof.galazka@intel.com> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> This is not Intel driver specific, so can be sent straight to net > --- > tools/testing/selftests/net/lib/csum.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tools/testing/selftests/net/lib/csum.c b/tools/testing/selftests/net/lib/csum.c > index b9f3fc3c3426..3dbaf2ecd59e 100644 > --- a/tools/testing/selftests/net/lib/csum.c > +++ b/tools/testing/selftests/net/lib/csum.c > @@ -658,6 +658,9 @@ static int recv_verify_packet_ipv4(void *nh, int len) > if (len < sizeof(*iph) || iph->protocol != proto) > return -1; > > + /* For short packets recvmsg returns length with padding, fix that */ > + len = ntohs(iph->tot_len); > + Are you running into this while running the standard testsuite in csum.py, or a specific custom invocation? Since the checksum is an L3 feature, trusting the L3 length field for this makes sense (as long as the packet wasn't truncated). > iph_addr_p = &iph->saddr; > if (proto == IPPROTO_TCP) > return recv_verify_packet_tcp(iph + 1, len - sizeof(*iph)); > @@ -673,6 +676,9 @@ static int recv_verify_packet_ipv6(void *nh, int len) > if (len < sizeof(*ip6h) || ip6h->nexthdr != proto) > return -1; > > + /* For short packets recvmsg returns length with padding, fix that */ > + len = sizeof(*ip6h) + ntohs(ip6h->payload_len); > + > iph_addr_p = &ip6h->saddr; > > if (proto == IPPROTO_TCP) > -- > 2.43.0 >
On Wed, Aug 21, 2024 at 04:24:09PM +0200, Krzysztof Galazka wrote: > For IPv4 and IPv6 packets shorter than minimum Ethernet > frame payload, recvmsg returns lenght including padding. > Use length from header for checksum verification to avoid > csum test failing on correct packets. > > Fixes: 1d0dc857b5d8 (selftests: drv-net: add checksum tests) nit: I think the correct format for the tag is Fixes: 1d0dc857b5d8 ("selftests: drv-net: add checksum tests") > Signed-off-by: Krzysztof Galazka <krzysztof.galazka@intel.com> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> ...
On 2024-08-21 16:32, Willem de Bruijn wrote: > Krzysztof Galazka wrote: >> For IPv4 and IPv6 packets shorter than minimum Ethernet >> frame payload, recvmsg returns lenght including padding. > > nit: length > >> Use length from header for checksum verification to avoid >> csum test failing on correct packets. >> >> Fixes: 1d0dc857b5d8 (selftests: drv-net: add checksum tests) >> Signed-off-by: Krzysztof Galazka <krzysztof.galazka@intel.com> >> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > This is not Intel driver specific, so can be sent straight to net > >> --- >> tools/testing/selftests/net/lib/csum.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/tools/testing/selftests/net/lib/csum.c b/tools/testing/selftests/net/lib/csum.c >> index b9f3fc3c3426..3dbaf2ecd59e 100644 >> --- a/tools/testing/selftests/net/lib/csum.c >> +++ b/tools/testing/selftests/net/lib/csum.c >> @@ -658,6 +658,9 @@ static int recv_verify_packet_ipv4(void *nh, int len) >> if (len < sizeof(*iph) || iph->protocol != proto) >> return -1; >> >> + /* For short packets recvmsg returns length with padding, fix that */ >> + len = ntohs(iph->tot_len); >> + > > Are you running into this while running the standard testsuite in > csum.py, or a specific custom invocation? It was with standard testsuite in csum.py. Not on every run but quite often. > > Since the checksum is an L3 feature, trusting the L3 length field for > this makes sense (as long as the packet wasn't truncated). Ah, I haven't thought about truncated packets. I'll add condition to update length returned by recvmsg only if it's greater then expected. Thanks, Krzysiek
diff --git a/tools/testing/selftests/net/lib/csum.c b/tools/testing/selftests/net/lib/csum.c index b9f3fc3c3426..3dbaf2ecd59e 100644 --- a/tools/testing/selftests/net/lib/csum.c +++ b/tools/testing/selftests/net/lib/csum.c @@ -658,6 +658,9 @@ static int recv_verify_packet_ipv4(void *nh, int len) if (len < sizeof(*iph) || iph->protocol != proto) return -1; + /* For short packets recvmsg returns length with padding, fix that */ + len = ntohs(iph->tot_len); + iph_addr_p = &iph->saddr; if (proto == IPPROTO_TCP) return recv_verify_packet_tcp(iph + 1, len - sizeof(*iph)); @@ -673,6 +676,9 @@ static int recv_verify_packet_ipv6(void *nh, int len) if (len < sizeof(*ip6h) || ip6h->nexthdr != proto) return -1; + /* For short packets recvmsg returns length with padding, fix that */ + len = sizeof(*ip6h) + ntohs(ip6h->payload_len); + iph_addr_p = &ip6h->saddr; if (proto == IPPROTO_TCP)