PATCH: VLAN support for 3c59x/3c90x

PATCH: VLAN support for 3c59x/3c90x

[Alan Cox]

This adds VLAN support to the 3c59x/90x series hardware.

Stefan de Konink ported this code from the 2.4 VLAN patches and tested it
extensively. I cleaned up the ifdefs and fixed a problem with bracketing
that made older cards fail.

--

Developer's Certificate of Origin 1.0

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the
right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my
knowledge, is covered under an appropriate open source license and I have
the right under that license to submit that work with modifications,
whether created in whole or in part by me, under the same open source
license (unless I am permitted to submit under a different license), as
indicated in the file; or

(c) The contribution was provided directly to me by some other person who
certified (a), (b) or (c) and I have not modified it.


I, Stefan de Konink, certify that:
 The contribution is based upon previous work that, again is based on GPL
code and I have the right under that license to submit that work with
modifications, whether created in whole or in part by me, under the same
open source license.

I, Alan Cox, certify likewise.


---


--- drivers/net/3c59x.c.old	2004-07-28 12:54:03.048561056 +0100
+++ drivers/net/3c59x.c	2004-07-28 13:36:31.500137528 +0100
@@ -695,7 +695,7 @@
 	Wn2_ResetOptions=12,
 };
 enum Window3 {			/* Window 3: MAC/config bits. */
-	Wn3_Config=0, Wn3_MAC_Ctrl=6, Wn3_Options=8,
+	Wn3_Config=0, Wn3_MaxPktSize=4, Wn3_MAC_Ctrl=6, Wn3_Options=8,
 };
 
 #define BFEXT(value, offset, bitcount)  \
@@ -723,7 +723,8 @@
 	Media_LnkBeat = 0x0800,
 };
 enum Window7 {					/* Window 7: Bus Master control. */
-	Wn7_MasterAddr = 0, Wn7_MasterLen = 6, Wn7_MasterStatus = 12,
+	Wn7_MasterAddr = 0, Wn7_VlanEtherType=4, Wn7_MasterLen = 6,
+	Wn7_MasterStatus = 12,
 };
 /* Boomerang bus master control registers. */
 enum MasterCtrl {
@@ -819,7 +820,8 @@
 		pm_state_valid:1,				/* power_state[] has sane contents */
 		open:1,
 		medialock:1,
-		must_free_region:1;				/* Flag: if zero, Cardbus owns the I/O region */
+		must_free_region:1,				/* Flag: if zero, Cardbus owns the I/O region */
+		large_frames:1;			/* accept large frames */
 	int drv_flags;
 	u16 status_enable;
 	u16 intr_enable;
@@ -904,6 +906,8 @@
 static void vortex_tx_timeout(struct net_device *dev);
 static void acpi_set_WOL(struct net_device *dev);
 static struct ethtool_ops vortex_ethtool_ops;
+static void set_8021q_mode(struct net_device *dev, int enable);
+
 \f
 /* This driver uses 'options' to pass the media type, full-duplex flag, etc. */
 /* Option count limit only -- unlimited interfaces are supported. */
@@ -1164,6 +1168,7 @@
 	dev->base_addr = ioaddr;
 	dev->irq = irq;
 	dev->mtu = mtu;
+	vp->large_frames = mtu > 1500;
 	vp->drv_flags = vci->drv_flags;
 	vp->has_nway = (vci->drv_flags & HAS_NWAY) ? 1 : 0;
 	vp->io_size = vci->io_size;
@@ -1616,7 +1621,7 @@
 
 	/* Set the full-duplex bit. */
 	outw(	((vp->info1 & 0x8000) || vp->full_duplex ? 0x20 : 0) |
-		 	(dev->mtu > 1500 ? 0x40 : 0) |
+		 	(vp->large_frames ? 0x40 : 0) |
 			((vp->full_duplex && vp->flow_ctrl && vp->partner_flow_ctrl) ? 0x100 : 0),
 			ioaddr + Wn3_MAC_Ctrl);
 
@@ -1700,6 +1705,8 @@
 	}
 	/* Set receiver mode: presumably accept b-case and phys addr only. */
 	set_rx_mode(dev);
+	/* enable 802.1q tagged frames */
+	set_8021q_mode(dev, 1);
 	outw(StatsEnable, ioaddr + EL3_CMD); /* Turn on statistics. */
 
 //	issue_and_wait(dev, SetTxStart|0x07ff);
@@ -1842,7 +1849,7 @@
 						/* Set the full-duplex bit. */
 						EL3WINDOW(3);
 						outw(	(vp->full_duplex ? 0x20 : 0) |
-								(dev->mtu > 1500 ? 0x40 : 0) |
+								(vp->large_frames ? 0x40 : 0) |
 								((vp->full_duplex && vp->flow_ctrl && vp->partner_flow_ctrl) ? 0x100 : 0),
 								ioaddr + Wn3_MAC_Ctrl);
 						if (vortex_debug > 1)
@@ -2068,6 +2075,8 @@
 			issue_and_wait(dev, RxReset|0x07);
 			/* Set the Rx filter to the current state. */
 			set_rx_mode(dev);
+			/* enable 802.1q VLAN tagged frames */
+			set_8021q_mode(dev, 1);
 			outw(RxEnable, ioaddr + EL3_CMD); /* Re-enable the receiver. */
 			outw(AckIntr | HostError, ioaddr + EL3_CMD);
 		}
@@ -2672,6 +2681,9 @@
 	outw(RxDisable, ioaddr + EL3_CMD);
 	outw(TxDisable, ioaddr + EL3_CMD);
 
+	/* Disable receiving 802.1q tagged frames */
+	set_8021q_mode(dev, 0);
+
 	if (dev->if_port == XCVR_10base2)
 		/* Turn off thinnet power.  Green! */
 		outw(StopCoax, ioaddr + EL3_CMD);
