2.4 tulip jumbo frames

PATCH In review Rask Ingemann Lambertsen 3 participants 10 messages Dec 9, 2003 – May 27, 2004

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Rask Ingemann Lambertsen

author

2003-12-09 15:06 UTC #0 lore raw

PATCH 1/1 2.4 tulip jumbo frames

Attached is a patch which enables jumbo frames on tulip based boards. It makes it possible to set an MTU of up to 2025 bytes. However, testing against an NE3200 board using 10base-2 shows assorted problems when going above 2018 bytes (for frames without VLAN tags). The problems show up as framing errors, bad CRCs and such (in both directions).

The patch is against 2.4.22-bk45.

--
Regards,
Rask Ingemann Lambertsen

drivers/net/tulip/tulip.h renamed from drivers/net/tulip/tulip.h-orig +7 -0

@@ -190,6 +190,12 @@ enum desc_status_bits {
 	DescOwned = 0x80000000,
 	RxDescFatalErr = 0x8000,
 	RxWholePkt = 0x0300,
+	RxError = (1 << 15),
+	RxErrLong = (1 << 7),
+	RxErrCRC = (1 << 1),
+	RxErrFIFO = (1 << 0),
+	RxErrRunt = (1 << 11),
+	RxErrFrame = (1 << 14),
 };

