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

[Charlie Jenkins <charlie@rivosinc.com>]

On Wed, Feb 28, 2024 at 07:40:43AM -0800, Guenter Roeck wrote:
> On 2/28/24 02:15, Geert Uytterhoeven wrote:
> > CC testing
> > 
> > On Wed, Feb 28, 2024 at 8:59 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > > 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.
> > 
> > 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?
> > 
> 
> Tough one. I can't enable CONFIG_NET_TEST on nios2, parisc, and arm with THUMB
> enabled due to crashes or hangs in gso tests. I accept that. Downside is that I
> have to disable CONFIG_NET_TEST on those architectures/platforms entirely,
> meaning a whole class of tests are missing for those architectures. I would
> prefer to have a configuration option such as CONFIG_NET_GSO_TEST to let me
> disable the problematic tests for the affected platforms so I can run all
> the other network unit tests. Yes, obviously something is wrong either with
> the affected tests or with the implementation of the tested functionality
> on the affected systems, but that could be handled separately if a separate
> configuration option existed, and new regressions in other tests on the affected
> architectures could be identified as they happen.

I think I got confused here, is this an issue with the tests included in
this patch or is it unrelated?

- Charlie

> 
> This case is similar. I'd prefer to have a separate configuration option,
> say, CONFIG_CHECKSUM_MISALIGNED_KUNIT, which I can disable to be able to
> run the common checksum tests on platforms / architectures which don't
> support unaligned accesses.
> 
> However, as I said, if the community wants to take a harsh stance, I have no
> problem with just disabling groups of tests entirely on platforms which have
> a problem with part of it.
> 
> Guenter
>