[PATCH v15 5/5] kunit: Add tests for csum_ipv6_magic and ip_fast_csum
Palmer Dabbelt
palmer at dabbelt.com
Mon Jan 22 15:39:16 PST 2024
On Mon, 22 Jan 2024 13:41:48 PST (-0800), David.Laight at ACULAB.COM wrote:
> From: Guenter Roeck
>> Sent: 22 January 2024 17:16
>>
>> On 1/22/24 08:52, David Laight wrote:
>> > From: Guenter Roeck
>> >> Sent: 22 January 2024 16:40
>> >>
>> >> Hi,
>> >>
>> >> On Mon, Jan 08, 2024 at 03:57:06PM -0800, Charlie Jenkins wrote:
>> >>> Supplement existing checksum tests with tests for csum_ipv6_magic and
>> >>> ip_fast_csum.
>> >>>
>> >>> Signed-off-by: Charlie Jenkins <charlie at rivosinc.com>
>> >>> ---
>> >>
>> >> With this patch in the tree, the arm:mps2-an385 qemu emulation gets a bad hiccup.
>> >>
>> >> [ 1.839556] Unhandled exception: IPSR = 00000006 LR = fffffff1
>> >> [ 1.839804] CPU: 0 PID: 164 Comm: kunit_try_catch Tainted: G N 6.8.0-rc1 #1
>> >> [ 1.839948] Hardware name: Generic DT based system
>> >> [ 1.840062] PC is at __csum_ipv6_magic+0x8/0xb4
>> >> [ 1.840408] LR is at test_csum_ipv6_magic+0x3d/0xa4
>> >> [ 1.840493] pc : [<21212f34>] lr : [<21117fd5>] psr: 0100020b
>> >> [ 1.840586] sp : 2180bebc ip : 46c7f0d2 fp : 21275b38
>> >> [ 1.840664] r10: 21276b60 r9 : 21275b28 r8 : 21465cfc
>> >> [ 1.840751] r7 : 00003085 r6 : 21275b4e r5 : 2138702c r4 : 00000001
>> >> [ 1.840847] r3 : 2c000000 r2 : 1ac7f0d2 r1 : 21275b39 r0 : 21275b29
>> >> [ 1.840942] xPSR: 0100020b
>> >>
>> >> This translates to:
>> >>
>> >> PC is at __csum_ipv6_magic (arch/arm/lib/csumipv6.S:15)
>> >> LR is at test_csum_ipv6_magic (./arch/arm/include/asm/checksum.h:60
>> >> ./arch/arm/include/asm/checksum.h:163 lib/checksum_kunit.c:617)
>> >>
>> >> Obviously I can not say if this is a problem with qemu or a problem with
>> >> the Linux kernel. Given that, and the presumably low interest in
>> >> running mps2-an385 with Linux, I'll simply disable that test. Just take
>> >> it as a heads up that there _may_ be a problem with this on arm
>> >> nommu systems.
>> >
>> > Can you drop in a disassembly of __csum_ipv6_magic ?
>> > Actually I think it is:
>>
>> It is, as per the PC pointer above. I don't know anything about arm assembler,
>> much less about its behavior with THUMB code.
>
> Doesn't look like thumb to me (offset 8 is two 4-byte instructions) and
> the code I found looks like arm to me.
> (I haven't written any arm asm since before they invented thumb!)
>
>> > ENTRY(__csum_ipv6_magic)
>> > str lr, [sp, #-4]!
>> > adds ip, r2, r3
>> > ldmia r1, {r1 - r3, lr}
>> >
>> > So the fault is (probably) a misaligned ldmia ?
>> > Are they ever supported?
>> >
>>
>> Good question. My primary guess is that this never worked. As I said,
>> this was just intended to be informational, (probably) no reason to bother.
>>
>> Of course one might ask if it makes sense to even keep the arm nommu code
>> in the kernel, but that is of course a different question. I do wonder though
>> if anyone but me is running it.
>
> If it is an alignment fault it isn't a 'nommu' bug.
>
> And traditionally arm didn't support misaligned transfers (well not
> in anyway any other cpu did!).
> It might be that the kernel assumes that all ethernet packets are
> aligned, but the test suite isn't aligning the buffer.
> Which would make it a test suite bug.
>From talking to Evan and Vineet, I think you're right and this is a test
suite bug: specifically the tests weren't respecting NET_IP_ALIGN. That
didn't crop up for ip_fast_csum() as it just uses ldr which supports
misaligned accesses on the M3 (at least as far as I can tell).
So I think the right fix is something like
diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
index 225bb7701460..2dd282e27dd4 100644
--- a/lib/checksum_kunit.c
+++ b/lib/checksum_kunit.c
@@ -5,6 +5,7 @@
#include <kunit/test.h>
#include <asm/checksum.h>
+#include <asm/checksum.h>
#include <net/ip6_checksum.h>
#define MAX_LEN 512
@@ -15,6 +16,7 @@
#define IPv4_MAX_WORDS 15
#define NUM_IPv6_TESTS 200
#define NUM_IP_FAST_CSUM_TESTS 181
+#define SUPPORTED_ALIGNMENT (1 << NET_IP_ALIGN)
/* Values for a little endian CPU. Byte swap each half on big endian CPU. */
static const u32 random_init_sum = 0x2847aab;
@@ -486,7 +488,7 @@ static void test_csum_fixed_random_inputs(struct kunit *test)
__sum16 result, expec;
assert_setup_correct(test);
- for (align = 0; align < TEST_BUFLEN; ++align) {
+ for (align = 0; align < TEST_BUFLEN; align += SUPPORTED_ALIGNMENT) {
memcpy(&tmp_buf[align], random_buf,
min(MAX_LEN, TEST_BUFLEN - align));
for (len = 0; len < MAX_LEN && (align + len) < TEST_BUFLEN;
@@ -513,7 +515,7 @@ static void test_csum_all_carry_inputs(struct kunit *test)
assert_setup_correct(test);
memset(tmp_buf, 0xff, TEST_BUFLEN);
- for (align = 0; align < TEST_BUFLEN; ++align) {
+ for (align = 0; align < TEST_BUFLEN; align += SUPPORTED_ALIGNMENT) {
for (len = 0; len < MAX_LEN && (align + len) < TEST_BUFLEN;
++len) {
/*
@@ -553,7 +555,7 @@ static void test_csum_no_carry_inputs(struct kunit *test)
assert_setup_correct(test);
memset(tmp_buf, 0x4, TEST_BUFLEN);
- for (align = 0; align < TEST_BUFLEN; ++align) {
+ for (align = 0; align < TEST_BUFLEN; align += SUPPORTED_ALIGNMENT) {
for (len = 0; len < MAX_LEN && (align + len) < TEST_BUFLEN;
++len) {
/*
but I haven't even build tested it...
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
More information about the linux-riscv
mailing list