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

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

[Krzysztof Galazka]

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(+)

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)
-- 
2.43.0

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

[Willem de Bruijn]

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
> 

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

[Galazka, Krzysztof]

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

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

[Simon Horman]

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>

...