RE: [PATCH 0/1] Add KUnit tests for lib/crc16.c

[David Laight]

From: Vinicius Peixoto
> Sent: 23 September 2024 00:27
> 
> Hi all,
> 
> This patch was developed during a hackathon organized by LKCAMP [1],
> with the objective of writing KUnit tests, both to introduce people to
> the kernel development process and to learn about different subsystems
> (with the positive side effect of improving the kernel test coverage, of
> course).
> 
> We noticed there were tests for CRC32 in lib/crc32test.c and thought it
> would be nice to have something similar for CRC16, since it seems to be
> widely used in network drivers (as well as in some ext4 code).
> 
> Although this patch turned out quite big, most of the LOCs come from
> tables containing randomly-generated test data that we use to validate
> the kernel's implementation of CRC-16.
> 
> We would really appreciate any feedback/suggestions on how to improve
> this. Thanks! :-)

Would is be better to use a trivial PRNG to generate repeatable 'random enough'
data, rather than having a large static array?

As a matter of interest, how in crc16 implemented (I know I could look).
The code version:

uint32_t
crc_step(uint32_t crc, uint32_t byte_val)
{
    uint32_t t = crc ^ (byte_val & 0xff);
    t = (t ^ t << 4) & 0xff;
    return crc >> 8 ^ t << 8 ^ t << 3 ^ t >> 4;
}

may well be faster than a lookup table version.
Especially on modern multi-issue cpu and/or for small buffers where the lookup
table won't necessarily be resident in the D-cache.

It is slightly slower than the table lookup on the simple Nios-II cpu.
But we use a custom instruction to do it in one clock.

	David

> 
> Vinicius Peixoto (1):
>   lib/crc16_kunit.c: add KUnit tests for crc16
> 
>  lib/Kconfig.debug |   8 +
>  lib/Makefile      |   1 +
>  lib/crc16_kunit.c | 715 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 724 insertions(+)
>  create mode 100644 lib/crc16_kunit.c
> 
> --
> 2.43.0
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)