diff mbox series

[iwl-net] selftests/net: Fix csum test for short packets

Message ID 20240821142409.958668-1-krzysztof.galazka@intel.com
State Changes Requested
Headers show
Series [iwl-net] selftests/net: Fix csum test for short packets | expand

Commit Message

Galazka, Krzysztof Aug. 21, 2024, 2:24 p.m. UTC
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)
Signed-off-by: Krzysztof Galazka <krzysztof.galazka@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 tools/testing/selftests/net/lib/csum.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Simon Horman Aug. 21, 2024, 5:43 p.m. UTC | #1
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>

...
Galazka, Krzysztof Aug. 27, 2024, 5:54 p.m. UTC | #2
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 mbox series

Patch

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)