Re: [PATCH v10] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests

[Christophe Leroy]

Le 28/02/2024 à 08:59, Guenter Roeck a écrit :
> On 2/27/24 23:25, Christophe Leroy wrote:
> [ ... ]
>>>
>>> This test case is supposed to be as true to the "general case" as
>>> possible, so I have aligned the data along 14 + NET_IP_ALIGN. On ARM
>>> this will be a 16-byte boundary since NET_IP_ALIGN is 2. A driver that
>>> does not follow this may not be appropriately tested by this test case,
>>> but anyone is welcome to submit additional test cases that address this
>>> additional alignment concern.
>>
>> But then this test case is becoming less and less true to the "general
>> case" with this patch, whereas your initial implementation was almost
>> perfect as it was covering most cases, a lot more than what we get with
>> that patch applied.
>>
> NP with me if that is where people want to go. I'll simply disable checksum
> tests on all architectures which don't support unaligned accesses (so far
> it looks like that is only arm with thumb instructions, and possibly 
> nios2).
> I personally find that less desirable and would have preferred a second
> configurable set of tests for unaligned accesses, but I have no problem
> with it.
> 

Can you tell more about the symptoms you encounter on ARM ? According to 
Russell (ARM Maintainer) it should work, quoting him below:

	However, that may not always be the case for incoming packets, and what
	saves 32-bit Arm is the ability to do unaligned loads in later revisions
	of the architecture, or the alignment fault handler (slow) on older
	revisions.


NIOS2 doesn't have her how functions and relies on CONFIG_GENERIC_CSUM. 
Which means that ip_fast_csum() is from lib/checksum.c and is 
implemented using do_csum() which handles unaligned accesses by 
splitting accesses into smaller aligned accesses.
Therefore, ip_fast_csum() shouldn't be an issue for NIOS2.

Regarding csum_ipv6_magic(), NIOS2 uses the function in 
net/ipv6/ip6_checksum.c
This function dereferences saddr and daddr with 32-bits accesses: 
saddr->s6_addr32[0], is that a problem when saddr and daddr are not 
32-bits aligned ? Does it Oops ?