Fourth test release of Tigon3 driver

Hey kiddies, uncle DaveM has a new release for ya:

ftp://ftp.kernel.org/pub/linux/kernel/people/davem/TIGON3/tg3-0.93.patch.gz

Changes in this release:

[FIX] Fix minimum MTU enforcement and streamline Jumbo handling

      (Jeff)

[FEATURE] Acenic gets VLAN hw acceleration support. (DaveM) [FEATURE] 8139CP gets VLAN hw acceleration support. (Jeff) [FEATURE] 8139CP gets MTU changing updates. (Jeff) [FEATURE] 8139CP gets RX checksum offload. (Jeff)
[FIX] Do not program Tigon3 MTU if it is not up yet. (DaveM) [API] VLAN hw acceleration interfaces enhanced:
1) netdev->vlan_rx_register returns now a void, it cannot fail
2) Add NETIF_F_HW_VLAN_FILTER and netdev->vlan_rx_add_vid for RX VLAN vid filtering acceleration as discussed yesterday on this list.
3) VLAN layer does destroy the VLAN group when the last VLAN device in it is unregistered. Implemented for hw acceleration by calling vlan_rx_register with NULL group argument.
[CLEANUP] Lots of private vlan kruft moved from
include/linux/if_vlan.h to net/8021q/vlan.h
Lots of totally unused stuff deleted.
[FIX] All the locking in the 8021q code has been audited, fixed, and
cleaned up.
Also fix refcounting, once VLAN device is made the reference to its' hardware device is leaked forever. Good luck bringing down any interface that had VLANs attached :( This bug has been in the VLAN code since it went into the tree.
[FEATURE] Use a small hash table for VLAN group lookups. [FEATURE] Add Robert Olsson's packet generator for stress testing.
See Documentation/networking/pktgen.txt for how to use it. Soon I will add Robert's enhancements for bundling, this is just the older basic version.
[FIX] Kill work limit code, this could cause a situation that leaves
the receive ring empty which deadlocks the Tigon3 and we never get any new RX interrupts so it remains empty forever.

This is hoped to fix a hang condition reported by Lennert Buytenhek. He was using Robert's packet generator which is one of the reasons I have integrated it :-)
[FIX] Tigon3 TX vlan tagging improperly implemented. VLANs fully
tested with Tigon3 and appear to work :-)

Bugs known to not be fixed in this release:

[BUG] 5703 revision chips do not work at all.

I will work on that soon, hopefully.

Please test it hard, and if you find some problem not listed in the "not fixed yet" bug list do let us know about it. Do not hesitate to report "yeah it works, here is my setup" cases, I do log them and they do help us know what kind of coverage the driver is getting and exactly which chips have been tested.

Thanks.

While you guys are fixing up the drivers and the VLAN code, please consider some of these other driver patches:

(I haven't tested these personally, but they have been sent to me and/or the VLAN mailing list by folks who have been using VLAN with their various drivers.)

diff -urN linux-2.4.16/drivers/net/eepro100.c linux/drivers/net/eepro100.c
@@ -510,12 +510,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. */

@@ -308,6 +308,9 @@
    code size of a per-interface flag is not worthwhile. */
 static char mii_preamble_required;
 
+/* The Ethernet Type used for 802.1q tagged frames */
+#define VLAN_ETHER_TYPE 0x8100
+
 #define PFX DRV_NAME ": "
 
 

@@ -651,7 +654,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)  \

@@ -679,7 +682,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 {

@@ -776,7 +780,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;
@@ -844,6 +849,9 @@
static int vortex_ioctl(struct net_device *dev, struct ifreq *rq, int cmd); static void vortex_tx_timeout(struct net_device *dev);
static void acpi_set_WOL(struct net_device *dev);
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) +static void set_8021q_mode(struct net_device *dev, int enable); +#endif

/* This driver uses 'options' to pass the media type, full-duplex flag, etc. */ /* Option count limit only -- unlimited interfaces are supported. */
@@ -1030,6 +1038,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;
@@ -1461,7 +1470,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);

