* [PATCH] cleanup large frame handling for natsemi.c
@ 2004-06-20 12:14 Manfred Spraul
2004-06-20 16:30 ` Jeff Garzik
0 siblings, 1 reply; 5+ messages in thread
From: Manfred Spraul @ 2004-06-20 12:14 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
Hi,
The DP83815/6 by default rejects frames longer that 1518 bytes
(including crc). This means that a special flag must be set for 8021q -
otherwise mtu sized packets are dropped.
The current driver enables this flag only if the buffer size is above
1536 bytes - this is wrong. Additionally, the nic writes up to two bytes
behind the indicated end of the buffer. This is not documented, thus
I've added 64 bytes - just to be safe.
The patch also removes RX_OFFSET from the rx buffer allocation: The nic
can only receive to 32-bit aligned addresses, it's a left over from a
skeleton driver.
Jeff, could you apply it? I've stress tested vlan for an hour with
tbench and parallel kernel compiles, not obvious problems.
--
Manfred
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] cleanup large frame handling for natsemi.c 2004-06-20 12:14 [PATCH] cleanup large frame handling for natsemi.c Manfred Spraul @ 2004-06-20 16:30 ` Jeff Garzik 2004-06-20 16:43 ` Manfred Spraul 0 siblings, 1 reply; 5+ messages in thread From: Jeff Garzik @ 2004-06-20 16:30 UTC (permalink / raw) To: Manfred Spraul; +Cc: netdev On Sun, Jun 20, 2004 at 02:14:59PM +0200, Manfred Spraul wrote: > Jeff, could you apply it? I've stress tested vlan for an hour with > tbench and parallel kernel compiles, not obvious problems. I would if you had actually included a patch ;-) Jeff ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cleanup large frame handling for natsemi.c 2004-06-20 16:30 ` Jeff Garzik @ 2004-06-20 16:43 ` Manfred Spraul 2004-06-20 18:38 ` Jeff Garzik 0 siblings, 1 reply; 5+ messages in thread From: Manfred Spraul @ 2004-06-20 16:43 UTC (permalink / raw) To: Jeff Garzik; +Cc: netdev [-- Attachment #1: Type: text/plain, Size: 338 bytes --] Jeff Garzik wrote: >On Sun, Jun 20, 2004 at 02:14:59PM +0200, Manfred Spraul wrote: > > >>Jeff, could you apply it? I've stress tested vlan for an hour with >>tbench and parallel kernel compiles, not obvious problems. >> >> > >I would if you had actually included a patch ;-) > > > Ups, sorry. Here's the patch. -- Manfred [-- Attachment #2: patch-natsemi-long --] [-- Type: text/plain, Size: 2797 bytes --] --- 2.6/drivers/net/natsemi.c 2004-06-20 11:24:02.959200463 +0200 +++ build-2.6/drivers/net/natsemi.c 2004-06-20 12:11:31.170607816 +0200 @@ -236,7 +236,14 @@ #define NATSEMI_REGS_SIZE (NATSEMI_NREGS * sizeof(u32)) #define NATSEMI_EEPROM_SIZE 24 /* 12 16-bit values */ -#define PKT_BUF_SZ 1536 /* Size of each temporary Rx buffer. */ +/* Buffer sizes: + * The nic writes 32-bit values, even if the upper bytes of + * a 32-bit value are beyond the end of the buffer. + */ +#define NATSEMI_HEADERS 22 /* 2*mac,type,vlan,crc */ +#define NATSEMI_PADDING 64 /* 2 bytes should be sufficient */ +#define NATSEMI_LONGPKT 1518 /* limit for long packets */ +#define NATSEMI_RX_LIMIT 2046 /* maximum supported by hardware */ /* These identify the driver base version and may not be removed. */ static char version[] __devinitdata = @@ -1688,7 +1695,7 @@ */ np->rx_config = RxMxdma_256 | 0x20; /* if receive ring now has bigger buffers than normal, enable jumbo */ - if (np->rx_buf_sz > PKT_BUF_SZ) + if (np->rx_buf_sz > NATSEMI_LONGPKT) np->rx_config |= RxAcceptLong; writel(np->rx_config, ioaddr + RxConfig); @@ -1870,7 +1877,7 @@ struct sk_buff *skb; int entry = np->dirty_rx % RX_RING_SIZE; if (np->rx_skbuff[entry] == NULL) { - unsigned int buflen = np->rx_buf_sz + RX_OFFSET; + unsigned int buflen = np->rx_buf_sz+NATSEMI_PADDING; skb = dev_alloc_skb(buflen); np->rx_skbuff[entry] = skb; if (skb == NULL) @@ -1909,9 +1916,13 @@ np->dirty_rx = 0; np->cur_rx = RX_RING_SIZE; np->oom = 0; - np->rx_buf_sz = PKT_BUF_SZ; - if (dev->mtu > ETH_DATA_LEN) - np->rx_buf_sz += dev->mtu - ETH_DATA_LEN; + if (dev->mtu <= ETH_DATA_LEN) + np->rx_buf_sz = ETH_DATA_LEN + NATSEMI_HEADERS; + else + np->rx_buf_sz = dev->mtu + NATSEMI_HEADERS; + if (np->rx_buf_sz > NATSEMI_RX_LIMIT) + np->rx_buf_sz = NATSEMI_RX_LIMIT; + np->rx_head_desc = &np->rx_ring[0]; /* Please be carefull before changing this loop - at least gcc-2.95.1 @@ -1949,7 +1960,7 @@ static void drain_ring(struct net_device *dev) { struct netdev_private *np = dev->priv; - unsigned int buflen = np->rx_buf_sz + RX_OFFSET; + unsigned int buflen = np->rx_buf_sz; int i; /* Free all the skbuffs in the Rx queue. */ @@ -2154,7 +2165,7 @@ int entry = np->cur_rx % RX_RING_SIZE; int boguscnt = np->dirty_rx + RX_RING_SIZE - np->cur_rx; s32 desc_status = le32_to_cpu(np->rx_head_desc->cmd_status); - unsigned int buflen = np->rx_buf_sz + RX_OFFSET; + unsigned int buflen = np->rx_buf_sz; /* If the driver owns the next entry it's a new packet. Send it up. */ while (desc_status < 0) { /* e.g. & DescOwn */ @@ -2881,6 +2892,7 @@ } return 0; } + static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) { struct mii_ioctl_data *data = if_mii(rq); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cleanup large frame handling for natsemi.c 2004-06-20 16:43 ` Manfred Spraul @ 2004-06-20 18:38 ` Jeff Garzik 2004-06-20 19:31 ` Manfred Spraul 0 siblings, 1 reply; 5+ messages in thread From: Jeff Garzik @ 2004-06-20 18:38 UTC (permalink / raw) To: Manfred Spraul; +Cc: netdev On Sun, Jun 20, 2004 at 06:43:07PM +0200, Manfred Spraul wrote: > @@ -1909,9 +1916,13 @@ > np->dirty_rx = 0; > np->cur_rx = RX_RING_SIZE; > np->oom = 0; > - np->rx_buf_sz = PKT_BUF_SZ; > - if (dev->mtu > ETH_DATA_LEN) > - np->rx_buf_sz += dev->mtu - ETH_DATA_LEN; > + if (dev->mtu <= ETH_DATA_LEN) > + np->rx_buf_sz = ETH_DATA_LEN + NATSEMI_HEADERS; > + else > + np->rx_buf_sz = dev->mtu + NATSEMI_HEADERS; > + if (np->rx_buf_sz > NATSEMI_RX_LIMIT) > + np->rx_buf_sz = NATSEMI_RX_LIMIT; > + Double NAK: 1) Use PKT_BUF_SZ, don't alloc smaller than that. 2) The final check should not be needed. The code should guarantee that np->rx_buf_sz never exceeds NATSEMI_RX_LIMIT. Jeff ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cleanup large frame handling for natsemi.c 2004-06-20 18:38 ` Jeff Garzik @ 2004-06-20 19:31 ` Manfred Spraul 0 siblings, 0 replies; 5+ messages in thread From: Manfred Spraul @ 2004-06-20 19:31 UTC (permalink / raw) To: Jeff Garzik; +Cc: netdev Jeff Garzik wrote: >On Sun, Jun 20, 2004 at 06:43:07PM +0200, Manfred Spraul wrote: > > >>@@ -1909,9 +1916,13 @@ >> np->dirty_rx = 0; >> np->cur_rx = RX_RING_SIZE; >> np->oom = 0; >>- np->rx_buf_sz = PKT_BUF_SZ; >>- if (dev->mtu > ETH_DATA_LEN) >>- np->rx_buf_sz += dev->mtu - ETH_DATA_LEN; >>+ if (dev->mtu <= ETH_DATA_LEN) >>+ np->rx_buf_sz = ETH_DATA_LEN + NATSEMI_HEADERS; >>+ else >>+ np->rx_buf_sz = dev->mtu + NATSEMI_HEADERS; >>+ if (np->rx_buf_sz > NATSEMI_RX_LIMIT) >>+ np->rx_buf_sz = NATSEMI_RX_LIMIT; >>+ >> >> > >Double NAK: > >1) Use PKT_BUF_SZ, don't alloc smaller than that. > > > The alloc size is never smaller than PKT_BUF_SZ: ETH_DATA_LEN+NATSEMI_HEADERS+NATSEMI_PADDING is 1586 bytes [still smaller than the 1620 byte skb kmalloc cache] >2) The final check should not be needed. The code should guarantee that >np->rx_buf_sz never exceeds NATSEMI_RX_LIMIT. > > > You mean: implement a change_mtu callback and reject mtu values above 2020 byte? -- Manfred > Jeff > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2004-06-20 19:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-06-20 12:14 [PATCH] cleanup large frame handling for natsemi.c Manfred Spraul 2004-06-20 16:30 ` Jeff Garzik 2004-06-20 16:43 ` Manfred Spraul 2004-06-20 18:38 ` Jeff Garzik 2004-06-20 19:31 ` Manfred Spraul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).