Messages in this thread | | | Date | Tue, 27 Feb 2024 21:14:26 +0100 | Subject | Re: [PATCH] parisc: More csum_ipv6_magic fixes | From | Helge Deller <> |
| |
On 2/27/24 19:53, Guenter Roeck wrote: > IPv6 checksum tests with unaligned addresses resulted in unexpected > failures. > > Expected expected == csum_result, but > expected == 46591 (0xb5ff) > csum_result == 46381 (0xb52d) > with alignment offset 1 > > Oddly enough, the problem disappeared after adding test code into > the beginning of csum_ipv6_magic(). > > As it turns out, the 'sum' parameter of csum_ipv6_magic() is declared as > __wsum, which is a 32-bit variable type. However, it is treated as 64-bit > variable in the assembler code.
Nice catch! That kind of bugs is actually the reason why I start to prefer C-code over inline assembly, even if C might perform slower.
I've applied that patch to the parisc git tree, but do you think you can come up with a better patch title, e.g. "strip upper 32bits of sum in csum_ipv6_magic()" ?
Other than that you may add Acked-by: Helge Deller <deller@gmx.de>
Helge
> Tests showed that the upper 32 bit of > the register used to pass the variable are _not_ cleared when entering > the function. This can result in checksum calculation errors. > > Clearing the upper 32 bit of 'sum' as first operation in the assembler > code fixes the problem. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > Maybe there is a way to do this without additional instruction, but if so > I have not been able to find it. > > arch/parisc/include/asm/checksum.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/parisc/include/asm/checksum.h b/arch/parisc/include/asm/checksum.h > index c949aa20fa16..2aceebcd695c 100644 > --- a/arch/parisc/include/asm/checksum.h > +++ b/arch/parisc/include/asm/checksum.h > @@ -126,6 +126,7 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr, > ** Try to keep 4 registers with "live" values ahead of the ALU. > */ > > +" depdi 0, 31, 32, %0\n"/* clear upper half of incoming checksum */ > " 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"
| |