Re: [PATCH 05/17] Blackfin: convert to generic checksum code

[Mike Frysinger]

On Mon, Jun 15, 2009 at 07:04, Arnd Bergmann wrote:
> On Sunday 14 June 2009, Mike Frysinger wrote:
>> The Blackfin port only implemented an optimized version of the
>> csum_tcpudp_nofold function, so convert everything else to the new
>> generic code.
>>
>> Signed-off-by: Mike Frysinger <vapier@xxxxxxxxxx>
>
> Have you tested this one well? I was as careful as possible with the
> version I added, but it was basically only tested on microblaze, which
> has a different endianess from blackfin. Some areas of the code may be
> sensitive to this.

i did some tests and it looks like do_csum() is broken :(

here's the input:
do_csum({ 0xff 0xfb 0x01 }, 3)

and here's the output:
Blackfin: 0xfc00
generic: 0xfcff

if i run the two funcs on my x86, i see similar behavior.  the
attached csum-test.c contains the csum code from lib/checksum.c and
arch/blackfin/lib/checksum.c and shows the problem.
-mike
#include <stdio.h>

static inline unsigned short gen_from32to16(unsigned long x)
{
	/* add up 16-bit and 16-bit for 16+c bit */
	x = (x & 0xffff) + (x >> 16);
	/* add up carry.. */
	x = (x & 0xffff) + (x >> 16);
	return x;
}

static unsigned int gen_do_csum(const unsigned char *buff, int len)
{
	int odd, count;
	unsigned long result = 0;

	if (len <= 0)
		goto out;
	odd = 1 & (unsigned long) buff;
	if (odd) {
		result = *buff;
		len--;
		buff++;
	}
	count = len >> 1;		/* nr of 16-bit words.. */
	if (count) {
		if (2 & (unsigned long) buff) {
			result += *(unsigned short *) buff;
			count--;
			len -= 2;
			buff += 2;
		}
		count >>= 1;		/* nr of 32-bit words.. */
		if (count) {
			unsigned long carry = 0;
			do {
				unsigned long w = *(unsigned long *) buff;
				count--;
				buff += 4;
				result += carry;
				result += w;
				carry = (w > result);
			} while (count);
			result += carry;
			result = (result & 0xffff) + (result >> 16);
		}
		if (len & 2) {
			result += *(unsigned short *) buff;
			buff += 2;
		}
	}
	if (len & 1)
		result += (*buff << 8);
	result = gen_from32to16(result);
	if (odd)
		result = ((result >> 8) & 0xff) | ((result & 0xff) << 8);
out:
	return result;
}

static unsigned short bf_do_csum(const unsigned char *buff, int len)
{
	register unsigned long sum = 0;
	int swappem = 0;

	if (1 & (unsigned long)buff) {
		sum = *buff << 8;
		buff++;
		len--;
		++swappem;
	}

	while (len > 1) {
		sum += *(unsigned short *)buff;
		buff += 2;
		len -= 2;
	}

	if (len > 0)
		sum += *buff;

	/*  Fold 32-bit sum to 16 bits */
	while (sum >> 16)
		sum = (sum & 0xffff) + (sum >> 16);

	if (swappem)
		sum = ((sum & 0xff00) >> 8) + ((sum & 0x00ff) << 8);

	return sum;

}

int main()
{
	unsigned char buf[] = { 0xff, 0xfb, 0x01 };
	unsigned short gen = gen_do_csum(buf, 3);
	unsigned short bf = bf_do_csum(buf, 3);
	printf("%x vs %x\n", gen, bf);
	return 0;
}