@@ -347,6 +353,7 @@ struct tulip_private {
 	struct ring_info tx_buffers[TX_RING_SIZE];
 	/* The addresses of receive-in-place skbuffs. */
 	struct ring_info rx_buffers[RX_RING_SIZE];
+	uint rx_buf_sz;
 	u16 setup_frame[96];	/* Pseudo-Tx frame to init address table. */
 	int chip_id;
 	int revision;

drivers/net/tulip/interrupt.c renamed from drivers/net/tulip/interrupt.c-orig +12 ~~-13~~

@@ -74,11 +74,11 @@ int tulip_refill_rx(struct net_device *d
 			struct sk_buff *skb;
 			dma_addr_t mapping;
 
-			skb = tp->rx_buffers[entry].skb = dev_alloc_skb(PKT_BUF_SZ);
+			skb = tp->rx_buffers[entry].skb = dev_alloc_skb(tp->rx_buf_sz);
 			if (skb == NULL)
 				break;
 
-			mapping = pci_map_single(tp->pdev, skb->tail, PKT_BUF_SZ,
+			mapping = pci_map_single(tp->pdev, skb->tail, tp->rx_buf_sz,
 						 PCI_DMA_FROMDEVICE);
 			tp->rx_buffers[entry].mapping = mapping;

@@ -122,13 +122,21 @@ static int tulip_rx(struct net_device *d
 	/* If we own the next entry, it is a new packet. Send it up. */
 	while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
 		s32 status = le32_to_cpu(tp->rx_ring[entry].status);
+		s32 status_hacked;
 
 		if (tulip_debug > 5)
 			printk(KERN_DEBUG "%s: In tulip_rx(), entry %d %8.8x.\n",
 				   dev->name, entry, status);
 		if (--rx_work_limit < 0)
 			break;
-		if ((status & 0x38008300) != 0x0300) {
+
+		/* Ugly Jumbo frame hack. Remove error flag on long frames. */
+		if ((status & (RxErrLong | RxErrCRC | RxErrFIFO | RxErrRunt | RxErrFrame)) == RxErrLong)
+			status_hacked = status & ~(RxError | RxErrLong);
+		else
+			status_hacked = status;
+
+		if (unlikely((status_hacked & 0x38008300) != 0x0300)) {
 			if ((status & 0x38000300) != 0x0300) {
 				/* Ingore earlier buffers. */
 				if ((status & 0xffff) != 0x7fff) {

@@ -154,15 +162,6 @@ static int tulip_rx(struct net_device *d
 			short pkt_len = ((status >> 16) & 0x7ff) - 4;
 			struct sk_buff *skb;
 
-#ifndef final_version
-			if (pkt_len > 1518) {
-				printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
-					   dev->name, pkt_len, pkt_len);
-				pkt_len = 1518;
-				tp->stats.rx_length_errors++;
-			}
-#endif
-
 #ifdef CONFIG_NET_HW_FLOWCONTROL
                         drop = atomic_read(&netdev_dropping);
                         if (drop)

@@ -203,7 +202,7 @@ static int tulip_rx(struct net_device *d
 #endif
 
 				pci_unmap_single(tp->pdev, tp->rx_buffers[entry].mapping,
-						 PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+						 tp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 
 				tp->rx_buffers[entry].skb = NULL;
 				tp->rx_buffers[entry].mapping = 0;

drivers/net/tulip/tulip_core.c renamed from drivers/net/tulip/tulip_core.c-orig +26 -8

@@ -24,6 +24,7 @@
 #include <linux/pci.h>
 #include <linux/init.h>
 #include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
 #include <linux/delay.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>

@@ -60,11 +61,13 @@ const char * const medianame[32] = {
 	"","","","", "","","","",  "","","","Transceiver reset",
 };
 
+#define PKT_BUF_SZ_MAX		2047	/* Maximum Rx buffer size. */
+
 /* Set the copy breakpoint for the copy-only-tiny-buffer Rx structure. */
 #if defined(__alpha__) || defined(__arm__) || defined(__hppa__) \
 	|| defined(__sparc_) || defined(__ia64__) \
 	|| defined(__sh__) || defined(__mips__) || defined(__SH5__)
-static int rx_copybreak = 1518;
+static int rx_copybreak = PKT_BUF_SZ_MAX;
 #else
 static int rx_copybreak = 100;
 #endif

@@ -518,9 +521,7 @@ void tulip_xon(struct net_device *dev)
 static int
 tulip_open(struct net_device *dev)
 {
-#ifdef CONFIG_NET_HW_FLOWCONTROL
         struct tulip_private *tp = (struct tulip_private *)dev->priv;
-#endif
 	int retval;
 	MOD_INC_USE_COUNT;

@@ -529,6 +530,10 @@ tulip_open(struct net_device *dev)
 		return retval;
 	}
 
+ 	tp->rx_buf_sz = (dev->mtu <= 1500 ? PKT_BUF_SZ : dev->mtu + 32);
+	if (tp->rx_buf_sz > PKT_BUF_SZ_MAX)
+		tp->rx_buf_sz = PKT_BUF_SZ_MAX;
+
 	tulip_init_ring (dev);
 
 	tulip_up (dev);

@@ -673,13 +678,13 @@ static void tulip_init_ring(struct net_d
 
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		tp->rx_ring[i].status = 0x00000000;
-		tp->rx_ring[i].length = cpu_to_le32(PKT_BUF_SZ);
+		tp->rx_ring[i].length = cpu_to_le32(tp->rx_buf_sz);
 		tp->rx_ring[i].buffer2 = cpu_to_le32(tp->rx_ring_dma + sizeof(struct tulip_rx_desc) * (i + 1));
 		tp->rx_buffers[i].skb = NULL;
 		tp->rx_buffers[i].mapping = 0;
 	}
 	/* Mark the last entry as wrapping the ring. */
-	tp->rx_ring[i-1].length = cpu_to_le32(PKT_BUF_SZ | DESC_RING_WRAP);
+	tp->rx_ring[i-1].length = cpu_to_le32(tp->rx_buf_sz | DESC_RING_WRAP);
 	tp->rx_ring[i-1].buffer2 = cpu_to_le32(tp->rx_ring_dma);
 
 	for (i = 0; i < RX_RING_SIZE; i++) {

@@ -688,12 +693,12 @@ static void tulip_init_ring(struct net_d
 		/* Note the receive buffer must be longword aligned.
 		   dev_alloc_skb() provides 16 byte alignment.  But do *not*
 		   use skb_reserve() to align the IP header! */
-		struct sk_buff *skb = dev_alloc_skb(PKT_BUF_SZ);
+		struct sk_buff *skb = dev_alloc_skb(tp->rx_buf_sz);
 		tp->rx_buffers[i].skb = skb;
 		if (skb == NULL)
 			break;
 		mapping = pci_map_single(tp->pdev, skb->tail,
-					 PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+					 tp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 		tp->rx_buffers[i].mapping = mapping;
 		skb->dev = dev;			/* Mark as being used by this device. */
 		tp->rx_ring[i].status = cpu_to_le32(DescOwned);	/* Owned by Tulip chip */

@@ -876,7 +881,7 @@ static int tulip_close (struct net_devic
 		tp->rx_ring[i].length = 0;
 		tp->rx_ring[i].buffer1 = 0xBADF00D0;	/* An invalid address. */
 		if (skb) {
-			pci_unmap_single(tp->pdev, mapping, PKT_BUF_SZ,
+			pci_unmap_single(tp->pdev, mapping, tp->rx_buf_sz,
 					 PCI_DMA_FROMDEVICE);
 			dev_kfree_skb (skb);
 		}

@@ -1321,6 +1326,18 @@ out:
 }
 #endif
 
+static int tulip_change_mtu (struct net_device *dev, int mtu)
+{
+	if (netif_running (dev))
+		return (-EBUSY);
+
+	if (mtu < 0 || mtu > PKT_BUF_SZ_MAX - VLAN_ETH_HLEN - 4)
+		return (-EINVAL);
+
+	dev->mtu = mtu;
+	return (0);
+}
+
 static int __devinit tulip_init_one (struct pci_dev *pdev,
 				     const struct pci_device_id *ent)
 {

@@ -1727,6 +1744,7 @@ static int __devinit tulip_init_one (str
 	dev->get_stats = tulip_get_stats;
 	dev->do_ioctl = private_ioctl;
 	dev->set_multicast_list = set_rx_mode;
+	dev->change_mtu = tulip_change_mtu;
 
 	if (register_netdev(dev))
 		goto err_out_free_ring;

Jeff Garzik

2003-12-09 16:45 UTC #1 lore raw

Two questions and a comment...

Would you split this into two patches? The first simply adds, and uses, tp->rx_buf_sz. The second adds PKT_BUF_SZ_MAX and mtu-related changes.

Have you looked at Donald Becker's changes to tulip.c? He went through most of his drivers and made the changes necessary to support larger MTUs. IIRC his tulip.c changes (which should be easily translate-able to 2.6.x tulip) were a bit more minimal than your patch, but still served the purpose.

For the comment: I am curious why a VLAN_xxx constant is included in the calculation of max MTU, in the ->change_mtu hook? IMO ->change_mtu simply needs to bind the MTU to the min and max h/w limits. If VLAN_ETH_HLEN ever figures into the calculations, those calculations should occur elsewhere, not in ->change_mtu.

Thanks!

Jeff

Rask Ingemann Lambertsen

author

2003-12-09 21:32 UTC #2 lore raw

Quoted 4 lines · Jeff Garzik

Two questions and a comment...
Would you split this into two patches? The first simply adds, and uses, tp->rx_buf_sz. The second adds PKT_BUF_SZ_MAX and mtu-related changes.

That sounds like a good idea. I'll do that.
I tried to find a good place in the private structure to add the rx_buf_sz field. By good, I mean using the cache efficiently. Any comments about that?

Quoted 5 lines

Have you looked at Donald Becker's changes to tulip.c? He went through most of his drivers and made the changes necessary to support larger MTUs. IIRC his tulip.c changes (which should be easily translate-able to 2.6.x tulip) were a bit more minimal than your patch, but still served the purpose.

I have not looked at this particular part of Becker's tulip.c. I'll have a look at it.

Quoted 2 lines

For the comment: I am curious why a VLAN_xxx constant is included in the calculation of max MTU, in the ->change_mtu hook?

This is to ensure that the receive buffers are large enough to hold a frame of the requested MTU also when using VLAN tags.

Quoted 4 lines

IMO ->change_mtu
simply needs to bind the MTU to the min and max h/w limits. If VLAN_ETH_HLEN ever figures into the calculations, those calculations should occur elsewhere, not in ->change_mtu.

What do you propose? Do we need something like

int vlan_adjust_mtu (int mtu)
{

#ifdef CONFIG_VLANN_8021Q
	return (mtu - VLAN_HLEN);
#else
	return (mtu);

#endif
}

and

int foobar_change_mtu (struct net_device *dev, int mtu)

{
	mtu = vlan_adjust_mtu (mtu);
	/* check hardware limits. */
	...
	dev->mtu = mtu;
	return (0);
}

? Ben, this would also keep you happy, right?

Signature · 2 lines

Regards,
Rask Ingemann Lambertsen

Ben Greear

2003-12-09 22:38 UTC #3 lore raw

Quoted 22 lines · Rask Ingemann Lambertsen

What do you propose? Do we need something like
int vlan_adjust_mtu (int mtu)
{
#ifdef CONFIG_VLANN_8021Q
	return (mtu - VLAN_HLEN);
#else
	return (mtu);
#endif
}
and
int foobar_change_mtu (struct net_device *dev, int mtu)
{
	mtu = vlan_adjust_mtu (mtu);
	/* check hardware limits. */
	...
	dev->mtu = mtu;
	return (0);
}

Quoted 1 line

? Ben, this would also keep you happy, right?

I was thinking the check could be made run-time, but in reality, this is a very minor detail. It may be better to just hard-code it like you had it originally. I don't like the patch above, I'd rather see the #ifdef when checking for the maximum hardware limit, if anywhere.

Thanks,
Ben

Quoted 1 line

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Rask Ingemann Lambertsen

author

2003-12-09 23:40 UTC #4 lore raw

On Tue, 09 Dec 2003 14:38:01 -0800, Ben Greear wrote

Quoted 12 lines

Quoted 10 lines · Rask Ingemann Lambertsen
What do you propose? Do we need something like
int vlan_adjust_mtu (int mtu)
{
#ifdef CONFIG_VLANN_8021Q
	return (mtu - VLAN_HLEN);
#else
	return (mtu);
#endif
}

Quoted 2 lines

I was thinking the check could be made run-time, but in reality, this is a very minor detail.

You could use something like

        if (dev->priv_flags & IFF_802_1Q_VLAN)
                return (mtu - VLAN_HLEN);
        else
                return (mtu);

but then you have a problem if the MTU is set to the maximum with VLAN disabled and someone decides to enable VLAN on the device afterwards. A possible solution would be to set NETIF_F_VLAN_CHALLENGED when VLAN_HLEN extra bytes are not available.

That said, even checking CONFIG_VLAN_8021Q is probably flawed too, because ideally, even when building a kernel without VLAN support, you should be able to use the bridging support in a VLAN environment. IMHO. I mean, if this is not the case, please remind me why we need VLAN patches in the first place since setting an MTU of 1496 bytes works with every Ethernet board and driver.

Quoted 2 lines

I don't like the patch above, I'd rather see the
#ifdef when checking for the maximum hardware limit, if anywhere.

I was thinking that there could be other reasons than VLAN for reducing the MTU and that vlan_adjust_mtu() could take these into account as well. PPPoE comes to my mind, but I have no clue how that is implemented in Linux (or any other system).

Signature · 2 lines

Regards,
Rask Ingemann Lambertsen

Rask Ingemann Lambertsen

author

2003-12-19 14:32 UTC #5 lore raw

Quoted 5 lines · Rask Ingemann Lambertsen

That said, even checking CONFIG_VLAN_8021Q is probably flawed too, because ideally, even when building a kernel without VLAN support, you should be able to use the bridging support in a VLAN environment. IMHO. I mean, if this is not the case, please remind me why we need VLAN patches in the first place since setting an MTU of 1496 bytes works with every Ethernet board and driver.

Further, why is it not the responsibility of vconfig to ensure that the MTU of the VLAN device is 4 lower than that of the underlying, "bare" Ethernet device?

Signature · 2 lines

Regards,
Rask Ingemann Lambertsen

Ben Greear

2003-12-20 06:22 UTC #6 lore raw

Quoted 9 lines · Rask Ingemann Lambertsen

Quoted 1 line · Rask Ingemann Lambertsen
That said, even checking CONFIG_VLAN_8021Q is probably flawed too, because ideally, even when building a kernel without VLAN support, you should be able to use the bridging support in a VLAN environment. IMHO. I mean, if this is not the case, please remind me why we need VLAN patches in the first place since setting an MTU of 1496 bytes works with every Ethernet board and driver.
Further, why is it not the responsibility of vconfig to ensure that the MTU of the VLAN device is 4 lower than that of the underlying, "bare" Ethernet device?

Because correctly patched drivers can have eth0 with MTU 1500 and vlan eth0.5 with MTU of 1500 as well. I do not want to add artificial policy in vconfig or the kernel,though of course users can hack up the MTU settings manually as desired...

Ben

Signature · 3 lines

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

Rask Ingemann Lambertsen

author

2003-12-13 17:29 UTC #7 lore raw

In reply to Jeff Garzik

Quoted 4 lines · Jeff Garzik

Two questions and a comment...
Would you split this into two patches? The first simply adds, and uses, tp->rx_buf_sz. The second adds PKT_BUF_SZ_MAX and mtu-related changes.

The first patch is attached to this message. I'm not happy with the second part yet because it is too hackish and I would prefer to have a look at the tulip documentation first.

Quoted 5 lines

Have you looked at Donald Becker's changes to tulip.c? He went through most of his drivers and made the changes necessary to support larger MTUs. IIRC his tulip.c changes (which should be easily translate-able to 2.6.x tulip) were a bit more minimal than your patch, but still served the purpose.

I've just tested Becker's tulip.c v0.97 (7/22/2003) and it fails to receive packets larger than the standard 1514 bytes. "ping -s 1472" works while "ping -s 1473" doesn't (when pinging from another box).

Signature · 2 lines

Regards,
Rask Ingemann Lambertsen

drivers/net/tulip/tulip.h renamed from drivers/net/tulip/tulip.h-orig +1 -0

@@ -347,6 +353,7 @@ struct tulip_private {
 	struct ring_info tx_buffers[TX_RING_SIZE];
 	/* The addresses of receive-in-place skbuffs. */
 	struct ring_info rx_buffers[RX_RING_SIZE];
+	uint rx_buf_sz;
 	u16 setup_frame[96];	/* Pseudo-Tx frame to init address table. */
 	int chip_id;
 	int revision;

drivers/net/tulip/interrupt.c renamed from drivers/net/tulip/interrupt.c-orig +3 -3

@@ -74,11 +74,11 @@ int tulip_refill_rx(struct net_device *d
 			struct sk_buff *skb;
 			dma_addr_t mapping;
 
-			skb = tp->rx_buffers[entry].skb = dev_alloc_skb(PKT_BUF_SZ);
+			skb = tp->rx_buffers[entry].skb = dev_alloc_skb(tp->rx_buf_sz);
 			if (skb == NULL)
 				break;
 
-			mapping = pci_map_single(tp->pdev, skb->tail, PKT_BUF_SZ,
+			mapping = pci_map_single(tp->pdev, skb->tail, tp->rx_buf_sz,
 						 PCI_DMA_FROMDEVICE);
 			tp->rx_buffers[entry].mapping = mapping;

@@ -203,7 +202,7 @@ static int tulip_rx(struct net_device *d
 #endif
 
 				pci_unmap_single(tp->pdev, tp->rx_buffers[entry].mapping,
-						 PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+						 tp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 
 				tp->rx_buffers[entry].skb = NULL;
 				tp->rx_buffers[entry].mapping = 0;

drivers/net/tulip/tulip_core.c renamed from drivers/net/tulip/tulip_core.c-orig +6 -7

@@ -518,9 +521,7 @@ void tulip_xon(struct net_device *dev)
 static int
 tulip_open(struct net_device *dev)
 {
-#ifdef CONFIG_NET_HW_FLOWCONTROL
         struct tulip_private *tp = (struct tulip_private *)dev->priv;
-#endif
 	int retval;
 	MOD_INC_USE_COUNT;

@@ -529,6 +530,7 @@ tulip_open(struct net_device *dev)
 		return retval;
 	}
 
+ 	tp->rx_buf_sz = PKT_BUF_SZ;
 	tulip_init_ring (dev);
 
 	tulip_up (dev);

@@ -673,13 +678,13 @@ static void tulip_init_ring(struct net_d
 
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		tp->rx_ring[i].status = 0x00000000;
-		tp->rx_ring[i].length = cpu_to_le32(PKT_BUF_SZ);
+		tp->rx_ring[i].length = cpu_to_le32(tp->rx_buf_sz);
 		tp->rx_ring[i].buffer2 = cpu_to_le32(tp->rx_ring_dma + sizeof(struct tulip_rx_desc) * (i + 1));
 		tp->rx_buffers[i].skb = NULL;
 		tp->rx_buffers[i].mapping = 0;
 	}
 	/* Mark the last entry as wrapping the ring. */
-	tp->rx_ring[i-1].length = cpu_to_le32(PKT_BUF_SZ | DESC_RING_WRAP);
+	tp->rx_ring[i-1].length = cpu_to_le32(tp->rx_buf_sz | DESC_RING_WRAP);
 	tp->rx_ring[i-1].buffer2 = cpu_to_le32(tp->rx_ring_dma);
 
 	for (i = 0; i < RX_RING_SIZE; i++) {

@@ -688,12 +693,12 @@ static void tulip_init_ring(struct net_d
 		/* Note the receive buffer must be longword aligned.
 		   dev_alloc_skb() provides 16 byte alignment.  But do *not*
 		   use skb_reserve() to align the IP header! */
-		struct sk_buff *skb = dev_alloc_skb(PKT_BUF_SZ);
+		struct sk_buff *skb = dev_alloc_skb(tp->rx_buf_sz);
 		tp->rx_buffers[i].skb = skb;
 		if (skb == NULL)
 			break;
 		mapping = pci_map_single(tp->pdev, skb->tail,
-					 PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+					 tp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 		tp->rx_buffers[i].mapping = mapping;
 		skb->dev = dev;			/* Mark as being used by this device. */
 		tp->rx_ring[i].status = cpu_to_le32(DescOwned);	/* Owned by Tulip chip */

@@ -876,7 +881,7 @@ static int tulip_close (struct net_devic
 		tp->rx_ring[i].length = 0;
 		tp->rx_ring[i].buffer1 = 0xBADF00D0;	/* An invalid address. */
 		if (skb) {
-			pci_unmap_single(tp->pdev, mapping, PKT_BUF_SZ,
+			pci_unmap_single(tp->pdev, mapping, tp->rx_buf_sz,
 					 PCI_DMA_FROMDEVICE);
 			dev_kfree_skb (skb);
 		}

Ben Greear

2004-05-27 19:29 UTC #8 lore raw

Quoted 11 lines · Rask Ingemann Lambertsen

Quoted 4 lines · Jeff Garzik
Two questions and a comment...
Would you split this into two patches? The first simply adds, and uses, tp->rx_buf_sz. The second adds PKT_BUF_SZ_MAX and mtu-related changes.
The first patch is attached to this message. I'm not happy with the second part yet because it is too hackish and I would prefer to have a look at the tulip documentation first.

Whatever became of this? For what it's worth, I have been using a patch originally by Ben McKeegan that seems to work fine. But, I'd be happy for either of the patches to go in so long as we can finally get the tulip driver to work with VLANs.

drivers/net/tulip/interrupt.c +27 ~~-17~~

@@ -1,4 +1,4 @@
-/*
+/*  -*-linux-c-*-
 	drivers/net/tulip/interrupt.c
 
 	Maintained by Jeff Garzik <[email protected]>

@@ -122,14 +122,34 @@
 	/* If we own the next entry, it is a new packet. Send it up. */
 	while ( ! (tp->rx_ring[entry].status & cpu_to_le32(DescOwned))) {
 		s32 status = le32_to_cpu(tp->rx_ring[entry].status);
-
+		short pkt_len;
+
 		if (tulip_debug > 5)
 			printk(KERN_DEBUG "%s: In tulip_rx(), entry %d %8.8x.\n",
 				   dev->name, entry, status);
 		if (--rx_work_limit < 0)
 			break;
-		if ((status & 0x38008300) != 0x0300) {
-			if ((status & 0x38000300) != 0x0300) {
+		/*
+		  Omit the four octet CRC from the length.
+		  (May not be considered valid until we have
+		  checked status for RxLengthOver2047 bits)
+                */
+		pkt_len = ((status >> 16) & 0x7ff) - 4;
+
+		/*
+		  Maximum pkt_len is 1518 (1514 + vlan header)
+		  Anything higher than this is always invalid
+		  regardless of RxLengthOver2047 bits
+		*/
+
+		if (((status & (RxLengthOver2047 |
+				RxDescCRCError |
+				RxDescCollisionSeen |
+				RxDescRunt |
+				RxDescDescErr |
+				RxWholePkt)) != RxWholePkt)
+		    || (pkt_len > 1518) ) {
+			if ((status & (RxLengthOver2047 | RxWholePkt)) != RxWholePkt) {
 				/* Ingore earlier buffers. */
 				if ((status & 0xffff) != 0x7fff) {
 					if (tulip_debug > 1)

@@ -138,31 +158,21 @@
 							   dev->name, status);
 					tp->stats.rx_length_errors++;
 				}
-			} else if (status & RxDescFatalErr) {
+			} else {
 				/* There was a fatal error. */
 				if (tulip_debug > 2)
 					printk(KERN_DEBUG "%s: Receive error, Rx status %8.8x.\n",
 						   dev->name, status);
 				tp->stats.rx_errors++; /* end of a packet.*/
-				if (status & 0x0890) tp->stats.rx_length_errors++;
+				if ((pkt_len > 1518) || (status & RxDescRunt))
+					tp->stats.rx_length_errors++;
 				if (status & 0x0004) tp->stats.rx_frame_errors++;
 				if (status & 0x0002) tp->stats.rx_crc_errors++;
 				if (status & 0x0001) tp->stats.rx_fifo_errors++;
 			}
 		} else {
-			/* Omit the four octet CRC from the length. */
-			short pkt_len = ((status >> 16) & 0x7ff) - 4;
 			struct sk_buff *skb;
 
-#ifndef final_version
-			if (pkt_len > 1518) {
-				printk(KERN_WARNING "%s: Bogus packet size of %d (%#x).\n",
-					   dev->name, pkt_len, pkt_len);
-				pkt_len = 1518;
-				tp->stats.rx_length_errors++;
-			}
-#endif
-
 #ifdef CONFIG_NET_HW_FLOWCONTROL
                         drop = atomic_read(&netdev_dropping);
                         if (drop)

drivers/net/tulip/tulip.h +33 -1

@@ -188,8 +188,40 @@
 
 enum desc_status_bits {
 	DescOwned = 0x80000000,
-	RxDescFatalErr = 0x8000,
+
+        /*
+	   Error summary flag is logical or of 'CRC Error',
+	   'Collision Seen', 'Frame Too Long', 'Runt' and
+	   'Descriptor Error' flags generated within tulip chip.
+	*/
+        RxDescErrorSummary = 0x8000,
+	
+	RxDescCRCError = 0x0002,
+        RxDescCollisionSeen = 0x0040,
+	
+        /*
+	   'Frame Too Long' flag is set if packet length including CRC
+	   exceeds 1518.  However, a full sized VLAN tagged frame is
+	   1522 bytes including CRC.
+	
+	   The tulip chip does not block oversized frames, and if this
+	   flag is set on a receive descriptor it does not indicate
+	   the frame has been truncated.  The receive descriptor also
+	   includes the actual length.  Therefore we can safety ignore
+	   this flag and check the length ourselves.
+        */
+        RxDescFrameTooLong = 0x0080,
+	RxDescRunt = 0x0800,
+	RxDescDescErr = 0x4000,
 	RxWholePkt = 0x0300,
+	
+	/*
+	  Top three bits of 14 bit frame length (status bits 27-29)
+          should never be set as that would make frame over 2047 bytes.
+	  The Receive Watchdog flag (bit 4) may indicate the length is
+          over 2048 and the length field is invalid.
+	*/
+	RxLengthOver2047 = 0x38000010
 };

Quoted 108 lines

Quoted 3 lines

Have you looked at Donald Becker's changes to tulip.c? He went through most of his drivers and made the changes necessary to support larger MTUs. IIRC his tulip.c changes (which should be easily translate-able to 2.6.x tulip) were a bit more minimal than your patch, but still served the purpose.

------------------------------------------------------------------------

drivers/net/tulip/tulip.h renamed from drivers/net/tulip/tulip.h-orig +1 -0

@@ -347,6 +353,7 @@ struct tulip_private {
 	struct ring_info tx_buffers[TX_RING_SIZE];
 	/* The addresses of receive-in-place skbuffs. */
 	struct ring_info rx_buffers[RX_RING_SIZE];
+	uint rx_buf_sz;
 	u16 setup_frame[96];	/* Pseudo-Tx frame to init address table. */
 	int chip_id;
 	int revision;

drivers/net/tulip/interrupt.c renamed from drivers/net/tulip/interrupt.c-orig +3 -3

@@ -74,11 +74,11 @@ int tulip_refill_rx(struct net_device *d
 			struct sk_buff *skb;
 			dma_addr_t mapping;
 
-			skb = tp->rx_buffers[entry].skb = dev_alloc_skb(PKT_BUF_SZ);
+			skb = tp->rx_buffers[entry].skb = dev_alloc_skb(tp->rx_buf_sz);
 			if (skb == NULL)
 				break;
 
-			mapping = pci_map_single(tp->pdev, skb->tail, PKT_BUF_SZ,
+			mapping = pci_map_single(tp->pdev, skb->tail, tp->rx_buf_sz,
 						 PCI_DMA_FROMDEVICE);
 			tp->rx_buffers[entry].mapping = mapping;

@@ -203,7 +202,7 @@ static int tulip_rx(struct net_device *d
 #endif
 
 				pci_unmap_single(tp->pdev, tp->rx_buffers[entry].mapping,
-						 PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+						 tp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 
 				tp->rx_buffers[entry].skb = NULL;
 				tp->rx_buffers[entry].mapping = 0;

drivers/net/tulip/tulip_core.c renamed from drivers/net/tulip/tulip_core.c-orig +6 -7

@@ -518,9 +521,7 @@ void tulip_xon(struct net_device *dev)
 static int
 tulip_open(struct net_device *dev)
 {
-#ifdef CONFIG_NET_HW_FLOWCONTROL
         struct tulip_private *tp = (struct tulip_private *)dev->priv;
-#endif
 	int retval;
 	MOD_INC_USE_COUNT;

@@ -529,6 +530,7 @@ tulip_open(struct net_device *dev)
 		return retval;
 	}
 
+ 	tp->rx_buf_sz = PKT_BUF_SZ;
 	tulip_init_ring (dev);
 
 	tulip_up (dev);

@@ -673,13 +678,13 @@ static void tulip_init_ring(struct net_d
 
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		tp->rx_ring[i].status = 0x00000000;
-		tp->rx_ring[i].length = cpu_to_le32(PKT_BUF_SZ);
+		tp->rx_ring[i].length = cpu_to_le32(tp->rx_buf_sz);
 		tp->rx_ring[i].buffer2 = cpu_to_le32(tp->rx_ring_dma + sizeof(struct tulip_rx_desc) * (i + 1));
 		tp->rx_buffers[i].skb = NULL;
 		tp->rx_buffers[i].mapping = 0;
 	}
 	/* Mark the last entry as wrapping the ring. */
-	tp->rx_ring[i-1].length = cpu_to_le32(PKT_BUF_SZ | DESC_RING_WRAP);
+	tp->rx_ring[i-1].length = cpu_to_le32(tp->rx_buf_sz | DESC_RING_WRAP);
 	tp->rx_ring[i-1].buffer2 = cpu_to_le32(tp->rx_ring_dma);
 
 	for (i = 0; i < RX_RING_SIZE; i++) {

@@ -688,12 +693,12 @@ static void tulip_init_ring(struct net_d
 		/* Note the receive buffer must be longword aligned.
 		   dev_alloc_skb() provides 16 byte alignment.  But do *not*
 		   use skb_reserve() to align the IP header! */
-		struct sk_buff *skb = dev_alloc_skb(PKT_BUF_SZ);
+		struct sk_buff *skb = dev_alloc_skb(tp->rx_buf_sz);
 		tp->rx_buffers[i].skb = skb;
 		if (skb == NULL)
 			break;
 		mapping = pci_map_single(tp->pdev, skb->tail,
-					 PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+					 tp->rx_buf_sz, PCI_DMA_FROMDEVICE);
 		tp->rx_buffers[i].mapping = mapping;
 		skb->dev = dev;			/* Mark as being used by this device. */
 		tp->rx_ring[i].status = cpu_to_le32(DescOwned);	/* Owned by Tulip chip */

@@ -876,7 +881,7 @@ static int tulip_close (struct net_devic
 		tp->rx_ring[i].length = 0;
 		tp->rx_ring[i].buffer1 = 0xBADF00D0;	/* An invalid address. */
 		if (skb) {
-			pci_unmap_single(tp->pdev, mapping, PKT_BUF_SZ,
+			pci_unmap_single(tp->pdev, mapping, tp->rx_buf_sz,
 					 PCI_DMA_FROMDEVICE);
 			dev_kfree_skb (skb);
 		}

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Ben Greear

2003-12-09 17:28 UTC #9 lore raw

In reply to Rask Ingemann Lambertsen

Quoted 7 lines · Rask Ingemann Lambertsen

Attached is a patch which enables jumbo frames on tulip based boards. It makes it possible to set an MTU of up to 2025 bytes. However, testing against an NE3200 board using 10base-2 shows assorted problems when going
above 2018 bytes (for frames without VLAN tags). The problems show up as
framing errors, bad CRCs and such (in both directions).
The patch is against 2.4.22-bk45.

drivers/net/tulip/tulip_core.c out: · line 1335

Quoted 7 lines

drivers/net/tulip/tulip_core.c renamed from drivers/net/tulip/tulip_core.c-orig +7 -0

+static int tulip_change_mtu (struct net_device *dev, int mtu)
+{
+	if (netif_running (dev))
+		return (-EBUSY);
+
+	if (mtu < 0 || mtu > PKT_BUF_SZ_MAX - VLAN_ETH_HLEN - 4)
+		return (-EINVAL);

I suppose you could let them ignore the VLAN_ETH_HLEN if you really wanted to (in case they are not running VLANs).

But, this should work fine too.

--Ben

Quoted 5 lines

drivers/net/tulip/tulip_core.c renamed from drivers/net/tulip/tulip_core.c-orig +5 -0
+
+	dev->mtu = mtu;
+	return (0);
+}
+

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com