Received-SPF: pass (google.com: domain of linux-kernel+bounces-58647-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 8 Feb 2024 12:09:33 -0800
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
Message-ID: <ZcU0/ZsCAwxW/oTo@ghost>
References: <20240130-fix_sparse_errors_checksum_tests-v5-0-4d8a0a337e5e@rivosinc.com>
 <20240130-fix_sparse_errors_checksum_tests-v5-2-4d8a0a337e5e@rivosinc.com>
 <ec44bf32-8b66-40c4-bc62-4deed3702f99@roeck-us.net>
 <ZcQeyigDWwvnc4Nu@ghost>
 <0b78e69bf4ef4e52a61ffe21d2c08c96@AcuMS.aculab.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
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)
>