Rask Ingemann Lambertsen wrote:
> On Mon, Nov 24, 2003 at 11:55:26PM -0800, Ben Greear wrote:
> 
>>@@ -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?

>> 		} 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...

> 
> 
>>+/**
>>+ * 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.
> 
> 
>>+/* 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 <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com