Messages in this thread | | | From | Mike Frysinger <> | Date | Wed, 24 Jun 2009 10:45:33 -0400 | Subject | Re: [PATCH] add checksum selftest |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |