linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
@ 2024-02-08  0:22 Charlie Jenkins
  2024-02-08  0:22 ` [PATCH v6 1/2] lib: checksum: Fix type casting in checksum kunits Charlie Jenkins
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Charlie Jenkins @ 2024-02-08  0:22 UTC (permalink / raw)
  To: Guenter Roeck, David Laight, Palmer Dabbelt, Andrew Morton
  Cc: linux-kernel, Charlie Jenkins, kernel test robot

The ip_fast_csum and csum_ipv6_magic tests did not have the data
types properly casted, and improperly misaligned data.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
Changes in v6:
- Fix for big endian (Guenter)
- Link to v5: https://lore.kernel.org/r/20240130-fix_sparse_errors_checksum_tests-v5-0-4d8a0a337e5e@rivosinc.com

Changes in v5:
- Add Guenter's tested-by
- CC Andrew Morton
- Link to v4: https://lore.kernel.org/r/20240124-fix_sparse_errors_checksum_tests-v4-0-bc2b8d23a35c@rivosinc.com

Changes in v4:
- Pad test values with zeroes (David)
- Link to v3: https://lore.kernel.org/r/20240123-fix_sparse_errors_checksum_tests-v3-0-efecc7f94297@rivosinc.com

Changes in v3:
- Don't read memory out of bounds
- Link to v2: https://lore.kernel.org/r/20240123-fix_sparse_errors_checksum_tests-v2-0-b306b6ce7da5@rivosinc.com

Changes in v2:
- Add additional patch to fix alignment issues
- Link to v1: https://lore.kernel.org/r/20240119-fix_sparse_errors_checksum_tests-v1-1-2d3df86d8d78@rivosinc.com

---
Charlie Jenkins (2):
      lib: checksum: Fix type casting in checksum kunits
      lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests

 lib/checksum_kunit.c | 409 +++++++++++++++++++--------------------------------
 1 file changed, 149 insertions(+), 260 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240119-fix_sparse_errors_checksum_tests-26b86b34d784
-- 
- Charlie


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v6 1/2] lib: checksum: Fix type casting in checksum kunits
  2024-02-08  0:22 [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Charlie Jenkins
@ 2024-02-08  0:22 ` Charlie Jenkins
  2024-02-08  0:22 ` [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests Charlie Jenkins
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Charlie Jenkins @ 2024-02-08  0:22 UTC (permalink / raw)
  To: Guenter Roeck, David Laight, Palmer Dabbelt, Andrew Morton
  Cc: linux-kernel, Charlie Jenkins, kernel test robot

The checksum functions use the types __wsum and __sum16. These need to
be explicitly casted to.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202401200106.PMTn6g56-lkp@intel.com/
---
 lib/checksum_kunit.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
index 225bb7701460..776ad3d6d5a1 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,
@@ -582,7 +582,7 @@ static void test_ip_fast_csum(struct kunit *test)
 	for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) {
 		for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) {
 			csum_result = ip_fast_csum(random_buf + index, len);
-			expected =
+			expected = (__force __sum16)
 				expected_fast_csum[(len - IPv4_MIN_WORDS) *
 						   NUM_IP_FAST_CSUM_TESTS +
 						   index];
@@ -614,8 +614,9 @@ static void test_csum_ipv6_magic(struct kunit *test)
 		len = *(unsigned int *)(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_ipv6_magic(saddr, daddr, len, proto, csum));
+		CHECK_EQ((__force __sum16)expected_csum_ipv6_magic[i],
+			 csum_ipv6_magic(saddr, daddr, len, proto,
+					 (__force __wsum)csum));
 	}
 #endif /* !CONFIG_NET */
 }

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-08  0:22 [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Charlie Jenkins
  2024-02-08  0:22 ` [PATCH v6 1/2] lib: checksum: Fix type casting in checksum kunits Charlie Jenkins
@ 2024-02-08  0:22 ` Charlie Jenkins
  2024-02-08  2:45   ` Guenter Roeck
  2024-02-12  6:26   ` Al Viro
  2024-02-08  1:45 ` [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Andrew Morton
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 22+ messages in thread
From: Charlie Jenkins @ 2024-02-08  0:22 UTC (permalink / raw)
  To: Guenter Roeck, David Laight, Palmer Dabbelt, Andrew Morton
  Cc: linux-kernel, Charlie Jenkins

The test cases for ip_fast_csum and csum_ipv6_magic were using arbitrary
alignment of data to iterate through random inputs. ip_fast_csum should
have the data aligned along (14 + NET_IP_ALIGN) bytes and
csum_ipv6_magic should have data aligned along 32-bit boundaries.

While this is being changed, fix up the awkward offset code in
test_csum_ipv6_magic and use a struct instead.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
---
 lib/checksum_kunit.c | 406 +++++++++++++++++++--------------------------------
 1 file changed, 147 insertions(+), 259 deletions(-)

diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
index 776ad3d6d5a1..acce285a4959 100644
--- a/lib/checksum_kunit.c
+++ b/lib/checksum_kunit.c
@@ -13,8 +13,9 @@
 
 #define IPv4_MIN_WORDS 5
 #define IPv4_MAX_WORDS 15
-#define NUM_IPv6_TESTS 200
-#define NUM_IP_FAST_CSUM_TESTS 181
+#define WORD_ALIGNMENT 4
+/* Ethernet headers are 14 bytes and NET_IP_ALIGN is used to align them */
+#define IP_ALIGNMENT (14 + NET_IP_ALIGN)
 
 /* Values for a little endian CPU. Byte swap each half on big endian CPU. */
 static const u32 random_init_sum = 0x2847aab;
@@ -216,234 +217,103 @@ static const u32 init_sums_no_overflow[] = {
 };
 
 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,
-	0xdf81, 0x8fd5, 0x3b5d, 0x8324, 0xf471, 0x83be, 0x1daf, 0x8c46, 0xe682,
-	0xd1fb, 0x6b2e, 0xe687, 0x2a33, 0x4833, 0x2d67, 0x660f, 0x2e79, 0xd65e,
-	0x6b62, 0x6672, 0x5dbd, 0x8680, 0xbaa5, 0x2229, 0x2125, 0x2d01, 0x1cc0,
-	0x6d36, 0x33c0, 0xee36, 0xd832, 0x9820, 0x8a31, 0x53c5, 0x2e2,	0xdb0e,
-	0x49ed, 0x17a7, 0x77a0, 0xd72e, 0x3d72, 0x7dc8, 0x5b17, 0xf55d, 0xa4d9,
-	0x1446, 0x5d56, 0x6b2e, 0x69a5, 0xadb6, 0xff2a, 0x92e,	0xe044, 0x3402,
-	0xbb60, 0xec7f, 0xe7e6, 0x1986, 0x32f4, 0x8f8,	0x5e00, 0x47c6, 0x3059,
-	0x3969, 0xe957, 0x4388, 0x2854, 0x3334, 0xea71, 0xa6de, 0x33f9, 0x83fc,
-	0x37b4, 0x5531, 0x3404, 0x1010, 0xed30, 0x610a, 0xc95,	0x9aed, 0x6ff,
-	0x5136, 0x2741, 0x660e, 0x8b80, 0xf71,	0xa263, 0x88af, 0x7a73, 0x3c37,
-	0x1908, 0x6db5, 0x2e92, 0x1cd2, 0x70c8, 0xee16, 0xe80,	0xcd55, 0x6e6,
-	0x6434, 0x127,	0x655d, 0x2ea0, 0xb4f4, 0xdc20, 0x5671, 0xe462, 0xe52b,
-	0xdb44, 0x3589, 0xc48f, 0xe60b, 0xd2d2, 0x66ad, 0x498,	0x436,	0xb917,
-	0xf0ca, 0x1a6e, 0x1cb7, 0xbf61, 0x2870, 0xc7e8, 0x5b30, 0xe4a5, 0x168,
-	0xadfc, 0xd035, 0xe690, 0xe283, 0xfb27, 0xe4ad, 0xb1a5, 0xf2d5, 0xc4b6,
-	0x8a30, 0xd7d5, 0x7df9, 0x91d5, 0x63ed, 0x2d21, 0x312b, 0xab19, 0xa632,
-	0x8d2e, 0xef06, 0x57b9, 0xc373, 0xbd1f, 0xa41f, 0x8444, 0x9975, 0x90cb,
-	0xc49c, 0xe965, 0x4eff, 0x5a,	0xef6d, 0xe81a, 0xe260, 0x853a, 0xff7a,
-	0x99aa, 0xb06b, 0xee19, 0xcc2c, 0xf34c, 0x7c49, 0xdac3, 0xa71e, 0xc988,
-	0x3845, 0x1014
+	0x3045, 0xb681, 0xc210, 0xe77b, 0x41cc, 0xa904, 0x4367, 0xe8d8, 0x5809,
+	0x5901, 0x5a40, 0xd3f4, 0xe467, 0xddde, 0xa609, 0xae05, 0xed14, 0x9133,
+	0x8007, 0x89b6, 0x97b0, 0x8927, 0x1e43, 0x7903, 0x4772, 0xd3a4, 0x457b,
+	0x83d0, 0x4ce1, 0x3656, 0x2dfc, 0xb678, 0x9a83, 0xd523, 0x61db, 0x379e,
+	0x3880, 0xbb23, 0xa38b, 0xd2eb, 0x991a, 0xcf73, 0x13ea, 0x890f, 0x20ce,
+	0x60ad, 0x5688, 0x4b13, 0x9399, 0x8a36, 0x75ae, 0x513a, 0xb222, 0xf3bb,
+	0x80d4, 0x1f98, 0xc2bc, 0xf566, 0x796a, 0x268a, 0xe67f, 0xb917, 0xd716,
+	0x3a16, 0x1858, 0x9d5b, 0x6fb4, 0x72b4, 0x1196, 0xb3d0, 0x89dc, 0xdd07,
+	0x1a8d, 0xfa6d, 0x1507, 0xafab, 0x7d37, 0xa623, 0x72dd, 0x2083, 0xdfc4,
+	0xa267, 0x92c9, 0xc8ad,
 };
 
 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,