@@ -2947,6 +2959,61 @@
 	outw(new_mode, ioaddr + EL3_CMD);
 }
 
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+/* Setup the card so that it can receive frames with an 802.1q VLAN tag.
+   Note that this must be done after each RxReset due to some backwards
+   compatibility logic in the Cyclone and Tornado ASICs */
+
+/* The Ethernet Type used for 802.1q tagged frames */
+#define VLAN_ETHER_TYPE 0x8100
+
+static void set_8021q_mode(struct net_device *dev, int enable)
+{
+	struct vortex_private *vp = (struct vortex_private *)dev->priv;
+	long ioaddr = dev->base_addr;
+	int old_window = inw(ioaddr + EL3_CMD);
+	int mac_ctrl;
+	
+	if ((vp->drv_flags&IS_CYCLONE) || (vp->drv_flags&IS_TORNADO)) {
+		/* cyclone and tornado chipsets can recognize 802.1q
+		 * tagged frames and treat them correctly */
+
+		int max_pkt_size = dev->mtu+14;	/* MTU+Ethernet header */
+		if (enable)
+			max_pkt_size += 4;	/* 802.1Q VLAN tag */
+
+		EL3WINDOW(3);
+		outw(max_pkt_size, ioaddr+Wn3_MaxPktSize);
+
+		/* set VlanEtherType to let the hardware checksumming
+		   treat tagged frames correctly */
+		EL3WINDOW(7);
+		outw(VLAN_ETHER_TYPE, ioaddr+Wn7_VlanEtherType);
+	} else {
+		/* on older cards we have to enable large frames */
+
+		vp->large_frames = dev->mtu > 1500 || enable;
+
+		EL3WINDOW(3);
+		mac_ctrl = inw(ioaddr+Wn3_MAC_Ctrl);
+		if (vp->large_frames)
+			mac_ctrl |= 0x40;
+		else
+			mac_ctrl &= ~0x40;
+		outw(mac_ctrl, ioaddr+Wn3_MAC_Ctrl);
+	}
+
+	EL3WINDOW(old_window);
+}
+#else
+
+static void set_8021q_mode(struct net_device *dev, int enable)
+{
+}
+
+
+#endif
+
 /* MII transceiver control section.
    Read and write the MII registers using software-generated serial
    MDIO protocol.  See the MII specifications or DP83840A data sheet

Re: PATCH: VLAN support for 3c59x/3c90x

[Ben Greear]

Alan Cox wrote:
> This adds VLAN support to the 3c59x/90x series hardware.
> 
> Stefan de Konink ported this code from the 2.4 VLAN patches and tested it
> extensively. I cleaned up the ifdefs and fixed a problem with bracketing
> that made older cards fail.

I am sure this will be appreciated by the VLAN users!

Also, do you happen to know how large of an MTU these cards
can support?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: VLAN support for 3c59x/3c90x

[Alan Cox]

On Wed, Jul 28, 2004 at 02:33:56PM -0700, Ben Greear wrote:
> >Stefan de Konink ported this code from the 2.4 VLAN patches and tested it
> >extensively. I cleaned up the ifdefs and fixed a problem with bracketing
> >that made older cards fail.
> 
> I am sure this will be appreciated by the VLAN users!
> 
> Also, do you happen to know how large of an MTU these cards
> can support?

In VLAN mode they support just the extra VLAN bits, with the length
checking turned off its either FDDI or jumbo frame size but I don't remember
which. I think FDDI.

Re: PATCH: VLAN support for 3c59x/3c90x

[Ben Greear]

Alan Cox wrote:
> On Wed, Jul 28, 2004 at 02:33:56PM -0700, Ben Greear wrote:
> 
>>>Stefan de Konink ported this code from the 2.4 VLAN patches and tested it
>>>extensively. I cleaned up the ifdefs and fixed a problem with bracketing
>>>that made older cards fail.
>>
>>I am sure this will be appreciated by the VLAN users!
>>
>>Also, do you happen to know how large of an MTU these cards
>>can support?
> 
> 
> In VLAN mode they support just the extra VLAN bits, with the length
> checking turned off its either FDDI or jumbo frame size but I don't remember
> which. I think FDDI.

So if you try to configure the MTU to 1504 using ifconfig or whatever,
will that actually work?

One reason I ask is that I saw a report of a bug/issue where it was
not possible to send a raw-packet from user space that is bigger than
the MTU.  This would preclude user-space from generating a VLAN packet
with the extra 4 bytes.  Being able to change the MTU would at least
partially work around this issue.

Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: VLAN support for 3c59x/3c90x

[Alan Cox]

On Wed, Jul 28, 2004 at 03:30:53PM -0700, Ben Greear wrote:
> So if you try to configure the MTU to 1504 using ifconfig or whatever,
> will that actually work?

It should work fine yes.

Re: PATCH: VLAN support for 3c59x/3c90x

[Andrew Morton]

Alan Cox <alan@redhat.com> wrote:
>
> This adds VLAN support to the 3c59x/90x series hardware.

Sigh.  This has been floating about for ever.  My reluctance stemmed
from largely-theoretical-sounding objections from Don Becker which I
didn't fully understand at the time and have now forgotten.

Ben, does the patch look complete/correct to you?

Re: PATCH: VLAN support for 3c59x/3c90x

[Ben Greear]

Andrew Morton wrote:
> Alan Cox <alan@redhat.com> wrote:
> 
>>This adds VLAN support to the 3c59x/90x series hardware.
> 
> 
> Sigh.  This has been floating about for ever.  My reluctance stemmed
> from largely-theoretical-sounding objections from Don Becker which I
> didn't fully understand at the time and have now forgotten.
> 
> Ben, does the patch look complete/correct to you?

I don't have such hardware, but I don't see any obvious problems
with the patch.

In my opinion Becker's complaints were invalid, or maybe I just
didn't understand what he was trying to say.  At any rate, lots of
other NICs have supported larger MTUs and VLANs w/out problem, so
it is unlikely that there is a fundamental flaw in accepting larger
frames.

There are patches for tulip floating around too.  I have been running
traffic on these patches for a while with no obvious problems
(on 2.4 kernel, however).  Jeff, if you want me to re-send this to you,
please let me know!

Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

Hi !

On Wed, Jul 28, 2004 at 02:45:47PM -0700, Ben Greear wrote:
 
> In my opinion Becker's complaints were invalid, or maybe I just
> didn't understand what he was trying to say.  At any rate, lots of
> other NICs have supported larger MTUs and VLANs w/out problem, so
> it is unlikely that there is a fundamental flaw in accepting larger
> frames.

I've already used these cards with larger MTU to experiment some tunnelling
without decreasing the tunnel's MTU. I believe I used a NIC MTU around 1518
so that it could pass a cisco switch limited to 1536 bytes per frame. It
never went into production but it worked well.

> There are patches for tulip floating around too.  I have been running
> traffic on these patches for a while with no obvious problems
> (on 2.4 kernel, however).  Jeff, if you want me to re-send this to you,
> please let me know!

I noticed a bug in the 2.4 tulip driver concerning MTU. The parameter
is correctly declared as a static int, initialized with default values,
checked by the code, but not declared as MODULE_PARM, so the user cannot
change it ! I wanted to send a patch but didn't find time to work on it
yet. So if your vlan patch fixes it, it's welcome :-)

Cheers,
Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Herbert Xu]

Willy Tarreau <willy@w.ods.org> wrote:
> 
> I noticed a bug in the 2.4 tulip driver concerning MTU. The parameter
> is correctly declared as a static int, initialized with default values,
> checked by the code, but not declared as MODULE_PARM, so the user cannot
> change it ! I wanted to send a patch but didn't find time to work on it
> yet. So if your vlan patch fixes it, it's welcome :-)

Why is this a module parameter at all? Can't you set it using ifconfig?
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

On Fri, Jul 30, 2004 at 12:20:17PM +1000, Herbert Xu wrote:
> Willy Tarreau <willy@w.ods.org> wrote:
> > 
> > I noticed a bug in the 2.4 tulip driver concerning MTU. The parameter
> > is correctly declared as a static int, initialized with default values,
> > checked by the code, but not declared as MODULE_PARM, so the user cannot
> > change it ! I wanted to send a patch but didn't find time to work on it
> > yet. So if your vlan patch fixes it, it's welcome :-)
> 
> Why is this a module parameter at all? Can't you set it using ifconfig?

no, because the driver has no change_mtu() function, so it uses the generic
one, eth_change_mtu(), which is bound to 1500.

Regards,
Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Herbert Xu]

Willy Tarreau <willy@w.ods.org> wrote:
>
>> Why is this a module parameter at all? Can't you set it using ifconfig?
> 
> no, because the driver has no change_mtu() function, so it uses the generic
> one, eth_change_mtu(), which is bound to 1500.

What is preventing you from implementing a change_mtu() function?
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

Hi Herbert,

On Sat, Jul 31, 2004 at 01:57:04PM +1000, Herbert Xu wrote:
> Willy Tarreau <willy@w.ods.org> wrote:
> > no, because the driver has no change_mtu() function, so it uses the generic
> > one, eth_change_mtu(), which is bound to 1500.
> 
> What is preventing you from implementing a change_mtu() function?

well, now I see where you want to bring me :-)

So several reasons :
  - the change_mtu() function might be called at any time after driver
    initialization. I don't know at all if there are things to do to
    "lock" the hardware during such changes, as well as I don't know
    what parts of the code I will need to extract to change the hard
    MTU. The initial MTU is really different since it's used to
    initialize hardware registers. The generic change_mtu() function
    only plays with dev->mtu and not hardware since it never goes
    above standard size. I could try, but if it works I would offer
    no warranties for other hardware.

  - I really, really, really... lack time. I would do this during
    my few hours nighty sleep and I wouldn't want to use the resulting
    code :-)

  - many (all ?) other drivers already have an MTU parameter, and many
    of them don't have a problem with using generic change_mtu(). So why
    would this one in particular need such a change ? (and please don't
    tell me that *I* will have to do this for all others :-))

As previously said, I can take a few minutes to add the 'MODULE_PARM'
line, it's not much more than replying to this mail. At least it will
be a good start.

Cheers,
Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

Herbert, your mailbox is bouncing back. I hope you'll be able to see this
right here.

Cheers,
Willy

On Sat, Jul 31, 2004 at 05:46:03AM -0300, Herbert Xu wrote:
> Erro ao enviar o email para Herbert Xu <herbert@gondor.apana.org.au>
> A caixa postal do destinatario esta cheia. O email foi recusado
> 
> The mailbox is full. The email was rejected.

Re: PATCH: VLAN support for 3c59x/3c90x

[Jeff Garzik]

Willy Tarreau wrote:
>   - many (all ?) other drivers already have an MTU parameter, and many

s/many/almost none/


>     of them don't have a problem with using generic change_mtu(). So why
>     would this one in particular need such a change ? (and please don't
>     tell me that *I* will have to do this for all others :-))

For VLAN support you definitely want to let the user increase the size 
above 1500, and for that you need ->change_mtu

	Jeff

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

Hi Jeff,

On Sat, Jul 31, 2004 at 05:34:41AM -0400, Jeff Garzik wrote:
> Willy Tarreau wrote:
> >  - many (all ?) other drivers already have an MTU parameter, and many
> 
> s/many/almost none/

Ok, sorry, I've just checked, they are 6. But I incidentely used the feature
on 2 of them (dl2k and starfire). But more drivers still have the
'static int mtu=1500' preceeded by a comment stating "allow the user to change
the mtu". Why is it not a #define then, if nobody can change it anymore ?

> For VLAN support you definitely want to let the user increase the size 
> above 1500, and for that you need ->change_mtu

I agree, but my point was that adding MODULE_PARM was only a one liner and
would have done the job too. But since everyone prefers a change_mtu(), I'll
do it.

Jeff, do you know the absolute hardware limit on the tulip ? I've seen the
limitation to PKT_BUF_SZ (1536), but I don't know for example if the
hardware stores the FCS in the buffer or not, nor if the IP headers risk
being aligned or not (which would consume 2 more bytes).
Or does 1536 - 14 (ethernet) - 2 (iphdr alignment) - 4 (FCS) = 1516 seem a
reasonable conservative higher bound ?

Cheers,
Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Matti Aarnio]

On Sat, Jul 31, 2004 at 12:11:52PM +0200, Willy Tarreau wrote:
> Hi Jeff,
> 
> On Sat, Jul 31, 2004 at 05:34:41AM -0400, Jeff Garzik wrote:
> > Willy Tarreau wrote:
> > >  - many (all ?) other drivers already have an MTU parameter, and many
> > 
> > s/many/almost none/
> 
> Ok, sorry, I've just checked, they are 6. But I incidentely used the feature
> on 2 of them (dl2k and starfire). But more drivers still have the
> 'static int mtu=1500' preceeded by a comment stating "allow the user to change
> the mtu". Why is it not a #define then, if nobody can change it anymore ?

In the older kernels that allowed for module parameter loading
sufficiently, I recall.  Now couple additional macroes are needed
to publish such parameters.  APIs do change in Linux kernel.

I have been pondering on the issue of usefullness of ->change_mtu
for this use.   One of the bigger issues is, like Willy notes, that
the MTU change information is given to the driver after it is already
up and about, which requires then running setup magics which usually
need running reset...

I do myself prefer MODULE_PARAM.

Also the Linux kernel isn't very well happy with multi-path stacking
of layer-2 driver modules.  A side-effect from all of these things might
be, that setting up some dozen VLANs to an ethernet with result in
two, or in worst case, a dozen+1 setup runs.  And if last VLAN setup
is (for any reason) smaller MTU value than 1500, it might even result
in entire driver to be configured for e.g. 1280+4 byte MTU..

To prevent that from happening, it is sufficient in the eth driver to
not to shrink its MTU except via card shutdown -- but then, IFCONFIG
data for e.g. IP layer needs separation from the hardware driver layer.

The way that VLAN (the 8021q module) is implemented in Linux does allow
the primary card to be downed (stopped), but not unloaded without downing
all vlans attached on it (and this preventing pointers to point into 
junk.. preventing chaos, madness and PANIC from such..)


For this IFCONFIG MTU issue, I would rather have the VLAN code to ask
the underlaying driver of what MTU can be supported, than just blindly
presume that 1500 will be functional for e.g. eth0.2  (like it does now)

Things would just magically work, if the uping of  eth0.2  would setup
itself to MTU 1496, unless the underlaying eth0 can handle real 1504 byte
802.1q frames.

> > For VLAN support you definitely want to let the user increase the size 
> > above 1500, and for that you need ->change_mtu
> 
> I agree, but my point was that adding MODULE_PARM was only a one liner and
> would have done the job too. But since everyone prefers a change_mtu(), I'll
> do it.
> 
> Jeff, do you know the absolute hardware limit on the tulip ? I've seen the
> limitation to PKT_BUF_SZ (1536), but I don't know for example if the
> hardware stores the FCS in the buffer or not, nor if the IP headers risk
> being aligned or not (which would consume 2 more bytes).
> Or does 1536 - 14 (ethernet) - 2 (iphdr alignment) - 4 (FCS) = 1516 seem a
> reasonable conservative higher bound ?

The Tulip (21143 at least) can do chained block receive; if first memory
block is too short, it can continue to next one.  This way maximum frame
size is at least 2560 bytes.  For transmit the Jabber timer seems to
trip at 2560, including preamples and crcs.  Also, there is a receive 
watchdog, that is guaranteed to pass 2048 byte frames, and timeout at
2560 byte frames.  (When the watchdog is not disabled, that is.) See 
CSR15<4>.  For transmit the Jabber-Clock bites at 2048-2560 bytes,
OR at timer of 2.6-3.3 ms (of 100 Mbps) which means at least 32 000 bytes.
( CSR15<2> )

In the receive descriptors there might appear a TL bit (Frame Too Long),
which is just telling that frame size exceeds 1518 bytes.
If RW (Receive Watchdog; RDES0<4>) has tripped, then there is at least
2048 bytes long frame, most likely longer than 2560 bytes.

Based on my reading of  ds21143hrm.pdf  (copy of which I have), I do
think it is safe to just receive larger frames with Tulip, and IGNORE
the "TL" bit.

Receiving 1522 byte frames from ethernet with Tulip should be trivial.
Will that be true with 21140 -- oddly I don't have a copy of 21140 HRM
in PDF form...  Possibly I got it in paper long ago.

> Cheers,
> Willy

/Matti Aarnio

Re: PATCH: VLAN support for 3c59x/3c90x

[Jeff Garzik]

Matti Aarnio wrote:
> On Sat, Jul 31, 2004 at 12:11:52PM +0200, Willy Tarreau wrote:
>>Ok, sorry, I've just checked, they are 6. But I incidentely used the feature
>>on 2 of them (dl2k and starfire). But more drivers still have the
>>'static int mtu=1500' preceeded by a comment stating "allow the user to change
>>the mtu". Why is it not a #define then, if nobody can change it anymore ?
> 
> 
> In the older kernels that allowed for module parameter loading
> sufficiently, I recall.  Now couple additional macroes are needed
> to publish such parameters.  APIs do change in Linux kernel.
> 
> I have been pondering on the issue of usefullness of ->change_mtu
> for this use.   One of the bigger issues is, like Willy notes, that
> the MTU change information is given to the driver after it is already
> up and about, which requires then running setup magics which usually
> need running reset...

First, MTU change need not occur while the interface is up.

Second, modern hardware deals a lot better with MTU changes.  Some only 
need a write into a register.  Some need no reset at all, as long as you 
don't exceed the hardware limit.


> Also the Linux kernel isn't very well happy with multi-path stacking
> of layer-2 driver modules.  A side-effect from all of these things might

Elaboration?  The 2.6.x net drivers do proper refcounting, unlike a lot 
of other drivers.


> To prevent that from happening, it is sufficient in the eth driver to
> not to shrink its MTU except via card shutdown -- but then, IFCONFIG
> data for e.g. IP layer needs separation from the hardware driver layer.

In general ifconfig data should definitely -not- be separate from the 
driver.  In particular changing MTU definitely needs to be tightly 
integrated with the driver.


> For this IFCONFIG MTU issue, I would rather have the VLAN code to ask
> the underlaying driver of what MTU can be supported, than just blindly
> presume that 1500 will be functional for e.g. eth0.2  (like it does now)

The VLAN code could certainly be updated to poke at the lower level 
driver MTU.


>>>For VLAN support you definitely want to let the user increase the size 
>>>above 1500, and for that you need ->change_mtu
>>
>>I agree, but my point was that adding MODULE_PARM was only a one liner and
>>would have done the job too. But since everyone prefers a change_mtu(), I'll
>>do it.
>>
>>Jeff, do you know the absolute hardware limit on the tulip ? I've seen the
>>limitation to PKT_BUF_SZ (1536), but I don't know for example if the
>>hardware stores the FCS in the buffer or not, nor if the IP headers risk
>>being aligned or not (which would consume 2 more bytes).
>>Or does 1536 - 14 (ethernet) - 2 (iphdr alignment) - 4 (FCS) = 1516 seem a
>>reasonable conservative higher bound ?
> 
> 
> The Tulip (21143 at least) can do chained block receive; if first memory
> block is too short, it can continue to next one.  This way maximum frame

Yes, but receiving packets not wholly contained in a single frame is SO 
sub-optimal that it is to be avoided at all costs.

Maybe when receive scatter-gather is fully supported this can change, 
but for now the driver should not be returning multi-frag frames to the 
kernel.


> size is at least 2560 bytes.  For transmit the Jabber timer seems to
> trip at 2560, including preamples and crcs.  Also, there is a receive 
> watchdog, that is guaranteed to pass 2048 byte frames, and timeout at
> 2560 byte frames.  (When the watchdog is not disabled, that is.) See 
> CSR15<4>.  For transmit the Jabber-Clock bites at 2048-2560 bytes,
> OR at timer of 2.6-3.3 ms (of 100 Mbps) which means at least 32 000 bytes.
> ( CSR15<2> )
> 
> In the receive descriptors there might appear a TL bit (Frame Too Long),
> which is just telling that frame size exceeds 1518 bytes.
> If RW (Receive Watchdog; RDES0<4>) has tripped, then there is at least
> 2048 bytes long frame, most likely longer than 2560 bytes.
> 
> Based on my reading of  ds21143hrm.pdf  (copy of which I have), I do
> think it is safe to just receive larger frames with Tulip, and IGNORE
> the "TL" bit.

That covers one of seven or eight tulip chips driven by the driver.

Once you exceed the ethernet norm there are tons of chip-specific quirks 
and details to deal with.  In addition to the details you mention, the 
on-chip FIFO sizes and behaviors become important.  As does the 
multi-fragment frame issue.  Some chips with checksumming features only 
work when the MTU is less than an unknown magic number (less than you 
would think, but higher than 1500).

All these reasons are why I want to dive into the 3c59x documentation, 
and also do some testing on older models, before we merge Alan's patch 
from $subject.

	Jeff

Re: PATCH: VLAN support for 3c59x/3c90x

[Ben Greear]

Jeff Garzik wrote:
> Matti Aarnio wrote:
> 

>> For this IFCONFIG MTU issue, I would rather have the VLAN code to ask
>> the underlaying driver of what MTU can be supported, than just blindly
>> presume that 1500 will be functional for e.g. eth0.2  (like it does now)
> 
> 
> The VLAN code could certainly be updated to poke at the lower level 
> driver MTU.

Please do not do this without more discussion.  This could easily
break the current ability to use the regular ethernet device in the
normal manner, regardless of the existence of VLANs.  I'd rather
not loose this capability across the board just to work around a few
broken drivers.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: VLAN support for 3c59x/3c90x

[Ben Greear]

Matti Aarnio wrote:

> Also the Linux kernel isn't very well happy with multi-path stacking
> of layer-2 driver modules.  A side-effect from all of these things might
> be, that setting up some dozen VLANs to an ethernet with result in
> two, or in worst case, a dozen+1 setup runs.  And if last VLAN setup
> is (for any reason) smaller MTU value than 1500, it might even result
> in entire driver to be configured for e.g. 1280+4 byte MTU..

If the VLAN code shrinks the MTU on the underlying device, I'd
consider that a bug.


> For this IFCONFIG MTU issue, I would rather have the VLAN code to ask
> the underlaying driver of what MTU can be supported, than just blindly
> presume that 1500 will be functional for e.g. eth0.2  (like it does now)
> 
> Things would just magically work, if the uping of  eth0.2  would setup
> itself to MTU 1496, unless the underlaying eth0 can handle real 1504 byte
> 802.1q frames.

Things would not magically work.  Sending larger frames almost always
works, it is receiving the larger frames that fails.  So you really
need to change the MTU of the peer device instead (and everything else
on the local VLAN).

>>>For VLAN support you definitely want to let the user increase the size 
>>>above 1500, and for that you need ->change_mtu
>>
>>I agree, but my point was that adding MODULE_PARM was only a one liner and
>>would have done the job too. But since everyone prefers a change_mtu(), I'll
>>do it.

VLAN allows you to continue using the ethX interface as a regular
ethernet interface, so you do not generally want it's MTU to be set
to 1504 because then the other peer ethernet interfaces would also
have to be set to 1504.  I believe it is much better to silently let
the extra 4 bytes pass but NOT advertise this extra 4 bytes to
anything that actually cares about MTU.


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

Hi Ben,

On Sat, Jul 31, 2004 at 10:03:31AM -0700, Ben Greear wrote:
 
> VLAN allows you to continue using the ethX interface as a regular
> ethernet interface, so you do not generally want it's MTU to be set
> to 1504 because then the other peer ethernet interfaces would also
> have to be set to 1504.  I believe it is much better to silently let
> the extra 4 bytes pass but NOT advertise this extra 4 bytes to
> anything that actually cares about MTU.

I 100% agree with you on this one, but I don't see how playing with
change_mtu() would change anything. Ideally, we would need to export
the level 2 limit (imposed by hardware and intermediate switches) to
other drivers such as 802_1q, and let only the IP stack rely on dev->mtu.

I've seen several drivers which silently add 4 bytes to the hardware
config when CONFIG_VLAN is set. I find it better than fooling the IP
stack into using 1504 bytes, which is a disaster on UDP !

Cheers,
Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Ben Greear]

Willy Tarreau wrote:
> Hi Ben,
> 
> On Sat, Jul 31, 2004 at 10:03:31AM -0700, Ben Greear wrote:
>  
> 
>>VLAN allows you to continue using the ethX interface as a regular
>>ethernet interface, so you do not generally want it's MTU to be set
>>to 1504 because then the other peer ethernet interfaces would also
>>have to be set to 1504.  I believe it is much better to silently let
>>the extra 4 bytes pass but NOT advertise this extra 4 bytes to
>>anything that actually cares about MTU.
> 
> 
> I 100% agree with you on this one, but I don't see how playing with
> change_mtu() would change anything. Ideally, we would need to export
> the level 2 limit (imposed by hardware and intermediate switches) to
> other drivers such as 802_1q, and let only the IP stack rely on dev->mtu.

Ok, I agree that it would be good to have a hard limit exported.
I am less certain that VLAN should modify any MTU based on this
information, but at the very least, it could warn the user that
some action needs to be taken and let the user make an informed
decision.

Also, it seems that most (all?) ethernet chips can handle the extra
4 bytes, but the patches are varying degrees of ugliness and so
many have not made it into the kernel proper.


> I've seen several drivers which silently add 4 bytes to the hardware
> config when CONFIG_VLAN is set. I find it better than fooling the IP
> stack into using 1504 bytes, which is a disaster on UDP !

It would be a disaster with any IP protocol, not just UDP.

Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: PATCH: VLAN support for 3c59x/3c90x

[Lee Revell]

On Sat, 2004-07-31 at 13:21, Ben Greear wrote:
> Willy Tarreau wrote:
> > I've seen several drivers which silently add 4 bytes to the hardware
> > config when CONFIG_VLAN is set. I find it better than fooling the IP
> > stack into using 1504 bytes, which is a disaster on UDP !
> 
> It would be a disaster with any IP protocol, not just UDP.

UDP is prone to *much* weirded behavior than TCP in the face of things
like this.  I once had an NFS server and client using UDP.  A had its
block size set to 8K, B to 32K.  For some reason the mount succeeded
with these options, but when you copied a file from A to B (like, oh,
say, /etc/passwd), it "worked", but the file was truncated to 8K!  The
only indication that anything was wrong (other than hundreds of users
unable to log in) was a mild warning in the logs.

I am not sure what would have happened with a TCP mount, but not that!

Lee

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

On Sat, Jul 31, 2004 at 04:16:29PM -0400, Lee Revell wrote:
> On Sat, 2004-07-31 at 13:21, Ben Greear wrote:
> > Willy Tarreau wrote:
> > > I've seen several drivers which silently add 4 bytes to the hardware
> > > config when CONFIG_VLAN is set. I find it better than fooling the IP
> > > stack into using 1504 bytes, which is a disaster on UDP !
> > 
> > It would be a disaster with any IP protocol, not just UDP.
> 
> UDP is prone to *much* weirded behavior than TCP in the face of things
> like this.  I once had an NFS server and client using UDP.  A had its
> block size set to 8K, B to 32K.  For some reason the mount succeeded
> with these options, but when you copied a file from A to B (like, oh,
> say, /etc/passwd), it "worked", but the file was truncated to 8K!  The
> only indication that anything was wrong (other than hundreds of users
> unable to log in) was a mild warning in the logs.
> 
> I am not sure what would have happened with a TCP mount, but not that!

TCP negociates the MSS which is in some sort the min of both MTU - headers.
So TCP between hosts with MTUs of 1504 and 1500 bytes will negociate an
MSS of 1460 because it's what the smaller can do. TCP is far less sensible
to MTU problems, although it often takes lors of retransmits to fix the
problem when it's not local.

Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Alan Cox]

On Sat, Jul 31, 2004 at 04:16:29PM -0400, Lee Revell wrote:
> UDP is prone to *much* weirded behavior than TCP in the face of things
> like this.  I once had an NFS server and client using UDP.  A had its
> block size set to 8K, B to 32K.  For some reason the mount succeeded

Thats NFS weirdness. NFS (especially older Linux NFS) is the problem not
the UDP layer. UDP is wonderfully bug free in most situations because
its so simple it forces the bugs up a protocol layer

Re: PATCH: VLAN support for 3c59x/3c90x

[Lee Revell]

On Sat, 2004-07-31 at 16:25, Alan Cox wrote:
> On Sat, Jul 31, 2004 at 04:16:29PM -0400, Lee Revell wrote:
> > UDP is prone to *much* weirded behavior than TCP in the face of things
> > like this.  I once had an NFS server and client using UDP.  A had its
> > block size set to 8K, B to 32K.  For some reason the mount succeeded
> 
> Thats NFS weirdness. NFS (especially older Linux NFS) is the problem not
> the UDP layer. UDP is wonderfully bug free in most situations because
> its so simple it forces the bugs up a protocol layer
> 

Yes, it seems like there are two choices at that point - bail, or take a
wild guess as to what you're supposed to do.  I think the mount
succeeded because no one was required to send more than 8K at once. 
IIRC A was BSD/OS 4.x, B was Linux 2.4.x.

Man, that was a weird bug.  Took me *days* to pin down, because it would
mysteriously disappear as long as you only dealt with files < 8K.  Which
as it turns out is almost all of them except the password file.

Lee

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

Hi Matti,

On Sat, Jul 31, 2004 at 05:12:22PM +0300, Matti Aarnio wrote:
<...> 
> In the receive descriptors there might appear a TL bit (Frame Too Long),
> which is just telling that frame size exceeds 1518 bytes.
> If RW (Receive Watchdog; RDES0<4>) has tripped, then there is at least
> 2048 bytes long frame, most likely longer than 2560 bytes.
> 
> Based on my reading of  ds21143hrm.pdf  (copy of which I have), I do
> think it is safe to just receive larger frames with Tulip, and IGNORE
> the "TL" bit.
> 
> Receiving 1522 byte frames from ethernet with Tulip should be trivial.
> Will that be true with 21140 -- oddly I don't have a copy of 21140 HRM
> in PDF form...  Possibly I got it in paper long ago.

I've just found a document on intel's site comparing 21140 and 21143.
Its reference is 27810701 and its title : "21140-AF to 21143-xD Upgrade".

Since there's nothing about the subject in this document, I assume that
they behave equally. I don't have the time right now, but probably will
during this week-end to make new tests with the doc. For those
interested, the 21143 HRM's reference is 27807401.

Regards,
Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Jeff Garzik]

Willy Tarreau wrote:
> Hi Jeff,
> 
> On Sat, Jul 31, 2004 at 05:34:41AM -0400, Jeff Garzik wrote:
> 
>>Willy Tarreau wrote:
>>
>>> - many (all ?) other drivers already have an MTU parameter, and many
>>
>>s/many/almost none/
> 
> 
> Ok, sorry, I've just checked, they are 6. But I incidentely used the feature
> on 2 of them (dl2k and starfire). But more drivers still have the
> 'static int mtu=1500' preceeded by a comment stating "allow the user to change
> the mtu". Why is it not a #define then, if nobody can change it anymore ?

People can change it all the time with ifconfig.


> Jeff, do you know the absolute hardware limit on the tulip ? I've seen the

It depends on the tulip.

Look at Donald Becker's tulip.c for reference, it has full ->change_mtu 
support (as does 3c59x.c I believe).

	Jeff

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

On Sat, Jul 31, 2004 at 12:05:03PM -0400, Jeff Garzik wrote:
> >Ok, sorry, I've just checked, they are 6. But I incidentely used the 
> >feature
> >on 2 of them (dl2k and starfire). But more drivers still have the
> >'static int mtu=1500' preceeded by a comment stating "allow the user to 
> >change
> >the mtu". Why is it not a #define then, if nobody can change it anymore ?
> 
> People can change it all the time with ifconfig.

no, they change dev->mtu, not the 'static' mtu. I think it has been
inherited from the old days without MODULE_PARM.

> >Jeff, do you know the absolute hardware limit on the tulip ? I've seen 
> >the
> 
> It depends on the tulip.
> 
> Look at Donald Becker's tulip.c for reference, it has full ->change_mtu 
> support (as does 3c59x.c I believe).

ok thanks, I'll look at this.

Regards,
Willy

Re: PATCH: VLAN support for 3c59x/3c90x

[Jeff Garzik]

Willy Tarreau wrote:
> On Sat, Jul 31, 2004 at 12:05:03PM -0400, Jeff Garzik wrote:
> 
>>>Ok, sorry, I've just checked, they are 6. But I incidentely used the 
>>>feature
>>>on 2 of them (dl2k and starfire). But more drivers still have the
>>>'static int mtu=1500' preceeded by a comment stating "allow the user to 
>>>change
>>>the mtu". Why is it not a #define then, if nobody can change it anymore ?
>>
>>People can change it all the time with ifconfig.
> 
> 
> no, they change dev->mtu, not the 'static' mtu. I think it has been

Correct.  The user _should not_ be allowed to change the driver limit, 
because it is precisely that a driver (and hardware) limit.


> inherited from the old days without MODULE_PARM.

Using MODULE_PARM to select max MTU limit was always the wrong thing to 
do.  The user generally has no idea of the hardware limits involve.

Patches to remove these incorrect module parameters are welcome.

	Jeff

PATCH-2.4: MTU fix for tulip driver

[Willy Tarreau]

On Sat, Jul 31, 2004 at 12:26:05PM -0400, Jeff Garzik wrote:
 
> Using MODULE_PARM to select max MTU limit was always the wrong thing to 
> do.  The user generally has no idea of the hardware limits involve.

That's right but sometimes useful, especially when there's no other choice :-)
I've used with it a lot when playing with jumbo frames on dl2k.

> Patches to remove these incorrect module parameters are welcome.

OK, here are two patches :
  - the first one, against 2.4.27-rc4, removes the unused static mtu
  - the second one, which has to be applied on top of the previous one,
    implements the change_mtu() function.

I've tried it, and indeed my firewall currently runs on it.

For the higher bound, I looked at Donald Becker's driver, and noticed that
he considers 18 bytes between dev->mtu and PKT_BUF_SZ. So I set the limit
to (PKT_BUF_SZ - 18). BTW, at first, I used netif_running() to prevent
MTU changes when the device was up, but since the code never uses it
anywhere, I simply removed this annoying constraint.

Now I could successfully change the MTU on my firewall up to 1518 and SEND
frames normally (1532 bytes frames).

The problem is with incoming frames larger than 1514 bytes. They get
accounted as errors. Matti talked about a 'TL' bit which should be ignored,
but I couldn't find anything related to it. Even the way the rx_missed_errors
are increased is magics to me.

That's exactly the part I didn't want to get into, since I don't know
how the driver nor the hardware works.

I past the 2 trivial patches here anyway, hoping that someone else knows
how to unlock the receive limit.

Regards,
Willy

#### patch 1 : remove the unused mtu[] array

--- ./drivers/net/tulip/tulip_core.c~	Sat Jul 31 18:45:59 2004
+++ ./drivers/net/tulip/tulip_core.c	Sat Jul 31 18:47:50 2004
@@ -48,7 +48,6 @@
 /* Used to pass the full-duplex flag, etc. */
 static int full_duplex[MAX_UNITS];
 static int options[MAX_UNITS];
-static int mtu[MAX_UNITS];			/* Jumbo MTU for interfaces. */
 
 /*  The possible media types that can be set in options[] are: */
 const char * const medianame[32] = {
@@ -1671,8 +1670,6 @@
 			tp->default_port = options[board_idx] & MEDIA_MASK;
 		if ((options[board_idx] & FullDuplex) || full_duplex[board_idx] > 0)
 			tp->full_duplex = 1;
-		if (mtu[board_idx] > 0)
-			dev->mtu = mtu[board_idx];
 	}
 	if (dev->mem_start & MEDIA_MASK)
 		tp->default_port = dev->mem_start & MEDIA_MASK;


#### patch 2 : implement change_mtu()

--- linux-2.4.27-rc4-tulip-no-mtu/drivers/net/tulip/tulip.h	Mon May 10 01:11:16 2004
+++ linux-2.4.27-rc4-tulip-mtu/drivers/net/tulip/tulip.h	Sat Jul 31 18:56:21 2004
@@ -267,6 +267,8 @@
 #define MEDIA_MASK     31
 
 #define PKT_BUF_SZ		1536	/* Size of each temporary Rx buffer. */
+#define TULIP_MTU_MIN		68
+#define TULIP_MTU_MAX		(PKT_BUF_SZ - 18) /* from D.Becker's tulip.c */
 
 #define TULIP_MIN_CACHE_LINE	8	/* in units of 32-bit words */
 
