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

From: Guenter Roeck
Date: Mon Feb 12 2024 - 13:34:43 EST

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);
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.