-	0x3a70, 0x9f3a, 0xe89e, 0x75e8, 0x7976, 0xfa79, 0x2cfa, 0x3c2c, 0x463c,
-	0x7146, 0x7a71, 0x547a, 0xfd53, 0x99fc, 0xb699, 0x92b6, 0xdb91, 0xe8da,
-	0x5fe9, 0x1e60, 0xae1d, 0x39ae, 0xf439, 0xa1f4, 0xdda1, 0xede,	0x790f,
-	0x579,	0x1206, 0x9012, 0x2490, 0xd224, 0x5cd2, 0xa65d, 0xca7,	0x220d,
-	0xf922, 0xbf9,	0x920b, 0x1b92, 0x361c, 0x2e36, 0x4d2e, 0x24d,	0x2,
-	0xcfff, 0x90cf, 0xa591, 0x93a5, 0x7993, 0x9579, 0xc894, 0x50c8, 0x5f50,
-	0xd55e, 0xcad5, 0xf3c9, 0x8f4,	0x4409, 0x5043, 0x5b50, 0x55b,	0x2205,
-	0x1e22, 0x801e, 0x3780, 0xe137, 0x7ee0, 0xf67d, 0x3cf6, 0xa53c, 0x2ea5,
-	0x472e, 0x5147, 0xcf51, 0x1bcf, 0x951c, 0x1e95, 0xc71e, 0xe4c7, 0xc3e4,
-	0x3dc3, 0xee3d, 0xa4ed, 0xf9a4, 0xcbf8, 0x75cb, 0xb375, 0x50b4, 0x3551,
-	0xf835, 0x19f8, 0x8c1a, 0x538c, 0xad52, 0xa3ac, 0xb0a3, 0x5cb0, 0x6c5c,
-	0x5b6c, 0xc05a, 0x92c0, 0x4792, 0xbe47, 0x53be, 0x1554, 0x5715, 0x4b57,
-	0xe54a, 0x20e5, 0x21,	0xd500, 0xa1d4, 0xa8a1, 0x57a9, 0xca57, 0x5ca,
-	0x1c06, 0x4f1c, 0xe24e, 0xd9e2, 0xf0d9, 0x4af1, 0x474b, 0x8146, 0xe81,
-	0xfd0e, 0x84fd, 0x7c85, 0xba7c, 0x17ba, 0x4a17, 0x964a, 0xf595, 0xff5,
-	0x5310, 0x3253, 0x6432, 0x4263, 0x2242, 0xe121, 0x32e1, 0xf632, 0xc5f5,
-	0x21c6, 0x7d22, 0x8e7c, 0x418e, 0x5641, 0x3156, 0x7c31, 0x737c, 0x373,
-	0x2503, 0xc22a, 0x3c2,	0x4a04, 0x8549, 0x5285, 0xa352, 0xe8a3, 0x6fe8,
-	0x1a6f, 0x211a, 0xe021, 0x38e0, 0x7638, 0xf575, 0x9df5, 0x169e, 0xf116,
-	0x23f1, 0xcd23, 0xece,	0x660f, 0x4866, 0x6a48, 0x716a, 0xee71, 0xa2ee,
-	0xb8a2, 0x61b9, 0xa361, 0xf7a2, 0x26f7, 0x1127, 0x6611, 0xe065, 0x36e0,
-	0x1837, 0x3018, 0x1c30, 0x721b, 0x3e71, 0xe43d, 0x99e4, 0x9e9a, 0xb79d,
-	0xa9b7, 0xcaa,	0xeb0c, 0x4eb,	0x1305, 0x8813, 0xb687, 0xa9b6, 0xfba9,
-	0xd7fb, 0xccd8, 0x2ecd, 0x652f, 0xae65, 0x3fae, 0x3a40, 0x563a, 0x7556,
-	0x2776, 0x1228, 0xef12, 0xf9ee, 0xcef9, 0x56cf, 0xa956, 0x24a9, 0xba24,
-	0x5fba, 0x665f, 0xf465, 0x8ff4, 0x6d8f, 0x346d, 0x5f34, 0x385f, 0xd137,
-	0xb8d0, 0xacb8, 0x55ac, 0x7455, 0xe874, 0x89e8, 0xd189, 0xa0d1, 0xb2a0,
-	0xb8b2, 0x36b8, 0x5636, 0xd355, 0x8d3,	0x1908, 0x2118, 0xc21,	0x990c,
-	0x8b99, 0x158c, 0x7815, 0x9e78, 0x6f9e, 0x4470, 0x1d44, 0x341d, 0x2634,
-	0x3f26, 0x793e, 0xc79,	0xcc0b, 0x26cc, 0xd126, 0x1fd1, 0xb41f, 0xb6b4,
-	0x22b7, 0xa122, 0xa1,	0x7f01, 0x837e, 0x3b83, 0xaf3b, 0x6fae, 0x916f,
-	0xb490, 0xffb3, 0xceff, 0x50cf, 0x7550, 0x7275, 0x1272, 0x2613, 0xaa26,
-	0xd5aa, 0x7d5,	0x9607, 0x96,	0xb100, 0xf8b0, 0x4bf8, 0xdd4c, 0xeddd,
-	0x98ed, 0x2599, 0x9325, 0xeb92, 0x8feb, 0xcc8f, 0x2acd, 0x392b, 0x3b39,
-	0xcb3b, 0x6acb, 0xd46a, 0xb8d4, 0x6ab8, 0x106a, 0x2f10, 0x892f, 0x789,
-	0xc806, 0x45c8, 0x7445, 0x3c74, 0x3a3c, 0xcf39, 0xd7ce, 0x58d8, 0x6e58,
-	0x336e, 0x1034, 0xee10, 0xe9ed, 0xc2e9, 0x3fc2, 0xd53e, 0xd2d4, 0xead2,
-	0x8fea, 0x2190, 0x1162, 0xbe11, 0x8cbe, 0x6d8c, 0xfb6c, 0x6dfb, 0xd36e,
-	0x3ad3, 0xf3a,	0x870e, 0xc287, 0x53c3, 0xc54,	0x5b0c, 0x7d5a, 0x797d,
-	0xec79, 0x5dec, 0x4d5e, 0x184e, 0xd618, 0x60d6, 0xb360, 0x98b3, 0xf298,
-	0xb1f2, 0x69b1, 0xf969, 0xef9,	0xab0e, 0x21ab, 0xe321, 0x24e3, 0x8224,
-	0x5481, 0x5954, 0x7a59, 0xff7a, 0x7dff, 0x1a7d, 0xa51a, 0x46a5, 0x6b47,
-	0xe6b,	0x830e, 0xa083, 0xff9f, 0xd0ff, 0xffd0, 0xe6ff, 0x7de7, 0xc67d,
-	0xd0c6, 0x61d1, 0x3a62, 0xc3b,	0x150c, 0x1715, 0x4517, 0x5345, 0x3954,
-	0xdd39, 0xdadd, 0x32db, 0x6a33, 0xd169, 0x86d1, 0xb687, 0x3fb6, 0x883f,
-	0xa487, 0x39a4, 0x2139, 0xbe20, 0xffbe, 0xedfe, 0x8ded, 0x368e, 0xc335,
-	0x51c3, 0x9851, 0xf297, 0xd6f2, 0xb9d6, 0x95ba, 0x2096, 0xea1f, 0x76e9,
-	0x4e76, 0xe04d, 0xd0df, 0x80d0, 0xa280, 0xfca2, 0x75fc, 0xef75, 0x32ef,
-	0x6833, 0xdf68, 0xc4df, 0x76c4, 0xb77,	0xb10a, 0xbfb1, 0x58bf, 0x5258,
-	0x4d52, 0x6c4d, 0x7e6c, 0xb67e, 0xccb5, 0x8ccc, 0xbe8c, 0xc8bd, 0x9ac8,
-	0xa99b, 0x52a9, 0x2f53, 0xc30,	0x3e0c, 0xb83d, 0x83b7, 0x5383, 0x7e53,
-	0x4f7e, 0xe24e, 0xb3e1, 0x8db3, 0x618e, 0xc861, 0xfcc8, 0x34fc, 0x9b35,
-	0xaa9b, 0xb1aa, 0x5eb1, 0x395e, 0x8639, 0xd486, 0x8bd4, 0x558b, 0x2156,
-	0xf721, 0x4ef6, 0x14f,	0x7301, 0xdd72, 0x49de, 0x894a, 0x9889, 0x8898,
-	0x7788, 0x7b77, 0x637b, 0xb963, 0xabb9, 0x7cab, 0xc87b, 0x21c8, 0xcb21,
-	0xdfca, 0xbfdf, 0xf2bf, 0x6af2, 0x626b, 0xb261, 0x3cb2, 0xc63c, 0xc9c6,
-	0xc9c9, 0xb4c9, 0xf9b4, 0x91f9, 0x4091, 0x3a40, 0xcc39, 0xd1cb, 0x7ed1,
-	0x537f, 0x6753, 0xa167, 0xba49, 0x88ba, 0x7789, 0x3877, 0xf037, 0xd3ef,
-	0xb5d4, 0x55b6, 0xa555, 0xeca4, 0xa1ec, 0xb6a2, 0x7b7,	0x9507, 0xfd94,
-	0x82fd, 0x5c83, 0x765c, 0x9676, 0x3f97, 0xda3f, 0x6fda, 0x646f, 0x3064,
-	0x5e30, 0x655e, 0x6465, 0xcb64, 0xcdca, 0x4ccd, 0x3f4c, 0x243f, 0x6f24,
-	0x656f, 0x6065, 0x3560, 0x3b36, 0xac3b, 0x4aac, 0x714a, 0x7e71, 0xda7e,
-	0x7fda, 0xda7f, 0x6fda, 0xff6f, 0xc6ff, 0xedc6, 0xd4ed, 0x70d5, 0xeb70,
-	0xa3eb, 0x80a3, 0xca80, 0x3fcb, 0x2540, 0xf825, 0x7ef8, 0xf87e, 0x73f8,
-	0xb474, 0xb4b4, 0x92b5, 0x9293, 0x93,	0x3500, 0x7134, 0x9071, 0xfa8f,
-	0x51fa, 0x1452, 0xba13, 0x7ab9, 0x957a, 0x8a95, 0x6e8a, 0x6d6e, 0x7c6d,
-	0x447c, 0x9744, 0x4597, 0x8945, 0xef88, 0x8fee, 0x3190, 0x4831, 0x8447,
-	0xa183, 0x1da1, 0xd41d, 0x2dd4, 0x4f2e, 0xc94e, 0xcbc9, 0xc9cb, 0x9ec9,
-	0x319e, 0xd531, 0x20d5, 0x4021, 0xb23f, 0x29b2, 0xd828, 0xecd8, 0x5ded,
-	0xfc5d, 0x4dfc, 0xd24d, 0x6bd2, 0x5f6b, 0xb35e, 0x7fb3, 0xee7e, 0x56ee,
-	0xa657, 0x68a6, 0x8768, 0x7787, 0xb077, 0x4cb1, 0x764c, 0xb175, 0x7b1,
-	0x3d07, 0x603d, 0x3560, 0x3e35, 0xb03d, 0xd6b0, 0xc8d6, 0xd8c8, 0x8bd8,
-	0x3e8c, 0x303f, 0xd530, 0xf1d4, 0x42f1, 0xca42, 0xddca, 0x41dd, 0x3141,
-	0x132,	0xe901, 0x8e9,	0xbe09, 0xe0bd, 0x2ce0, 0x862d, 0x3986, 0x9139,
-	0x6d91, 0x6a6d, 0x8d6a, 0x1b8d, 0xac1b, 0xedab, 0x54ed, 0xc054, 0xcebf,
-	0xc1ce, 0x5c2,	0x3805, 0x6038, 0x5960, 0xd359, 0xdd3,	0xbe0d, 0xafbd,
-	0x6daf, 0x206d, 0x2c20, 0x862c, 0x8e86, 0xec8d, 0xa2ec, 0xa3a2, 0x51a3,
-	0x8051, 0xfd7f, 0x91fd, 0xa292, 0xaf14, 0xeeae, 0x59ef, 0x535a, 0x8653,
-	0x3986, 0x9539, 0xb895, 0xa0b8, 0x26a0, 0x2227, 0xc022, 0x77c0, 0xad77,
-	0x46ad, 0xaa46, 0x60aa, 0x8560, 0x4785, 0xd747, 0x45d7, 0x2346, 0x5f23,
-	0x25f,	0x1d02, 0x71d,	0x8206, 0xc82,	0x180c, 0x3018, 0x4b30, 0x4b,
-	0x3001, 0x1230, 0x2d12, 0x8c2d, 0x148d, 0x4015, 0x5f3f, 0x3d5f, 0x6b3d,
-	0x396b, 0x473a, 0xf746, 0x44f7, 0x8945, 0x3489, 0xcb34, 0x84ca, 0xd984,
-	0xf0d9, 0xbcf0, 0x63bd, 0x3264, 0xf332, 0x45f3, 0x7346, 0x5673, 0xb056,
-	0xd3b0, 0x4ad4, 0x184b, 0x7d18, 0x6c7d, 0xbb6c, 0xfeba, 0xe0fe, 0x10e1,
-	0x5410, 0x2954, 0x9f28, 0x3a9f, 0x5a3a, 0xdb59, 0xbdc,	0xb40b, 0x1ab4,
-	0x131b, 0x5d12, 0x6d5c, 0xe16c, 0xb0e0, 0x89b0, 0xba88, 0xbb,	0x3c01,
-	0xe13b, 0x6fe1, 0x446f, 0xa344, 0x81a3, 0xfe81, 0xc7fd, 0x38c8, 0xb38,
-	0x1a0b, 0x6d19, 0xf36c, 0x47f3, 0x6d48, 0xb76d, 0xd3b7, 0xd8d2, 0x52d9,
-	0x4b53, 0xa54a, 0x34a5, 0xc534, 0x9bc4, 0xed9b, 0xbeed, 0x3ebe, 0x233e,
-	0x9f22, 0x4a9f, 0x774b, 0x4577, 0xa545, 0x64a5, 0xb65,	0x870b, 0x487,
-	0x9204, 0x5f91, 0xd55f, 0x35d5, 0x1a35, 0x71a,	0x7a07, 0x4e7a, 0xfc4e,
-	0x1efc, 0x481f, 0x7448, 0xde74, 0xa7dd, 0x1ea7, 0xaa1e, 0xcfaa, 0xfbcf,
-	0xedfb, 0x6eee, 0x386f, 0x4538, 0x6e45, 0xd96d, 0x11d9, 0x7912, 0x4b79,
-	0x494b, 0x6049, 0xac5f, 0x65ac, 0x1366, 0x5913, 0xe458, 0x7ae4, 0x387a,
-	0x3c38, 0xb03c, 0x76b0, 0x9376, 0xe193, 0x42e1, 0x7742, 0x6476, 0x3564,
-	0x3c35, 0x6a3c, 0xcc69, 0x94cc, 0x5d95, 0xe5e,	0xee0d, 0x4ced, 0xce4c,
-	0x52ce, 0xaa52, 0xdaaa, 0xe4da, 0x1de5, 0x4530, 0x5445, 0x3954, 0xb639,
-	0x81b6, 0x7381, 0x1574, 0xc215, 0x10c2, 0x3f10, 0x6b3f, 0xe76b, 0x7be7,
-	0xbc7b, 0xf7bb, 0x41f7, 0xcc41, 0x38cc, 0x4239, 0xa942, 0x4a9,	0xc504,
-	0x7cc4, 0x437c, 0x6743, 0xea67, 0x8dea, 0xe88d, 0xd8e8, 0xdcd8, 0x17dd,
-	0x5718, 0x958,	0xa609, 0x41a5, 0x5842, 0x159,	0x9f01, 0x269f, 0x5a26,
-	0x405a, 0xc340, 0xb4c3, 0xd4b4, 0xf4d3, 0xf1f4, 0x39f2, 0xe439, 0x67e4,
-	0x4168, 0xa441, 0xdda3, 0xdedd, 0x9df,	0xab0a, 0xa5ab, 0x9a6,	0xba09,
-	0x9ab9, 0xad9a, 0x5ae,	0xe205, 0xece2, 0xecec, 0x14ed, 0xd614, 0x6bd5,
-	0x916c, 0x3391, 0x6f33, 0x206f, 0x8020, 0x780,	0x7207, 0x2472, 0x8a23,
-	0xb689, 0x3ab6, 0xf739, 0x97f6, 0xb097, 0xa4b0, 0xe6a4, 0x88e6, 0x2789,
-	0xb28,	0x350b, 0x1f35, 0x431e, 0x1043, 0xc30f, 0x79c3, 0x379,	0x5703,
-	0x3256, 0x4732, 0x7247, 0x9d72, 0x489d, 0xd348, 0xa4d3, 0x7ca4, 0xbf7b,
-	0x45c0, 0x7b45, 0x337b, 0x4034, 0x843f, 0xd083, 0x35d0, 0x6335, 0x4d63,
-	0xe14c, 0xcce0, 0xfecc, 0x35ff, 0x5636, 0xf856, 0xeef8, 0x2def, 0xfc2d,
-	0x4fc,	0x6e04, 0xb66d, 0x78b6, 0xbb78, 0x3dbb, 0x9a3d, 0x839a, 0x9283,
-	0x593,	0xd504, 0x23d5, 0x5424, 0xd054, 0x61d0, 0xdb61, 0x17db, 0x1f18,
-	0x381f, 0x9e37, 0x679e, 0x1d68, 0x381d, 0x8038, 0x917f, 0x491,	0xbb04,
-	0x23bb, 0x4124, 0xd41,	0xa30c, 0x8ba3, 0x8b8b, 0xc68b, 0xd2c6, 0xebd2,
-	0x93eb, 0xbd93, 0x99bd, 0x1a99, 0xea19, 0x58ea, 0xcf58, 0x73cf, 0x1073,
-	0x9e10, 0x139e, 0xea13, 0xcde9, 0x3ecd, 0x883f, 0xf89,	0x180f, 0x2a18,
-	0x212a, 0xce20, 0x73ce, 0xf373, 0x60f3, 0xad60, 0x4093, 0x8e40, 0xb98e,
-	0xbfb9, 0xf1bf, 0x8bf1, 0x5e8c, 0xe95e, 0x14e9, 0x4e14, 0x1c4e, 0x7f1c,
-	0xe77e, 0x6fe7, 0xf26f, 0x13f2, 0x8b13, 0xda8a, 0x5fda, 0xea5f, 0x4eea,
-	0xa84f, 0x88a8, 0x1f88, 0x2820, 0x9728, 0x5a97, 0x3f5b, 0xb23f, 0x70b2,
-	0x2c70, 0x232d, 0xf623, 0x4f6,	0x905,	0x7509, 0xd675, 0x28d7, 0x9428,
-	0x3794, 0xf036, 0x2bf0, 0xba2c, 0xedb9, 0xd7ed, 0x59d8, 0xed59, 0x4ed,
-	0xe304, 0x18e3, 0x5c19, 0x3d5c, 0x753d, 0x6d75, 0x956d, 0x7f95, 0xc47f,
-	0x83c4, 0xa84,	0x2e0a, 0x5f2e, 0xb95f, 0x77b9, 0x6d78, 0xf46d, 0x1bf4,
-	0xed1b, 0xd6ed, 0xe0d6, 0x5e1,	0x3905, 0x5638, 0xa355, 0x99a2, 0xbe99,
-	0xb4bd, 0x85b4, 0x2e86, 0x542e, 0x6654, 0xd765, 0x73d7, 0x3a74, 0x383a,
-	0x2638, 0x7826, 0x7677, 0x9a76, 0x7e99, 0x2e7e, 0xea2d, 0xa6ea, 0x8a7,
-	0x109,	0x3300, 0xad32, 0x5fad, 0x465f, 0x2f46, 0xc62f, 0xd4c5, 0xad5,
-	0xcb0a, 0x4cb,	0xb004, 0x7baf, 0xe47b, 0x92e4, 0x8e92, 0x638e, 0x1763,
-	0xc17,	0xf20b, 0x1ff2, 0x8920, 0x5889, 0xcb58, 0xf8cb, 0xcaf8, 0x84cb,
-	0x9f84, 0x8a9f, 0x918a, 0x4991, 0x8249, 0xff81, 0x46ff, 0x5046, 0x5f50,
-	0x725f, 0xf772, 0x8ef7, 0xe08f, 0xc1e0, 0x1fc2, 0x9e1f, 0x8b9d, 0x108b,
-	0x411,	0x2b04, 0xb02a, 0x1fb0, 0x1020, 0x7a0f, 0x587a, 0x8958, 0xb188,
-	0xb1b1, 0x49b2, 0xb949, 0x7ab9, 0x917a, 0xfc91, 0xe6fc, 0x47e7, 0xbc47,
-	0x8fbb, 0xea8e, 0x34ea, 0x2635, 0x1726, 0x9616, 0xc196, 0xa6c1, 0xf3a6,
-	0x11f3, 0x4811, 0x3e48, 0xeb3e, 0xf7ea, 0x1bf8, 0xdb1c, 0x8adb, 0xe18a,
-	0x42e1, 0x9d42, 0x5d9c, 0x6e5d, 0x286e, 0x4928, 0x9a49, 0xb09c, 0xa6b0,
-	0x2a7,	0xe702, 0xf5e6, 0x9af5, 0xf9b,	0x810f, 0x8080, 0x180,	0x1702,
-	0x5117, 0xa650, 0x11a6, 0x1011, 0x550f, 0xd554, 0xbdd5, 0x6bbe, 0xc66b,
-	0xfc7,	0x5510, 0x5555, 0x7655, 0x177,	0x2b02, 0x6f2a, 0xb70,	0x9f0b,
-	0xcf9e, 0xf3cf, 0x3ff4, 0xcb40, 0x8ecb, 0x768e, 0x5277, 0x8652, 0x9186,
-	0x9991, 0x5099, 0xd350, 0x93d3, 0x6d94, 0xe6d,	0x530e, 0x3153, 0xa531,
-	0x64a5, 0x7964, 0x7c79, 0x467c, 0x1746, 0x3017, 0x3730, 0x538,	0x5,
-	0x1e00, 0x5b1e, 0x955a, 0xae95, 0x3eaf, 0xff3e, 0xf8ff, 0xb2f9, 0xa1b3,
-	0xb2a1, 0x5b2,	0xad05, 0x7cac, 0x2d7c, 0xd32c, 0x80d2, 0x7280, 0x8d72,
-	0x1b8e, 0x831b, 0xac82, 0xfdac, 0xa7fd, 0x15a8, 0xd614, 0xe0d5, 0x7be0,
-	0xb37b, 0x61b3, 0x9661, 0x9d95, 0xc79d, 0x83c7, 0xd883, 0xead7, 0xceb,
-	0xf60c, 0xa9f5, 0x19a9, 0xa019, 0x8f9f, 0xd48f, 0x3ad5, 0x853a, 0x985,
-	0x5309, 0x6f52, 0x1370, 0x6e13, 0xa96d, 0x98a9, 0x5198, 0x9f51, 0xb69f,
-	0xa1b6, 0x2ea1, 0x672e, 0x2067, 0x6520, 0xaf65, 0x6eaf, 0x7e6f, 0xee7e,
-	0x17ef, 0xa917, 0xcea8, 0x9ace, 0xff99, 0x5dff, 0xdf5d, 0x38df, 0xa39,
-	0x1c0b, 0xe01b, 0x46e0, 0xcb46, 0x90cb, 0xba90, 0x4bb,	0x9104, 0x9d90,
-	0xc89c, 0xf6c8, 0x6cf6, 0x886c, 0x1789, 0xbd17, 0x70bc, 0x7e71, 0x17e,
-	0x1f01, 0xa01f, 0xbaa0, 0x14bb, 0xfc14, 0x7afb, 0xa07a, 0x3da0, 0xbf3d,
-	0x48bf, 0x8c48, 0x968b, 0x9d96, 0xfd9d, 0x96fd, 0x9796, 0x6b97, 0xd16b,
-	0xf4d1, 0x3bf4, 0x253c, 0x9125, 0x6691, 0xc166, 0x34c1, 0x5735, 0x1a57,
-	0xdc19, 0x77db, 0x8577, 0x4a85, 0x824a, 0x9182, 0x7f91, 0xfd7f, 0xb4c3,
-	0xb5b4, 0xb3b5, 0x7eb3, 0x617e, 0x4e61, 0xa4f,	0x530a, 0x3f52, 0xa33e,
-	0x34a3, 0x9234, 0xf091, 0xf4f0, 0x1bf5, 0x311b, 0x9631, 0x6a96, 0x386b,
-	0x1d39, 0xe91d, 0xe8e9, 0x69e8, 0x426a, 0xee42, 0x89ee, 0x368a, 0x2837,
-	0x7428, 0x5974, 0x6159, 0x1d62, 0x7b1d, 0xf77a, 0x7bf7, 0x6b7c, 0x696c,
-	0xf969, 0x4cf9, 0x714c, 0x4e71, 0x6b4e, 0x256c, 0x6e25, 0xe96d, 0x94e9,
-	0x8f94, 0x3e8f, 0x343e, 0x4634, 0xb646, 0x97b5, 0x8997, 0xe8a,	0x900e,
-	0x8090, 0xfd80, 0xa0fd, 0x16a1, 0xf416, 0xebf4, 0x95ec, 0x1196, 0x8911,
-	0x3d89, 0xda3c, 0x9fd9, 0xd79f, 0x4bd7, 0x214c, 0x3021, 0x4f30, 0x994e,
-	0x5c99, 0x6f5d, 0x326f, 0xab31, 0x6aab, 0xe969, 0x90e9, 0x1190, 0xff10,
-	0xa2fe, 0xe0a2, 0x66e1, 0x4067, 0x9e3f, 0x2d9e, 0x712d, 0x8170, 0xd180,
-	0xffd1, 0x25ff, 0x3826, 0x2538, 0x5f24, 0xc45e, 0x1cc4, 0xdf1c, 0x93df,
-	0xc793, 0x80c7, 0x2380, 0xd223, 0x7ed2, 0xfc7e, 0x22fd, 0x7422, 0x1474,
-	0xb714, 0x7db6, 0x857d, 0xa85,	0xa60a, 0x88a6, 0x4289, 0x7842, 0xc278,
-	0xf7c2, 0xcdf7, 0x84cd, 0xae84, 0x8cae, 0xb98c, 0x1aba, 0x4d1a, 0x884c,
-	0x4688, 0xcc46, 0xd8cb, 0x2bd9, 0xbe2b, 0xa2be, 0x72a2, 0xf772, 0xd2f6,
-	0x75d2, 0xc075, 0xa3c0, 0x63a3, 0xae63, 0x8fae, 0x2a90, 0x5f2a, 0xef5f,
-	0x5cef, 0xa05c, 0x89a0, 0x5e89, 0x6b5e, 0x736b, 0x773,	0x9d07, 0xe99c,
-	0x27ea, 0x2028, 0xc20,	0x980b, 0x4797, 0x2848, 0x9828, 0xc197, 0x48c2,
-	0x2449, 0x7024, 0x570,	0x3e05, 0xd3e,	0xf60c, 0xbbf5, 0x69bb, 0x3f6a,
-	0x740,	0xf006, 0xe0ef, 0xbbe0, 0xadbb, 0x56ad, 0xcf56, 0xbfce, 0xa9bf,
-	0x205b, 0x6920, 0xae69, 0x50ae, 0x2050, 0xf01f, 0x27f0, 0x9427, 0x8993,
-	0x8689, 0x4087, 0x6e40, 0xb16e, 0xa1b1, 0xe8a1, 0x87e8, 0x6f88, 0xfe6f,
-	0x4cfe, 0xe94d, 0xd5e9, 0x47d6, 0x3148, 0x5f31, 0xc35f, 0x13c4, 0xa413,
-	0x5a5,	0x2405, 0xc223, 0x66c2, 0x3667, 0x5e37, 0x5f5e, 0x2f5f, 0x8c2f,
-	0xe48c, 0xd0e4, 0x4d1,	0xd104, 0xe4d0, 0xcee4, 0xfcf,	0x480f, 0xa447,
-	0x5ea4, 0xff5e, 0xbefe, 0x8dbe, 0x1d8e, 0x411d, 0x1841, 0x6918, 0x5469,
-	0x1155, 0xc611, 0xaac6, 0x37ab, 0x2f37, 0xca2e, 0x87ca, 0xbd87, 0xabbd,
-	0xb3ab, 0xcb4,	0xce0c, 0xfccd, 0xa5fd, 0x72a5, 0xf072, 0x83f0, 0xfe83,
-	0x97fd, 0xc997, 0xb0c9, 0xadb0, 0xe6ac, 0x88e6, 0x1088, 0xbe10, 0x16be,
-	0xa916, 0xa3a8, 0x46a3, 0x5447, 0xe953, 0x84e8, 0x2085, 0xa11f, 0xfa1,
-	0xdd0f, 0xbedc, 0x5abe, 0x805a, 0xc97f, 0x6dc9, 0x826d, 0x4a82, 0x934a,
-	0x5293, 0xd852, 0xd3d8, 0xadd3, 0xf4ad, 0xf3f4, 0xfcf3, 0xfefc, 0xcafe,
-	0xb7ca, 0x3cb8, 0xa13c, 0x18a1, 0x1418, 0xea13, 0x91ea, 0xf891, 0x53f8,
-	0xa254, 0xe9a2, 0x87ea, 0x4188, 0x1c41, 0xdc1b, 0xf5db, 0xcaf5, 0x45ca,
-	0x6d45, 0x396d, 0xde39, 0x90dd, 0x1e91, 0x1e,	0x7b00, 0x6a7b, 0xa46a,
-	0xc9a3, 0x9bc9, 0x389b, 0x1139, 0x5211, 0x1f52, 0xeb1f, 0xabeb, 0x48ab,
-	0x9348, 0xb392, 0x17b3, 0x1618, 0x5b16, 0x175b, 0xdc17, 0xdedb, 0x1cdf,
-	0xeb1c, 0xd1ea, 0x4ad2, 0xd4b,	0xc20c, 0x24c2, 0x7b25, 0x137b, 0x8b13,
-	0x618b, 0xa061, 0xff9f, 0xfffe, 0x72ff, 0xf572, 0xe2f5, 0xcfe2, 0xd2cf,
-	0x75d3, 0x6a76, 0xc469, 0x1ec4, 0xfc1d, 0x59fb, 0x455a, 0x7a45, 0xa479,
-	0xb7a4
+	0x83da, 0x2ac2, 0x6211, 0x49ba, 0x14af, 0x3045, 0x9340, 0x9cb0, 0xc3b4,
+	0x5b20, 0x2cdf, 0x4e03, 0x8552, 0x6cfb, 0x37f0, 0x5386, 0xb681, 0xbff1,
+	0xe6f5, 0x7e61, 0x5020, 0x916a, 0x8771, 0x6f1a, 0x3a0f, 0x55a5, 0xb8a0,
+	0xc210, 0xe914, 0x8080, 0x523f, 0x9389, 0x704a, 0x6d81, 0x3876, 0x540c,
+	0xb707, 0xc077, 0xe77b, 0x7ee7, 0x50a6, 0x91f0, 0x6eb1, 0xc58a, 0xfb5a,
+	0x16f1, 0x79ec, 0x835c, 0xaa60, 0x41cc, 0x138b, 0x54d5, 0x3196, 0x886f,
+	0x545c, 0xac6a, 0x0f66, 0x18d6, 0x3fda, 0xd745, 0xa904, 0xea4e, 0xc70f,
+	0x1de9, 0xe9d5, 0x06ab, 0x687e, 0x71ee, 0x98f2, 0x305e, 0x021d, 0x4367,
+	0x2028, 0x7701, 0x42ee, 0x5fc3, 0x3c73, 0x3a9f, 0x61a3, 0xf90e, 0xcacd,
+	0x0c18, 0xe8d8, 0x3fb2, 0x0b9f, 0x2874, 0x0524, 0x1f07, 0x79fa, 0x1166,
+	0xe324, 0x246f, 0x0130, 0x5809, 0x23f6, 0x40cb, 0x1d7b, 0x375e, 0x57d9,
+	0x4671, 0x1830, 0x597a, 0x363b, 0x8d14, 0x5901, 0x75d6, 0x5286, 0x6c69,
+	0x8ce4, 0xec7a, 0xfc99, 0x3de4, 0x1aa5, 0x717e, 0x3d6b, 0x5a40, 0x36f0,
+	0x50d3, 0x714e, 0xd0e4, 0xaa9f, 0xdae8, 0xb7a9, 0x0e83, 0xda6f, 0xf744,
+	0xd3f4, 0xedd7, 0x0e53, 0x6de9, 0x47a4, 0xc7fd, 0xae39, 0x0513, 0xd0ff,
+	0xedd4, 0xca84, 0xe467, 0x04e3, 0x6479, 0x3e34, 0xbe8d, 0x3f6d, 0xde0e,
+	0xa9fb, 0xc6d0, 0xa380, 0xbd63, 0xddde, 0x3d75, 0x1730, 0x9789, 0x1869,
+	0x5e16, 0x1290, 0x2f65, 0x0c15, 0x25f8, 0x4673, 0xa609, 0x7fc4, 0x001e,
+	0x80fd, 0xc6aa, 0x9c46, 0x5da6, 0x3a56, 0x5439, 0x74b4, 0xd44a, 0xae05,
+	0x2e5f, 0xaf3e, 0xf4eb, 0xca87, 0xf556, 0xf90b, 0x12ef, 0x336a, 0x9300,
+	0x6cbb, 0xed14, 0x6df4, 0xb3a1, 0x893d, 0xb40c, 0x8233, 0x362e, 0x56a9,
+	0xb63f, 0x8ffa, 0x1054, 0x9133, 0xd6e0, 0xac7c, 0xd74b, 0xa572, 0x5471,
+	0xffcf, 0x5f66, 0x3921, 0xb97a, 0x3a5a, 0x8007, 0x55a3, 0x8072, 0x4e99,
+	0xfd97, 0x78d3, 0x9379, 0x6d34, 0xed8d, 0x6e6d, 0xb41a, 0x89b6, 0xb485,
+	0x82ac, 0x31ab, 0xace6, 0xdaeb, 0x505f, 0xd0b8, 0x5198, 0x9745, 0x6ce1,
+	0x97b0, 0x65d7, 0x14d6, 0x9011, 0xbe16, 0x2404, 0xf408, 0x74e8, 0xba95,
+	0x9031, 0xbb00, 0x8927, 0x3826, 0xb361, 0xe166, 0x4754, 0x3984, 0x5b05,
+	0xa0b2, 0x764e, 0xa11d, 0x6f44, 0x1e43, 0x997e, 0xc783, 0x2d71, 0x1fa1,
+	0xded5, 0x8037, 0x55d3, 0x80a2, 0x4ec9, 0xfdc7, 0x7903, 0xa708, 0x0cf6,
+	0xff25, 0xbe5a, 0xcd18, 0xf63c, 0x210c, 0xef32, 0x9e31, 0x196d, 0x4772,
+	0xad5f, 0x9f8f, 0x5ec4, 0x6d82, 0x2c93, 0x4751, 0x1578, 0xc476, 0x3fb2,
+	0x6db7, 0xd3a4, 0xc5d4, 0x8509, 0x93c7, 0x52d8, 0x754d, 0x951e, 0x441d,
+	0xbf58, 0xed5d, 0x534b, 0x457b, 0x04b0, 0x136e, 0xd27e, 0xf4f3, 0x4b97,
+	0xc33d, 0x3e79, 0x6c7e, 0xd26b, 0xc49b, 0x83d0, 0x928e, 0x519f, 0x7414,
+	0xcab7, 0x5dc1, 0xf8cb, 0x26d1, 0x8cbe, 0x7eee, 0x3e23, 0x4ce1, 0x0bf2,
+	0x2e67, 0x850a, 0x1814, 0xcdef, 0x5135, 0xb722, 0xa952, 0x6887, 0x7745,
+	0x3656, 0x58cb, 0xaf6e, 0x4278, 0xf853, 0xb310, 0x8c53, 0x7e83, 0x3db8,
+	0x4c76, 0x0b87, 0x2dfc, 0x849f, 0x17a9, 0xcd84, 0x8841, 0xc3f1, 0xb05c,
+	0x6f91, 0x7e4f, 0x3d60, 0x5fd5, 0xb678, 0x4982, 0xff5d, 0xba1a, 0xf5ca,
+	0xe8dc, 0xc092, 0xcf50, 0x8e61, 0xb0d6, 0x077a, 0x9a83, 0x505f, 0x0b1c,
+	0x46cc, 0x39de, 0x7bb6, 0x5415, 0x1326, 0x359b, 0x8c3e, 0x1f48, 0xd523,
+	0x8fe0, 0xcb90, 0xbea2, 0x007b, 0xf7a0, 0xe520, 0x0796, 0x5e39, 0xf142,
+	0xa71e, 0x61db, 0x9d8b, 0x909d, 0xd275, 0xc99b, 0xb80c, 0xa1a8, 0xf84b,
+	0x8b55, 0x4131, 0xfbed, 0x379e, 0x2ab0, 0x6c88, 0x63ae, 0x521f, 0x3ac3,
+	0x061c, 0x9925, 0x4f01, 0x09be, 0x456e, 0x3880, 0x7a58, 0x717e, 0x5fef,
+	0x4893, 0xd58f, 0xd9f0, 0x8fcc, 0x4a89, 0x8639, 0x794b, 0xbb23, 0xb249,
+	0xa0ba, 0x895e, 0x165b, 0xedda, 0x810e, 0x3bcb, 0x777b, 0x6a8d, 0xac65,
+	0xa38b, 0x91fc, 0x7aa0, 0x079d, 0xdf1c, 0x0a1e, 0x7cba, 0xb86a, 0xab7c,
+	0xed54, 0xe47a, 0xd2eb, 0xbb8f, 0x488c, 0x200c, 0x4b0d, 0x9d88, 0x95f5,
+	0x8907, 0xcadf, 0xc205, 0xb076, 0x991a, 0x2617, 0xfd96, 0x2898, 0x7b13,
+	0xf6a2, 0x3264, 0x743c, 0x6b62, 0x59d3, 0x4277, 0xcf73, 0xa6f3, 0xd1f4,
+	0x2470, 0x9fff, 0x88ec, 0xe132, 0xd858, 0xc6c9, 0xaf6d, 0x3c6a, 0x13ea,
+	0x3eeb, 0x9166, 0x0cf6, 0xf5e2, 0x2c46, 0x227d, 0x10ee, 0xf991, 0x868e,
+	0x5e0e, 0x890f, 0xdb8a, 0x571a, 0x4007, 0x766a, 0xb616, 0x5631, 0x3ed5,
+	0xcbd1, 0xa351, 0xce52, 0x20ce, 0x9c5d, 0x854a, 0xbbad, 0xfb59, 0xe067,
+	0x0325, 0x9021, 0x67a1, 0x92a2, 0xe51d, 0x60ad, 0x499a, 0x7ffd, 0xbfa9,
+	0xa4b7, 0x78d3, 0x9d0f, 0x748f, 0x9f90, 0xf20b, 0x6d9b, 0x5688, 0x8ceb,
+	0xcc97, 0xb1a5, 0x85c1, 0xee49, 0x32b7, 0x5db8, 0xb033, 0x2bc3, 0x14b0,
+	0x4b13, 0x8abf, 0x6fcd, 0x43e9, 0xac71, 0xe4d9, 0x6692, 0xb90d, 0x349d,
+	0x1d8a, 0x53ed, 0x9399, 0x78a7, 0x4cc3, 0xb54b, 0xedb3, 0x5a4e, 0xca9c,
+	0x462c, 0x2f19, 0x657c, 0xa528, 0x8a36, 0x5e52, 0xc6da, 0xff42, 0x6bdd,
+	0x4b71, 0x5d88, 0x4675, 0x7cd8, 0xbc84, 0xa192, 0x75ae, 0xde36, 0x169f,
+	0x8339, 0x62cd, 0x7d20, 0xb978, 0xefdb, 0x2f88, 0x1496, 0xe8b1, 0x513a,
+	0x89a2, 0xf63c, 0xd5d0, 0xf023, 0x7c35, 0x185c, 0x5808, 0x3d16, 0x1132,
+	0x79ba, 0xb222, 0x1ebd, 0xfe50, 0x18a4, 0xa4b5, 0x0dc3, 0x2d07, 0x1215,
+	0xe630, 0x4eb9, 0x8721, 0xf3bb, 0xd34f, 0xeda2, 0x79b4, 0xe2c1, 0xa671,
+	0xbf99, 0x93b5, 0xfc3d, 0x34a6, 0xa140, 0x80d4, 0x9b27, 0x2739, 0x9046,
+	0x53f6, 0xe623, 0x1826, 0x80ae, 0xb916, 0x25b1, 0x0545, 0x1f98, 0xaba9,
+	0x14b7, 0xd866, 0x6a94, 0xfea5, 0x97c1, 0xd029, 0x3cc4, 0x1c58, 0x36ab,
+	0xc2bc, 0x2bca, 0xef79, 0x81a7, 0x15b9, 0x50d9, 0x99c6, 0x0661, 0xe5f4,
+	0x0048, 0x8c59, 0xf566, 0xb916, 0x4b44, 0xdf55, 0x1a76, 0x70dd, 0xc6b4,
+	0xa648, 0xc09b, 0x4cad, 0xb5ba, 0x796a, 0x0b98, 0x9fa9, 0xdac9, 0x3131,
+	0x00cb, 0xc13a, 0xdb8d, 0x679f, 0xd0ac, 0x945c, 0x268a, 0xba9b, 0xf5bb,
+	0x4c23, 0x1bbd, 0xd992, 0x0772, 0x9383, 0xfc90, 0xc040, 0x526e, 0xe67f,
+	0x21a0, 0x7807, 0x47a1, 0x0577, 0xa4cd, 0x2afb, 0x9408, 0x57b8, 0xe9e5,
+	0x7df7, 0xb917, 0x0f7f, 0xdf18, 0x9cee, 0x3c45, 0x9aaf, 0x5ba0, 0x1f50,
+	0xb17d, 0x458f, 0x80af, 0xd716, 0xa6b0, 0x6486, 0x03dd, 0x6247, 0xce54,
+	0xb2b5, 0x44e3, 0xd8f4, 0x1415, 0x6a7c, 0x3a16, 0xf7eb, 0x9742, 0xf5ac,
+	0x61ba, 0xc44b, 0x654f, 0xf960, 0x3481, 0x8ae8, 0x5a82, 0x1858, 0xb7ae,
+	0x1619, 0x8226, 0xe4b7, 0x1920, 0xdf0d, 0x1a2e, 0x7095, 0x402f, 0xfe04,
+	0x9d5b, 0xfbc5, 0x67d3, 0xca64, 0xfecc, 0x68ea, 0x8e1c, 0xe483, 0xb41d,
+	0x71f3, 0x114a, 0x6fb4, 0xdbc1, 0x3e53, 0x72bb, 0xdcd8, 0x6b24, 0x7b76,
+	0x4b10, 0x08e6, 0xa83c, 0x06a7, 0x72b4, 0xd545, 0x09ae, 0x73cb, 0x0217,
+	0x9603,
 };
 
 static u8 tmp_buf[TEST_BUFLEN];
@@ -578,15 +448,19 @@ static void test_csum_no_carry_inputs(struct kunit *test)
 static void test_ip_fast_csum(struct kunit *test)
 {
 	__sum16 csum_result, expected;
-
-	for (int len = IPv4_MIN_WORDS; len < IPv4_MAX_WORDS; len++) {
-		for (int index = 0; index < NUM_IP_FAST_CSUM_TESTS; index++) {
-			csum_result = ip_fast_csum(random_buf + index, len);
-			expected = (__force __sum16)
-				expected_fast_csum[(len - IPv4_MIN_WORDS) *
-						   NUM_IP_FAST_CSUM_TESTS +
-						   index];
-			CHECK_EQ(expected, csum_result);
+	int num_tests = (MAX_LEN / WORD_ALIGNMENT - IPv4_MAX_WORDS * WORD_ALIGNMENT);
+
+	for (int i = 0; i < num_tests; i++) {
+		memcpy(&tmp_buf[IP_ALIGNMENT],
+		       random_buf + (i * WORD_ALIGNMENT),
+		       IPv4_MAX_WORDS * WORD_ALIGNMENT);
+
+		for (int len = IPv4_MIN_WORDS; len <= IPv4_MAX_WORDS; len++) {
+			int index = (len - IPv4_MIN_WORDS) +
+				 i * ((IPv4_MAX_WORDS - IPv4_MIN_WORDS) + 1);
+			csum_result = ip_fast_csum(tmp_buf + IP_ALIGNMENT, len);
+			expected = (__force __sum16)htons(expected_fast_csum[index]);
+			CHECK_EQ(csum_result, expected);
 		}
 	}
 }
@@ -594,29 +468,42 @@ static void test_ip_fast_csum(struct kunit *test)
 static void test_csum_ipv6_magic(struct kunit *test)
 {
 #if defined(CONFIG_NET)
-	const struct in6_addr *saddr;
-	const struct in6_addr *daddr;
-	unsigned int len;
-	unsigned char proto;
-	unsigned int csum;
-
-	const int daddr_offset = sizeof(struct in6_addr);
-	const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr);
-	const int proto_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr) +
-			     sizeof(int);
-	const int csum_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr) +
-			    sizeof(int) + sizeof(char);
-
-	for (int i = 0; i < NUM_IPv6_TESTS; i++) {
-		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);
-		proto = *(random_buf + i + proto_offset);
-		csum = *(unsigned int *)(random_buf + i + csum_offset);
-		CHECK_EQ((__force __sum16)expected_csum_ipv6_magic[i],
-			 csum_ipv6_magic(saddr, daddr, len, proto,
-					 (__force __wsum)csum));
+	__sum16 csum_result, expected;
+	struct csum_ipv6_magic_data {
+		const struct in6_addr saddr;
+		const struct in6_addr daddr;
+		unsigned int len;
+		__wsum csum;
+		unsigned char proto;
+	} data, *data_ptr;
+	int num_tests = MAX_LEN / WORD_ALIGNMENT - sizeof(struct csum_ipv6_magic_data);
+
+	for (int i = 0; i < num_tests; i++) {
+		data_ptr = (struct csum_ipv6_magic_data *)(random_buf + (i * WORD_ALIGNMENT));
+
+		cpu_to_be32_array((__be32 *)&data.saddr, (const u32 *)&data_ptr->saddr,
+				  sizeof_field(struct csum_ipv6_magic_data, saddr) / 4);
+		cpu_to_be32_array((__be32 *)&data.daddr, (const u32 *)&data_ptr->daddr,
+				  sizeof_field(struct csum_ipv6_magic_data, daddr) / 4);
+		data.len = data_ptr->len;
+		data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
+		/*
+		 * proto must be zero to be compatible between big-endian and
+		 * little-endian CPUs. On little-endian CPUs, proto is
+		 * converted to a big-endian 32-bit value before the checksum
+		 * operation. This causes proto to be in the most significant
+		 * 8 bits on a little-endian CPU. On big-endian CPUs proto will
+		 * remain in the least significant 8 bits. There does not exist
+		 * a transformation to an arbitrary proto that will allow
+		 * csum_ipv6_magic to return the same value on a big-endian and
+		 * little-endian CPUs.
+		 */
+		data.proto = 0;
+		csum_result = csum_ipv6_magic(&data.saddr, &data.daddr,
+					      data.len, data.proto,
+					      data.csum);
+		expected = (__force __sum16)expected_csum_ipv6_magic[i];
+		CHECK_EQ(csum_result, expected);
 	}
 #endif /* !CONFIG_NET */
 }