@@ -1545,6 +1554,10 @@
}
/* Set receiver mode: presumably accept b-case and phys addr only. */ set_rx_mode(dev);
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+ /* enable 802.1q tagged frames */
+ set_8021q_mode(dev, 1);
+#endif
outw(StatsEnable, ioaddr + EL3_CMD); /* Turn on statistics. */

// issue_and_wait(dev, SetTxStart|0x07ff);
@@ -1680,7 +1693,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)

@@ -1900,6 +1913,10 @@

	issue_and_wait(dev, RxReset|0x07);
 	 
	/* Set the Rx filter to the current state. */
 	 
	set_rx_mode(dev);
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+ 
		/* enable 802.1q VLAN tagged frames */
+ 
		set_8021q_mode(dev, 1);

+#endif

	outw(RxEnable, ioaddr + EL3_CMD); /* Re-enable the receiver. */
 	 
	outw(AckIntr | HostError, ioaddr + EL3_CMD);

}
@@ -2497,6 +2514,11 @@
outw(RxDisable, ioaddr + EL3_CMD);
outw(TxDisable, ioaddr + EL3_CMD);

+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+ /* Disable receiving 802.1q tagged frames */
+ set_8021q_mode(dev, 0);
+#endif
+
if (dev->if_port == XCVR_10base2)
/* Turn off thinnet power. Green! */ outw(StopCoax, ioaddr + EL3_CMD);
@@ -2760,6 +2782,50 @@

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 */
+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);
+}
+#endif

/* MII transceiver control section.
Read and write the MII registers using software-generated serial

diff -u --recursive --new-file linux/drivers/net/tulip/interrupt.c linux.dev/drivers/net/tulip/interrupt.c
@@ -128,8 +128,8 @@
 	 

		   dev->name, entry, status);

if (--rx_work_limit < 0)

	break;
- 
	if ((status & 0x38008300) != 0x0300) {
- 
		if ((status & 0x38000300) != 0x0300) {
+ 
	if ((status & (0x38000000 | RxDescFatalErr | RxWholePkt)) != RxWholePkt) {
+ 
		if ((status & (0x38000000 | RxWholePkt)) != RxWholePkt) {
 	 
		/* Ingore earlier buffers. */
 	 
		if ((status & 0xffff) != 0x7fff) {
 	 
			if (tulip_debug > 1)

@@ -155,10 +155,10 @@

	struct sk_buff *skb;

#ifndef final_version
-

		if (pkt_len > 1518) {
+ 
		if (pkt_len > 1522) {
 	 
		printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
 	 
			   dev->name, pkt_len, pkt_len);
- 
			pkt_len = 1518;
+ 
			pkt_len = 1522;
 	 
		tp->stats.rx_length_errors++;
 	 
	}

#endif

diff -u --recursive --new-file linux/drivers/net/tulip/tulip.h linux.dev/drivers/net/tulip/tulip.h
@@ -186,7 +186,7 @@
 
 enum desc_status_bits {
 	DescOwned = 0x80000000,
- RxDescFatalErr = 0x8000,
+ RxDescFatalErr = 0x4842,
 	RxWholePkt = 0x0300,
 };
 

diff -u --recursive --new-file linux/drivers/net/tulip/tulip_core.c linux.dev/drivers/net/tulip/tulip_core.c
@@ -63,7 +63,7 @@
 #if defined(__alpha__) || defined(__arm__) || defined(__hppa__) \
 	|| defined(__sparc_) || defined(__ia64__) \
 	|| defined(__sh__) || defined(__mips__)
-static int rx_copybreak = 1518;
+static int rx_copybreak = 1522;
 #else
 static int rx_copybreak = 100;
 #endif

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

Your mail client wraps long lines so the patches are totally unusuable, please resend without line wrapping. Thanks.

Most of the diffs haven't been applied because they still need some work. At least that is what Jeff told me about the 3c59x changes.

Hopefully this will work better.

I don't have hardware to test with this driver, so I can't really vouch for the patch. However, it does work for some people, and there are definate problems with the stock driver, so we should try to get some kind of patch in there eventually...

Resend of the patches:

@@ -308,6 +308,9 @@
    code size of a per-interface flag is not worthwhile. */
 static char mii_preamble_required;
 
+/* The Ethernet Type used for 802.1q tagged frames */
+#define VLAN_ETHER_TYPE 0x8100
+
 #define PFX DRV_NAME ": "
 
 

@@ -651,7 +654,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)  \

@@ -679,7 +682,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 {

@@ -776,7 +780,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;
@@ -844,6 +849,9 @@
static int vortex_ioctl(struct net_device *dev, struct ifreq *rq, int cmd); static void vortex_tx_timeout(struct net_device *dev);
static void acpi_set_WOL(struct net_device *dev);
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE) +static void set_8021q_mode(struct net_device *dev, int enable); +#endif

/* This driver uses 'options' to pass the media type, full-duplex flag, etc. */ /* Option count limit only -- unlimited interfaces are supported. */
@@ -1030,6 +1038,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;
@@ -1461,7 +1470,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);

@@ -1545,6 +1554,10 @@
}
/* Set receiver mode: presumably accept b-case and phys addr only. */ set_rx_mode(dev);
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+ /* enable 802.1q tagged frames */
+ set_8021q_mode(dev, 1);
+#endif
outw(StatsEnable, ioaddr + EL3_CMD); /* Turn on statistics. */

// issue_and_wait(dev, SetTxStart|0x07ff);
@@ -1680,7 +1693,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)

@@ -1900,6 +1913,10 @@

	issue_and_wait(dev, RxReset|0x07);
 	 
	/* Set the Rx filter to the current state. */
 	 
	set_rx_mode(dev);
+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+ 
		/* enable 802.1q VLAN tagged frames */
+ 
		set_8021q_mode(dev, 1);

+#endif

	outw(RxEnable, ioaddr + EL3_CMD); /* Re-enable the receiver. */
 	 
	outw(AckIntr | HostError, ioaddr + EL3_CMD);

}
@@ -2497,6 +2514,11 @@
outw(RxDisable, ioaddr + EL3_CMD);
outw(TxDisable, ioaddr + EL3_CMD);

+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
+ /* Disable receiving 802.1q tagged frames */
+ set_8021q_mode(dev, 0);
+#endif
+
if (dev->if_port == XCVR_10base2)
/* Turn off thinnet power. Green! */ outw(StopCoax, ioaddr + EL3_CMD);
@@ -2760,6 +2782,50 @@

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 */
+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);
+}
+#endif

/* MII transceiver control section.
Read and write the MII registers using software-generated serial

diff -urN linux-2.4.16/drivers/net/eepro100.c linux/drivers/net/eepro100.c
@@ -510,12 +510,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. */

diff -u --recursive --new-file linux/drivers/net/tulip/interrupt.c linux.dev/drivers/net/tulip/interrupt.c
@@ -128,8 +128,8 @@
 	 

		   dev->name, entry, status);

if (--rx_work_limit < 0)

	break;
