Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH] add checksum selftest | Date | Wed, 24 Jun 2009 16:14:02 +0200 |
| |
On Wednesday 24 June 2009, Mike Frysinger wrote: > Start a checksum internal testsuite for arch porters and people mucking > about in the checksum code -- regressions are bad. > > Signed-off-by: Mike Frysinger <vapier@gentoo.org> > --- > Arnd: i've tested this on my Blackfin and it seems to work ... can you > check for any places i missed little endian swapping ?
Endian swapping looks correct, and I like the patch a lot in general, but:
> + > +/* > + * The do_csum() interface is "internal" to the generic checksum code. > + * Do not require it if the arch has not switched over. > + */ > +extern unsigned short do_csum(const unsigned char *buff, int len);
Just nitpicking: this prototype should to into asm-generic/checksum.h, extern declarations have no place in .c files.
> +static unsigned char __initdata do_csum_data1[] = { > + 0x20, > +}; > +static unsigned char __initdata do_csum_data2[] = { > + 0x0d, 0x0a, > +}; > +static unsigned char __initdata do_csum_data3[] = { > + 0xff, 0xfb, 0x01, > +};
You define separate test vectors for each of the three cases, which looks like it could be optimized by reusing the same test vectors for each case.
> +static struct csum_partial_data __initdata csum_partial_data[] = { > + CSUM_PARTIAL_DATA(1, 0x00000074, 0x0), > + CSUM_PARTIAL_DATA(2, 0x00000a0d, 0x0), > + CSUM_PARTIAL_DATA(3, 0x0000fe00, 0x0), > + CSUM_PARTIAL_DATA(5, 0x00005084, 0x0), > + CSUM_PARTIAL_DATA(8, 0x1101eefe, 0x11016a80), > + CSUM_PARTIAL_DATA(8b, 0x00008781, 0x847e), > + CSUM_PARTIAL_DATA(9, 0x1101eefe, 0x11016b80), > +};
For partial checksums, the result has to be folded into a 16-bit number using csum_fold(), because csum_partial and other functions return a 32-bit __wsum that can take many equivalent values taht are all correct.
> +static int __init csum_tcpudp_nofold_selftest(void) > +{ > + int i, ret; > + unsigned short tret, eret; > + > + ret = 0; > + for (i = 0; i < ARRAY_SIZE(csum_tcpudp_nofold_data); ++i) { > + eret = le16_to_cpu(csum_tcpudp_nofold_data[i].ret); > + tret = csum_tcpudp_nofold( > + csum_tcpudp_nofold_data[i].saddr, > + csum_tcpudp_nofold_data[i].daddr, > + csum_tcpudp_nofold_data[i].len, > + csum_tcpudp_nofold_data[i].proto, > + csum_tcpudp_nofold_data[i].sum); > + if (tret != eret) { > + pr_err("%s: test %i: %#x != %#x: FAIL\n", > + __func__, i, tret, eret); > + ret = 1; > + } > + } > + > + return ret; > +}
same here, but you can easily use csum_tcpudp_magic() instead of csum_tcpudp_nofold here.
Thanks,
Arnd <><
| |