Re: [PATCH] mv643xx: fix byte order when checksum offload is enabled

[Al Viro <viro@ZenIV.linux.org.uk>]

On Sat, Jan 19, 2008 at 07:27:38PM +0000, Byron Bradley wrote:
>  		case IPPROTO_UDP:
>  			cmd_sts |= ETH_UDP_FRAME;
> -			desc->l4i_chk = udp_hdr(skb)->check;
> +			desc->l4i_chk = htons(udp_hdr(skb)->check);
>  			break;
>  		case IPPROTO_TCP:
> -			desc->l4i_chk = tcp_hdr(skb)->check;
> +			desc->l4i_chk = htons(tcp_hdr(skb)->check);
>  			break;

The first part was OK, but this one...  AFAICS, the sucker byteswaps
64bit words in descriptors wholesale, right?  Then the right way to spell
that would be ntohs((__force __be16)....->check).

What's happening here is that we take a fixed-endian (checksum) and then
correct for conversion done in hardware - i.e. we store it in something
that expects a _host_-endian value and would convert that to fixed-endian.
So we need to counter that correction and that's where the damn thing is
coming from.

It's not particulary rare - drivers that do hardware byteswap tend to need
it.  For now I'd suggest explicit form (with force-cast from __csum to
__be16 and ntohs() on top of it); if anybody has good ideas for helper
names, though...  csum_as_le() and csum_as_be(), perhaps?  Interpret
__csum (fixed-endian) and another fixed-endian type, with proper checks;
i.e. something along the lines of
static inline __be16 csum_as_be(__csum sum)
{
	return (__force __be16)sum;
}
and this stuff becoming ntohs(csum_as_be(udp_hdr(skb)->check)), etc.