Rask Ingemann Lambertsen wrote:

On Mon, Nov 24, 2003 at 11:55:26PM -0800, Ben Greear wrote:

quoted

@@ -2052,13 +2065,26 @@

			if (bdp->rev_id >= D102_REV_ID) {
				skb->ip_summed = e100_D102_check_checksum(rfd);
			} else {
-				skb->ip_summed = e100_D101M_checksum(bdp, skb);
+				skb->ip_summed = e100_D101M_checksum(bdp, skb, !!(dev->priv_flags & IFF_ACCEPT_ALL_FRAMES));
			}

Shouldn't that be IFF_SAVE_FCS rather than IFF_ACCEPT_ALL_FRAMES?

Yes, will fix, actually, considering your next comment, it should not even
be needed at all, eh?

quoted

		} else {
			skb->ip_summed = CHECKSUM_NONE;
		}

+		/* Show the FCS? */
+		if (unlikely(dev->priv_flags & IFF_SAVE_FCS)) {
+			if (bdp->rev_id < D102_REV_ID) {
+				/* Have to over-write the two IP checksum bytes
+				 * TODO:  Will this break vlan_hwaccel_rx???
+				 */
+				skb->tail[-4] = skb->tail[-2];
+				skb->tail[-3] = skb->tail[-1];
+				skb->tail[-2] = skb->tail[0];
+				skb->tail[-1] = skb->tail[1];
+			}
+		}
+		

I don't understand this part of the code. The 55x docs say that the IP
checksum bytes are transferred to memory _following_ the FCS.

I can't find this in the docs, but it could easily be true.  If you have a page/section
number, please let me know.  I don't appear to have hardware that takes this
branch at any rate.  Anyone know which chipset/NIC has this particular rev-id?

Also, this should invalidate all of the hacks from the e100_D101M_checksum code...

quoted

+/**
+ * e100_config_promisc - configure promiscuous mode
+ * @bdp: atapter's private data struct
+ * @enable: should we enable this option or not

s/atapter/adapter/g

s/etherne /ethernet /g too, somewhere, IIRC.

quoted

+/* Only valid for 82558 and 82559.  Must be zero for 82557 */
+#define CB_CFIG_LONG_RX_OK          BIT_3	/* OK to receive Long frames */

I find it disappointing that the good, old 82586 transfers long frames to
memory without complaint while newer chips such as the 82557 and tulip
can't/don't.

Tulip can at least work with VLANs with a hack or two, but not sure how it's
max longness.

There may be other ways to get the 82557 to do VLANs, but the docs definately
indicate the rx-long bit does not exist for 82557 (page 62-64, byte 18, bit 3)
of the e100 docs from sourceforge.

Thanks for the thorough review.  Attached is a new patch that I believe
addresses these problems.

Ben

-- 
Ben Greear [off-list ref]
Candela Technologies Inc  http://www.candelatech.com