Re: sungem - ifconfig eth0 mtu 1300 -> oops

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[Herbert Xu]

Soeren Sonnenburg <kernel@nn7.de> wrote:
> 
> When I have some ethernet connection and then do:
> 
> ifconfig eth0 mtu 1300
> 
> I get an immediate kernel panic (kernel 2.6.6) on a powerbook g4 15"
> 1ghz.
> 
> xmon trace (jpeg) is here: http://www.nn7.de/kernel/mtu1300.jpg  (use a
> webbrowser to view it as it is a redirect)
> 
> this is 100% reproducable here so I hope it is more easy to fix.

Does this patch fix your problems?

Cheers,
-- 
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
--
===== drivers/net/sungem.c 1.56 vs edited =====
--- 1.56/drivers/net/sungem.c	2004-06-19 17:16:13 +10:00
+++ edited/drivers/net/sungem.c	2004-06-21 22:28:40 +10:00
@@ -33,6 +33,7 @@
 #include <linux/crc32.h>
 #include <linux/random.h>
 #include <linux/workqueue.h>
+#include <linux/if_vlan.h>
 
 #include <asm/system.h>
 #include <asm/bitops.h>
@@ -742,7 +743,7 @@
 				       PCI_DMA_FROMDEVICE);
 			gp->rx_skbs[entry] = new_skb;
 			new_skb->dev = gp->dev;
-			skb_put(new_skb, (ETH_FRAME_LEN + RX_OFFSET));
+			skb_put(new_skb, (VLAN_ETH_FRAME_LEN + RX_OFFSET));
 			rxd->buffer = cpu_to_le64(pci_map_page(gp->pdev,
 							       virt_to_page(new_skb->data),
 							       offset_in_page(new_skb->data),
@@ -1482,6 +1483,9 @@
 
 	gem_clean_rings(gp);
 
+	gp->rx_buf_sz = min(dev->mtu + ETH_HLEN + VLAN_HLEN,
+			    (unsigned)VLAN_ETH_FRAME_LEN);
+
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		struct sk_buff *skb;
 		struct gem_rxd *rxd = &gb->rxd[i];
@@ -1495,7 +1499,7 @@
 
 		gp->rx_skbs[i] = skb;
 		skb->dev = dev;
-		skb_put(skb, (ETH_FRAME_LEN + RX_OFFSET));
+		skb_put(skb, (VLAN_ETH_FRAME_LEN + RX_OFFSET));
 		dma_addr = pci_map_page(gp->pdev,
 					virt_to_page(skb->data),
 					offset_in_page(skb->data),
===== drivers/net/sungem.h 1.14 vs edited =====
--- 1.14/drivers/net/sungem.h	2004-01-26 18:03:59 +11:00
+++ edited/drivers/net/sungem.h	2004-06-21 22:24:46 +10:00
@@ -911,7 +911,7 @@
 	  (GP)->tx_old - (GP)->tx_new - 1)
 
 #define RX_OFFSET          2
-#define RX_BUF_ALLOC_SIZE(gp)	((gp)->dev->mtu + 46 + RX_OFFSET + 64)
+#define RX_BUF_ALLOC_SIZE(gp)	((gp)->rx_buf_sz + 32 + RX_OFFSET + 64)
 
 #define RX_COPY_THRESHOLD  256
 
@@ -979,6 +979,7 @@
 	int			rx_fifo_sz;
 	int			rx_pause_off;
 	int			rx_pause_on;
+	int			rx_buf_sz;
 	int			mii_phy_addr;
 
 	u32			mac_rx_cfg;

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 488 bytes --]

On Mon, Jun 21, 2004 at 10:33:50PM +1000, Herbert Xu wrote:
> 
> Does this patch fix your problems?

Oops, I had a thinko about min vs. max.  I've also decided to make the
bigger MTU useful by adjusting the arguments to skb_put() as well.
Please try this one instead.

Cheers,
-- 
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

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 2464 bytes --]

===== drivers/net/sungem.c 1.56 vs edited =====
--- 1.56/drivers/net/sungem.c	2004-06-19 17:16:13 +10:00
+++ edited/drivers/net/sungem.c	2004-06-21 22:57:09 +10:00
@@ -33,6 +33,7 @@
 #include <linux/crc32.h>
 #include <linux/random.h>
 #include <linux/workqueue.h>
+#include <linux/if_vlan.h>
 
 #include <asm/system.h>
 #include <asm/bitops.h>
@@ -742,7 +743,7 @@
 				       PCI_DMA_FROMDEVICE);
 			gp->rx_skbs[entry] = new_skb;
 			new_skb->dev = gp->dev;
