2003-01-17 14:45:09

by Florian Lohoff

[permalink] [raw]
Subject: eepro100 - 802.1q - mtu size


Hi,
it seems the current (2.4.20) eepro100 driver is not capable of doing
802.1q as the max received packet size seems to be to low - Receiving
packets with more that 1514 bytes get dropped. The vlan page lists a
patch against a 2.2 kernel which is still valid for 2.4.20 and even
works.

Why is this patch not integerated yet ?

Rediffed:

--- linux-2.4.20/drivers/net/eepro100.c Fri Nov 29 00:53:13 2002
+++ linux-2.4.20.flo/drivers/net/eepro100.c Fri Jan 17 15:30:52 2003
@@ -502,12 +502,12 @@
static const char i82557_config_cmd[CONFIG_DATA_SIZE] = {
22, 0x08, 0, 0, 0, 0, 0x32, 0x03, 1, /* 1=Use MII 0=Use AUI */
0, 0x2E, 0, 0x60, 0,
- 0xf2, 0x48, 0, 0x40, 0xf2, 0x80, /* 0x40=Force full-duplex */
+ 0xf2, 0x48, 0, 0x40, 0xfa, 0x80, /* 0x40=Force full-duplex */
0x3f, 0x05, };
static const char i82558_config_cmd[CONFIG_DATA_SIZE] = {
22, 0x08, 0, 1, 0, 0, 0x22, 0x03, 1, /* 1=Use MII 0=Use AUI */
0, 0x2E, 0, 0x60, 0x08, 0x88,
- 0x68, 0, 0x40, 0xf2, 0x84, /* Disable FC */
+ 0x68, 0, 0x40, 0xfa, 0x84, /* Disable FC */
0x31, 0x05, };

/* PHY media interface chips. */



I tried on this chip:

02:08.0 Ethernet controller: Intel Corp. 82820 (ICH2) Chipset Ethernet Controller (rev 03)
Subsystem: Intel Corp.: Unknown device 3013
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B-
Status: Cap+ 66Mhz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
Latency: 66 (2000ns min, 14000ns max), cache line size 08
Interrupt: pin A routed to IRQ 9
Region 0: Memory at f4900000 (32-bit, non-prefetchable) [size=4K]
Region 1: I/O ports at 3000 [size=64]
Capabilities: [dc] Power Management version 2
Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
Status: D0 PME-Enable- DSel=0 DScale=2 PME-

Flo
--
Florian Lohoff [email protected] +49-5201-669912
Heisenberg may have been here.


Attachments:
(No filename) (2.01 kB)
(No filename) (232.00 B)
Download all attachments

2003-01-17 15:59:45

by Paul Komkoff

[permalink] [raw]
Subject: Re: eepro100 - 802.1q - mtu size

Replying to Florian Lohoff:
> Why is this patch not integerated yet ?

Because newer and better e100 driver, which accepts tagged frames and
handles it properly, already in the tree

--
Paul P 'Stingray' Komkoff Jr /// (icq)23200764 /// (http)stingr.net
This message represents the official view of the voices in my head

2003-01-17 16:19:27

by Jeff Garzik

[permalink] [raw]
Subject: Re: eepro100 - 802.1q - mtu size

On Fri, Jan 17, 2003 at 07:08:40PM +0300, Paul P Komkoff Jr wrote:
> Replying to Florian Lohoff:
> > Why is this patch not integerated yet ?
>
> Because newer and better e100 driver, which accepts tagged frames and
> handles it properly, already in the tree

Regardless, people still use eepro100, so I would still like to get
eepro100 doing VLAN.

The reason why the patch was not accepted is that it changes one magic
number to another magic number, and without chipset docs, I had no idea
what either magic number really meant.

Now that Intel has released chipset docs, this is an excellent time to
re-evaluate those eepro100 VLAN changes. I still refuse to accept a
"change the magic numbers" patch... any change will need to define
a constant that describes the bits we wish to set/clear.

Download the e100 documentation from the e1000 sourceforge site:
http://sourceforge.net/projects/e1000
["8255x Developer Manual"]

Jeff




2003-01-17 17:21:06

by Dave Jones

[permalink] [raw]
Subject: Re: eepro100 - 802.1q - mtu size

On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:

> The reason why the patch was not accepted is that it changes one magic
> number to another magic number, and without chipset docs, I had no idea
> what either magic number really meant.

Whilst on the subject of magic numbers in net drivers, did we ever get
to the bottom of 2.4's ChangeSet 1.587.9.20

ftp://ftp.kernel.org/pub/linux/kernel/people/davej/patches/2.5/2.5.48/split-dj1/net-cs89x0-media-corrections.diff

?

Dave

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2003-01-17 17:40:35

by Jeff Garzik

[permalink] [raw]
Subject: cs89x0 in 2.5 (was Re: eepro100 - 802.1q - mtu size)

On Fri, Jan 17, 2003 at 05:27:19PM +0000, Dave Jones wrote:
> On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:
>
> > The reason why the patch was not accepted is that it changes one magic
> > number to another magic number, and without chipset docs, I had no idea
> > what either magic number really meant.
>
> Whilst on the subject of magic numbers in net drivers, did we ever get
> to the bottom of 2.4's ChangeSet 1.587.9.20
>
> ftp://ftp.kernel.org/pub/linux/kernel/people/davej/patches/2.5/2.5.48/split-dj1/net-cs89x0-media-corrections.diff


IIRC it came from -ac tree without explanation, and I think akpm said it
broke stuff. Since it has an alive maintainer (akpm), I would rather
let Alan and Andrew fight it out :) Whatever they decide is fine with
me for 2.5.

Jeff



2003-01-17 17:39:40

by Bruno Ducrot

[permalink] [raw]
Subject: Re: eepro100 - 802.1q - mtu size

Hi Jeff,

On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:
> On Fri, Jan 17, 2003 at 07:08:40PM +0300, Paul P Komkoff Jr wrote:
> > Replying to Florian Lohoff:
> > > Why is this patch not integerated yet ?
> >
> > Because newer and better e100 driver, which accepts tagged frames and
> > handles it properly, already in the tree
>
> Regardless, people still use eepro100, so I would still like to get
> eepro100 doing VLAN.
>
> The reason why the patch was not accepted is that it changes one magic
> number to another magic number, and without chipset docs, I had no idea
> what either magic number really meant.
>
> Now that Intel has released chipset docs, this is an excellent time to
> re-evaluate those eepro100 VLAN changes. I still refuse to accept a
> "change the magic numbers" patch... any change will need to define
> a constant that describes the bits we wish to set/clear.
>
> Download the e100 documentation from the e1000 sourceforge site:
> http://sourceforge.net/projects/e1000
> ["8255x Developer Manual"]
>
> Jeff

BTW, it look like i82557_config_cmd is never used...



--- linux-2.4/drivers/net/eepro100.c 2003/01/17 17:33:41 1.1
+++ linux-2.4/drivers/net/eepro100.c 2003/01/17 17:34:32
@@ -499,11 +499,13 @@
/* The parameters for a CmdConfigure operation.
There are so many options that it would be difficult to document each bit.
We mostly use the default or recommended settings. */
+#if 0
static const char i82557_config_cmd[CONFIG_DATA_SIZE] = {
22, 0x08, 0, 0, 0, 0, 0x32, 0x03, 1, /* 1=Use MII 0=Use AUI */
0, 0x2E, 0, 0x60, 0,
0xf2, 0x48, 0, 0x40, 0xf2, 0x80, /* 0x40=Force full-duplex */
0x3f, 0x05, };
+#endif
static const char i82558_config_cmd[CONFIG_DATA_SIZE] = {
22, 0x08, 0, 1, 0, 0, 0x22, 0x03, 1, /* 1=Use MII 0=Use AUI */
0, 0x2E, 0, 0x60, 0x08, 0x88,


--
Ducrot Bruno
http://www.poupinou.org Page profaissionelle
http://toto.tu-me-saoules.com Haume page

2003-01-17 19:17:19

by Florian Lohoff

[permalink] [raw]
Subject: Re: eepro100 - 802.1q - mtu size

On Fri, Jan 17, 2003 at 11:28:18AM -0500, Jeff Garzik wrote:
> Regardless, people still use eepro100, so I would still like to get
> eepro100 doing VLAN.
>
> The reason why the patch was not accepted is that it changes one magic
> number to another magic number, and without chipset docs, I had no idea
> what either magic number really meant.

I did the basic decoding of the config block - The result from looking
at the gdb output is the same except the IP ARP Filter basic address
which is now the documentations recommended default value and some
mandatory bits which are filled.

I also set the LONGRCV option which enables to use VLANs.

I tried this patch which is against 2.4.20 with IPv4, IPv6, 802.1q and
DHCP so i'd say full feature test ;). I did not try 82557 which
might need different initialization as some bits are unused but that
problem persists from before this change.



--- drivers/net/eepro100.c-vanilla Fri Jan 17 20:18:17 2003
+++ drivers/net/eepro100.c Fri Jan 17 20:19:39 2003
@@ -496,19 +496,66 @@
#endif
};

