lkml.org 
[lkml]   [2024]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] parisc: More csum_ipv6_magic fixes
From
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"


\
 
 \ /
  Last update: 2024-05-27 15:25    [W:0.077 / U:1.384 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site