diff -urN linux-2.4.27-rc4-tulip-no-mtu/drivers/net/tulip/tulip_core.c linux-2.4.27-rc4-tulip-mtu/drivers/net/tulip/tulip_core.c
--- linux-2.4.27-rc4-tulip-no-mtu/drivers/net/tulip/tulip_core.c	Sat Jul 31 18:58:54 2004
+++ linux-2.4.27-rc4-tulip-mtu/drivers/net/tulip/tulip_core.c	Sat Jul 31 18:56:31 2004
@@ -266,8 +266,16 @@
 static struct net_device_stats *tulip_get_stats(struct net_device *dev);
 static int private_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 static void set_rx_mode(struct net_device *dev);
+static int change_mtu(struct net_device *dev, int new_mtu);
 
 
+static int change_mtu(struct net_device *dev, int new_mtu)
+{
+	if ((new_mtu < TULIP_MTU_MIN) || (new_mtu > TULIP_MTU_MAX))
+		return -EINVAL;
+	dev->mtu = new_mtu;
+	return 0;
+}
 
 static void tulip_set_power_state (struct tulip_private *tp,
 				   int sleep, int snooze)
@@ -1726,6 +1734,7 @@
 	dev->get_stats = tulip_get_stats;
 	dev->do_ioctl = private_ioctl;
 	dev->set_multicast_list = set_rx_mode;