-			skb_put(new_skb, (ETH_FRAME_LEN + RX_OFFSET));
+			skb_put(new_skb, (gp->rx_buf_sz + RX_OFFSET));
 			rxd->buffer = cpu_to_le64(pci_map_page(gp->pdev,
 							       virt_to_page(new_skb->data),
 							       offset_in_page(new_skb->data),
@@ -1482,6 +1483,9 @@
 
 	gem_clean_rings(gp);
 
+	gp->rx_buf_sz = max(dev->mtu + ETH_HLEN + VLAN_HLEN,
+			    (unsigned)VLAN_ETH_FRAME_LEN);
+
 	for (i = 0; i < RX_RING_SIZE; i++) {
 		struct sk_buff *skb;
 		struct gem_rxd *rxd = &gb->rxd[i];
@@ -1495,7 +1499,7 @@
 
 		gp->rx_skbs[i] = skb;
 		skb->dev = dev;
-		skb_put(skb, (ETH_FRAME_LEN + RX_OFFSET));
+		skb_put(skb, (gp->rx_buf_sz + RX_OFFSET));
 		dma_addr = pci_map_page(gp->pdev,
 					virt_to_page(skb->data),
 					offset_in_page(skb->data),
@@ -1750,7 +1754,7 @@
 	writel(0x40, gp->regs + MAC_MINFSZ);
 
 	/* Ethernet payload + header + FCS + optional VLAN tag. */
-	writel(0x20000000 | (gp->dev->mtu + ETH_HLEN + 4 + 4), gp->regs + MAC_MAXFSZ);
+	writel(0x20000000 | (gp->rx_buf_sz + 4), gp->regs + MAC_MAXFSZ);
 
 	writel(0x07, gp->regs + MAC_PASIZE);
 	writel(0x04, gp->regs + MAC_JAMSIZE);
@@ -1827,7 +1831,7 @@
 	if (gp->rx_fifo_sz <= (2 * 1024)) {
 		gp->rx_pause_off = gp->rx_pause_on = gp->rx_fifo_sz;
 	} else {
-		int max_frame = (gp->dev->mtu + ETH_HLEN + 4 + 4 + 64) & ~63;
+		int max_frame = (gp->rx_buf_sz + 4 + 64) & ~63;
 		int off = (gp->rx_fifo_sz - (max_frame * 2));
 		int on = off - max_frame;
 
===== drivers/net/sungem.h 1.14 vs edited =====
--- 1.14/drivers/net/sungem.h	2004-01-26 18:03:59 +11:00
+++ edited/drivers/net/sungem.h	2004-06-21 22:59:38 +10:00
@@ -911,7 +911,7 @@
 	  (GP)->tx_old - (GP)->tx_new - 1)
 
 #define RX_OFFSET          2
-#define RX_BUF_ALLOC_SIZE(gp)	((gp)->dev->mtu + 46 + RX_OFFSET + 64)
+#define RX_BUF_ALLOC_SIZE(gp)	((gp)->rx_buf_sz + 28 + RX_OFFSET + 64)
 
 #define RX_COPY_THRESHOLD  256
 
@@ -979,6 +979,7 @@
 	int			rx_fifo_sz;
 	int			rx_pause_off;
 	int			rx_pause_on;
+	int			rx_buf_sz;
 	int			mii_phy_addr;
 
 	u32			mac_rx_cfg;

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[Manuel Arostegui Ramirez]

El Lunes 21 Junio 2004 14:33, Herbert Xu escribió:
> Soeren Sonnenburg <kernel@nn7.de> wrote:
> > When I have some ethernet connection and then do:
> >
> > ifconfig eth0 mtu 1300
> >
> > I get an immediate kernel panic (kernel 2.6.6) on a powerbook g4 15"
> > 1ghz.
> >
> > xmon trace (jpeg) is here: http://www.nn7.de/kernel/mtu1300.jpg  (use a
> > webbrowser to view it as it is a redirect)
> >
> > this is 100% reproducable here so I hope it is more easy to fix.
>
> Does this patch fix your problems?
>
> Cheers,
> >
> >===== drivers/net/sungem.c 1.56 vs edited =====
> >--- 1.56/drivers/net/sungem.c   2004-06-19 17:16:13 +10:00
> >+++ edited/drivers/net/sungem.c 2004-06-21 22:28:40 +10:00
> >@@ -33,6 +33,7 @@
> > #include <linux/crc32.h>
> > #include <linux/random.h>
> > #include <linux/workqueue.h>
> >+#include <linux/if_vlan.h>
> > 
> > #include <asm/system.h>
> > #include <asm/bitops.h>
> >@@ -742,7 +743,7 @@
> >                                       PCI_DMA_FROMDEVICE);
> >                        gp->rx_skbs[entry] = new_skb;
 > >                       new_skb->dev = gp->dev;
> >-                       skb_put(new_skb, (ETH_FRAME_LEN + RX_OFFSET));
> >+                       skb_put(new_skb, (VLAN_ETH_FRAME_LEN + RX_OFFSET));
> >                        rxd->buffer = cpu_to_le64(pci_map_page(gp->pdev,
> >                                                               > 
>virt_to_page(new_skb->data),
> >                                                               > 
>offset_in_page(new_skb->data),
> >@@ -1482,6 +1483,9 @@
> > 
> >        gem_clean_rings(gp);
> > 
> >+       gp->rx_buf_sz = min(dev->mtu + ETH_HLEN + VLAN_HLEN,
> >+                           (unsigned)VLAN_ETH_FRAME_LEN);
> >+
> >        for (i = 0; i < RX_RING_SIZE; i++) {
> >                struct sk_buff *skb;
> >                struct gem_rxd *rxd = &gb->rxd[i];
> >@@ -1495,7 +1499,7 @@
> > 
 > >               gp->rx_skbs[i] = skb;
 > >               skb->dev = dev;
> >-               skb_put(skb, (ETH_FRAME_LEN + RX_OFFSET));
> >+               skb_put(skb, (VLAN_ETH_FRAME_LEN + RX_OFFSET));
> >                dma_addr = pci_map_page(gp->pdev,
> >                                        virt_to_page(skb->data),
> >                                        offset_in_page(skb->data),
> >===== drivers/net/sungem.h 1.14 vs edited =====
> >--- 1.14/drivers/net/sungem.h   2004-01-26 18:03:59 +11:00
> >+++ edited/drivers/net/sungem.h 2004-06-21 22:24:46 +10:00
> >@@ -911,7 +911,7 @@
> >          (GP)->tx_old - (GP)->tx_new - 1)
 
> > #define RX_OFFSET          2
> >-#define RX_BUF_ALLOC_SIZE(gp)  ((gp)->dev->mtu + 46 + RX_OFFSET + 64)
> >+#define RX_BUF_ALLOC_SIZE(gp)  ((gp)->rx_buf_sz + 32 + RX_OFFSET + 64)
> > 
> > #define RX_COPY_THRESHOLD  256
> > 
> >@@ -979,6 +979,7 @@
> >        int                     rx_fifo_sz;
> >        int                     rx_pause_off;
> >        int                     rx_pause_on;
> >+       int                     rx_buf_sz;
 > >       int                     mii_phy_addr;
 > >
> >        u32                     mac_rx_cfg;

I've had the same problem, like Soeren, but if i put MTU=1200 there is not 
kernel panic.
I'm going to patch my 2.6.6 with this patch, thanks Herbert.

Best regards

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[Soeren Sonnenburg]

On Mon, 2004-06-21 at 15:03, Herbert Xu wrote:
> On Mon, Jun 21, 2004 at 10:33:50PM +1000, Herbert Xu wrote:
> > 
> > Does this patch fix your problems?
> 
> Oops, I had a thinko about min vs. max.  I've also decided to make the
> bigger MTU useful by adjusting the arguments to skb_put() as well.
> Please try this one instead.
> 
> Cheers,

yes that one works nicely... I tested several mtu's ranging from 1000 to
1500 while the interface was up... no oops.

thanks,
Soeren.

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[David S. Miller]

On Mon, 21 Jun 2004 23:03:16 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Mon, Jun 21, 2004 at 10:33:50PM +1000, Herbert Xu wrote:
> > 
> > Does this patch fix your problems?
> 
> Oops, I had a thinko about min vs. max.  I've also decided to make the
> bigger MTU useful by adjusting the arguments to skb_put() as well.
> Please try this one instead.

Applied, thanks Herbert.

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[Chris Friesen]

David S. Miller wrote:
> On Mon, 21 Jun 2004 23:03:16 +1000
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
>  > On Mon, Jun 21, 2004 at 10:33:50PM +1000, Herbert Xu wrote:
>  > >
>  > > Does this patch fix your problems?
>  >
>  > Oops, I had a thinko about min vs. max.  I've also decided to make the
>  > bigger MTU useful by adjusting the arguments to skb_put() as well.
>  > Please try this one instead.
> 
> Applied, thanks Herbert.

Just a quick question.  Does the sungem chip support jumbo frames?  I'd like to 
use MTU of 9000 to make large local transfers more efficient, but it didn't seem 
to work last time I checked.

Thanks,

Chris

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[Jeff Garzik]

Chris Friesen wrote:
> Just a quick question.  Does the sungem chip support jumbo frames?  I'd 
> like to use MTU of 9000 to make large local transfers more efficient, 
> but it didn't seem to work last time I checked.


Are you 100% certain you configured the other side to support jumbo?

Jumbo frames are non-standard, and sometimes require configuring MTU on 
the switch or remote network card (if directly connected).

	Jeff

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[Benjamin Herrenschmidt]

On Tue, 2004-06-22 at 10:04, Jeff Garzik wrote:
> Chris Friesen wrote:
> > Just a quick question.  Does the sungem chip support jumbo frames?  I'd 
> > like to use MTU of 9000 to make large local transfers more efficient, 
> > but it didn't seem to work last time I checked.
> 
> 
> Are you 100% certain you configured the other side to support jumbo?
> 
> Jumbo frames are non-standard, and sometimes require configuring MTU on 
> the switch or remote network card (if directly connected).

Well, it's not enabled in the driver I think, or at least it wasn't last
time I looked. Dave told me the chip fifo's are too small to do anything
useful with jumbo frames.

Ben.

Re: sungem - ifconfig eth0 mtu 1300 -> oops

[David S. Miller]

On Tue, 22 Jun 2004 10:53:23 -0400
Chris Friesen <cfriesen@nortelnetworks.com> wrote:

> Just a quick question.  Does the sungem chip support jumbo frames?

Nope, not at all.