- 
	if ((status & 0x38008300) != 0x0300) {
- 
		if ((status & 0x38000300) != 0x0300) {
+ 
	if ((status & (0x38000000 | RxDescFatalErr | RxWholePkt)) != RxWholePkt) {
+ 
		if ((status & (0x38000000 | RxWholePkt)) != RxWholePkt) {
 	 
		/* Ingore earlier buffers. */
 	 
		if ((status & 0xffff) != 0x7fff) {
 	 
			if (tulip_debug > 1)

@@ -155,10 +155,10 @@

	struct sk_buff *skb;

#ifndef final_version
-

		if (pkt_len > 1518) {
+ 
		if (pkt_len > 1522) {
 	 
		printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
 	 
			   dev->name, pkt_len, pkt_len);
- 
			pkt_len = 1518;
+ 
			pkt_len = 1522;
 	 
		tp->stats.rx_length_errors++;
 	 
	}

#endif

diff -u --recursive --new-file linux/drivers/net/tulip/tulip.h linux.dev/drivers/net/tulip/tulip.h
@@ -186,7 +186,7 @@
 
 enum desc_status_bits {
 	DescOwned = 0x80000000,
- RxDescFatalErr = 0x8000,
+ RxDescFatalErr = 0x4842,
 	RxWholePkt = 0x0300,
 };
 

diff -u --recursive --new-file linux/drivers/net/tulip/tulip_core.c linux.dev/drivers/net/tulip/tulip_core.c
@@ -63,7 +63,7 @@
 #if defined(__alpha__) || defined(__arm__) || defined(__hppa__) \
 	|| defined(__sparc_) || defined(__ia64__) \
 	|| defined(__sh__) || defined(__mips__)
-static int rx_copybreak = 1518;
+static int rx_copybreak = 1522;
 #else
 static int rx_copybreak = 100;
 #endif

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

From: Ben Greear <[email protected]>
Date: Fri, 01 Mar 2002 09:27:44 -0700

Resend of the patches:

Same line wrapping, try again.

No, it does not.

You MIGHT be able to send the patches as ATTACHMENTS.
I think Uncle DaveM will accept them, while Linus dislikes them immensely.

/Matti Aarnio

On Fri, 1 Mar 2002, Matti Aarnio wrote:

|   No, it does not.
|
| On Fri, Mar 01, 2002 at 09:27:44AM -0700, Ben Greear wrote:
| > User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.4) Gecko/20011019 Netscape6/6.2
|
|   You MIGHT be able to send the patches as ATTACHMENTS.
| I think Uncle DaveM will accept them, while Linus dislikes them immensely.

Attachment with Netscape 4.5/4.7 work fine. They look like plain text email (probably an option). jgarzik and I have used them successfully many times, including for Linus.

~Randy

Yep.

The rule in Netscape appears to be, as long as the file doesn't have unprintable characters, it is attached as text/plain, which is the desired effect.

Jeff

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

Ben,

Actually, would you just e-mail URLs to these patches?

It looks like the eepro100 one, mysterious as it is, is ok. The others I have different plans for: making them act similar to the recent 8139cp changes.

So, the other patches will be immensely useful to me, but only as reference, not to be directly applied...

Jeff

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

I'm pretty sure Andrew realizes this. I can see two valid complaints,

1) you don't need ugly ifdefs in the code itself -- define a no-op static inline function for when !VLAN

2) IIRC Alan or somebody is trying to get rid of CONFIG_xxx_MODULE, because it doesn't really cover the case of when somebody builds VLAN "later on" as a module, but disables it initially.

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

News to me

Oops, Arjan says it was David Woodhouse. (note I carefully covered my failing memory with "or somebody" above :):))

Anyway, using CONFIG_xxx_MODULE has the problem I describe above.

Jeff

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

From: Jeff Garzik <[email protected]>
Date: Fri, 01 Mar 2002 15:53:41 -0500

Anyway, using CONFIG_xxx_MODULE has the problem I describe above.

It actually gets used for all the wrong reasons.

For example, in 2.4.x it is used to keep struct sock from bloating up in include/net/sock.h unless you have all the protocols enabled.

Whereas with Arnaldo's changes in 2.5.x to split all the non-generic junk out from struct sock, *_MODULE testing is no longer is needed for that purpose.

Every existing reference I see in my 2.5.x tree for the networking is "Duh, delete the ifdef".

The 3c59x one is available on

http://www.myipv6.de/patches/vlan/3c59x.c-8021q.patch

Andreas

       Andreas Ferber - dev/consulting GmbH - Bielefeld, FRG
     ---------------------------------------------------------
         +49 521 1365800 - [email protected] - www.devcon.net

Ok, tulip patch attached as hopefully a plain-text attachment. The 3com link has already been sent, and the eepro100 patch was short enough not to wrap.

Thanks for your patience.

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

diff -u --recursive --new-file linux/drivers/net/tulip/interrupt.c linux.dev/drivers/net/tulip/interrupt.c
@@ -128,8 +128,8 @@
 				   dev->name, entry, status);
 		if (--rx_work_limit < 0)
 			break;
