[v9] lib: checksum: Fix issues with checksum tests

[PATCH v9 0/2] lib: checksum: Fix issues with checksum tests

Posted by Charlie Jenkins 6 months, 2 weeks ago

The ip_fast_csum and csum_ipv6_magic tests did not work on all
architectures due to differences in endianness and misaligned access
support. Fix those issues by changing endianness of data and aligning
the data.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
Changes in v9:
- Revert back to v7, the changes to v8 were not needed
- Link to v8: https://lore.kernel.org/r/20240214-fix_sparse_errors_checksum_tests-v8-0-36b60e673593@rivosinc.com

Changes in v8:
- Change ipv6_magic test case to use memcpy to avoid misalignment
- Add Guenter's tested-by to patch 1 but not patch 2 since the later has
  changed
- Link to v7: https://lore.kernel.org/r/20240212-fix_sparse_errors_checksum_tests-v7-0-bbd3b8f64746@rivosinc.com

Changes in v7:
- Incorporate feedback for test_csum_ipv6_magic from Guenter and Al
- Link to v6: https://lore.kernel.org/r/20240207-fix_sparse_errors_checksum_tests-v6-0-4caa9629705b@rivosinc.com

Changes in v6:
- Fix for big endian (Guenter)
- Link to v5: https://lore.kernel.org/r/20240130-fix_sparse_errors_checksum_tests-v5-0-4d8a0a337e5e@rivosinc.com

Changes in v5:
- Add Guenter's tested-by
- CC Andrew Morton
- Link to v4: https://lore.kernel.org/r/20240124-fix_sparse_errors_checksum_tests-v4-0-bc2b8d23a35c@rivosinc.com

Changes in v4:
- Pad test values with zeroes (David)
- Link to v3: https://lore.kernel.org/r/20240123-fix_sparse_errors_checksum_tests-v3-0-efecc7f94297@rivosinc.com

Changes in v3:
- Don't read memory out of bounds
- Link to v2: https://lore.kernel.org/r/20240123-fix_sparse_errors_checksum_tests-v2-0-b306b6ce7da5@rivosinc.com

Changes in v2:
- Add additional patch to fix alignment issues
- Link to v1: https://lore.kernel.org/r/20240119-fix_sparse_errors_checksum_tests-v1-1-2d3df86d8d78@rivosinc.com

---
Charlie Jenkins (2):
      lib: checksum: Fix type casting in checksum kunits
      lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests

 lib/checksum_kunit.c | 396 ++++++++++++++++++---------------------------------
 1 file changed, 136 insertions(+), 260 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240119-fix_sparse_errors_checksum_tests-26b86b34d784
-- 
- Charlie

Re: [PATCH v9 0/2] lib: checksum: Fix issues with checksum tests

Posted by Palmer Dabbelt 6 months, 2 weeks ago

