| Message ID | 1486199020-59977-1-git-send-email-shile.zhang@nokia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, Feb 04, 2017 at 05:03:40PM +0800, Shile Zhang wrote: > fix the missed point in Paul's patch: > "powerpc/64: Fix checksum folding in csum_tcpudp_nofold and > ip_fast_csum_nofold" > > Signed-off-by: Shile Zhang <shile.zhang@nokia.com> Acked-by: Paul Mackerras <paulus@ozlabs.org>
On Sat, 2017-02-04 at 09:03:40 UTC, Shile Zhang wrote: > fix the missed point in Paul's patch: > "powerpc/64: Fix checksum folding in csum_tcpudp_nofold and > ip_fast_csum_nofold" > > Signed-off-by: Shile Zhang <shile.zhang@nokia.com> > Acked-by: Paul Mackerras <paulus@ozlabs.org> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/6ad966d7303b70165228dba1ee8da1 cheers
Shile Zhang <shile.zhang@nokia.com> a écrit : > fix the missed point in Paul's patch: > "powerpc/64: Fix checksum folding in csum_tcpudp_nofold and > ip_fast_csum_nofold" > > Signed-off-by: Shile Zhang <shile.zhang@nokia.com> > --- > arch/powerpc/include/asm/checksum.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/include/asm/checksum.h > b/arch/powerpc/include/asm/checksum.h > index 5b1a6e3..430d038 100644 > --- a/arch/powerpc/include/asm/checksum.h > +++ b/arch/powerpc/include/asm/checksum.h > @@ -108,7 +108,7 @@ static inline __wsum csum_add(__wsum csum, __wsum addend) > > #ifdef __powerpc64__ > res += (__force u64)addend; > - return (__force __wsum)((u32)res + (res >> 32)); > + return (__force __wsum) from64to32(res); Did you encounter a bug due to that ? As far as i understand, csum and addend are 32 bits so can't exceed 0xffffffff Then their sum won't exceed 0x1fffffffe. So the sum of upper and lower part won't carry Christophe > #else > asm("addc %0,%0,%1;" > "addze %0,%0;" > -- > 2.6.2
On Tue, Mar 27, 2018 at 05:22:32PM +0200, LEROY Christophe wrote: > Shile Zhang <shile.zhang@nokia.com> a écrit : > > >fix the missed point in Paul's patch: > >"powerpc/64: Fix checksum folding in csum_tcpudp_nofold and > >ip_fast_csum_nofold" > > > >Signed-off-by: Shile Zhang <shile.zhang@nokia.com> > >--- > > arch/powerpc/include/asm/checksum.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/arch/powerpc/include/asm/checksum.h > >b/arch/powerpc/include/asm/checksum.h > >index 5b1a6e3..430d038 100644 > >--- a/arch/powerpc/include/asm/checksum.h > >+++ b/arch/powerpc/include/asm/checksum.h > >@@ -108,7 +108,7 @@ static inline __wsum csum_add(__wsum csum, __wsum addend) > > > > #ifdef __powerpc64__ > > res += (__force u64)addend; > >- return (__force __wsum)((u32)res + (res >> 32)); > >+ return (__force __wsum) from64to32(res); > > Did you encounter a bug due to that ? > As far as i understand, csum and addend are 32 bits so can't exceed 0xffffffff > Then their sum won't exceed 0x1fffffffe. So the sum of upper and lower part > won't carry If the sum of the two halves was 0x1fffffffe, then that previously got truncated to 32 bits and returned as 0xfffffffe, which is wrong - the result should be 0xffffffff. Paul.
Paul Mackerras <paulus@ozlabs.org> a écrit : > On Tue, Mar 27, 2018 at 05:22:32PM +0200, LEROY Christophe wrote: >> Shile Zhang <shile.zhang@nokia.com> a écrit : >> >> >fix the missed point in Paul's patch: >> >"powerpc/64: Fix checksum folding in csum_tcpudp_nofold and >> >ip_fast_csum_nofold" >> > >> >Signed-off-by: Shile Zhang <shile.zhang@nokia.com> >> >--- >> > arch/powerpc/include/asm/checksum.h | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> >diff --git a/arch/powerpc/include/asm/checksum.h >> >b/arch/powerpc/include/asm/checksum.h >> >index 5b1a6e3..430d038 100644 >> >--- a/arch/powerpc/include/asm/checksum.h >> >+++ b/arch/powerpc/include/asm/checksum.h >> >@@ -108,7 +108,7 @@ static inline __wsum csum_add(__wsum csum, >> __wsum addend) >> > >> > #ifdef __powerpc64__ >> > res += (__force u64)addend; >> >- return (__force __wsum)((u32)res + (res >> 32)); >> >+ return (__force __wsum) from64to32(res); >> >> Did you encounter a bug due to that ? >> As far as i understand, csum and addend are 32 bits so can't exceed >> 0xffffffff >> Then their sum won't exceed 0x1fffffffe. So the sum of upper and lower part >> won't carry > > If the sum of the two halves was 0x1fffffffe, then that previously got > truncated to 32 bits and returned as 0xfffffffe, which is wrong - the > result should be 0xffffffff. > It is the sum of the two arguments of csum_add() which can't exceed 0x1fffffffe So the sum of the two halves will be 0xffffffff which is the expected result. The issue would start with res = 0x1ffffffff, in that case the sum of the two halves would be 0x100000000 which would have been truncated to 0 whereas we expect the result to be 1 in that case. But in order to obtain res = 0x1ffffffff or higher, you have to sum at least three 32 bits numbers. You can't obtain such a value with a sum of two 32 bits numbers. Christophe
diff --git a/arch/powerpc/include/asm/checksum.h b/arch/powerpc/include/asm/checksum.h index 5b1a6e3..430d038 100644 --- a/arch/powerpc/include/asm/checksum.h +++ b/arch/powerpc/include/asm/checksum.h @@ -108,7 +108,7 @@ static inline __wsum csum_add(__wsum csum, __wsum addend) #ifdef __powerpc64__ res += (__force u64)addend; - return (__force __wsum)((u32)res + (res >> 32)); + return (__force __wsum) from64to32(res); #else asm("addc %0,%0,%1;" "addze %0,%0;"
fix the missed point in Paul's patch: "powerpc/64: Fix checksum folding in csum_tcpudp_nofold and ip_fast_csum_nofold" Signed-off-by: Shile Zhang <shile.zhang@nokia.com> --- arch/powerpc/include/asm/checksum.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)