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

[Charlie Jenkins <charlie@rivosinc.com>]

On Tue, Feb 27, 2024 at 11:31:01AM -0800, Guenter Roeck wrote:
> On 2/27/24 09:54, Charlie Jenkins wrote:
> 
> > > It's been suggested during the discussion that alignment tests should be
> > > added later in a follow-up patch. So for the time being I'm trying to
> > > find a compromise and get the existing tests working on all platforms
> > > but with a smaller alignment than the 16-bytes alignment brought by
> > > Charlie's v10 patch. And a 4 bytes alignment seemed to me to be a good
> > > compromise for this fix. The idea is also to make the fix as minimal as
> > > possible, unlike Charlie's patch that is churning up the tests quite
> > > heavily.
> > 
> > Do you have a list of platforms this is failing on? I haven't seen any
> > reports that haven't been fixed.
> > 
> 
> This is what I carry locally on top of v6.8-rc6:
> 
> 097b149e4acb parisc: More csum_ipv6_magic fixes
> 15bf67a115eb kunit: Fix again checksum tests on big endian CPUs
> bebe776d36ea parisc: Fix csum_ipv6_magic on 64-bit systems
> 523208f03063 parisc: Fix csum_ipv6_magic on 32-bit systems
> a9dda1971c72 parisc: Fix ip_fast_csum
> 2ad0a6850b64 Revert "sh: Handle calling csum_partial with misaligned data"
> 7113cc414860 lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
> 
> I also have
> 0dd01a364cb7 lib: checksum: Add some corner cases to IPv6 checksum tests
> e767cce6598b lib: checksum: Add tests for unaligned IPv6 addresses
> 
> which I may submit or not depending on the outcome of this discussion.
> 
> In other words, parisc and sh4 are currently known to be broken in the
> upstream kernel, with fixes pending. On top of that, arm:mps2-an385
> (probably all arm:nommu systems) crashes hard if csum_ipv6_magic()
> is called with an unaligned address.
> 
> This is the "known" list of failures. I don't currently run kunit tests
> on nios2 or riscv32, for example, nor on any architectures with no qemu
> support.
> 
> On a side note, most architectures don't handle "len + proto" overflows.
> While 'len' is a 32-bit parameter, IPv6 only allows for a 16-bit length
> field. Many implementations of csum_ipv6_magic() specifically do
> not handle such overflows because that would be pointless and require
> extra code for no good reason. The current test code doesn't generate
> such overflows, but its 'len' parameter is almost always larger than
> 16 bit and thus not realistic. Maybe it would make sense to limit
> the range of 'len' to 16 bit when calling csum_ipv6_magic().

Thank you for the suggestion, I can limit len to 16-bit.

- Charlie

> 
> Thanks,
> Guenter
>