@@ -639,3 +526,4 @@ kunit_test_suites(&checksum_test_suite);
 
 MODULE_AUTHOR("Noah Goldstein <goldstein.w.n@gmail.com>");
 MODULE_LICENSE("GPL");
+

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-08  0:22 [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Charlie Jenkins
  2024-02-08  0:22 ` [PATCH v6 1/2] lib: checksum: Fix type casting in checksum kunits Charlie Jenkins
  2024-02-08  0:22 ` [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests Charlie Jenkins
@ 2024-02-08  1:45 ` Andrew Morton
  2024-02-08  2:09   ` Charlie Jenkins
  2024-02-08  4:15 ` Guenter Roeck
  2024-02-11 19:18 ` Guenter Roeck
  4 siblings, 1 reply; 22+ messages in thread
From: Andrew Morton @ 2024-02-08  1:45 UTC (permalink / raw)
  To: Charlie Jenkins
  Cc: Guenter Roeck, David Laight, Palmer Dabbelt, linux-kernel,
	kernel test robot

On Wed, 07 Feb 2024 16:22:49 -0800 Charlie Jenkins <charlie@rivosinc.com> wrote:

> The ip_fast_csum and csum_ipv6_magic tests did not have the data
> types properly casted, and improperly misaligned data.

Neither this nor the two patch's changelogs describe *why* these changes
are needed.  They merely assert that we need to do this.

IOW, when fixing a bug please always describe the user-visible effects
of that bug.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-08  1:45 ` [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Andrew Morton
@ 2024-02-08  2:09   ` Charlie Jenkins
  2024-02-08  2:44     ` Guenter Roeck
  0 siblings, 1 reply; 22+ messages in thread
From: Charlie Jenkins @ 2024-02-08  2:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Guenter Roeck, David Laight, Palmer Dabbelt, linux-kernel,
	kernel test robot

On Wed, Feb 07, 2024 at 05:45:22PM -0800, Andrew Morton wrote:
> On Wed, 07 Feb 2024 16:22:49 -0800 Charlie Jenkins <charlie@rivosinc.com> wrote:
> 
> > The ip_fast_csum and csum_ipv6_magic tests did not have the data
> > types properly casted, and improperly misaligned data.
> 
> Neither this nor the two patch's changelogs describe *why* these changes
> are needed.  They merely assert that we need to do this.
> 
> IOW, when fixing a bug please always describe the user-visible effects
> of that bug.
> 

I can add a description that says that the tests are being fixed because
they caused failures on systems without misaligned access support. As
for the casting patch it's a bit less pertinent but I can add that it
allows the code to pass sparse checks.

- Charlie


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-08  2:09   ` Charlie Jenkins
@ 2024-02-08  2:44     ` Guenter Roeck
  2024-02-08  2:47       ` Palmer Dabbelt
  0 siblings, 1 reply; 22+ messages in thread
From: Guenter Roeck @ 2024-02-08  2:44 UTC (permalink / raw)
  To: Charlie Jenkins, Andrew Morton
  Cc: David Laight, Palmer Dabbelt, linux-kernel, kernel test robot

On 2/7/24 18:09, Charlie Jenkins wrote:
> On Wed, Feb 07, 2024 at 05:45:22PM -0800, Andrew Morton wrote:
>> On Wed, 07 Feb 2024 16:22:49 -0800 Charlie Jenkins <charlie@rivosinc.com> wrote:
>>
>>> The ip_fast_csum and csum_ipv6_magic tests did not have the data
>>> types properly casted, and improperly misaligned data.
>>
>> Neither this nor the two patch's changelogs describe *why* these changes
>> are needed.  They merely assert that we need to do this.
>>
>> IOW, when fixing a bug please always describe the user-visible effects
>> of that bug.
>>
> 
> I can add a description that says that the tests are being fixed because
> they caused failures on systems without misaligned access support. As
> for the casting patch it's a bit less pertinent but I can add that it
> allows the code to pass sparse checks.
> 
> - Charlie
> 
I don't know exactly what Andrew is asking for, but maybe including the
error log from the failed selftests and/or the sparse errors would be
sufficient ?

Not sure though if any of those count as "user visible".

Guenter




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-08  0:22 ` [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests Charlie Jenkins
@ 2024-02-08  2:45   ` Guenter Roeck
  2024-02-12  6:26   ` Al Viro
  1 sibling, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2024-02-08  2:45 UTC (permalink / raw)
  To: Charlie Jenkins; +Cc: David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel

On Wed, Feb 07, 2024 at 04:22:51PM -0800, Charlie Jenkins wrote:
> The test cases for ip_fast_csum and csum_ipv6_magic were using arbitrary
> alignment of data to iterate through random inputs. ip_fast_csum should
> have the data aligned along (14 + NET_IP_ALIGN) bytes and
> csum_ipv6_magic should have data aligned along 32-bit boundaries.
> 
> While this is being changed, fix up the awkward offset code in
> test_csum_ipv6_magic and use a struct instead.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> Fixes: 6f4c45cbcb00 ("kunit: Add tests for csum_ipv6_magic and ip_fast_csum")
> ---
[ ... ]
> @@ -639,3 +526,4 @@ kunit_test_suites(&checksum_test_suite);
>  
>  MODULE_AUTHOR("Noah Goldstein <goldstein.w.n@gmail.com>");
>  MODULE_LICENSE("GPL");
> +

git doesn't like the new extra empty line at the end.

Guenter

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-08  2:44     ` Guenter Roeck
@ 2024-02-08  2:47       ` Palmer Dabbelt
  0 siblings, 0 replies; 22+ messages in thread
From: Palmer Dabbelt @ 2024-02-08  2:47 UTC (permalink / raw)
  To: linux; +Cc: Charlie Jenkins, akpm, David.Laight, linux-kernel, lkp

On Wed, 07 Feb 2024 18:44:42 PST (-0800), linux@roeck-us.net wrote:
> On 2/7/24 18:09, Charlie Jenkins wrote:
>> On Wed, Feb 07, 2024 at 05:45:22PM -0800, Andrew Morton wrote:
>>> On Wed, 07 Feb 2024 16:22:49 -0800 Charlie Jenkins <charlie@rivosinc.com> wrote:
>>>
>>>> The ip_fast_csum and csum_ipv6_magic tests did not have the data
>>>> types properly casted, and improperly misaligned data.
>>>
>>> Neither this nor the two patch's changelogs describe *why* these changes
>>> are needed.  They merely assert that we need to do this.
>>>
>>> IOW, when fixing a bug please always describe the user-visible effects
>>> of that bug.
>>>
>>
>> I can add a description that says that the tests are being fixed because
>> they caused failures on systems without misaligned access support. As
>> for the casting patch it's a bit less pertinent but I can add that it
>> allows the code to pass sparse checks.
>>
>> - Charlie
>>
> I don't know exactly what Andrew is asking for, but maybe including the
> error log from the failed selftests and/or the sparse errors would be
> sufficient ?
>
> Not sure though if any of those count as "user visible".

Ya, for compiler warning/error workarounds I usually just include 
something like "without this, I get $ERROR".  Something like 
28ea54bade76 ("RISC-V: Don't rely on positional structure 
initialization").

For the aligned access on there was a fairly interesting discussion on 
why this hadn't tripped up before, I forget if it was on LKML or IRC (or 
Slack or just in the office).  That's worth including...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-08  0:22 [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Charlie Jenkins
                   ` (2 preceding siblings ...)
  2024-02-08  1:45 ` [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Andrew Morton
@ 2024-02-08  4:15 ` Guenter Roeck
  2024-02-11 19:18 ` Guenter Roeck
  4 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2024-02-08  4:15 UTC (permalink / raw)
  To: Charlie Jenkins
  Cc: David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel,
	kernel test robot

On Wed, Feb 07, 2024 at 04:22:49PM -0800, Charlie Jenkins wrote:
> The ip_fast_csum and csum_ipv6_magic tests did not have the data
> types properly casted, and improperly misaligned data.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>

I still get:

Failed unit tests:
	mips:malta:checksum
	mips64:malta:checksum
	mipsel:malta:checksum
	mipsel64:malta:checksum
	parisc:B160L:checksum
	parisc:C3700:checksum
	parisc64:C3700:checksum
	sh:rts7751r2dplus_defconfig:checksum

on parisc/parisc64:

    # test_ip_fast_csum: ASSERTION FAILED at lib/checksum_kunit.c:463
    Expected ( u64)csum_result == ( u64)expected, but
        ( u64)csum_result == 33754 (0x83da)
        ( u64)expected == 10946 (0x2ac2)
    not ok 4 test_ip_fast_csum
    ok 5 test_csum_ipv6_magic
# checksum: pass:4 fail:1 skip:0 total:5
# Totals: pass:4 fail:1 skip:0 total:5

mipsel/mipsel64 (little endian):

    # test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:506
    Expected ( u64)csum_result == ( u64)expected, but
        ( u64)csum_result == 18588 (0x489c)
        ( u64)expected == 12357 (0x3045)
    not ok 5 test_csum_ipv6_magic
# checksum: pass:4 fail:1 skip:0 total:5
# Totals: pass:4 fail:1 skip:0 total:5

mips (big endian):

    # test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:506
    Expected ( u64)csum_result == ( u64)expected, but
        ( u64)csum_result == 59728 (0xe950)
        ( u64)expected == 12357 (0x3045)
    not ok 5 test_csum_ipv6_magic
# checksum: pass:4 fail:1 skip:0 total:5
# Totals: pass:4 fail:1 skip:0 total:5

I noticed that csum_result varies across tests for some reason. On parisc/parisc64
the value is unexpected but always the same.

sh (little endian; ok, this isn't entirely fair, this test wasn't enabled before):

    KTAP version 1
    # Subtest: checksum
    # module: checksum_kunit
    1..5
    # test_csum_fixed_random_inputs: ASSERTION FAILED at lib/checksum_kunit.c:370
    Expected ( u64)result == ( u64)expec, but
        ( u64)result == 53378 (0xd082)
        ( u64)expec == 33488 (0x82d0)
    not ok 1 test_csum_fixed_random_inputs
    # test_csum_all_carry_inputs: ASSERTION FAILED at lib/checksum_kunit.c:395
    Expected ( u64)result == ( u64)expec, but
        ( u64)result == 65281 (0xff01)
        ( u64)expec == 65280 (0xff00)
    not ok 2 test_csum_all_carry_inputs
    # test_csum_no_carry_inputs: ASSERTION FAILED at lib/checksum_kunit.c:443
    Expected ( u64)result == ( u64)expec, but
        ( u64)result == 65535 (0xffff)
        ( u64)expec == 65534 (0xfffe)
    not ok 3 test_csum_no_carry_inputs
    ok 4 test_ip_fast_csum
    ok 5 test_csum_ipv6_magic
# checksum: pass:2 fail:3 skip:0 total:5
# Totals: pass:2 fail:3 skip:0 total:5

The result/expected values are always the same in the sh4 tests.
I'd take the test results for sh4 with a grain of salt; there is
at least some possibility that this is an emulation problem.

Guenter

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-08  0:22 [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Charlie Jenkins
                   ` (3 preceding siblings ...)
  2024-02-08  4:15 ` Guenter Roeck
@ 2024-02-11 19:18 ` Guenter Roeck
  2024-02-12  5:26   ` Charlie Jenkins
  2024-02-12 12:46   ` Geert Uytterhoeven
  4 siblings, 2 replies; 22+ messages in thread
From: Guenter Roeck @ 2024-02-11 19:18 UTC (permalink / raw)
  To: Charlie Jenkins, David Laight, Palmer Dabbelt, Andrew Morton
  Cc: linux-kernel, kernel test robot

Hi,

On 2/7/24 16:22, Charlie Jenkins wrote:
> The ip_fast_csum and csum_ipv6_magic tests did not have the data
> types properly casted, and improperly misaligned data.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>

I sorted out most of the problems with this version, but I still get:

     # test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:513
     Expected ( u64)csum_result == ( u64)expected, but
         ( u64)csum_result == 16630 (0x40f6)
         ( u64)expected == 65535 (0xffff)
     not ok 5 test_csum_ipv6_magic

on m68k:q800. This is suspicious because there is no 0xffff in
expected_csum_ipv6_magic[]. With some debugging information:

####### num_tests=86 i=84 expect array size=84
####### MAX_LEN=512 WORD_ALIGNMENT=4 magic data size=42

That means the loop

	for (int i = 0; i < num_tests; i++) {
		...
		expected = (__force __sum16)expected_csum_ipv6_magic[i];
		...
	}

will access data beyond the end of the expected_csum_ipv6_magic[] array,
possibly because m68k doesn't pad struct csum_ipv6_magic_data to 44 bytes.

In this context, is the comment about proto having to be 0 really true ?
It seems to me that the calculated checksum must be identical on both
little and big endian systems. After all, they need to be able to talk
to each other.

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-11 19:18 ` Guenter Roeck
@ 2024-02-12  5:26   ` Charlie Jenkins
  2024-02-12 17:10     ` Guenter Roeck
  2024-02-12 12:46   ` Geert Uytterhoeven
  1 sibling, 1 reply; 22+ messages in thread
From: Charlie Jenkins @ 2024-02-12  5:26 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel,
	kernel test robot

On Sun, Feb 11, 2024 at 11:18:36AM -0800, Guenter Roeck wrote:
> Hi,
> 
> On 2/7/24 16:22, Charlie Jenkins wrote:
> > The ip_fast_csum and csum_ipv6_magic tests did not have the data
> > types properly casted, and improperly misaligned data.
> > 
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> 
> I sorted out most of the problems with this version, but I still get:
> 
>     # test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:513
>     Expected ( u64)csum_result == ( u64)expected, but
>         ( u64)csum_result == 16630 (0x40f6)
>         ( u64)expected == 65535 (0xffff)
>     not ok 5 test_csum_ipv6_magic
> 
> on m68k:q800. This is suspicious because there is no 0xffff in
> expected_csum_ipv6_magic[]. With some debugging information:
> 
> ####### num_tests=86 i=84 expect array size=84
> ####### MAX_LEN=512 WORD_ALIGNMENT=4 magic data size=42
> 
> That means the loop
> 
> 	for (int i = 0; i < num_tests; i++) {
> 		...
> 		expected = (__force __sum16)expected_csum_ipv6_magic[i];
> 		...
> 	}
> 
> will access data beyond the end of the expected_csum_ipv6_magic[] array,
> possibly because m68k doesn't pad struct csum_ipv6_magic_data to 44 bytes.

Okay I will check that out.

> 
> In this context, is the comment about proto having to be 0 really true ?
> It seems to me that the calculated checksum must be identical on both
> little and big endian systems. After all, they need to be able to talk
> to each other.

I agree, but I couldn't find a solution other than setting it to zero.
Maybe I am missing something simple...

- Charlie

> 
> Thanks,
> Guenter
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-08  0:22 ` [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests Charlie Jenkins
  2024-02-08  2:45   ` Guenter Roeck
@ 2024-02-12  6:26   ` Al Viro
  2024-02-12 17:18     ` Guenter Roeck
  1 sibling, 1 reply; 22+ messages in thread
From: Al Viro @ 2024-02-12  6:26 UTC (permalink / raw)
  To: Charlie Jenkins
  Cc: Guenter Roeck, David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel

On Wed, Feb 07, 2024 at 04:22:51PM -0800, Charlie Jenkins wrote:
> +	struct csum_ipv6_magic_data {
> +		const struct in6_addr saddr;
> +		const struct in6_addr daddr;
> +		unsigned int len;
> +		__wsum csum;
> +		unsigned char proto;
> +	} data, *data_ptr;

Huh?

> +	int num_tests = MAX_LEN / WORD_ALIGNMENT - sizeof(struct csum_ipv6_magic_data);
> +
> +	for (int i = 0; i < num_tests; i++) {
> +		data_ptr = (struct csum_ipv6_magic_data *)(random_buf + (i * WORD_ALIGNMENT));
> +
> +		cpu_to_be32_array((__be32 *)&data.saddr, (const u32 *)&data_ptr->saddr,
> +				  sizeof_field(struct csum_ipv6_magic_data, saddr) / 4);
> +		cpu_to_be32_array((__be32 *)&data.daddr, (const u32 *)&data_ptr->daddr,
> +				  sizeof_field(struct csum_ipv6_magic_data, daddr) / 4);
> +		data.len = data_ptr->len;
> +		data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);

What are those cpu_to_be32() about?  Checksum calculations *DO* *NOT* involve
any endianness conversions.  At any point.

Replace those assignments with memcpy() and be done with that - that will take
care of unaligned accesses.

Result will have host-independent memory representation.  The only place where you
might want to play with byteswaps (only 16-bit ones) is if you initialized the
array of expected results with u16 constants.  That will have opposite memory
representations on l-e and b-e, so you'll need to byteswap to compare with
what you get from function.  Alternatively, make it an array of bytes and
do
	sum16 = csum_ipv6_magic(saddr, daddr, len, proto, csum);
	if (memcmp(sum16, expected_csum_ipv6_magic + i * 2, 2))
		complain

That's it.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-11 19:18 ` Guenter Roeck
  2024-02-12  5:26   ` Charlie Jenkins
@ 2024-02-12 12:46   ` Geert Uytterhoeven
  1 sibling, 0 replies; 22+ messages in thread
From: Geert Uytterhoeven @ 2024-02-12 12:46 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Charlie Jenkins, David Laight, Palmer Dabbelt, Andrew Morton,
	linux-kernel, kernel test robot

Hi Günter,

On Sun, Feb 11, 2024 at 8:18 PM Guenter Roeck <linux@roeck-us.net> wrote:
> On 2/7/24 16:22, Charlie Jenkins wrote:
> > The ip_fast_csum and csum_ipv6_magic tests did not have the data
> > types properly casted, and improperly misaligned data.
> >
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
>
> I sorted out most of the problems with this version, but I still get:
>
>      # test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:513
>      Expected ( u64)csum_result == ( u64)expected, but
>          ( u64)csum_result == 16630 (0x40f6)
>          ( u64)expected == 65535 (0xffff)
>      not ok 5 test_csum_ipv6_magic
>
> on m68k:q800. This is suspicious because there is no 0xffff in
> expected_csum_ipv6_magic[]. With some debugging information:
>
> ####### num_tests=86 i=84 expect array size=84
> ####### MAX_LEN=512 WORD_ALIGNMENT=4 magic data size=42
>
> That means the loop
>
>         for (int i = 0; i < num_tests; i++) {
>                 ...
>                 expected = (__force __sum16)expected_csum_ipv6_magic[i];
>                 ...
>         }
>
> will access data beyond the end of the expected_csum_ipv6_magic[] array,
> possibly because m68k doesn't pad struct csum_ipv6_magic_data to 44 bytes.

Exactly, sizeof(struct csum_ipv6_magic_data) = 42 on m68k.
Hence struct csum_ipv6_magic_data needs an extra field "unsigned char pad[3];"
at the end.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-12  5:26   ` Charlie Jenkins
@ 2024-02-12 17:10     ` Guenter Roeck
  2024-02-16  9:01       ` Geert Uytterhoeven
  0 siblings, 1 reply; 22+ messages in thread
From: Guenter Roeck @ 2024-02-12 17:10 UTC (permalink / raw)
  To: Charlie Jenkins
  Cc: David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel,
	kernel test robot

On Mon, Feb 12, 2024 at 12:26:08AM -0500, Charlie Jenkins wrote:
> On Sun, Feb 11, 2024 at 11:18:36AM -0800, Guenter Roeck wrote:
> > Hi,
> > 
> > On 2/7/24 16:22, Charlie Jenkins wrote:
> > > The ip_fast_csum and csum_ipv6_magic tests did not have the data
> > > types properly casted, and improperly misaligned data.
> > > 
> > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > 
> > I sorted out most of the problems with this version, but I still get:
> > 
> >     # test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:513
> >     Expected ( u64)csum_result == ( u64)expected, but
> >         ( u64)csum_result == 16630 (0x40f6)
> >         ( u64)expected == 65535 (0xffff)
> >     not ok 5 test_csum_ipv6_magic
> > 
> > on m68k:q800. This is suspicious because there is no 0xffff in
> > expected_csum_ipv6_magic[]. With some debugging information:
> > 
> > ####### num_tests=86 i=84 expect array size=84
> > ####### MAX_LEN=512 WORD_ALIGNMENT=4 magic data size=42
> > 
> > That means the loop
> > 
> > 	for (int i = 0; i < num_tests; i++) {
> > 		...
> > 		expected = (__force __sum16)expected_csum_ipv6_magic[i];
> > 		...
> > 	}
> > 
> > will access data beyond the end of the expected_csum_ipv6_magic[] array,
> > possibly because m68k doesn't pad struct csum_ipv6_magic_data to 44 bytes.
> 
> Okay I will check that out.
> 
> > 
> > In this context, is the comment about proto having to be 0 really true ?
> > It seems to me that the calculated checksum must be identical on both
> > little and big endian systems. After all, they need to be able to talk
> > to each other.
> 
> I agree, but I couldn't find a solution other than setting it to zero.
> Maybe I am missing something simple...
> 

Try the patch below on top of yours. It should work on both big and little
endian systems.

Key changes:
- use random_buf directly instead of copying anything
- no need to convert source / destination addresses
- csum in the buffer is in network byte order and needs
  to stay that way
- len in the buffer is in network byte order and needs to be
  converted to host byte order since that is expected by
  csum_ipv6_magic()
- the expected value is in host byte order and needs to be
  converted to network byte order for comparison
- protocol is just fine and converted by csum_ipv6_magic()
  as needed

Hope this helps,
Guenter

---
diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
index acce285a4959..dec60e97e87a 100644
--- a/lib/checksum_kunit.c
+++ b/lib/checksum_kunit.c
@@ -217,16 +217,19 @@ static const u32 init_sums_no_overflow[] = {
 };
 
 static const u16 expected_csum_ipv6_magic[] = {
-	0x3045, 0xb681, 0xc210, 0xe77b, 0x41cc, 0xa904, 0x4367, 0xe8d8, 0x5809,
-	0x5901, 0x5a40, 0xd3f4, 0xe467, 0xddde, 0xa609, 0xae05, 0xed14, 0x9133,
-	0x8007, 0x89b6, 0x97b0, 0x8927, 0x1e43, 0x7903, 0x4772, 0xd3a4, 0x457b,
-	0x83d0, 0x4ce1, 0x3656, 0x2dfc, 0xb678, 0x9a83, 0xd523, 0x61db, 0x379e,
-	0x3880, 0xbb23, 0xa38b, 0xd2eb, 0x991a, 0xcf73, 0x13ea, 0x890f, 0x20ce,
-	0x60ad, 0x5688, 0x4b13, 0x9399, 0x8a36, 0x75ae, 0x513a, 0xb222, 0xf3bb,
-	0x80d4, 0x1f98, 0xc2bc, 0xf566, 0x796a, 0x268a, 0xe67f, 0xb917, 0xd716,
-	0x3a16, 0x1858, 0x9d5b, 0x6fb4, 0x72b4, 0x1196, 0xb3d0, 0x89dc, 0xdd07,
-	0x1a8d, 0xfa6d, 0x1507, 0xafab, 0x7d37, 0xa623, 0x72dd, 0x2083, 0xdfc4,
-	0xa267, 0x92c9, 0xc8ad,
+	0x2fbd, 0xb5d0, 0xc16e, 0xe6c1, 0x412e, 0xa836, 0x433b, 0xe87c, 0x57ea,
+	0x5875, 0x5a21, 0xd361, 0xe422, 0xdd50, 0xa57f, 0xad6b, 0xecd1, 0x90b5,
+	0x7fda, 0x88db, 0x979e, 0x8916, 0x1df0, 0x7853, 0x473e, 0xd2b3, 0x4526,
+	0x8304, 0x4c34, 0x363d, 0x2dc1, 0xb66c, 0x9a28, 0xd4f2, 0x615d, 0x36dd,
+	0x3784, 0xbadd, 0xa2c6, 0xd293, 0x9830, 0xcea8, 0x1349, 0x886d, 0x20a3,
+	0x6001, 0x5607, 0x4a30, 0x9365, 0x893c, 0x7505, 0x50a1, 0xb200, 0xf3ad,
+	0x80bf, 0x1f48, 0xc1d9, 0xf55d, 0x7871, 0x262a, 0xe606, 0xb894, 0xd6cd,
+	0x39ed, 0x1817, 0x9d20, 0x6f93, 0x722d, 0x1116, 0xb3b4, 0x88ec, 0xdcb5,
+	0x1a46, 0xfa1d, 0x141e, 0xaef7, 0x7cee, 0xa583, 0x72ad, 0x201b, 0xdece,
+	0xa1ee, 0x92bd, 0xc7ba, 0x403e, 0x50a9, 0xcb5a, 0xff3b, 0x1b41, 0xa06b,
+	0xcf2d, 0xcc9a, 0xf42a, 0x0c61, 0x7654, 0xf3d4, 0xcc25, 0x4985, 0x7606,
+	0xc8a2, 0x6636, 0x610e, 0xc454, 0xefa4, 0xf3a6, 0x43a7, 0xd8e2, 0xe31e,
+	0x150b, 0x445d, 0x57d5, 0x253c, 0x6adf, 0x3c53, 0x502c, 0x9ee5, 0x8422,
 };
 
 static const u16 expected_fast_csum[] = {
@@ -465,44 +468,36 @@ static void test_ip_fast_csum(struct kunit *test)
 	}
 }
 
+#define IPV6_NUM_TESTS ((MAX_LEN - sizeof(struct in6_addr) * 2 - sizeof(int) * 3) / WORD_ALIGNMENT)
+
 static void test_csum_ipv6_magic(struct kunit *test)
 {
 #if defined(CONFIG_NET)
+	const struct in6_addr *saddr;
+	const struct in6_addr *daddr;
+	unsigned int len;
 	__sum16 csum_result, expected;
-	struct csum_ipv6_magic_data {
-		const struct in6_addr saddr;
-		const struct in6_addr daddr;
-		unsigned int len;
-		__wsum csum;
-		unsigned char proto;
-	} data, *data_ptr;
-	int num_tests = MAX_LEN / WORD_ALIGNMENT - sizeof(struct csum_ipv6_magic_data);
+	unsigned char proto;
+	unsigned int csum;
 
-	for (int i = 0; i < num_tests; i++) {
-		data_ptr = (struct csum_ipv6_magic_data *)(random_buf + (i * WORD_ALIGNMENT));
+	const int daddr_offset = sizeof(struct in6_addr);
+	const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr);
+	const int csum_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr) +
+	  sizeof(int);
+	const int proto_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr) +
+	  sizeof(int) * 2;
 
-		cpu_to_be32_array((__be32 *)&data.saddr, (const u32 *)&data_ptr->saddr,
-				  sizeof_field(struct csum_ipv6_magic_data, saddr) / 4);
-		cpu_to_be32_array((__be32 *)&data.daddr, (const u32 *)&data_ptr->daddr,
-				  sizeof_field(struct csum_ipv6_magic_data, daddr) / 4);
-		data.len = data_ptr->len;
-		data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
-		/*
-		 * proto must be zero to be compatible between big-endian and
-		 * little-endian CPUs. On little-endian CPUs, proto is
-		 * converted to a big-endian 32-bit value before the checksum
-		 * operation. This causes proto to be in the most significant
-		 * 8 bits on a little-endian CPU. On big-endian CPUs proto will
-		 * remain in the least significant 8 bits. There does not exist
-		 * a transformation to an arbitrary proto that will allow
-		 * csum_ipv6_magic to return the same value on a big-endian and
-		 * little-endian CPUs.
-		 */
-		data.proto = 0;
-		csum_result = csum_ipv6_magic(&data.saddr, &data.daddr,
-					      data.len, data.proto,
-					      data.csum);
-		expected = (__force __sum16)expected_csum_ipv6_magic[i];
+	for (int i = 0; i < IPV6_NUM_TESTS; i++) {
+		int index = i * WORD_ALIGNMENT;
+
+		saddr = (const struct in6_addr *)(random_buf + index);
+		daddr = (const struct in6_addr *)(random_buf + index + daddr_offset);
+		len = ntohl(*(unsigned int *)(random_buf + index + len_offset));
+		csum = *(unsigned int *)(random_buf + index + csum_offset);
+		proto = *(random_buf + index + proto_offset);
+
+		csum_result = csum_ipv6_magic(saddr, daddr, len, proto, csum);
+		expected = (__force __sum16)htons(expected_csum_ipv6_magic[i]);
 		CHECK_EQ(csum_result, expected);
 	}
 #endif /* !CONFIG_NET */

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-12  6:26   ` Al Viro
@ 2024-02-12 17:18     ` Guenter Roeck
  2024-02-12 18:09       ` Charlie Jenkins
  2024-02-12 18:12       ` Al Viro
  0 siblings, 2 replies; 22+ messages in thread
From: Guenter Roeck @ 2024-02-12 17:18 UTC (permalink / raw)
  To: Al Viro
  Cc: Charlie Jenkins, David Laight, Palmer Dabbelt, Andrew Morton,
	linux-kernel

On Mon, Feb 12, 2024 at 06:26:14AM +0000, Al Viro wrote:
> On Wed, Feb 07, 2024 at 04:22:51PM -0800, Charlie Jenkins wrote:
> > +	struct csum_ipv6_magic_data {
> > +		const struct in6_addr saddr;
> > +		const struct in6_addr daddr;
> > +		unsigned int len;
> > +		__wsum csum;
> > +		unsigned char proto;
> > +	} data, *data_ptr;
> 
> Huh?
> 
> > +	int num_tests = MAX_LEN / WORD_ALIGNMENT - sizeof(struct csum_ipv6_magic_data);
> > +
> > +	for (int i = 0; i < num_tests; i++) {
> > +		data_ptr = (struct csum_ipv6_magic_data *)(random_buf + (i * WORD_ALIGNMENT));
> > +
> > +		cpu_to_be32_array((__be32 *)&data.saddr, (const u32 *)&data_ptr->saddr,
> > +				  sizeof_field(struct csum_ipv6_magic_data, saddr) / 4);
> > +		cpu_to_be32_array((__be32 *)&data.daddr, (const u32 *)&data_ptr->daddr,
> > +				  sizeof_field(struct csum_ipv6_magic_data, daddr) / 4);
> > +		data.len = data_ptr->len;
> > +		data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
> 
> What are those cpu_to_be32() about?  Checksum calculations *DO* *NOT* involve
> any endianness conversions.  At any point.
> 
> Replace those assignments with memcpy() and be done with that - that will take
> care of unaligned accesses.
> 
> Result will have host-independent memory representation.  The only place where you
> might want to play with byteswaps (only 16-bit ones) is if you initialized the
> array of expected results with u16 constants.  That will have opposite memory
> representations on l-e and b-e, so you'll need to byteswap to compare with
> what you get from function.  Alternatively, make it an array of bytes and
> do
> 	sum16 = csum_ipv6_magic(saddr, daddr, len, proto, csum);
> 	if (memcmp(sum16, expected_csum_ipv6_magic + i * 2, 2))
> 		complain
> 
> That's it.

Almost. Turns out the csum parameter of csum_ipv6_magic() needs to be in
network byte order, and the length parameter needs to be in host byte order.
So instead of
	data.len = data_ptr->len;
	data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
it needs to be something like
	data.len = ntohl(data_ptr->len);
	data.csum = data_ptr->csum;

Also, as you mentioned, either the returned checksum or the expected
checksum needs to be converted for the comparison because one is in
network byte order and the other in host byte order.

Address conversions are indeed not needed.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-12 17:18     ` Guenter Roeck
@ 2024-02-12 18:09       ` Charlie Jenkins
  2024-02-12 18:26         ` Guenter Roeck
  2024-02-12 18:12       ` Al Viro
  1 sibling, 1 reply; 22+ messages in thread
From: Charlie Jenkins @ 2024-02-12 18:09 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Al Viro, David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel

On Mon, Feb 12, 2024 at 09:18:14AM -0800, Guenter Roeck wrote:
> On Mon, Feb 12, 2024 at 06:26:14AM +0000, Al Viro wrote:
> > On Wed, Feb 07, 2024 at 04:22:51PM -0800, Charlie Jenkins wrote:
> > > +	struct csum_ipv6_magic_data {
> > > +		const struct in6_addr saddr;
> > > +		const struct in6_addr daddr;
> > > +		unsigned int len;
> > > +		__wsum csum;
> > > +		unsigned char proto;
> > > +	} data, *data_ptr;
> > 
> > Huh?
> > 
> > > +	int num_tests = MAX_LEN / WORD_ALIGNMENT - sizeof(struct csum_ipv6_magic_data);
> > > +
> > > +	for (int i = 0; i < num_tests; i++) {
> > > +		data_ptr = (struct csum_ipv6_magic_data *)(random_buf + (i * WORD_ALIGNMENT));
> > > +
> > > +		cpu_to_be32_array((__be32 *)&data.saddr, (const u32 *)&data_ptr->saddr,
> > > +				  sizeof_field(struct csum_ipv6_magic_data, saddr) / 4);
> > > +		cpu_to_be32_array((__be32 *)&data.daddr, (const u32 *)&data_ptr->daddr,
> > > +				  sizeof_field(struct csum_ipv6_magic_data, daddr) / 4);
> > > +		data.len = data_ptr->len;
> > > +		data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
> > 
> > What are those cpu_to_be32() about?  Checksum calculations *DO* *NOT* involve
> > any endianness conversions.  At any point.
> > 
> > Replace those assignments with memcpy() and be done with that - that will take
> > care of unaligned accesses.
> > 
> > Result will have host-independent memory representation.  The only place where you
> > might want to play with byteswaps (only 16-bit ones) is if you initialized the
> > array of expected results with u16 constants.  That will have opposite memory
> > representations on l-e and b-e, so you'll need to byteswap to compare with
> > what you get from function.  Alternatively, make it an array of bytes and
> > do
> > 	sum16 = csum_ipv6_magic(saddr, daddr, len, proto, csum);
> > 	if (memcmp(sum16, expected_csum_ipv6_magic + i * 2, 2))
> > 		complain
> > 
> > That's it.
> 
> Almost. Turns out the csum parameter of csum_ipv6_magic() needs to be in
> network byte order, and the length parameter needs to be in host byte order.
> So instead of
> 	data.len = data_ptr->len;
> 	data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
> it needs to be something like
> 	data.len = ntohl(data_ptr->len);
> 	data.csum = data_ptr->csum;
> 
> Also, as you mentioned, either the returned checksum or the expected
> checksum needs to be converted for the comparison because one is in
> network byte order and the other in host byte order.
> 
> Address conversions are indeed not needed.
> 
> Thanks,
> Guenter

Aww that makes sense thank you. I was reversing everything except len
and the solution was to only reverse len... Thank you for figuring that
out for me.

I will send out another version with your change. Should I add a
signed-off-by with your tag for this patch?

- Charlie


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-12 17:18     ` Guenter Roeck
  2024-02-12 18:09       ` Charlie Jenkins
@ 2024-02-12 18:12       ` Al Viro
  2024-02-12 18:34         ` Guenter Roeck
  2024-02-12 18:43         ` Al Viro
  1 sibling, 2 replies; 22+ messages in thread
From: Al Viro @ 2024-02-12 18:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Charlie Jenkins, David Laight, Palmer Dabbelt, Andrew Morton,
	linux-kernel

On Mon, Feb 12, 2024 at 09:18:14AM -0800, Guenter Roeck wrote:

> Almost. Turns out the csum parameter of csum_ipv6_magic() needs to be in
> network byte order, and the length parameter needs to be in host byte order.
> So instead of
> 	data.len = data_ptr->len;
> 	data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
> it needs to be something like
> 	data.len = ntohl(data_ptr->len);
> 	data.csum = data_ptr->csum;
> 
> Also, as you mentioned, either the returned checksum or the expected
> checksum needs to be converted for the comparison because one is in
> network byte order and the other in host byte order.

        for (int i = 0; i < NUM_IPv6_TESTS; i++) {
		struct args {
			struct in6_addr saddr;
			struct in6_addr daddr;
			__be32 len;
			__wsum csum;
			unsigned char proto;
		} __packed data = (struct args *)(random_buf + i);
                CHECK_EQ(cpu_to_le16(expected_csum_ipv6_magic[i]),
                         csum_ipv6_magic(data.saddr, data.daddr, ntohl(data.len),
					 data.proto, data.sum));
        }
and to hell with field-by-field copying.  __packed here will tell the compiler
that alignment of the entire thing is 1 - the total size of fields is 41 bytes,
so "no padding" translates into "can't even assume that address is even".

And I'd probably turn that expect_csum....[] array into an array of bytes,
with *(__sum16*)(expected_csum_ipv6_magic + 2 * i) in your CHECK_EQ instead
of arch-dependent byteswaps.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-12 18:09       ` Charlie Jenkins
@ 2024-02-12 18:26         ` Guenter Roeck
  0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2024-02-12 18:26 UTC (permalink / raw)
  To: Charlie Jenkins
  Cc: Al Viro, David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel

On 2/12/24 10:09, Charlie Jenkins wrote:
[ ... ]

>> Almost. Turns out the csum parameter of csum_ipv6_magic() needs to be in
>> network byte order, and the length parameter needs to be in host byte order.
>> So instead of
>> 	data.len = data_ptr->len;
>> 	data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
>> it needs to be something like
>> 	data.len = ntohl(data_ptr->len);
>> 	data.csum = data_ptr->csum;
>>
>> Also, as you mentioned, either the returned checksum or the expected
>> checksum needs to be converted for the comparison because one is in
>> network byte order and the other in host byte order.
>>
>> Address conversions are indeed not needed.
>>
>> Thanks,
>> Guenter
> 
> Aww that makes sense thank you. I was reversing everything except len
> and the solution was to only reverse len... Thank you for figuring that
> out for me.
> 
> I will send out another version with your change. Should I add a
> signed-off-by with your tag for this patch?
> 

No need. I'll send you a Tested-by: tag on the final version (if it works ;-).

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-12 18:12       ` Al Viro
@ 2024-02-12 18:34         ` Guenter Roeck
  2024-02-12 18:41           ` Charlie Jenkins
  2024-02-12 18:43         ` Al Viro
  1 sibling, 1 reply; 22+ messages in thread
From: Guenter Roeck @ 2024-02-12 18:34 UTC (permalink / raw)
  To: Al Viro
  Cc: Charlie Jenkins, David Laight, Palmer Dabbelt, Andrew Morton,
	linux-kernel

On 2/12/24 10:12, Al Viro wrote:
> On Mon, Feb 12, 2024 at 09:18:14AM -0800, Guenter Roeck wrote:
> 
>> Almost. Turns out the csum parameter of csum_ipv6_magic() needs to be in
>> network byte order, and the length parameter needs to be in host byte order.
>> So instead of
>> 	data.len = data_ptr->len;
>> 	data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
>> it needs to be something like
>> 	data.len = ntohl(data_ptr->len);
>> 	data.csum = data_ptr->csum;
>>
>> Also, as you mentioned, either the returned checksum or the expected
>> checksum needs to be converted for the comparison because one is in
>> network byte order and the other in host byte order.
> 
>          for (int i = 0; i < NUM_IPv6_TESTS; i++) {
> 		struct args {
> 			struct in6_addr saddr;
> 			struct in6_addr daddr;
> 			__be32 len;
> 			__wsum csum;
> 			unsigned char proto;
> 		} __packed data = (struct args *)(random_buf + i);
>                  CHECK_EQ(cpu_to_le16(expected_csum_ipv6_magic[i]),
>                           csum_ipv6_magic(data.saddr, data.daddr, ntohl(data.len),
> 					 data.proto, data.sum));
>          }
> and to hell with field-by-field copying.  __packed here will tell the compiler
> that alignment of the entire thing is 1 - the total size of fields is 41 bytes,
> so "no padding" translates into "can't even assume that address is even".
> 

Except that the data pointer needs to be aligned because otherwise architectures
not supporting unaligned accesses will bail out (as observed on mps2-an385).
But then I am no longer sure if that is correct - maybe csum_ipv6_magic()
is supposed to be able to handle unaligned accesses. If so, maybe it would be
more appropriate to skip this test on the affected architectures.

Guenter


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-12 18:34         ` Guenter Roeck
@ 2024-02-12 18:41           ` Charlie Jenkins
  0 siblings, 0 replies; 22+ messages in thread
From: Charlie Jenkins @ 2024-02-12 18:41 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Al Viro, David Laight, Palmer Dabbelt, Andrew Morton, linux-kernel

On Mon, Feb 12, 2024 at 10:34:10AM -0800, Guenter Roeck wrote:
> On 2/12/24 10:12, Al Viro wrote:
> > On Mon, Feb 12, 2024 at 09:18:14AM -0800, Guenter Roeck wrote:
> > 
> > > Almost. Turns out the csum parameter of csum_ipv6_magic() needs to be in
> > > network byte order, and the length parameter needs to be in host byte order.
> > > So instead of
> > > 	data.len = data_ptr->len;
> > > 	data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
> > > it needs to be something like
> > > 	data.len = ntohl(data_ptr->len);
> > > 	data.csum = data_ptr->csum;
> > > 
> > > Also, as you mentioned, either the returned checksum or the expected
> > > checksum needs to be converted for the comparison because one is in
> > > network byte order and the other in host byte order.
> > 
> >          for (int i = 0; i < NUM_IPv6_TESTS; i++) {
> > 		struct args {
> > 			struct in6_addr saddr;
> > 			struct in6_addr daddr;
> > 			__be32 len;
> > 			__wsum csum;
> > 			unsigned char proto;
> > 		} __packed data = (struct args *)(random_buf + i);
> >                  CHECK_EQ(cpu_to_le16(expected_csum_ipv6_magic[i]),
> >                           csum_ipv6_magic(data.saddr, data.daddr, ntohl(data.len),
> > 					 data.proto, data.sum));
> >          }
> > and to hell with field-by-field copying.  __packed here will tell the compiler
> > that alignment of the entire thing is 1 - the total size of fields is 41 bytes,
> > so "no padding" translates into "can't even assume that address is even".
> > 
> 
> Except that the data pointer needs to be aligned because otherwise architectures
> not supporting unaligned accesses will bail out (as observed on mps2-an385).
> But then I am no longer sure if that is correct - maybe csum_ipv6_magic()
> is supposed to be able to handle unaligned accesses. If so, maybe it would be
> more appropriate to skip this test on the affected architectures.
> 
> Guenter
> 

We can align this against IP_ALIGNMENT since it should always be aligned
against the IP header + NET_IP_ALIGN.

- Charlie


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
  2024-02-12 18:12       ` Al Viro
  2024-02-12 18:34         ` Guenter Roeck
@ 2024-02-12 18:43         ` Al Viro
  1 sibling, 0 replies; 22+ messages in thread
From: Al Viro @ 2024-02-12 18:43 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Charlie Jenkins, David Laight, Palmer Dabbelt, Andrew Morton,
	linux-kernel

On Mon, Feb 12, 2024 at 06:12:17PM +0000, Al Viro wrote:
>                          csum_ipv6_magic(data.saddr, data.daddr, ntohl(data.len),
					   &data.saddr, &data.daddr, ntohl(data.len),

obviously...

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests
  2024-02-12 17:10     ` Guenter Roeck
@ 2024-02-16  9:01       ` Geert Uytterhoeven
  0 siblings, 0 replies; 22+ messages in thread
From: Geert Uytterhoeven @ 2024-02-16  9:01 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Charlie Jenkins, David Laight, Palmer Dabbelt, Andrew Morton,
	linux-kernel, kernel test robot

Hi Günter,

On Mon, Feb 12, 2024 at 6:13 PM Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Feb 12, 2024 at 12:26:08AM -0500, Charlie Jenkins wrote:
> > On Sun, Feb 11, 2024 at 11:18:36AM -0800, Guenter Roeck wrote:
> > > On 2/7/24 16:22, Charlie Jenkins wrote:
> > > > The ip_fast_csum and csum_ipv6_magic tests did not have the data
> > > > types properly casted, and improperly misaligned data.
> > > >
> > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > >
> > > I sorted out most of the problems with this version, but I still get:
> > >
> > >     # test_csum_ipv6_magic: ASSERTION FAILED at lib/checksum_kunit.c:513
> > >     Expected ( u64)csum_result == ( u64)expected, but
> > >         ( u64)csum_result == 16630 (0x40f6)
> > >         ( u64)expected == 65535 (0xffff)
> > >     not ok 5 test_csum_ipv6_magic
> > >
> > > on m68k:q800. This is suspicious because there is no 0xffff in
> > > expected_csum_ipv6_magic[]. With some debugging information:
> > >
> > > ####### num_tests=86 i=84 expect array size=84
> > > ####### MAX_LEN=512 WORD_ALIGNMENT=4 magic data size=42
> > >
> > > That means the loop
> > >
> > >     for (int i = 0; i < num_tests; i++) {
> > >             ...
> > >             expected = (__force __sum16)expected_csum_ipv6_magic[i];
> > >             ...
> > >     }
> > >
> > > will access data beyond the end of the expected_csum_ipv6_magic[] array,
> > > possibly because m68k doesn't pad struct csum_ipv6_magic_data to 44 bytes.
> >
> > Okay I will check that out.
> >
> > >
> > > In this context, is the comment about proto having to be 0 really true ?
> > > It seems to me that the calculated checksum must be identical on both
> > > little and big endian systems. After all, they need to be able to talk
> > > to each other.
> >
> > I agree, but I couldn't find a solution other than setting it to zero.
> > Maybe I am missing something simple...
> >
>
> Try the patch below on top of yours. It should work on both big and little
> endian systems.
>
> Key changes:
> - use random_buf directly instead of copying anything
> - no need to convert source / destination addresses
> - csum in the buffer is in network byte order and needs
>   to stay that way
> - len in the buffer is in network byte order and needs to be
>   converted to host byte order since that is expected by
>   csum_ipv6_magic()
> - the expected value is in host byte order and needs to be
>   converted to network byte order for comparison
> - protocol is just fine and converted by csum_ipv6_magic()
>   as needed

Thanks for your patch!

> --- a/lib/checksum_kunit.c
> +++ b/lib/checksum_kunit.c

> @@ -465,44 +468,36 @@ static void test_ip_fast_csum(struct kunit *test)
>         }
>  }
>
> +#define IPV6_NUM_TESTS ((MAX_LEN - sizeof(struct in6_addr) * 2 - sizeof(int) * 3) / WORD_ALIGNMENT)
> +
>  static void test_csum_ipv6_magic(struct kunit *test)
>  {
>  #if defined(CONFIG_NET)
> +       const struct in6_addr *saddr;
> +       const struct in6_addr *daddr;
> +       unsigned int len;
>         __sum16 csum_result, expected;
> -       struct csum_ipv6_magic_data {
> -               const struct in6_addr saddr;
> -               const struct in6_addr daddr;
> -               unsigned int len;
> -               __wsum csum;
> -               unsigned char proto;
> -       } data, *data_ptr;
> -       int num_tests = MAX_LEN / WORD_ALIGNMENT - sizeof(struct csum_ipv6_magic_data);
> +       unsigned char proto;
> +       unsigned int csum;
>
> -       for (int i = 0; i < num_tests; i++) {
> -               data_ptr = (struct csum_ipv6_magic_data *)(random_buf + (i * WORD_ALIGNMENT));
> +       const int daddr_offset = sizeof(struct in6_addr);
> +       const int len_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr);
> +       const int csum_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr) +
> +         sizeof(int);
> +       const int proto_offset = sizeof(struct in6_addr) + sizeof(struct in6_addr) +
> +         sizeof(int) * 2;

Please no manual offset calculations.
Please fix the csum_ipv6_magic_data structure definition instead.

>
> -               cpu_to_be32_array((__be32 *)&data.saddr, (const u32 *)&data_ptr->saddr,
> -                                 sizeof_field(struct csum_ipv6_magic_data, saddr) / 4);
> -               cpu_to_be32_array((__be32 *)&data.daddr, (const u32 *)&data_ptr->daddr,
> -                                 sizeof_field(struct csum_ipv6_magic_data, daddr) / 4);
> -               data.len = data_ptr->len;
> -               data.csum = (__force __wsum)htonl((__force u32)data_ptr->csum);
> -               /*
> -                * proto must be zero to be compatible between big-endian and
> -                * little-endian CPUs. On little-endian CPUs, proto is
> -                * converted to a big-endian 32-bit value before the checksum
> -                * operation. This causes proto to be in the most significant
> -                * 8 bits on a little-endian CPU. On big-endian CPUs proto will
> -                * remain in the least significant 8 bits. There does not exist
> -                * a transformation to an arbitrary proto that will allow
> -                * csum_ipv6_magic to return the same value on a big-endian and
> -                * little-endian CPUs.
> -                */
> -               data.proto = 0;
> -               csum_result = csum_ipv6_magic(&data.saddr, &data.daddr,
> -                                             data.len, data.proto,
> -                                             data.csum);
> -               expected = (__force __sum16)expected_csum_ipv6_magic[i];
> +       for (int i = 0; i < IPV6_NUM_TESTS; i++) {
> +               int index = i * WORD_ALIGNMENT;
> +
> +               saddr = (const struct in6_addr *)(random_buf + index);
> +               daddr = (const struct in6_addr *)(random_buf + index + daddr_offset);
> +               len = ntohl(*(unsigned int *)(random_buf + index + len_offset));
> +               csum = *(unsigned int *)(random_buf + index + csum_offset);
> +               proto = *(random_buf + index + proto_offset);
> +
> +               csum_result = csum_ipv6_magic(saddr, daddr, len, proto, csum);
> +               expected = (__force __sum16)htons(expected_csum_ipv6_magic[i]);
>                 CHECK_EQ(csum_result, expected);
>         }
>  #endif /* !CONFIG_NET */

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2024-02-16  9:01 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-08  0:22 [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Charlie Jenkins
2024-02-08  0:22 ` [PATCH v6 1/2] lib: checksum: Fix type casting in checksum kunits Charlie Jenkins
2024-02-08  0:22 ` [PATCH v6 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests Charlie Jenkins
2024-02-08  2:45   ` Guenter Roeck
2024-02-12  6:26   ` Al Viro
2024-02-12 17:18     ` Guenter Roeck
2024-02-12 18:09       ` Charlie Jenkins
2024-02-12 18:26         ` Guenter Roeck
2024-02-12 18:12       ` Al Viro
2024-02-12 18:34         ` Guenter Roeck
2024-02-12 18:41           ` Charlie Jenkins
2024-02-12 18:43         ` Al Viro
2024-02-08  1:45 ` [PATCH v6 0/2] lib: checksum: Fix issues with checksum tests Andrew Morton
2024-02-08  2:09   ` Charlie Jenkins
2024-02-08  2:44     ` Guenter Roeck
2024-02-08  2:47       ` Palmer Dabbelt
2024-02-08  4:15 ` Guenter Roeck
2024-02-11 19:18 ` Guenter Roeck
2024-02-12  5:26   ` Charlie Jenkins
2024-02-12 17:10     ` Guenter Roeck
2024-02-16  9:01       ` Geert Uytterhoeven
2024-02-12 12:46   ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).