-/* The parameters for a CmdConfigure operation.
- There are so many options that it would be difficult to document each bit.
- We mostly use the default or recommended settings. */
-static const char i82557_config_cmd[CONFIG_DATA_SIZE] = {
- 22, 0x08, 0, 0, 0, 0, 0x32, 0x03, 1, /* 1=Use MII 0=Use AUI */
- 0, 0x2E, 0, 0x60, 0,
- 0xf2, 0x48, 0, 0x40, 0xf2, 0x80, /* 0x40=Force full-duplex */
- 0x3f, 0x05, };
+#define i55x_CB1_TXFIFO_LIMIT(x) ((x)<<4)
+#define i55x_CB1_RXFIFO_LIMIT(x) ((x)&0xf)
+
+#define i55x_CB3_MWIENABLE (1<<0)
+
+#define i55x_CB6_EXTSTATCOUNT (1<<5)
+#define i55x_CB6_MANDBITS (1<<1)
+
+#define i55x_CB7_DISCARDSHORT (1<<0)
+#define i55x_CB7_UNDERRUNRETRY(x) ((x)<<1)
+
+#define i557_CB8_USEMII (1<<0)
+#define i558_CB8_MANDBITS i557_CB8_USEMII
+
+#define i55x_CB10_MANDBITS (3<<1)
+#define i55x_CB10_PREAMBLELEN(x) ((x&3)<<4)
+#define i55x_CB10_NSAI (1<<3)
+
+#define i55x_CB12_INTERFRAMESPACE(x) ((x)<<4)
+
+#define i558_CB15_MANDBITS (1<<3|1<<6)
+#define i559_CB16_CRC16 (1<<5)
+
+#define i558_CB18_MANDBITS (1<<7)
+#define i558_CB18_FCTHRESH(x) ((x&7)<<4)
+#define i558_CB18_LONGRCV (1<<3)
+#define i55x_CB18_PAD (1<<1)
+
+#define i558_CB19_TXFC (1<<2)
+#define i558_CB19_AUTOFDX (1<<7)
+
+#define i558_CB20_PRIFCLOC (1<<5)
+#define i558_CB20_MANDBITS (0x1f)
+
+#define i558_CB21_MANDBITS (0x05)
+
static const char i82558_config_cmd[CONFIG_DATA_SIZE] = {
- 22, 0x08, 0, 1, 0, 0, 0x22, 0x03, 1, /* 1=Use MII 0=Use AUI */
- 0, 0x2E, 0, 0x60, 0x08, 0x88,
- 0x68, 0, 0x40, 0xf2, 0x84, /* Disable FC */
- 0x31, 0x05, };
+ CONFIG_DATA_SIZE,
+ i55x_CB1_TXFIFO_LIMIT(0)|i55x_CB1_RXFIFO_LIMIT(8),
+ 0,
+ i55x_CB3_MWIENABLE,
+ 0,
+ 0,
+ i55x_CB6_EXTSTATCOUNT|i55x_CB6_MANDBITS,
+ i55x_CB7_UNDERRUNRETRY(1)|i55x_CB7_DISCARDSHORT,
+ i558_CB8_MANDBITS,
+ 0,
+ i55x_CB10_MANDBITS|i55x_CB10_NSAI|i55x_CB10_PREAMBLELEN(2),
+ 0,
+ i55x_CB12_INTERFRAMESPACE(6),
+ 0x00, /* cb 13 - ARP Filter IP Address Low */
+ 0xf2, /* cb 14 - ARP Filter IP Address High */
+ i558_CB15_MANDBITS|i559_CB16_CRC16,
+ 0, /* cb 16 - FC Delay - LSB */
+ 0x40, /* cb 17 - FC Delay - MSB */
+ i558_CB18_MANDBITS|i558_CB18_FCTHRESH(7)|i558_CB18_LONGRCV|i55x_CB18_PAD,
+ i558_CB19_TXFC|i558_CB19_AUTOFDX,
+ i558_CB20_PRIFCLOC|i558_CB20_MANDBITS,
+ i558_CB21_MANDBITS,
+ };

