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 <[email protected]>
Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
Signed-off-by: Christophe Leroy <[email protected]>
---
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
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 <[email protected]>
> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> 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 <[email protected]>
>> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>> 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 <[email protected]>
> >> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
> >> Signed-off-by: Christophe Leroy <[email protected]>
> >> ---
> >> 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 <[email protected]>
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 <[email protected]>
>>>> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
>>>> Signed-off-by: Christophe Leroy <[email protected]>
>>>> ---
>>>> 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 <[email protected]>
>
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 <[email protected]>
> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
> Signed-off-by: Christophe Leroy <[email protected]>
netdev checkpatch complains about "1 blamed authors not CCed:
[email protected]; 1 maintainers not CCed: [email protected] "
Palmer was copied but as Palmer Dabbelt <[email protected]>. 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 <[email protected]>
> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
> Signed-off-by: Christophe Leroy <[email protected]>
Tested-by: Guenter Roeck <[email protected]>
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 <[email protected]>
> > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
> > Signed-off-by: Christophe Leroy <[email protected]>
>
> netdev checkpatch complains about "1 blamed authors not CCed:
> [email protected]; 1 maintainers not CCed: [email protected] "
>
> Palmer was copied but as Palmer Dabbelt <[email protected]>. Hope it is
> not a show stopper.
No, it's not.
Acked-by: Paolo Abeni <[email protected]>
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 <[email protected]>
> > > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
> > > Signed-off-by: Christophe Leroy <[email protected]>
> >
> > netdev checkpatch complains about "1 blamed authors not CCed:
> > [email protected]; 1 maintainers not CCed: [email protected] "
> >
> > Palmer was copied but as Palmer Dabbelt <[email protected]>. Hope it is
> > not a show stopper.
>
> No, it's not.
>
> Acked-by: Paolo Abeni <[email protected]>
>
> 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), [email protected] 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 <[email protected]>
>> > > Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
>> > > Signed-off-by: Christophe Leroy <[email protected]>
>> >
>> > netdev checkpatch complains about "1 blamed authors not CCed:
>> > [email protected]; 1 maintainers not CCed: [email protected] "
>> >
>> > Palmer was copied but as Palmer Dabbelt <[email protected]>. Hope it is
>> > not a show stopper.
>>
>> No, it's not.
>>
>> Acked-by: Paolo Abeni <[email protected]>
>>
>> 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 <[email protected]>
Thanks!
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html