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

[Charlie Jenkins <charlie@rivosinc.com>]

On Fri, Mar 01, 2024 at 07:17:38AM +0000, Christophe Leroy wrote:
> +CC netdev ARM Russell
> 
> Le 29/02/2024 à 23:46, Charlie Jenkins a écrit :
> > The test cases for ip_fast_csum and csum_ipv6_magic were not properly
> > aligning the IP header, which were causing failures on architectures
> > that do not support misaligned accesses like some ARM platforms. To
> > solve this, align the data along (14 + NET_IP_ALIGN) bytes which is the
> > standard alignment of an IP header and must be supported by the
> > architecture.
> 
> In your description, please provide more details on platforms that have 
> a problem, what the problem is exactly (Failed calculation, slowliness, 
> kernel Oops, panic, ....) on each platform.
> 
> And please copy maintainers and lists of platforms your are specifically 
> addressing with this change. And as this is network related, netdev list 
> should have been copied as well.
> 
> I still think that your patch is not the good approach, it looks like 
> you are ignoring all the discussion. Below is a quote of what Geert said 
> and I fully agree with that:
> 
> 	IMHO the tests should validate the expected functionality.  If a test
> 	fails, either functionality is missing or behaves wrong, or the test
> 	is wrong.
> 
> 	What is the point of writing tests for a core functionality like network
> 	checksumming that do not match the expected functionality?
> 
> 
> So we all agree that there is something to fix, because today's test 
> does odd-address accesses which is unexpected for those functions, but 
> 2-byte alignments should be supported hence tested by the test. Limiting 
> the test to a 16-bytes alignment deeply reduces the usefullness of the test.
> 

Maybe I am lost in the conversations. This isn't limited to 16-bytes
alignment? It aligns along 14 + NET_IP_ALIGN. That is 16 on some
platforms and 14 on platforms where unaligned accesses are desired.
These functions are expected to be called with this offset. Testing with
any other alignment is not the expected behavior. These tests are
testing the expected functionality.

- Charlie

> Christophe