| Message ID | 73df3a9e95c2179119398ad1b4c84cdacbd8dfb6.1708684443.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3d6423ef8d517e8924bec3f22c40285a90d652f3 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] kunit: Fix again checksum tests on big endian CPUs | expand |
On Fri, Feb 23, 2024 at 11:41:52AM +0100, Christophe Leroy wrote: > Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") > fixed endianness issues with kunit checksum tests, but then > commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and > ip_fast_csum") introduced new issues on big endian CPUs. Those issues > are once again reflected by the warnings reported by sparse. > > So, fix them with the same approach, perform proper conversion in > order to support both little and big endian CPUs. Once the conversions > are properly done and the right types used, the sparse warnings are > cleared as well. > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > lib/checksum_kunit.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c > index 225bb7701460..bf70850035c7 100644 > --- a/lib/checksum_kunit.c > +++ b/lib/checksum_kunit.c > @@ -215,7 +215,7 @@ static const u32 init_sums_no_overflow[] = { > 0xffff0000, 0xfffffffb, > }; > > -static const __sum16 expected_csum_ipv6_magic[] = { > +static const u16 expected_csum_ipv6_magic[] = { > 0x18d4, 0x3085, 0x2e4b, 0xd9f4, 0xbdc8, 0x78f, 0x1034, 0x8422, 0x6fc0, > 0xd2f6, 0xbeb5, 0x9d3, 0x7e2a, 0x312e, 0x778e, 0xc1bb, 0x7cf2, 0x9d1e, > 0xca21, 0xf3ff, 0x7569, 0xb02e, 0xca86, 0x7e76, 0x4539, 0x45e3, 0xf28d, > @@ -241,7 +241,7 @@ static const __sum16 expected_csum_ipv6_magic[] = { > 0x3845, 0x1014 > }; > > -static const __sum16 expected_fast_csum[] = { > +static const u16 expected_fast_csum[] = { > 0xda83, 0x45da, 0x4f46, 0x4e4f, 0x34e, 0xe902, 0xa5e9, 0x87a5, 0x7187, > 0x5671, 0xf556, 0x6df5, 0x816d, 0x8f81, 0xbb8f, 0xfbba, 0x5afb, 0xbe5a, > 0xedbe, 0xabee, 0x6aac, 0xe6b, 0xea0d, 0x67ea, 0x7e68, 0x8a7e, 0x6f8a, > @@ -577,7 +577,8 @@ static void test_csum_no_carry_inputs(struct kunit *test) > > static void test_ip_fast_csum(struct kunit *test) > { > - __sum16 csum_result, expected; > + __sum16 csum_result; > + u16 expected; > > for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) { > for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) { > @@ -586,7 +587,7 @@ static void test_ip_fast_csum(struct kunit *test) > expected_fast_csum[(len - IPv4_MIN_WORDS) * > NUM_IP_FAST_CSUM_TESTS + > index]; > - CHECK_EQ(expected, csum_result); > + CHECK_EQ(to_sum16(expected), csum_result); > } > } > } > @@ -598,7 +599,7 @@ static void test_csum_ipv6_magic(struct kunit *test) > const struct in6_addr *daddr; > unsigned int len; > unsigned char proto; > - unsigned int csum; > + __wsum csum; > > const int daddr_offset = sizeof(struct in6_addr); > const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr); > @@ -611,10 +612,10 @@ static void test_csum_ipv6_magic(struct kunit *test) > saddr = (const struct in6_addr *)(random_buf + i); > daddr = (const struct in6_addr *)(random_buf + i + > daddr_offset); > - len = *(unsigned int *)(random_buf + i + len_offset); > + len = le32_to_cpu(*(__le32 *)(random_buf + i + len_offset)); > proto = *(random_buf + i + proto_offset); > - csum = *(unsigned int *)(random_buf + i + csum_offset); > - CHECK_EQ(expected_csum_ipv6_magic[i], > + csum = *(__wsum *)(random_buf + i + csum_offset); > + CHECK_EQ(to_sum16(expected_csum_ipv6_magic[i]), > csum_ipv6_magic(saddr, daddr, len, proto, csum)); > } > #endif /* !CONFIG_NET */ > -- > 2.43.0 > There is no need to duplicate efforts here. This has already been resolved by https://lore.kernel.org/lkml/20240221-fix_sparse_errors_checksum_tests-v9-2-bff4d73ab9d1@rivosinc.com/. - Charlie
Le 23/02/2024 à 18:57, Charlie Jenkins a écrit : > On Fri, Feb 23, 2024 at 11:41:52AM +0100, Christophe Leroy wrote: >> Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") >> fixed endianness issues with kunit checksum tests, but then >> commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and >> ip_fast_csum") introduced new issues on big endian CPUs. Those issues >> are once again reflected by the warnings reported by sparse. >> >> So, fix them with the same approach, perform proper conversion in >> order to support both little and big endian CPUs. Once the conversions >> are properly done and the right types used, the sparse warnings are >> cleared as well. >> >> Reported-by: Erhard Furtner <erhard_f@mailbox.org> >> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> lib/checksum_kunit.c | 17 +++++++++-------- >> 1 file changed, 9 insertions(+), 8 deletions(-) >> >> diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c >> index 225bb7701460..bf70850035c7 100644 >> --- a/lib/checksum_kunit.c >> +++ b/lib/checksum_kunit.c >> @@ -215,7 +215,7 @@ static const u32 init_sums_no_overflow[] = { >> 0xffff0000, 0xfffffffb, >> }; >> >> -static const __sum16 expected_csum_ipv6_magic[] = { >> +static const u16 expected_csum_ipv6_magic[] = { >> 0x18d4, 0x3085, 0x2e4b, 0xd9f4, 0xbdc8, 0x78f, 0x1034, 0x8422, 0x6fc0, >> 0xd2f6, 0xbeb5, 0x9d3, 0x7e2a, 0x312e, 0x778e, 0xc1bb, 0x7cf2, 0x9d1e, >> 0xca21, 0xf3ff, 0x7569, 0xb02e, 0xca86, 0x7e76, 0x4539, 0x45e3, 0xf28d, >> @@ -241,7 +241,7 @@ static const __sum16 expected_csum_ipv6_magic[] = { >> 0x3845, 0x1014 >> }; >> >> -static const __sum16 expected_fast_csum[] = { >> +static const u16 expected_fast_csum[] = { >> 0xda83, 0x45da, 0x4f46, 0x4e4f, 0x34e, 0xe902, 0xa5e9, 0x87a5, 0x7187, >> 0x5671, 0xf556, 0x6df5, 0x816d, 0x8f81, 0xbb8f, 0xfbba, 0x5afb, 0xbe5a, >> 0xedbe, 0xabee, 0x6aac, 0xe6b, 0xea0d, 0x67ea, 0x7e68, 0x8a7e, 0x6f8a, >> @@ -577,7 +577,8 @@ static void test_csum_no_carry_inputs(struct kunit *test) >> >> static void test_ip_fast_csum(struct kunit *test) >> { >> - __sum16 csum_result, expected; >> + __sum16 csum_result; >> + u16 expected; >> >> for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) { >> for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) { >> @@ -586,7 +587,7 @@ static void test_ip_fast_csum(struct kunit *test) >> expected_fast_csum[(len - IPv4_MIN_WORDS) * >> NUM_IP_FAST_CSUM_TESTS + >> index]; >> - CHECK_EQ(expected, csum_result); >> + CHECK_EQ(to_sum16(expected), csum_result); >> } >> } >> } >> @@ -598,7 +599,7 @@ static void test_csum_ipv6_magic(struct kunit *test) >> const struct in6_addr *daddr; >> unsigned int len; >> unsigned char proto; >> - unsigned int csum; >> + __wsum csum; >> >> const int daddr_offset = sizeof(struct in6_addr); >> const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr); >> @@ -611,10 +612,10 @@ static void test_csum_ipv6_magic(struct kunit *test) >> saddr = (const struct in6_addr *)(random_buf + i); >> daddr = (const struct in6_addr *)(random_buf + i + >> daddr_offset); >> - len = *(unsigned int *)(random_buf + i + len_offset); >> + len = le32_to_cpu(*(__le32 *)(random_buf + i + len_offset)); >> proto = *(random_buf + i + proto_offset); >> - csum = *(unsigned int *)(random_buf + i + csum_offset); >> - CHECK_EQ(expected_csum_ipv6_magic[i], >> + csum = *(__wsum *)(random_buf + i + csum_offset); >> + CHECK_EQ(to_sum16(expected_csum_ipv6_magic[i]), >> csum_ipv6_magic(saddr, daddr, len, proto, csum)); >> } >> #endif /* !CONFIG_NET */ >> -- >> 2.43.0 >> > > There is no need to duplicate efforts here. This has already been > resolved by > https://lore.kernel.org/lkml/20240221-fix_sparse_errors_checksum_tests-v9-2-bff4d73ab9d1@rivosinc.com/. > The idea here is to provide a fix which is similar to the one done previously and that uses the same approach and reuses the same helpers. This is to keep the code homogeneous. Christophe
On Fri, Feb 23, 2024 at 06:15:16PM +0000, Christophe Leroy wrote: > > > Le 23/02/2024 à 18:57, Charlie Jenkins a écrit : > > On Fri, Feb 23, 2024 at 11:41:52AM +0100, Christophe Leroy wrote: > >> Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") > >> fixed endianness issues with kunit checksum tests, but then > >> commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and > >> ip_fast_csum") introduced new issues on big endian CPUs. Those issues > >> are once again reflected by the warnings reported by sparse. > >> > >> So, fix them with the same approach, perform proper conversion in > >> order to support both little and big endian CPUs. Once the conversions > >> are properly done and the right types used, the sparse warnings are > >> cleared as well. > >> > >> Reported-by: Erhard Furtner <erhard_f@mailbox.org> > >> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") > >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > >> --- > >> lib/checksum_kunit.c | 17 +++++++++-------- > >> 1 file changed, 9 insertions(+), 8 deletions(-) > >> > >> diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c > >> index 225bb7701460..bf70850035c7 100644 > >> --- a/lib/checksum_kunit.c > >> +++ b/lib/checksum_kunit.c > >> @@ -215,7 +215,7 @@ static const u32 init_sums_no_overflow[] = { > >> 0xffff0000, 0xfffffffb, > >> }; > >> > >> -static const __sum16 expected_csum_ipv6_magic[] = { > >> +static const u16 expected_csum_ipv6_magic[] = { > >> 0x18d4, 0x3085, 0x2e4b, 0xd9f4, 0xbdc8, 0x78f, 0x1034, 0x8422, 0x6fc0, > >> 0xd2f6, 0xbeb5, 0x9d3, 0x7e2a, 0x312e, 0x778e, 0xc1bb, 0x7cf2, 0x9d1e, > >> 0xca21, 0xf3ff, 0x7569, 0xb02e, 0xca86, 0x7e76, 0x4539, 0x45e3, 0xf28d, > >> @@ -241,7 +241,7 @@ static const __sum16 expected_csum_ipv6_magic[] = { > >> 0x3845, 0x1014 > >> }; > >> > >> -static const __sum16 expected_fast_csum[] = { > >> +static const u16 expected_fast_csum[] = { > >> 0xda83, 0x45da, 0x4f46, 0x4e4f, 0x34e, 0xe902, 0xa5e9, 0x87a5, 0x7187, > >> 0x5671, 0xf556, 0x6df5, 0x816d, 0x8f81, 0xbb8f, 0xfbba, 0x5afb, 0xbe5a, > >> 0xedbe, 0xabee, 0x6aac, 0xe6b, 0xea0d, 0x67ea, 0x7e68, 0x8a7e, 0x6f8a, > >> @@ -577,7 +577,8 @@ static void test_csum_no_carry_inputs(struct kunit *test) > >> > >> static void test_ip_fast_csum(struct kunit *test) > >> { > >> - __sum16 csum_result, expected; > >> + __sum16 csum_result; > >> + u16 expected; > >> > >> for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) { > >> for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) { > >> @@ -586,7 +587,7 @@ static void test_ip_fast_csum(struct kunit *test) > >> expected_fast_csum[(len - IPv4_MIN_WORDS) * > >> NUM_IP_FAST_CSUM_TESTS + > >> index]; > >> - CHECK_EQ(expected, csum_result); > >> + CHECK_EQ(to_sum16(expected), csum_result); > >> } > >> } > >> } > >> @@ -598,7 +599,7 @@ static void test_csum_ipv6_magic(struct kunit *test) > >> const struct in6_addr *daddr; > >> unsigned int len; > >> unsigned char proto; > >> - unsigned int csum; > >> + __wsum csum; > >> > >> const int daddr_offset = sizeof(struct in6_addr); > >> const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr); > >> @@ -611,10 +612,10 @@ static void test_csum_ipv6_magic(struct kunit *test) > >> saddr = (const struct in6_addr *)(random_buf + i); > >> daddr = (const struct in6_addr *)(random_buf + i + > >> daddr_offset); > >> - len = *(unsigned int *)(random_buf + i + len_offset); > >> + len = le32_to_cpu(*(__le32 *)(random_buf + i + len_offset)); > >> proto = *(random_buf + i + proto_offset); > >> - csum = *(unsigned int *)(random_buf + i + csum_offset); > >> - CHECK_EQ(expected_csum_ipv6_magic[i], > >> + csum = *(__wsum *)(random_buf + i + csum_offset); > >> + CHECK_EQ(to_sum16(expected_csum_ipv6_magic[i]), > >> csum_ipv6_magic(saddr, daddr, len, proto, csum)); > >> } > >> #endif /* !CONFIG_NET */ > >> -- > >> 2.43.0 > >> > > > > There is no need to duplicate efforts here. This has already been > > resolved by > > https://lore.kernel.org/lkml/20240221-fix_sparse_errors_checksum_tests-v9-2-bff4d73ab9d1@rivosinc.com/. > > > > The idea here is to provide a fix which is similar to the one done > previously and that uses the same approach and reuses the same helpers. > > This is to keep the code homogeneous. > > Christophe htons makes more sense here since this is networking code, but I don't care enough to argue the point. I tested it on big endian SPARC and on riscv. I'll base my alignment patch on this. Tested-by: Charlie Jenkins <charlie@rivosinc.com>
Le 23/02/2024 à 21:05, Charlie Jenkins a écrit : > On Fri, Feb 23, 2024 at 06:15:16PM +0000, Christophe Leroy wrote: >> >> >> Le 23/02/2024 à 18:57, Charlie Jenkins a écrit : >>> On Fri, Feb 23, 2024 at 11:41:52AM +0100, Christophe Leroy wrote: >>>> Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") >>>> fixed endianness issues with kunit checksum tests, but then >>>> commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and >>>> ip_fast_csum") introduced new issues on big endian CPUs. Those issues >>>> are once again reflected by the warnings reported by sparse. >>>> >>>> So, fix them with the same approach, perform proper conversion in >>>> order to support both little and big endian CPUs. Once the conversions >>>> are properly done and the right types used, the sparse warnings are >>>> cleared as well. >>>> >>>> Reported-by: Erhard Furtner <erhard_f@mailbox.org> >>>> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") >>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>> --- >>>> lib/checksum_kunit.c | 17 +++++++++-------- >>>> 1 file changed, 9 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c >>>> index 225bb7701460..bf70850035c7 100644 >>>> --- a/lib/checksum_kunit.c >>>> +++ b/lib/checksum_kunit.c >>>> @@ -215,7 +215,7 @@ static const u32 init_sums_no_overflow[] = { >>>> 0xffff0000, 0xfffffffb, >>>> }; >>>> >>>> -static const __sum16 expected_csum_ipv6_magic[] = { >>>> +static const u16 expected_csum_ipv6_magic[] = { >>>> 0x18d4, 0x3085, 0x2e4b, 0xd9f4, 0xbdc8, 0x78f, 0x1034, 0x8422, 0x6fc0, >>>> 0xd2f6, 0xbeb5, 0x9d3, 0x7e2a, 0x312e, 0x778e, 0xc1bb, 0x7cf2, 0x9d1e, >>>> 0xca21, 0xf3ff, 0x7569, 0xb02e, 0xca86, 0x7e76, 0x4539, 0x45e3, 0xf28d, >>>> @@ -241,7 +241,7 @@ static const __sum16 expected_csum_ipv6_magic[] = { >>>> 0x3845, 0x1014 >>>> }; >>>> >>>> -static const __sum16 expected_fast_csum[] = { >>>> +static const u16 expected_fast_csum[] = { >>>> 0xda83, 0x45da, 0x4f46, 0x4e4f, 0x34e, 0xe902, 0xa5e9, 0x87a5, 0x7187, >>>> 0x5671, 0xf556, 0x6df5, 0x816d, 0x8f81, 0xbb8f, 0xfbba, 0x5afb, 0xbe5a, >>>> 0xedbe, 0xabee, 0x6aac, 0xe6b, 0xea0d, 0x67ea, 0x7e68, 0x8a7e, 0x6f8a, >>>> @@ -577,7 +577,8 @@ static void test_csum_no_carry_inputs(struct kunit *test) >>>> >>>> static void test_ip_fast_csum(struct kunit *test) >>>> { >>>> - __sum16 csum_result, expected; >>>> + __sum16 csum_result; >>>> + u16 expected; >>>> >>>> for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) { >>>> for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) { >>>> @@ -586,7 +587,7 @@ static void test_ip_fast_csum(struct kunit *test) >>>> expected_fast_csum[(len - IPv4_MIN_WORDS) * >>>> NUM_IP_FAST_CSUM_TESTS + >>>> index]; >>>> - CHECK_EQ(expected, csum_result); >>>> + CHECK_EQ(to_sum16(expected), csum_result); >>>> } >>>> } >>>> } >>>> @@ -598,7 +599,7 @@ static void test_csum_ipv6_magic(struct kunit *test) >>>> const struct in6_addr *daddr; >>>> unsigned int len; >>>> unsigned char proto; >>>> - unsigned int csum; >>>> + __wsum csum; >>>> >>>> const int daddr_offset = sizeof(struct in6_addr); >>>> const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr); >>>> @@ -611,10 +612,10 @@ static void test_csum_ipv6_magic(struct kunit *test) >>>> saddr = (const struct in6_addr *)(random_buf + i); >>>> daddr = (const struct in6_addr *)(random_buf + i + >>>> daddr_offset); >>>> - len = *(unsigned int *)(random_buf + i + len_offset); >>>> + len = le32_to_cpu(*(__le32 *)(random_buf + i + len_offset)); >>>> proto = *(random_buf + i + proto_offset); >>>> - csum = *(unsigned int *)(random_buf + i + csum_offset); >>>> - CHECK_EQ(expected_csum_ipv6_magic[i], >>>> + csum = *(__wsum *)(random_buf + i + csum_offset); >>>> + CHECK_EQ(to_sum16(expected_csum_ipv6_magic[i]), >>>> csum_ipv6_magic(saddr, daddr, len, proto, csum)); >>>> } >>>> #endif /* !CONFIG_NET */ >>>> -- >>>> 2.43.0 >>>> >>> >>> There is no need to duplicate efforts here. This has already been >>> resolved by >>> https://lore.kernel.org/lkml/20240221-fix_sparse_errors_checksum_tests-v9-2-bff4d73ab9d1@rivosinc.com/. >>> >> >> The idea here is to provide a fix which is similar to the one done >> previously and that uses the same approach and reuses the same helpers. >> >> This is to keep the code homogeneous. >> >> Christophe > > htons makes more sense here since this is networking code, but I don't > care enough to argue the point. I tested it on big endian SPARC and on > riscv. I'll base my alignment patch on this. I agree with you that htons() would have made more sense, but the expected values are in little endian and when I fixed the issue the first time I wanted to keep it as a minimal fix and didn't want to churn things too much and change all expected values. If we go for htons() this time this means again changing expected values to natural network byte order (big endian). Either we do it only for the tests to be fixed now, and we'll have some tests for with the results are in big endian and some that remains in little endian. That looks inconsistant to me. Or we do the change for all tests. In both case it would be a bulky change. As we are relatively late in 6.8 cycle my preference goes for a minimal fix. Why not then convert _all_ tests to network byte order expectation in a follow-up patch. > > Tested-by: Charlie Jenkins <charlie@rivosinc.com> > Thanks Christophe
Hi, Le 23/02/2024 à 11:41, Christophe Leroy a écrit : > Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") > fixed endianness issues with kunit checksum tests, but then > commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and > ip_fast_csum") introduced new issues on big endian CPUs. Those issues > are once again reflected by the warnings reported by sparse. > > So, fix them with the same approach, perform proper conversion in > order to support both little and big endian CPUs. Once the conversions > are properly done and the right types used, the sparse warnings are > cleared as well. > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> netdev checkpatch complains about "1 blamed authors not CCed: palmer@rivosinc.com; 1 maintainers not CCed: palmer@rivosinc.com " Palmer was copied but as Palmer Dabbelt <palmer@dabbelt.com>. Hope it is not a show stopper. Christophe > --- > lib/checksum_kunit.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c > index 225bb7701460..bf70850035c7 100644 > --- a/lib/checksum_kunit.c > +++ b/lib/checksum_kunit.c > @@ -215,7 +215,7 @@ static const u32 init_sums_no_overflow[] = { > 0xffff0000, 0xfffffffb, > }; > > -static const __sum16 expected_csum_ipv6_magic[] = { > +static const u16 expected_csum_ipv6_magic[] = { > 0x18d4, 0x3085, 0x2e4b, 0xd9f4, 0xbdc8, 0x78f, 0x1034, 0x8422, 0x6fc0, > 0xd2f6, 0xbeb5, 0x9d3, 0x7e2a, 0x312e, 0x778e, 0xc1bb, 0x7cf2, 0x9d1e, > 0xca21, 0xf3ff, 0x7569, 0xb02e, 0xca86, 0x7e76, 0x4539, 0x45e3, 0xf28d, > @@ -241,7 +241,7 @@ static const __sum16 expected_csum_ipv6_magic[] = { > 0x3845, 0x1014 > }; > > -static const __sum16 expected_fast_csum[] = { > +static const u16 expected_fast_csum[] = { > 0xda83, 0x45da, 0x4f46, 0x4e4f, 0x34e, 0xe902, 0xa5e9, 0x87a5, 0x7187, > 0x5671, 0xf556, 0x6df5, 0x816d, 0x8f81, 0xbb8f, 0xfbba, 0x5afb, 0xbe5a, > 0xedbe, 0xabee, 0x6aac, 0xe6b, 0xea0d, 0x67ea, 0x7e68, 0x8a7e, 0x6f8a, > @@ -577,7 +577,8 @@ static void test_csum_no_carry_inputs(struct kunit *test) > > static void test_ip_fast_csum(struct kunit *test) > { > - __sum16 csum_result, expected; > + __sum16 csum_result; > + u16 expected; > > for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) { > for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) { > @@ -586,7 +587,7 @@ static void test_ip_fast_csum(struct kunit *test) > expected_fast_csum[(len - IPv4_MIN_WORDS) * > NUM_IP_FAST_CSUM_TESTS + > index]; > - CHECK_EQ(expected, csum_result); > + CHECK_EQ(to_sum16(expected), csum_result); > } > } > } > @@ -598,7 +599,7 @@ static void test_csum_ipv6_magic(struct kunit *test) > const struct in6_addr *daddr; > unsigned int len; > unsigned char proto; > - unsigned int csum; > + __wsum csum; > > const int daddr_offset = sizeof(struct in6_addr); > const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr); > @@ -611,10 +612,10 @@ static void test_csum_ipv6_magic(struct kunit *test) > saddr = (const struct in6_addr *)(random_buf + i); > daddr = (const struct in6_addr *)(random_buf + i + > daddr_offset); > - len = *(unsigned int *)(random_buf + i + len_offset); > + len = le32_to_cpu(*(__le32 *)(random_buf + i + len_offset)); > proto = *(random_buf + i + proto_offset); > - csum = *(unsigned int *)(random_buf + i + csum_offset); > - CHECK_EQ(expected_csum_ipv6_magic[i], > + csum = *(__wsum *)(random_buf + i + csum_offset); > + CHECK_EQ(to_sum16(expected_csum_ipv6_magic[i]), > csum_ipv6_magic(saddr, daddr, len, proto, csum)); > } > #endif /* !CONFIG_NET */
On Fri, Feb 23, 2024 at 11:41:52AM +0100, Christophe Leroy wrote: > Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") > fixed endianness issues with kunit checksum tests, but then > commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and > ip_fast_csum") introduced new issues on big endian CPUs. Those issues > are once again reflected by the warnings reported by sparse. > > So, fix them with the same approach, perform proper conversion in > order to support both little and big endian CPUs. Once the conversions > are properly done and the right types used, the sparse warnings are > cleared as well. > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Tested-by: Guenter Roeck <linux@roeck-us.net>
On Sat, 2024-02-24 at 07:44 +0000, Christophe Leroy wrote: > Hi, > > Le 23/02/2024 à 11:41, Christophe Leroy a écrit : > > Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") > > fixed endianness issues with kunit checksum tests, but then > > commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and > > ip_fast_csum") introduced new issues on big endian CPUs. Those issues > > are once again reflected by the warnings reported by sparse. > > > > So, fix them with the same approach, perform proper conversion in > > order to support both little and big endian CPUs. Once the conversions > > are properly done and the right types used, the sparse warnings are > > cleared as well. > > > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > netdev checkpatch complains about "1 blamed authors not CCed: > palmer@rivosinc.com; 1 maintainers not CCed: palmer@rivosinc.com " > > Palmer was copied but as Palmer Dabbelt <palmer@dabbelt.com>. Hope it is > not a show stopper. No, it's not. Acked-by: Paolo Abeni <pabeni@redhat.com> I *think* this, despite the subject prefix, should go via Andrew's tree to avoid conflicts. @Andrew does the above fits you? Cheers, Paolo
On Tue, 27 Feb 2024 10:06:41 +0100 Paolo Abeni wrote: > On Sat, 2024-02-24 at 07:44 +0000, Christophe Leroy wrote: > > Hi, > > > > Le 23/02/2024 à 11:41, Christophe Leroy a écrit : > > > Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") > > > fixed endianness issues with kunit checksum tests, but then > > > commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and > > > ip_fast_csum") introduced new issues on big endian CPUs. Those issues > > > are once again reflected by the warnings reported by sparse. > > > > > > So, fix them with the same approach, perform proper conversion in > > > order to support both little and big endian CPUs. Once the conversions > > > are properly done and the right types used, the sparse warnings are > > > cleared as well. > > > > > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> > > > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") > > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > > > netdev checkpatch complains about "1 blamed authors not CCed: > > palmer@rivosinc.com; 1 maintainers not CCed: palmer@rivosinc.com " > > > > Palmer was copied but as Palmer Dabbelt <palmer@dabbelt.com>. Hope it is > > not a show stopper. > > No, it's not. > > Acked-by: Paolo Abeni <pabeni@redhat.com> > > I *think* this, despite the subject prefix, should go via Andrew's tree > to avoid conflicts. > > @Andrew does the above fits you? I don't see this in linux-next, so unless told otherwise I'll merge it and ship to Linus tomorrow.
On Wed, 28 Feb 2024 17:51:58 PST (-0800), kuba@kernel.org wrote: > On Tue, 27 Feb 2024 10:06:41 +0100 Paolo Abeni wrote: >> On Sat, 2024-02-24 at 07:44 +0000, Christophe Leroy wrote: >> > Hi, >> > >> > Le 23/02/2024 à 11:41, Christophe Leroy a écrit : >> > > Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") >> > > fixed endianness issues with kunit checksum tests, but then >> > > commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and >> > > ip_fast_csum") introduced new issues on big endian CPUs. Those issues >> > > are once again reflected by the warnings reported by sparse. >> > > >> > > So, fix them with the same approach, perform proper conversion in >> > > order to support both little and big endian CPUs. Once the conversions >> > > are properly done and the right types used, the sparse warnings are >> > > cleared as well. >> > > >> > > Reported-by: Erhard Furtner <erhard_f@mailbox.org> >> > > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") >> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> > >> > netdev checkpatch complains about "1 blamed authors not CCed: >> > palmer@rivosinc.com; 1 maintainers not CCed: palmer@rivosinc.com " >> > >> > Palmer was copied but as Palmer Dabbelt <palmer@dabbelt.com>. Hope it is >> > not a show stopper. >> >> No, it's not. >> >> Acked-by: Paolo Abeni <pabeni@redhat.com> >> >> I *think* this, despite the subject prefix, should go via Andrew's tree >> to avoid conflicts. >> >> @Andrew does the above fits you? > > I don't see this in linux-next, so unless told otherwise I'll merge it > and ship to Linus tomorrow. Sorry, I thought I'd Ack'd this at some point -- I might have just gotten lost, though, as IIRC there were a few of these floating around. I'm fine with anyone taking it, so Acked-by: Palmer Dabbelt <palmer@rivosinc.com> Thanks!
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Fri, 23 Feb 2024 11:41:52 +0100 you wrote: > Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") > fixed endianness issues with kunit checksum tests, but then > commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and > ip_fast_csum") introduced new issues on big endian CPUs. Those issues > are once again reflected by the warnings reported by sparse. > > So, fix them with the same approach, perform proper conversion in > order to support both little and big endian CPUs. Once the conversions > are properly done and the right types used, the sparse warnings are > cleared as well. > > [...] Here is the summary with links: - [net] kunit: Fix again checksum tests on big endian CPUs https://git.kernel.org/netdev/net/c/3d6423ef8d51 You are awesome, thank you!
diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c index 225bb7701460..bf70850035c7 100644 --- a/lib/checksum_kunit.c +++ b/lib/checksum_kunit.c @@ -215,7 +215,7 @@ static const u32 init_sums_no_overflow[] = { 0xffff0000, 0xfffffffb, }; -static const __sum16 expected_csum_ipv6_magic[] = { +static const u16 expected_csum_ipv6_magic[] = { 0x18d4, 0x3085, 0x2e4b, 0xd9f4, 0xbdc8, 0x78f, 0x1034, 0x8422, 0x6fc0, 0xd2f6, 0xbeb5, 0x9d3, 0x7e2a, 0x312e, 0x778e, 0xc1bb, 0x7cf2, 0x9d1e, 0xca21, 0xf3ff, 0x7569, 0xb02e, 0xca86, 0x7e76, 0x4539, 0x45e3, 0xf28d, @@ -241,7 +241,7 @@ static const __sum16 expected_csum_ipv6_magic[] = { 0x3845, 0x1014 }; -static const __sum16 expected_fast_csum[] = { +static const u16 expected_fast_csum[] = { 0xda83, 0x45da, 0x4f46, 0x4e4f, 0x34e, 0xe902, 0xa5e9, 0x87a5, 0x7187, 0x5671, 0xf556, 0x6df5, 0x816d, 0x8f81, 0xbb8f, 0xfbba, 0x5afb, 0xbe5a, 0xedbe, 0xabee, 0x6aac, 0xe6b, 0xea0d, 0x67ea, 0x7e68, 0x8a7e, 0x6f8a, @@ -577,7 +577,8 @@ static void test_csum_no_carry_inputs(struct kunit *test) static void test_ip_fast_csum(struct kunit *test) { - __sum16 csum_result, expected; + __sum16 csum_result; + u16 expected; for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) { for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) { @@ -586,7 +587,7 @@ static void test_ip_fast_csum(struct kunit *test) expected_fast_csum[(len - IPv4_MIN_WORDS) * NUM_IP_FAST_CSUM_TESTS + index]; - CHECK_EQ(expected, csum_result); + CHECK_EQ(to_sum16(expected), csum_result); } } } @@ -598,7 +599,7 @@ static void test_csum_ipv6_magic(struct kunit *test) const struct in6_addr *daddr; unsigned int len; unsigned char proto; - unsigned int csum; + __wsum csum; const int daddr_offset = sizeof(struct in6_addr); const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr); @@ -611,10 +612,10 @@ static void test_csum_ipv6_magic(struct kunit *test) saddr = (const struct in6_addr *)(random_buf + i); daddr = (const struct in6_addr *)(random_buf + i + daddr_offset); - len = *(unsigned int *)(random_buf + i + len_offset); + len = le32_to_cpu(*(__le32 *)(random_buf + i + len_offset)); proto = *(random_buf + i + proto_offset); - csum = *(unsigned int *)(random_buf + i + csum_offset); - CHECK_EQ(expected_csum_ipv6_magic[i], + csum = *(__wsum *)(random_buf + i + csum_offset); + CHECK_EQ(to_sum16(expected_csum_ipv6_magic[i]), csum_ipv6_magic(saddr, daddr, len, proto, csum)); } #endif /* !CONFIG_NET */
Commit b38460bc463c ("kunit: Fix checksum tests on big endian CPUs") fixed endianness issues with kunit checksum tests, but then commit 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") introduced new issues on big endian CPUs. Those issues are once again reflected by the warnings reported by sparse. So, fix them with the same approach, perform proper conversion in order to support both little and big endian CPUs. Once the conversions are properly done and the right types used, the sparse warnings are cleared as well. Reported-by: Erhard Furtner <erhard_f@mailbox.org> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum") Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- lib/checksum_kunit.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)