+	dev->change_mtu = &change_mtu;
 
 	if (register_netdev(dev))
 		goto err_out_free_ring;

Re: PATCH: VLAN support for 3c59x/3c90x

[Herbert Xu]

On Sat, Jul 31, 2004 at 10:33:08AM +0200, Willy Tarreau wrote:
> 
> So several reasons :
>   - the change_mtu() function might be called at any time after driver
>     initialization. I don't know at all if there are things to do to

See the sungem.c for a working implementation.

> As previously said, I can take a few minutes to add the 'MODULE_PARM'
> line, it's not much more than replying to this mail. At least it will
> be a good start.

BTW I presume this is for the tulip driver? Does it actually use the
mtu parameter for anything? It seems to just store it in dev->mtu and
then promptly forgets about it.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: PATCH: VLAN support for 3c59x/3c90x

[Willy Tarreau]

On Sat, Jul 31, 2004 at 07:35:45PM +1000, Herbert Xu wrote:
> On Sat, Jul 31, 2004 at 10:33:08AM +0200, Willy Tarreau wrote:
> > 
> > So several reasons :
> >   - the change_mtu() function might be called at any time after driver
> >     initialization. I don't know at all if there are things to do to
> 
> See the sungem.c for a working implementation.

Indeed, I remember having read parts of it several times because it was
very clean. I agree that the first half of the function does the same
thing as the initialization code. What is more of a problem is the second
half which resets the card, because inserting resets in the tulip driver
is not trivial (at least to me). But perhaps it would be acceptable to
only implement the dev->mtu change when the device is not up.

> BTW I presume this is for the tulip driver? Does it actually use the
> mtu parameter for anything? It seems to just store it in dev->mtu and
> then promptly forgets about it.

It's for the tulip driver. Now, you're right, it doesn't use dev->mtu at
all (just noticed now) ! So this is fairly simpler, since I then assume
that the driver will work up to PKT_BUF_SZ - 14 or 18.

Ok, I have the hardware, you've convinced me. I'll try it.

Cheers,
Willy