-		if ((status & 0x38008300) != 0x0300) {
-			if ((status & 0x38000300) != 0x0300) {
+		if ((status & (0x38000000 | RxDescFatalErr | RxWholePkt)) != RxWholePkt) {
+			if ((status & (0x38000000 | RxWholePkt)) != RxWholePkt) {
 				/* Ingore earlier buffers. */
 				if ((status & 0xffff) != 0x7fff) {
 					if (tulip_debug > 1)

@@ -155,10 +155,10 @@
 			struct sk_buff *skb;
 
 #ifndef final_version
-			if (pkt_len > 1518) {
+			if (pkt_len > 1522) {
 				printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
 					   dev->name, pkt_len, pkt_len);
-				pkt_len = 1518;
+				pkt_len = 1522;
 				tp->stats.rx_length_errors++;
 			}
 #endif

diff -u --recursive --new-file linux/drivers/net/tulip/tulip.h linux.dev/drivers/net/tulip/tulip.h
@@ -186,7 +186,7 @@
 
 enum desc_status_bits {
 	DescOwned = 0x80000000,
-	RxDescFatalErr = 0x8000,
+	RxDescFatalErr = 0x4842,
 	RxWholePkt = 0x0300,
 };
 

diff -u --recursive --new-file linux/drivers/net/tulip/tulip_core.c linux.dev/drivers/net/tulip/tulip_core.c
@@ -63,7 +63,7 @@
 #if defined(__alpha__) || defined(__arm__) || defined(__hppa__) \
 	|| defined(__sparc_) || defined(__ia64__) \
 	|| defined(__sh__) || defined(__mips__)
-static int rx_copybreak = 1518;
+static int rx_copybreak = 1522;
 #else
 static int rx_copybreak = 100;
 #endif

Looks OK, except all the additions to the driver are inside

+#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)

Is this avoidable somehow?

-

From: Andrew Morton <[email protected]>
Date: Fri, 01 Mar 2002 10:45:39 -0800

   +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)

Is this avoidable somehow?

It's stupid to have all the overhead when vlans aren't even being built into the kernel.

That was my original impetus.

It's costly in some cases, when you have the TXDs on the chip you can avoid an entire PIO for each packet.

Comments:

1) This patch is (like I mentioned earlier) for reference only, not for application. It is really halfway in between software and hardware VLAN support.

2) You don't want to set_8021q_mode if VLAN is not active. It's silly to activate it when VLAN is compiled as a module but no one is using vlans. That's going to be THE common case, because distros will inevitably build the VLAN module with their stock kernel.

3) 3c59x needs real dev->change_mtu support. This patch provides the info needed to do that... but doesn't do that.

4) It uses magic numbers instead of ETH_DATA_LEN and ETH_HLEN

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

This patch, from all I know using it, does exactly one thing: it permits receiving (and sending) slightly larger frames, for setting the MTU on the base interface to 1504, so the VLAN interfaces themselves can run the normal 1500 byte MTU.

I have been using the patch to this end on several eepro100 based systems, over the last year, with no surprises.

I agree that such an array of magic constants is very very undesirable.

best regards
Patrick

That's a good reason to object.

The patch turns on the reception for larger frames.
It only works for some chip revisions. Specifically, the i82557 does not document this bit.

It should have been handled like all of the other interface- and chip-specific settings -- modifying a copy of the table, not the original static table.

Have you tried sending slightly oversized non-VLAN frames? What about testing with '557, '558 and '559 chips?

I have a similar objection to the Tulip modification: the change just disables the overlength packet protection. This won't work as you expect with all chips.

The reason for the magic numbers is twofold: I got the documentation only with a negotiated NDA, and even with the documentation many of the bits have specified-but-undocumented values. Others bits are fundamentally uninteresting modifications of standard Ethernet: disabling preamble, extending the IFG1/IFG2 period, changing to linear back-off.

OTOH, this patch is a case where the setting should be documented. It is a change from the default/recommended value.

Donald Becker [email protected]
Scyld Computing Corporation http://www.scyld.com
410 Severn Ave. Suite 210 Second Generation Beowulf Clusters
Annapolis MD 21403 410-990-9993

Thanks.

Can you be more specific?
Does this (a) set eepro100 h/w max mtu to 1504, or (b) enable h/w vlan de-tagging, or (c) enable h/w support for non-standard frame sizes? Any idea what the max h/w frame size is?

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

Someone told me it disables something like: drop oversized frames. But
anyway I was wondering if this could be user changable, since for example I use 3 eepro100 and only one uses vlan 802.1q, and since I connect the others on WANs, I would like the card to still discard the "oversized frames" could this be possible?

