On Sat, Feb 10, 2024 at 09:55:26AM -0800, Guenter Roeck wrote:
> IP checksum unit tests report the following error when run on hppa/hppa64.
>
> # test_ip_fast_csum: ASSERTION FAILED at lib/checksum_kunit.c:463
> Expected ( u64)csum_result == ( u64)expected, but
> ( u64)csum_result == 33754 (0x83da)
> ( u64)expected == 10946 (0x2ac2)
> not ok 4 test_ip_fast_csum
>
> 0x83da is the expected result if the IP header length is 20 bytes. 0x2ac2
> is the expected result if the IP header length is 24 bytes. The test fails
> with an IP header length of 24 bytes. It appears that ip_fast_csum()
> always returns the checksum for a 20-byte header, no matter how long
> the header actually is.
>
> Code analysis shows a suspicious assembler sequence in ip_fast_csum().
>
> " addc %0, %3, %0\n"
> "1: ldws,ma 4(%1), %3\n"
> " addib,< 0, %2, 1b\n" <---
>
> While my understanding of HPPA assembler is limited, it does not seem
> to make much sense to subtract 0 from a register and to expect the result
> to ever be negative. Subtracting 1 from the length parameter makes more
> sense. On top of that, the operation should be repeated if and only if
> the result is still > 0, so change the suspicious instruction to
> " addib,> -1, %2, 1b\n"
>
> The IP checksum unit test passes after this change.
>
> Cc: Charlie Jenkins <charlie@xxxxxxxxxxxx>
> Cc: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
> Prerequisite for this patch is
> https://lore.kernel.org/lkml/20240207-fix_sparse_errors_checksum_tests-v6-0-4caa9629705b@xxxxxxxxxxxx/
>
> No idea how that was not detected before. Maybe I am missing something.
>
> Note that test_csum_ipv6_magic still fails on 32-bit parisc systems
> after this patch has been applied. That is a different problem.
>
> arch/parisc/include/asm/checksum.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/parisc/include/asm/checksum.h b/arch/parisc/include/asm/checksum.h
> index 3c43baca7b39..f705e5dd1074 100644
> --- a/arch/parisc/include/asm/checksum.h
> +++ b/arch/parisc/include/asm/checksum.h
> @@ -40,7 +40,7 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
> " addc %0, %5, %0\n"
> " addc %0, %3, %0\n"
> "1: ldws,ma 4(%1), %3\n"
> -" addib,< 0, %2, 1b\n"
> +" addib,> -1, %2, 1b\n"
> " addc %0, %3, %0\n"
> "\n"
> " extru %0, 31, 16, %4\n"
> --
> 2.39.2
>
I got my parisc setup working and this does indeed fix the issue.
Tested-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
Reviewed-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
