All of lore.kernel.org
 help / color / mirror / Atom feed
From: Erhard Furtner <erhard_f@mailbox.org>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	Charlie Jenkins <charlie@rivosinc.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Palmer Dabbelt <palmer@rivosinc.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: "test_ip_fast_csum: ASSERTION FAILED at lib/checksum_kunit.c:589" at boot with CONFIG_CHECKSUM_KUNIT=y enabled on a Talos II, kernel 6.8-rc5
Date: Fri, 23 Feb 2024 12:37:13 +0100	[thread overview]
Message-ID: <20240223123713.2e49b981@yea> (raw)
In-Reply-To: <b2a7b678-fc59-4d12-acc3-696866cfd7c2@csgroup.eu>

On Fri, 23 Feb 2024 09:06:56 +0000
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:

> Yes, with second patch is magically works, meaning the patch description 
> is not correct because the problem for powerpc it not at all related to 
> memory alignment but to endianness. And endianness should have been 
> fixed by patch 1, but instead of it, patch 1 just hides the problem by 
> forcing casts.
> 
> The real fix for endianness which should be your patch 1 is the 
> following change. With that change it works perfectly well without any 
> forced cast:
> 
> 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 */
> ---
> 
> Christophe

Your patch applied on top of 6.8-rc5 fixes the issue. Thanks!

And I take your remarks here as a hint for the other "drm_test_fb_xrgb8888_to_xrgb2101010 on Big Endian machines" issue I posted. ;) Let's see what I can do.

Regards,
Erhard

WARNING: multiple messages have this Message-ID (diff)
From: Erhard Furtner <erhard_f@mailbox.org>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Charlie Jenkins <charlie@rivosinc.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	Palmer Dabbelt <palmer@rivosinc.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: "test_ip_fast_csum: ASSERTION FAILED at lib/checksum_kunit.c:589" at boot with CONFIG_CHECKSUM_KUNIT=y enabled on a Talos II, kernel 6.8-rc5
Date: Fri, 23 Feb 2024 12:37:13 +0100	[thread overview]
Message-ID: <20240223123713.2e49b981@yea> (raw)
In-Reply-To: <b2a7b678-fc59-4d12-acc3-696866cfd7c2@csgroup.eu>

On Fri, 23 Feb 2024 09:06:56 +0000
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:

> Yes, with second patch is magically works, meaning the patch description 
> is not correct because the problem for powerpc it not at all related to 
> memory alignment but to endianness. And endianness should have been 
> fixed by patch 1, but instead of it, patch 1 just hides the problem by 
> forcing casts.
> 
> The real fix for endianness which should be your patch 1 is the 
> following change. With that change it works perfectly well without any 
> forced cast:
> 
> 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 */
> ---
> 
> Christophe

Your patch applied on top of 6.8-rc5 fixes the issue. Thanks!

And I take your remarks here as a hint for the other "drm_test_fb_xrgb8888_to_xrgb2101010 on Big Endian machines" issue I posted. ;) Let's see what I can do.

Regards,
Erhard

  reply	other threads:[~2024-02-23 11:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-23  1:26 "test_ip_fast_csum: ASSERTION FAILED at lib/checksum_kunit.c:589" at boot with CONFIG_CHECKSUM_KUNIT=y enabled on a Talos II, kernel 6.8-rc5 Erhard Furtner
2024-02-23  5:59 ` Christophe Leroy
2024-02-23  6:12   ` Charlie Jenkins
2024-02-23  6:12     ` Charlie Jenkins
2024-02-23  6:58     ` Christophe Leroy
2024-02-23  6:58       ` Christophe Leroy
2024-02-23  7:00       ` Charlie Jenkins
2024-02-23  7:00         ` Charlie Jenkins
2024-02-23  9:06         ` Christophe Leroy
2024-02-23  9:06           ` Christophe Leroy
2024-02-23 11:37           ` Erhard Furtner [this message]
2024-02-23 11:37             ` Erhard Furtner
2024-02-23 17:35           ` Charlie Jenkins
2024-02-23 17:35             ` Charlie Jenkins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240223123713.2e49b981@yea \
    --to=erhard_f@mailbox.org \
    --cc=charlie@rivosinc.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=palmer@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.