On Wed, 21 Feb 2024 18:55:48 PST (-0800), Charlie Jenkins wrote:
> The ip_fast_csum and csum_ipv6_magic tests did not work on all
> architectures due to differences in endianness and misaligned access
> support. Fix those issues by changing endianness of data and aligning
> the data.
>
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
> Changes in v9:
> - Revert back to v7, the changes to v8 were not needed
> - Link to v8: https://lore.kernel.org/r/20240214-fix_sparse_errors_checksum_tests-v8-0-36b60e673593@rivosinc.com
>
> Changes in v8:
> - Change ipv6_magic test case to use memcpy to avoid misalignment
> - Add Guenter's tested-by to patch 1 but not patch 2 since the later has
>   changed
> - Link to v7: https://lore.kernel.org/r/20240212-fix_sparse_errors_checksum_tests-v7-0-bbd3b8f64746@rivosinc.com
>
> Changes in v7:
> - Incorporate feedback for test_csum_ipv6_magic from Guenter and Al
> - Link to v6: https://lore.kernel.org/r/20240207-fix_sparse_errors_checksum_tests-v6-0-4caa9629705b@rivosinc.com
>
> Changes in v6:
> - Fix for big endian (Guenter)
> - Link to v5: https://lore.kernel.org/r/20240130-fix_sparse_errors_checksum_tests-v5-0-4d8a0a337e5e@rivosinc.com
>
> Changes in v5:
> - Add Guenter's tested-by
> - CC Andrew Morton
> - Link to v4: https://lore.kernel.org/r/20240124-fix_sparse_errors_checksum_tests-v4-0-bc2b8d23a35c@rivosinc.com
>
> Changes in v4:
> - Pad test values with zeroes (David)
> - Link to v3: https://lore.kernel.org/r/20240123-fix_sparse_errors_checksum_tests-v3-0-efecc7f94297@rivosinc.com
>
> Changes in v3:
> - Don't read memory out of bounds
> - Link to v2: https://lore.kernel.org/r/20240123-fix_sparse_errors_checksum_tests-v2-0-b306b6ce7da5@rivosinc.com
>
> Changes in v2:
> - Add additional patch to fix alignment issues
> - Link to v1: https://lore.kernel.org/r/20240119-fix_sparse_errors_checksum_tests-v1-1-2d3df86d8d78@rivosinc.com
>
> ---
> Charlie Jenkins (2):
>       lib: checksum: Fix type casting in checksum kunits
>       lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
>
>  lib/checksum_kunit.c | 396 ++++++++++++++++++---------------------------------
>  1 file changed, 136 insertions(+), 260 deletions(-)
> ---
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> change-id: 20240119-fix_sparse_errors_checksum_tests-26b86b34d784

I put a 

Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

on the v4, but looks like it got lost.  I'm happy to take this via the 
RISC-V tree, as that's how I merged the broken patches in the first 
place, but no big deal if someone else wants to pick it up.

It looks like the issues are all resolved and such, but there's been a 
long tail of them so I'm not 100% sure here...

Re: [PATCH v9 0/2] lib: checksum: Fix issues with checksum tests

Posted by Michael Ellerman 6 months, 2 weeks ago

Palmer Dabbelt <palmer@dabbelt.com> writes:
> On Wed, 21 Feb 2024 18:55:48 PST (-0800), Charlie Jenkins wrote:
>> The ip_fast_csum and csum_ipv6_magic tests did not work on all
>> architectures due to differences in endianness and misaligned access
>> support. Fix those issues by changing endianness of data and aligning
>> the data.
>>
>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
>> ---
>> Changes in v9:
>> - Revert back to v7, the changes to v8 were not needed
>> - Link to v8: https://lore.kernel.org/r/20240214-fix_sparse_errors_checksum_tests-v8-0-36b60e673593@rivosinc.com
...
>>
>> ---
>> Charlie Jenkins (2):
>>       lib: checksum: Fix type casting in checksum kunits
>>       lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
>>
>>  lib/checksum_kunit.c | 396 ++++++++++++++++++---------------------------------
>>  1 file changed, 136 insertions(+), 260 deletions(-)
>> ---
>> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
>> change-id: 20240119-fix_sparse_errors_checksum_tests-26b86b34d784
>
> I put a 
>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> on the v4, but looks like it got lost.  I'm happy to take this via the 
> RISC-V tree, as that's how I merged the broken patches in the first 
> place, but no big deal if someone else wants to pick it up.
>
> It looks like the issues are all resolved and such, but there's been a 
> long tail of them so I'm not 100% sure here...

I tested v9 on ppc32/64 BE, and it fixes the test failures and the
sparse errors, so LGTM.

Tested-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers

Re: [PATCH v9 0/2] lib: checksum: Fix issues with checksum tests

Posted by Christophe Leroy 6 months, 2 weeks ago


