Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH] add checksum selftest | Date | Wed, 24 Jun 2009 17:24:30 +0200 |
| |
On Wednesday 24 June 2009, Mike Frysinger wrote: > >> +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
I just mean you can consolidate
+struct do_csum_data { + unsigned short ret; + unsigned char *buff; + int len; +}; +#define DO_CSUM_DATA(_num, _ret) \ +{ \ + .ret = _ret, \ + .buff = do_csum_data##_num, \ + .len = ARRAY_SIZE(do_csum_data##_num), \ +}
and
+struct csum_partial_data { + __wsum ret; + const void *buff; + int len; + __wsum wsum; +}; +#define CSUM_PARTIAL_DATA(_num, _ret, _wsum) \ +{ \ + .ret = _ret, \ + .buff = csum_partial_data##_num, \ + .len = ARRAY_SIZE(csum_partial_data##_num), \ + .wsum = _wsum, \ +}
into something like
struct csum_combined_check_data { const char *buff; int len; unsigned short do_csum_ret; __wsum wsum; unsigned short csum_partial_ret; };
#define CSUM_COMBINED_TEST_DATA(_num, _do_csum_ret, \ _csum_partial_ret, _wsum) \ { \ .buff = csum_partial_data##_num, \ .len = ARRAY_SIZE(csum_partial_data##_num), \ .do_csum_ret = _do_csum_ret, \ .wsum = _wsum, \ .csum_partial_ret = csum_partial_ret, \ };
This could cut down the length of the module significantly, without changing any of the semantics.
> >> +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 ;)
The problem is that IP checksumming is only defined for 16-bit words. We use __wsum (32 bits) as an intermediate in the networking stack so we can consolidate the folding in one place. If you have a test vector that results in checksum 0xffff (as a well-formed packet should), the __wsum could be one of 0x0000ffff, 0xffff0000, 0xffffffff, 0x1234edcb, for any other value x where (((x >> 16) + (x & 0xffff)) >> 16 + ((x >> 16) + (x & 0xffff))) & 0xffff = 0xffff. The specific __wsum returned by csum_partial() is implementation specific, so you cannot compare it to a precomputed value unless sending it through csum_fold().
Arnd <><
| |