David S.Miller wrote:

From: Jeff Garzik <redacted>
Date: Fri, 20 May 2005 15:42:20 -0400

quoted

10) [additional review] doesn't bnx2_rx_int() need to move the rmb()
inside the loop?  Are you protecting against the compiler
reordering/caching loads/stores, or against SMP CPUs?

This rmb() is needed to strongly order the status block consumer
index read, from that of the descriptor data.  I'm pretty sure it's
in the right spot.

quoted

10.1) [additional review] is the rmb() even needed in bnx2_rx_int(),
since its caller also uses rmb()?

No, it's guarding status block consumer index read to the first
RX descriptor read, as explained above.

OK

quoted

13) [additional review] why is CHECKSUM_UNNECESSARY used when
cksum==0xffff or cksum==0 ?

+                       u16 cksum = rx_hdr->l2_fhdr_tcp_udp_xsum;
+
+                       if ((cksum == 0xffff) || (cksum == 0)) {
+                               skb->ip_summed = CHECKSUM_UNNECESSARY;
+                       }

For UDP, a checksum value of zero means no checksum at all.

Sure.  What I'm driving at is that a checksum of zero seems to imply 
CHECKSUM_NONE not CHECKSUM_UNNECESSARY.  tg3 only does the 0xffff check.

I am also a bit surprised that, if the actual checksum value is 
available, why not use CHECKSUM_HW like sunhme?

	Jeff