Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests

[Guenter Roeck]

On 2/12/24 10:12, Al Viro wrote:
> On Mon, Feb 12, 2024 at 09:18:14AM -0800, Guenter Roeck wrote:
>
> > Almost. Turns out the csum parameter of csum_ipv6_magic() needs to be in
> > network byte order, and the length parameter needs to be in host byte order.
> > So instead of
> > 	data.len = data_ptr->len;
> > 	data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
> > it needs to be something like
> > 	data.len = ntohl(data_ptr->len);
> > 	data.csum = data_ptr->csum;
> >
> > Also, as you mentioned, either the returned checksum or the expected
> > checksum needs to be converted for the comparison because one is in
> > network byte order and the other in host byte order.
>
>          for (int i = 0; i < NUM_IPv6_TESTS; i++) {
> 		struct args {
> 			struct in6_addr saddr;
> 			struct in6_addr daddr;
> 			__be32 len;
> 			__wsum csum;
> 			unsigned char proto;
> 		} __packed data = (struct args *)(random_buf + i);
>                  CHECK_EQ(cpu_to_le16(expected_csum_ipv6_magic[i]),
>                           csum_ipv6_magic(data.saddr, data.daddr, ntohl(data.len),
> 					 data.proto, data.sum));
>          }
> and to hell with field-by-field copying.  __packed here will tell the compiler
> that alignment of the entire thing is 1 - the total size of fields is 41 bytes,
> so "no padding" translates into "can't even assume that address is even".

Except that the data pointer needs to be aligned because otherwise architectures
not supporting unaligned accesses will bail out (as observed on mps2-an385).
But then I am no longer sure if that is correct - maybe csum_ipv6_magic()
is supposed to be able to handle unaligned accesses. If so, maybe it would be
more appropriate to skip this test on the affected architectures.

Guenter