Le 23/02/2024 à 08:22, Michael Ellerman a écrit :
> Palmer Dabbelt <palmer@dabbelt.com> writes:
>> On Wed, 21 Feb 2024 18:55:48 PST (-0800), Charlie Jenkins wrote:
>>> The ip_fast_csum and csum_ipv6_magic tests did not work on all
>>> architectures due to differences in endianness and misaligned access
>>> support. Fix those issues by changing endianness of data and aligning
>>> the data.
>>>
>>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
>>> ---
>>> Changes in v9:
>>> - Revert back to v7, the changes to v8 were not needed
>>> - Link to v8: https://lore.kernel.org/r/20240214-fix_sparse_errors_checksum_tests-v8-0-36b60e673593@rivosinc.com
> ...
>>>
>>> ---
>>> Charlie Jenkins (2):
>>>        lib: checksum: Fix type casting in checksum kunits
>>>        lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
>>>
>>>   lib/checksum_kunit.c | 396 ++++++++++++++++++---------------------------------
>>>   1 file changed, 136 insertions(+), 260 deletions(-)
>>> ---
>>> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
>>> change-id: 20240119-fix_sparse_errors_checksum_tests-26b86b34d784
>>
>> I put a
>>
>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>>
>> on the v4, but looks like it got lost.  I'm happy to take this via the
>> RISC-V tree, as that's how I merged the broken patches in the first
>> place, but no big deal if someone else wants to pick it up.
>>
>> It looks like the issues are all resolved and such, but there's been a
>> long tail of them so I'm not 100% sure here...
> 
> I tested v9 on ppc32/64 BE, and it fixes the test failures and the
> sparse errors, so LGTM.
> 
> Tested-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

It magically works, but patch 1 is wrong and patch 2 subject is 
unrelated to the problem on powerpc.

This fix series needs rework.

Christophe

Re: [PATCH v9 0/2] lib: checksum: Fix issues with checksum tests

Posted by Charlie Jenkins 6 months, 2 weeks ago

On Fri, Feb 23, 2024 at 09:22:58AM +0000, Christophe Leroy wrote:
> 
> 
> Le 23/02/2024 à 08:22, Michael Ellerman a écrit :
> > Palmer Dabbelt <palmer@dabbelt.com> writes:
> >> On Wed, 21 Feb 2024 18:55:48 PST (-0800), Charlie Jenkins wrote:
> >>> The ip_fast_csum and csum_ipv6_magic tests did not work on all
> >>> architectures due to differences in endianness and misaligned access
> >>> support. Fix those issues by changing endianness of data and aligning
> >>> the data.
> >>>
> >>> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> >>> ---
> >>> Changes in v9:
> >>> - Revert back to v7, the changes to v8 were not needed
> >>> - Link to v8: https://lore.kernel.org/r/20240214-fix_sparse_errors_checksum_tests-v8-0-36b60e673593@rivosinc.com
> > ...
> >>>
> >>> ---
> >>> Charlie Jenkins (2):
> >>>        lib: checksum: Fix type casting in checksum kunits
> >>>        lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
> >>>
> >>>   lib/checksum_kunit.c | 396 ++++++++++++++++++---------------------------------
> >>>   1 file changed, 136 insertions(+), 260 deletions(-)
> >>> ---
> >>> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> >>> change-id: 20240119-fix_sparse_errors_checksum_tests-26b86b34d784
> >>
> >> I put a
> >>
> >> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> >>
> >> on the v4, but looks like it got lost.  I'm happy to take this via the
> >> RISC-V tree, as that's how I merged the broken patches in the first
> >> place, but no big deal if someone else wants to pick it up.
> >>
> >> It looks like the issues are all resolved and such, but there's been a
> >> long tail of them so I'm not 100% sure here...
> > 
> > I tested v9 on ppc32/64 BE, and it fixes the test failures and the
> > sparse errors, so LGTM.
> > 
> > Tested-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> 
> It magically works, but patch 1 is wrong and patch 2 subject is 
> unrelated to the problem on powerpc.
> 
> This fix series needs rework.
> 
> Christophe

I will rearrange the patches so that patch 1 fixes the endianness and
patch 2 addresses alignment. The second patch is subject to endianness
and alignment and the title only including alignment (but the
body including both) is the source of confusion.

- Charlie