Re: [PATCH] add checksum selftest

[Mike Frysinger]

On Wed, Jun 24, 2009 at 10:14, Arnd Bergmann wrote:
> On Wednesday 24 June 2009, Mike Frysinger wrote:
>> +/*
>> + * 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.

yeah, i meant to move that after the last discussion, but forgot

>> +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.

i'm not really familiar with the interfaces to figure out how to do
this ... i just added some printks to dump arguments/buffers and then
copied & pasted ones that looked pretty different

>> +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.

i hear your words, but i understand them not ;)

>> +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.

this introduces redirection that would be annoying for people who
implement just csum_tcpudp_nofold() (like the Blackfin arch).  i dont
have a problem extending the test to also cover csum_tcpudp_magic(),
but i'd like to keep the csum_tcpudp_nofold() invocation as well.
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/