[240] in linux-net channel archive
Re: Packet-length patch to lance.c in 1.2.5?
daemon@ATHENA.MIT.EDU (Dave Platt)
Thu Apr 27 17:10:55 1995
Date: Thu, 27 Apr 95 11:20:21 PDT
From: dplatt@3do.com (Dave Platt)
To: linux-net@vger.rutgers.edu
Ah HAH! A light dawns. I just compared the data-sheet descriptions for
the Am79C960 (PCnet-ISA) and Am79C970 (PCnet-PCI). There is a subtle, but
very significant difference in the wording of the "Receive FCS checking"
paragraph. Based on Russ's comments, I believe that both chips actually
behave the way that the '970 data sheet describes, and that the '960 data
sheet is in error.
The bracketed phrases below appear in the '970 docs but not in the '960
docs.
"Note that if the Automatic Pad Stripping feature is enabled, the FCS
[for padded frames] will be verified against the value computed for the
incoming bit stream including pad characters, but the FCS value [for a
padded frame] will not be passed to the host."
Thus, the '970 version of this paragraph is consistent with the
"Automatic pad stripping" section, which indicates that receive frames
which have a length field of 46 or greater [and which therefore are not
padded] will be passed to the host unmodified. The '960 versions of
"Receive FCS checking" and "Automatic pad stripping" are not consistent
with one another, since the former says that the FCS is _always_
stripped if the Automatic Pad Stripping feature is enabled, and the
latter says that nothing is stripped if the packet isn't padded.
So... revised diagnosis. Russ's modification is probably correct for all
Lance chips, but ONLY for non-padded packets (which are delivered with their
FCS intact and are not stripped). It is probably causing a problem only for
short (padded) packets being received via PCnet chips.
Not by coincidence, this patch causes problems for people who are running
CAP, since the Ethertalk protocol can send short packets which have to be
padded out to the minimum legal Ethernet length. These packets are having
their last four significant bytes stripped off. IP, and I'd guess IPX and
most other protocols, don't send short packets, and thus Russ's patch
would be beneficial or at worst harmless for these protocols.
I can see a couple of possible fixes:
[1] Modify the software FCS stripper to strip the last four bytes from
non-padded Ethernet packets (embedded length >= 46, or perhaps based on
the actual received byte count) and not strip short packets (which had
presumably been padded, and have had the pad stripped) [except on the
older Lance chips, which won't have stripped the FCS and pad.]
[2] Turn off the Automatic Pad Stripping feature in the PCnet chips, so that
none of the chips strip off the FCS and pad. Always strip the last four
bytes via software.
Approach [2] leads to simpler code, and it's more consistent - all of
the FCS stripping would be done in the same way, for all Lance chips
and all packet lengths. The only possible disadvantage I can see to
it, is that when short (padded) packets are received, the code would
end up having to allocate a skbuff which is large enough for the padded
packet. This is probably a non-issue: the extra space is only a few
tens of bytes, and I can't imagine that any sane network code would try
to figure out the length of the received packet by looking at the size
of the skbuff.
Approach [1] would not work if anybody tries to run a weird Ethernet protocol
which doesn't use the "embedded length" field in the conventional way (the
PCnet data sheets indicate that this is possible, although infrequent).
So, I would propose that we leave Russ's patch in place, and simply change
the chip initialization code to turn off the FCS/pad stripping feature during
packet reception.
--- lance.c.1.2.6 Wed Apr 26 11:07:28 1995
+++ lance.c Thu Apr 27 11:18:02 1995
@@ -608,7 +608,7 @@
outw(((int)&lp->init_block) >> 16, ioaddr+LANCE_DATA);
outw(0x0004, ioaddr+LANCE_ADDR);
- outw(0x0d15, ioaddr+LANCE_DATA);
+ outw(0x0915, ioaddr+LANCE_DATA);
outw(0x0000, ioaddr+LANCE_ADDR);
outw(0x0001, ioaddr+LANCE_DATA);
I'll try this out and letcha know. Russ, could you try this and make sure
it doesn't break your own protocol?