* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-23 10:41 [PATCH net] kunit: Fix again checksum tests on big endian CPUs Christophe Leroy
@ 2024-02-23 17:57 ` Charlie Jenkins
2024-02-23 18:15 ` Christophe Leroy
2024-02-24 7:44 ` Christophe Leroy
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Charlie Jenkins @ 2024-02-23 17:57 UTC (permalink / raw)
To: Christophe Leroy
Cc: David S. Miller, Jakub Kicinski, Andrew Morton, linux-kernel,
netdev, Palmer Dabbelt, Erhard Furtner
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-23 17:57 ` Charlie Jenkins
@ 2024-02-23 18:15 ` Christophe Leroy
2024-02-23 20:05 ` Charlie Jenkins
0 siblings, 1 reply; 11+ messages in thread
From: Christophe Leroy @ 2024-02-23 18:15 UTC (permalink / raw)
To: Charlie Jenkins
Cc: David S. Miller, Jakub Kicinski, Andrew Morton, linux-kernel,
netdev, Palmer Dabbelt, Erhard Furtner
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-23 18:15 ` Christophe Leroy
@ 2024-02-23 20:05 ` Charlie Jenkins
2024-02-24 7:40 ` Christophe Leroy
0 siblings, 1 reply; 11+ messages in thread
From: Charlie Jenkins @ 2024-02-23 20:05 UTC (permalink / raw)
To: Christophe Leroy
Cc: David S. Miller, Jakub Kicinski, Andrew Morton, Guenter Roeck,
linux-kernel, netdev, Palmer Dabbelt, Erhard Furtner
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-23 20:05 ` Charlie Jenkins
@ 2024-02-24 7:40 ` Christophe Leroy
0 siblings, 0 replies; 11+ messages in thread
From: Christophe Leroy @ 2024-02-24 7:40 UTC (permalink / raw)
To: Charlie Jenkins
Cc: David S. Miller, Jakub Kicinski, Andrew Morton, Guenter Roeck,
linux-kernel, netdev, Palmer Dabbelt, Erhard Furtner
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-23 10:41 [PATCH net] kunit: Fix again checksum tests on big endian CPUs Christophe Leroy
2024-02-23 17:57 ` Charlie Jenkins
@ 2024-02-24 7:44 ` Christophe Leroy
2024-02-27 9:06 ` Paolo Abeni
2024-02-25 15:58 ` Guenter Roeck
2024-02-29 17:40 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 11+ messages in thread
From: Christophe Leroy @ 2024-02-24 7:44 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Andrew Morton
Cc: linux-kernel, netdev, Charlie Jenkins, Palmer Dabbelt, Erhard Furtner
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 */
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-24 7:44 ` Christophe Leroy
@ 2024-02-27 9:06 ` Paolo Abeni
2024-02-29 1:51 ` Jakub Kicinski
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Abeni @ 2024-02-27 9:06 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, netdev, Charlie Jenkins, Palmer Dabbelt,
Erhard Furtner, Christophe Leroy, David S. Miller,
Jakub Kicinski
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-27 9:06 ` Paolo Abeni
@ 2024-02-29 1:51 ` Jakub Kicinski
2024-02-29 1:55 ` Palmer Dabbelt
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2024-02-29 1:51 UTC (permalink / raw)
To: Andrew Morton
Cc: Paolo Abeni, linux-kernel, netdev, Charlie Jenkins,
Palmer Dabbelt, Erhard Furtner, Christophe Leroy,
David S. Miller
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.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-29 1:51 ` Jakub Kicinski
@ 2024-02-29 1:55 ` Palmer Dabbelt
0 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2024-02-29 1:55 UTC (permalink / raw)
To: kuba
Cc: akpm, pabeni, linux-kernel, netdev, Charlie Jenkins, erhard_f,
christophe.leroy, davem
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!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-23 10:41 [PATCH net] kunit: Fix again checksum tests on big endian CPUs Christophe Leroy
2024-02-23 17:57 ` Charlie Jenkins
2024-02-24 7:44 ` Christophe Leroy
@ 2024-02-25 15:58 ` Guenter Roeck
2024-02-29 17:40 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2024-02-25 15:58 UTC (permalink / raw)
To: Christophe Leroy
Cc: David S. Miller, Jakub Kicinski, Andrew Morton, linux-kernel,
netdev, Charlie Jenkins, Palmer Dabbelt, Erhard Furtner
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] kunit: Fix again checksum tests on big endian CPUs
2024-02-23 10:41 [PATCH net] kunit: Fix again checksum tests on big endian CPUs Christophe Leroy
` (2 preceding siblings ...)
2024-02-25 15:58 ` Guenter Roeck
@ 2024-02-29 17:40 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-02-29 17:40 UTC (permalink / raw)
To: Christophe Leroy
Cc: davem, kuba, akpm, linux-kernel, netdev, charlie, palmer, erhard_f
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 11+ messages in thread