/* PHY media interface chips. */
static const char *phys[] = {

--
Florian Lohoff [email protected] +49-5201-669912
Heisenberg may have been here.


Attachments:
(No filename) (3.51 kB)
(No filename) (232.00 B)
Download all attachments

2003-01-17 20:05:52

by Paul Komkoff

[permalink] [raw]
Subject: Re: cs89x0 in 2.5 (was Re: eepro100 - 802.1q - mtu size)

Replying to Jeff Garzik:
> > ftp://ftp.kernel.org/pub/linux/kernel/people/davej/patches/2.5/2.5.48/split-dj1/net-cs89x0-media-corrections.diff
>
>
> IIRC it came from -ac tree without explanation, and I think akpm said it
> broke stuff. Since it has an alive maintainer (akpm), I would rather
> let Alan and Andrew fight it out :) Whatever they decide is fine with
> me for 2.5.

It is not a magic number in net drivers. I was author of this bit, and
without it I cannot start onboard nic on IBM PC300 GL (a lot of them I
have here). Actually, to _understand_ what this bit do you just need
to look at the driver source and see _how_ it works with
A_CNF_MEDIA_10B_2 constants. More specific, it expects that there are
single bits, and before my patch A_CNF_MEDIA_10B_2 was 0x60. 0x60 is
not a single bit.

I've thought this change is obvious, and as far as I can remember
Andrew has nothing against it.

--
Paul P 'Stingray' Komkoff Jr /// (icq)23200764 /// (http)stingr.net
This message represents the official view of the voices in my head

2003-01-21 07:07:02

by Alan

[permalink] [raw]
Subject: Re: cs89x0 in 2.5 (was Re: eepro100 - 802.1q - mtu size)

On Fri, 2003-01-17 at 17:49, Jeff Garzik wrote:
> > Whilst on the subject of magic numbers in net drivers, did we ever get
> > to the bottom of 2.4's ChangeSet 1.587.9.20
> >
> > ftp://ftp.kernel.org/pub/linux/kernel/people/davej/patches/2.5/2.5.48/split-dj1/net-cs89x0-media-corrections.diff
>
>
> IIRC it came from -ac tree without explanation, and I think akpm said it
> broke stuff. Since it has an alive maintainer (akpm), I would rather
> let Alan and Andrew fight it out :) Whatever they decide is fine with
> me for 2.5.

I've had no reports I remember about it breaking stuff, and several that
it fixed stuff. It also seems to match the documentation. Its been in -ac
for ages

2003-01-21 23:31:34

by Jeff Garzik

[permalink] [raw]
Subject: Re: cs89x0 in 2.5 (was Re: eepro100 - 802.1q - mtu size)

Alan wrote:
> I've had no reports I remember about it breaking stuff, and several that
> it fixed stuff. It also seems to match the documentation. Its been in -ac
> for ages

Andrew wrote:
> I've seen no reports either way, and appear to have misplaced the
> datasheet.
>
> May as well run with it.


Sounds good. Dave, whatever your preference for patching handling is,
is all good. I guess yourself, myself, and akpm are all patch-mongering
candidates...

Jeff