Ack. Although it works like a charme in production here, and I did not receive any problem reports about it so far. So anyone who needs this now should apply the patch locally for the moment.

Not really. The MTU setting is something which must be done for any "dumb" card that has no real hardware VLAN support, and the only thing Cyclone and Tornado cards care about the VlanEtherType is that they start IP checksumming 4 octets later. They don't care a bit about the contents of the VLAN tag.

Well, this was discussed on the VLAN mailing list. The conclusion there was that it will not hurt on most cards if it is enabled unconditionally.

The only situation where it will probably hurt is if you have many frames exceeding ethernet MTU coming in through your ethernet cable. Those (up to FDDI size e.g. on 3COM cards without MaxPktSize support) will not be dropped by the card but instead handed up to the network stack and be dropped there. Are there any situations (except for failure cases) where a lot large frames are sent over normal ethernet?

And for the 802.1q recognition of Cyclone and Tornado cards, this only affects the IP checksumming done by the hardware. As plain IP is never transmitted in frames with an ethernet type of 0x8100, this also will not affect normal operation.

Anyway, if you want to enable it only if VLANs are really used on the card, what about adding a new hook to struct net_device, which can be used by the 802.1q core code to enable VLAN support on the card as needed (if supported by the driver)? Or, as this is only a boolean flag, maybe a generic feature_supported/set_feature hook pair that can be used for other similar situations in the future?

Intentionally, as this has nothing to do with VLAN support. The whole thing about the patch was that you /don't/ have to change the physical device MTU visible to the network stack. Otherwise the network stack would start sending oversized frames to the ethernet, which would render the untagged VLAN useless.

That's what the driver used before ;-)

Andreas

       Andreas Ferber - dev/consulting GmbH - Bielefeld, FRG
     ---------------------------------------------------------
         +49 521 1365800 - [email protected] - www.devcon.net

Good to know, thanks. My point still stands, though :) See below...

Well, conclusions like that slow down packet processing on the chip, quite often...

It does, but not directly. The infrastructure for VLAN and changing MTU share common elements, so both should be merged at the same time.

This is ESPECIALLY key with 3c59x, because we are turning on support for large frames, not specifically VLAN. That is obviously the same operation as changing MTU to a larger, non-standard one, and so should not be treated as something vlan-specific.

Thanks much for the patch, whoever the author(s) were though... it provides the necessary reference information to modify 3c59x for these things I describe, and also provide the framework for

Early next week, I will likely make a bombing run through several drivers, fixing up the large frame and MTU issues. That should be enough for software VLAN.

Jeff

Jeff Garzik      |
Building 1024    |
MandrakeSoft     | Choose life.

Do you expect to see slowdowns by just enabling large packet receives, or just when you actually receive a large packet?

The subtle difference is that we want to be able to have an MTU 4 bytes bigger than the VLAN device's MTU, but we most likely do NOT want the rest of the stack to know we are using the higher MTU, because then non-vlan packets may not be accepted at the other end. In other words, we want eth0's MTU to be 1500, and VLAN1's MTU to be 1500, as far as the layer 3 stacks are concerned. However, eth0 must really be able to handle a packet of 1504 length.

That will be a very welcome development!

Thanks,
Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

From: Ben Greear <[email protected]>
Date: Fri, 01 Mar 2002 14:19:09 -0700

The subtle difference is that we want to be able to have an MTU 4 bytes bigger than the VLAN device's MTU, but we most likely do NOT want the rest of the stack to know we are using the higher MTU

Jeff understands this Ben :-)

OK, from the chip driving side, yes, but not from the network stack side, as noted by Ben.

It was me actually ;-)

When doing the 3c59x modifications, please note the comment above set_8201q_mode

| Note that this must be done after each RxReset due to some backwards | compatibility logic in the Cyclone and Tornado ASICs */

The "backwards compatibility logic" mentioned there refers to that Cyclone and Tornado cards will reset MaxPktSize to standard ethernet or FDDI MTU (as determined by the allowLargePackets flag) on every RxReset.

Ack.

Andreas

       Andreas Ferber - dev/consulting GmbH - Bielefeld, FRG
     ---------------------------------------------------------
         +49 521 1365800 - [email protected] - www.devcon.net