On 2/12/24 16:32, Charlie Jenkins wrote:
On Mon, Feb 12, 2024 at 04:14:49PM -0800, Guenter Roeck wrote:
On 2/12/24 12:33, Charlie Jenkins wrote:
The test cases for ip_fast_csum and csum_ipv6_magic were failing on a
variety of architectures that are big endian or do not support
misalgined accesses. Both of these test cases are changed to support big
and little endian architectures.
The test for ip_fast_csum is changed to align the data along (14 +
NET_IP_ALIGN) bytes which is the alignment of an IP header. The test for
csum_ipv6_magic aligns the data using a struct. An extra padding field
is added to the struct to ensure that the size of the struct is the same
on all architectures (44 bytes).
Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
This thing really wants to annoy me. Now I get:
# test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:494
Expected ( u64)csum_result == ( u64)expected, but
( u64)csum_result == 46543 (0xb5cf)
( u64)expected == 46544 (0xb5d0)
not ok 5 test_csum_ipv6_magic
with the parisc64 tests. All other architectures / platforms work fine
after applying the various pending fixes. It looks like a carry gets
lost somewhere, but I have not been able to figure out where exactly
that happens. This only happens with the 64-bit hppa assembler code.
It turns out that hppa64 triggers an unaligned instruction trap if an
access is unaligned (where unaligned means not aligned to 64 bit).
It appears that this trap either wrongly drops or adds a carry flag
under some circumstances, and that this is causing the problem to be seen.
I can confirm this with:
diff --git a/arch/parisc/include/asm/checksum.h b/arch/parisc/include/asm/checksum.h
index e619e67440db..9bddf3231a28 100644
--- a/arch/parisc/include/asm/checksum.h
+++ b/arch/parisc/include/asm/checksum.h
@@ -128,9 +128,9 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
" ldd,ma 8(%1), %4\n" /* get 1st saddr word */
" ldd,ma 8(%2), %5\n" /* get 1st daddr word */
-" add %4, %0, %0\n"
" ldd,ma 8(%1), %6\n" /* 2nd saddr */
" ldd,ma 8(%2), %7\n" /* 2nd daddr */
+" add %4, %0, %0\n"
" add,dc %5, %0, %0\n"
" add,dc %6, %0, %0\n"
" add,dc %7, %0, %0\n"
or, simpler,
diff --git a/arch/parisc/include/asm/checksum.h b/arch/parisc/include/asm/checksum.h
index e619e67440db..7e2bb797dfe0 100644
--- a/arch/parisc/include/asm/checksum.h
+++ b/arch/parisc/include/asm/checksum.h
@@ -131,7 +131,7 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
" add %4, %0, %0\n"
" ldd,ma 8(%1), %6\n" /* 2nd saddr */
" ldd,ma 8(%2), %7\n" /* 2nd daddr */
-" add,dc %5, %0, %0\n"
+" add %5, %0, %0\n"
" add,dc %6, %0, %0\n"
" add,dc %7, %0, %0\n"
" add,dc %3, %0, %0\n" /* fold in proto+len | carry bit */
both of which make the problem disappear. Unless I am missing something, this
means that the unaligned access trap adds an additional carry flag.
Question now is if this is a bug in the qemu emulation or a bug in the unaligned
trap handler. FWIW, both changes above should be possible workarounds since the
first add operation should never overflow (it adds two 32 bit numbers into a 64
bit register). I'd rather fix the root cause, though.
Any thoughts ? I am adding the parisc maintainers and its e-mail list for
additional feedback.
Thanks,
Guenter
