From: Charlie Jenkins <charlie@rivosinc.com>
To: David Laight <David.Laight@aculab.com>
Cc: Guenter Roeck <linux@roeck-us.net>,
Palmer Dabbelt <palmer@dabbelt.com>,
Andrew Morton <akpm@linux-foundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests
Date: Thu, 8 Feb 2024 12:09:33 -0800 [thread overview]
Message-ID: <ZcU0/ZsCAwxW/oTo@ghost> (raw)
In-Reply-To: <0b78e69bf4ef4e52a61ffe21d2c08c96@AcuMS.aculab.com>
On Thu, Feb 08, 2024 at 09:54:39AM +0000, David Laight wrote:
> From: Charlie Jenkins
> > Sent: 08 February 2024 00:23
> >
> > On Sun, Feb 04, 2024 at 09:41:56AM -0800, Guenter Roeck wrote:
> > > Hi,
> > >
> > > On Tue, Jan 30, 2024 at 11:10:04AM -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.
> > > >
> > > >
> ...
> > >
> > > So this works on little endian systems. Unfortunately, I still get
> > >
> ...
> > >
> > > when running the test on big endian systems such as hppa/parisc or sparc.
> >
> > Hmm okay it was easy to get this to work on big endian for
> > test_ip_fast_csum but test_csum_ipv6_magic was trickier. I will send out
> > a new version with the changes.
>
> Instead of trying to save the expected results why not just
> calculate them with a 'really dumb' implementation.
> (eg: Add 16bit items and then fold.)
>
> For the generic tests, IIRC:
> Your test vectors looked random.
> They should probably contain some very specific tests cases.
> eg:
> - Zero length and all zeros - checksum should be zero (not 0xffff).
> - Buffers where the final 'fold' needs the carry added in.
The existing csum test cases have three tests, one for random and then
one for each of the two specific test cases you mentioned. I figured
having the random test case would most likely catch an error if one
existed.
The additional tests can be added in a future patch but I would like to
make sure the existing test cases work first.
- Charlie
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
prev parent reply other threads:[~2024-02-08 20:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 19:10 [PATCH v5 0/2] lib: checksum: Fix issues with checksum tests Charlie Jenkins
2024-01-30 19:10 ` [PATCH v5 1/2] lib: checksum: Fix type casting in checksum kunits Charlie Jenkins
2024-01-30 19:10 ` [PATCH v5 2/2] lib: checksum: Use aligned accesses for ip_fast_csum and csum_ipv6_magic tests Charlie Jenkins
2024-02-04 17:41 ` Guenter Roeck
2024-02-08 0:22 ` Charlie Jenkins
2024-02-08 9:54 ` David Laight
2024-02-08 20:09 ` Charlie Jenkins [this message]
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=ZcU0/ZsCAwxW/oTo@ghost \
--to=charlie@rivosinc.com \
--cc=David.Laight@aculab.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=palmer@dabbelt.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox