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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
