Netdev List
 help / color / mirror / Atom feed
* [PATCH] ip_gra_reasm() should set "err" incase of skb_clone() failure
From: Badari Pulavarty @ 2007-10-17 23:02 UTC (permalink / raw)
  To: netdev; +Cc: lkml

Simple error handling fix (against 2.26.23-git12).

Thanks,
Badari

Need to initialize "err" in case of skb_clone() failure.

net/ipv4/ip_fragment.c: In function `ip_defrag':
net/ipv4/ip_fragment.c:540: warning: `err' might be used uninitialized in this function

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
---
 net/ipv4/ip_fragment.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6.23/net/ipv4/ip_fragment.c
===================================================================
--- linux-2.6.23.orig/net/ipv4/ip_fragment.c	2007-10-17 15:33:27.000000000 -0700
+++ linux-2.6.23/net/ipv4/ip_fragment.c	2007-10-17 15:50:51.000000000 -0700
@@ -544,6 +544,7 @@ static int ip_frag_reasm(struct ipq *qp,
 	/* Make the one we just received the head. */
 	if (prev) {
 		head = prev->next;
+		err = -ENOMEM;
 		fp = skb_clone(head, GFP_ATOMIC);
 
 		if (!fp)

^ permalink raw reply

* [PATCH] pcnet32: fix non-napi packet reception
From: Don Fry @ 2007-10-17 22:59 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev

Recent changes to the driver for the new napi API broke the reception of packets when in
non-napi mode.  The initialization of napi.weight was removed for the non-napi case
leaving the value zero.

Tested NAPI and non-NAPI on x86_64.

Signed-off-by: Don Fry <pcnet32@verizon.net>
---
--- linux-2.6.23-git7/drivers/net/orig.pcnet32.c	2007-10-15 11:19:17.000000000 -0700
+++ linux-2.6.23-git7/drivers/net/pcnet32.c	2007-10-17 15:56:15.000000000 -0700
@@ -1849,6 +1849,9 @@ pcnet32_probe1(unsigned long ioaddr, int
 	lp->mii_if.mdio_read = mdio_read;
 	lp->mii_if.mdio_write = mdio_write;
 
+	/* napi.weight is used in both the napi and non-napi cases */
+	lp->napi.weight = lp->rx_ring_size / 2;
+
 #ifdef CONFIG_PCNET32_NAPI
 	netif_napi_add(dev, &lp->napi, pcnet32_poll, lp->rx_ring_size / 2);
 #endif



^ permalink raw reply

* Re: multicast: bug or "feature"
From: David Stevens @ 2007-10-17 23:11 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: Brian Haley, netdev
In-Reply-To: <47167DD4.3020108@hp.com>

You're joining the group on interface eth1, which is the
sender, right? You need to be a member on eth0 to receive it
there. I think your program needs another argument, to
specify the receiving interface, which you want to be
different from the sending interface.

                                        +-DLS


^ permalink raw reply

* [PATCH] vortex_up should initialize "err"
From: Badari Pulavarty @ 2007-10-17 23:15 UTC (permalink / raw)
  To: netdev; +Cc: lkml

Simple compile warning fix. (against 2.6.23-git12)

Thanks,
Badari

vortex_up() should initialize 'err' for a successful return.

drivers/net/3c59x.c: In function `vortex_up':
drivers/net/3c59x.c:1494: warning: `err' might be used uninitialized in this function


Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
---
 drivers/net/3c59x.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.23/drivers/net/3c59x.c
===================================================================
--- linux-2.6.23.orig/drivers/net/3c59x.c	2007-10-17 15:33:07.000000000 -0700
+++ linux-2.6.23/drivers/net/3c59x.c	2007-10-17 16:07:10.000000000 -0700
@@ -1491,7 +1491,7 @@ vortex_up(struct net_device *dev)
 	struct vortex_private *vp = netdev_priv(dev);
 	void __iomem *ioaddr = vp->ioaddr;
 	unsigned int config;
-	int i, mii_reg1, mii_reg5, err;
+	int i, mii_reg1, mii_reg5, err = 0;
 
 	if (VORTEX_PCI(vp)) {
 		pci_set_power_state(VORTEX_PCI(vp), PCI_D0);	/* Go active */



^ permalink raw reply

* [PATCH] fix EMAC driver for proper napi_synchronize API
From: Benjamin Herrenschmidt @ 2007-10-17 23:14 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jeff Garzik, netdev, David S. Miller, Roland Dreier,
	linuxppc-dev list
In-Reply-To: <20071017083107.6a1b1bd3@freepuppy.rosehill>

The EMAC driver "fix" was merged by mistake before the dust had settled on
the new napi synchronize interface (and before it got merged). The final
version of that function is spelled without underscores.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
--- 

Index: linux-work/drivers/net/ibm_newemac/mal.c
===================================================================
--- linux-work.orig/drivers/net/ibm_newemac/mal.c	2007-10-18 08:02:22.000000000 +1000
+++ linux-work/drivers/net/ibm_newemac/mal.c	2007-10-18 08:02:24.000000000 +1000
@@ -322,7 +322,7 @@ void mal_poll_disable(struct mal_instanc
 		msleep(1);
 
 	/* Synchronize with the MAL NAPI poller */
-	__napi_synchronize(&mal->napi);
+	napi_synchronize(&mal->napi);
 }
 
 void mal_poll_enable(struct mal_instance *mal, struct mal_commac *commac)



^ permalink raw reply

* Re: [RFD] iptables:  mangle table obsoletes filter table
From: Bill Davidsen @ 2007-10-17 23:24 UTC (permalink / raw)
  To: Bill Davidsen
  Cc: Al Boldi, Patrick McHardy, netfilter-devel, netdev, linux-net,
	linux-kernel
In-Reply-To: <47168EA1.1080300@tmr.com>

Bill Davidsen wrote:

>>>> If not, then shouldn't the filter table be obsoleted to avoid 
>>>> confusion?
>>> That would probably confuse people. Just don't use it if you don't
>>> need to.
>>
> That is a most practical suggestion.
> 
>> The problem is that people think they are safe with the filter table, 
>> when in fact they need the prerouting chain to seal things.  Right now 
>> this is only possible in the mangle table.
>>
> I'm not sure what you think is unsafe about using the filter table, and 
> the order of evaluation issues certainly seem to suggest that some 
> actions would take a major rethink at least. Perhaps you could avoid 
> breaking all of the setups which currently work, rather than force 
> everyone to do things differently because you feel that your way is better.
> 
It was my intention to suggest that unintentional breakage of existing 
setups should be avoided, not that removing the filter table was some 
evil plot. ;-)
On rereading my original post I failed to make that clear, please take 
it as intended.

-- 
Bill Davidsen <davidsen@tmr.com>
   "We have more to fear from the bungling of the incompetent than from
the machinations of the wicked."  - from Slashdot

^ permalink raw reply

* [PATCH] pcnet32:  remove compile warnings in non-napi mode
From: Don Fry @ 2007-10-17 23:10 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev

Remove compile warning when in non-napi mode.

Signed-off-by:  Don Fry <pcnet32@verizon.net>
---
--- linux-2.6.23-git7/drivers/net/napi.pcnet32.c	2007-10-17 15:56:15.000000000 -0700
+++ linux-2.6.23-git7/drivers/net/pcnet32.c	2007-10-17 16:00:44.000000000 -0700
@@ -442,7 +442,9 @@ static struct pcnet32_access pcnet32_dwi
 
 static void pcnet32_netif_stop(struct net_device *dev)
 {
+#ifdef CONFIG_PCNET32_NAPI
 	struct pcnet32_private *lp = netdev_priv(dev);
+#endif
 	dev->trans_start = jiffies;
 #ifdef CONFIG_PCNET32_NAPI
 	napi_disable(&lp->napi);
@@ -452,7 +454,9 @@ static void pcnet32_netif_stop(struct ne
 
 static void pcnet32_netif_start(struct net_device *dev)
 {
+#ifdef CONFIG_PCNET32_NAPI
 	struct pcnet32_private *lp = netdev_priv(dev);
+#endif
 	netif_wake_queue(dev);
 #ifdef CONFIG_PCNET32_NAPI
 	napi_enable(&lp->napi);



^ permalink raw reply

* Re: [PATCH] net: Add napi_sycnhronize() to sync with napi poll
From: Josh Boyer @ 2007-10-17 23:36 UTC (permalink / raw)
  To: benh
  Cc: Stephen Hemminger, netdev, Roland Dreier, David S. Miller,
	Jeff Garzik, linuxppc-dev list
In-Reply-To: <1192658654.6783.43.camel@pasglop>

On Thu, 2007-10-18 at 08:04 +1000, Benjamin Herrenschmidt wrote:
> net: Add __napi_synchronize() to sync with napi poll
> 
> The EMAC driver which needs to handle multiple devices with one
> NAPI instance implements its own per-channel disable bit. However,
> when setting such a bit, it needs to synchronize with the poller
> (that is make sure that any pending poller instance has completed,
> or is started late enough to see that disable bit).
> 
> This implements a low level __napi_synchronize() function to acheive
> that. The underscores are to emphasis the low level aspect of it and
> to discourage driver writers who don't know what they are doing to
> use it (to please DaveM :-)

Erm.. your commit log calls it __napi_synchronize still.

josh


^ permalink raw reply

* Re: [PATCH] pcnet32: fix non-napi packet reception
From: Jeff Garzik @ 2007-10-17 23:55 UTC (permalink / raw)
  To: Don Fry; +Cc: netdev
In-Reply-To: <1192661962.474.10.camel@Linux.home>

Don Fry wrote:
> Recent changes to the driver for the new napi API broke the reception of packets when in
> non-napi mode.  The initialization of napi.weight was removed for the non-napi case
> leaving the value zero.
> 
> Tested NAPI and non-NAPI on x86_64.
> 
> Signed-off-by: Don Fry <pcnet32@verizon.net>
> ---
> --- linux-2.6.23-git7/drivers/net/orig.pcnet32.c	2007-10-15 11:19:17.000000000 -0700
> +++ linux-2.6.23-git7/drivers/net/pcnet32.c	2007-10-17 15:56:15.000000000 -0700
> @@ -1849,6 +1849,9 @@ pcnet32_probe1(unsigned long ioaddr, int
>  	lp->mii_if.mdio_read = mdio_read;
>  	lp->mii_if.mdio_write = mdio_write;
>  
> +	/* napi.weight is used in both the napi and non-napi cases */
> +	lp->napi.weight = lp->rx_ring_size / 2;
> +
>  #ifdef CONFIG_PCNET32_NAPI
>  	netif_napi_add(dev, &lp->napi, pcnet32_poll, lp->rx_ring_size / 2);
>  #endif

In general, we're hoping to remove as many CONFIG_FOO_NAPI as possible, 
pushing everybody towards using NAPI.

Any objection to heading in this direction with pcnet32?

	Jeff




^ permalink raw reply

* Re: [PATCH] net: Add napi_sycnhronize() to sync with napi poll
From: Benjamin Herrenschmidt @ 2007-10-17 23:59 UTC (permalink / raw)
  To: Josh Boyer
  Cc: Stephen Hemminger, netdev, Roland Dreier, David S. Miller,
	Jeff Garzik, linuxppc-dev list
In-Reply-To: <1192664183.13993.30.camel@localhost.localdomain>


On Wed, 2007-10-17 at 18:36 -0500, Josh Boyer wrote:
> On Thu, 2007-10-18 at 08:04 +1000, Benjamin Herrenschmidt wrote:
> > net: Add __napi_synchronize() to sync with napi poll
> > 
> > The EMAC driver which needs to handle multiple devices with one
> > NAPI instance implements its own per-channel disable bit. However,
> > when setting such a bit, it needs to synchronize with the poller
> > (that is make sure that any pending poller instance has completed,
> > or is started late enough to see that disable bit).
> > 
> > This implements a low level __napi_synchronize() function to acheive
> > that. The underscores are to emphasis the low level aspect of it and
> > to discourage driver writers who don't know what they are doing to
> > use it (to please DaveM :-)
> 
> Erm.. your commit log calls it __napi_synchronize still.

The commit is a mistake, I sent a patch to rename it, or did I miss
something ?

Ben.



^ permalink raw reply

* Re: [PATCH] pcnet32: fix non-napi packet reception
From: Don Fry @ 2007-10-18  0:04 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev

>In general, we're hoping to remove as many CONFIG_FOO_NAPI as possible,
>pushing everybody towards using NAPI.
>
>Any objection to heading in this direction with pcnet32?
>
>        Jeff

I have no objections myself.  It has been slowly moving that direction.
First with the napi implementation, default off, labeled experimental.
Then removing experimental and then making the default on.

If any other user of the pcnet32 has objections, now is the time to
speak loudly!

Don


^ permalink raw reply

* [PATCH] pcnet32:  remove private net_device_stats structure
From: Don Fry @ 2007-10-18  0:06 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev

Remove the statistics from the private structure.
Use the net_device_stats in netn_device structure.

Following Jeff Garzik's massive cleanup Sep 01.
pcnet32 was not "low-hanging fruit".

Tested x86_64.

Signed-off-by:  Don Fry <pcnet32@verizon.net>
---
--- linux-2.6.23-git7/drivers/net/cleaner.pcnet32.c	2007-10-17 16:00:44.000000000 -0700
+++ linux-2.6.23-git7/drivers/net/pcnet32.c	2007-10-17 16:44:50.000000000 -0700
@@ -282,7 +282,6 @@ struct pcnet32_private {
 
 	struct net_device	*dev;
 	struct napi_struct	napi;
-	struct net_device_stats	stats;
 	char			tx_full;
 	char			phycount;	/* number of phys found */
 	int			options;
@@ -1182,15 +1181,15 @@ static void pcnet32_rx_entry(struct net_
 		 * buffers, with only the last correctly noting the error.
 		 */
 		if (status & 0x01)	/* Only count a general error at the */
-			lp->stats.rx_errors++;	/* end of a packet. */
+			dev->stats.rx_errors++;	/* end of a packet. */
 		if (status & 0x20)
-			lp->stats.rx_frame_errors++;
+			dev->stats.rx_frame_errors++;
 		if (status & 0x10)
-			lp->stats.rx_over_errors++;
+			dev->stats.rx_over_errors++;
 		if (status & 0x08)
-			lp->stats.rx_crc_errors++;
+			dev->stats.rx_crc_errors++;
 		if (status & 0x04)
-			lp->stats.rx_fifo_errors++;
+			dev->stats.rx_fifo_errors++;
 		return;
 	}
 
@@ -1201,13 +1200,13 @@ static void pcnet32_rx_entry(struct net_
 		if (netif_msg_drv(lp))
 			printk(KERN_ERR "%s: Impossible packet size %d!\n",
 			       dev->name, pkt_len);
-		lp->stats.rx_errors++;
+		dev->stats.rx_errors++;
 		return;
 	}
 	if (pkt_len < 60) {
 		if (netif_msg_rx_err(lp))
 			printk(KERN_ERR "%s: Runt packet!\n", dev->name);
-		lp->stats.rx_errors++;
+		dev->stats.rx_errors++;
 		return;
 	}
 
@@ -1241,7 +1240,7 @@ static void pcnet32_rx_entry(struct net_
 			printk(KERN_ERR
 			       "%s: Memory squeeze, dropping packet.\n",
 			       dev->name);
-		lp->stats.rx_dropped++;
+		dev->stats.rx_dropped++;
 		return;
 	}
 	skb->dev = dev;
@@ -1260,7 +1259,7 @@ static void pcnet32_rx_entry(struct net_
 					       pkt_len,
 					       PCI_DMA_FROMDEVICE);
 	}
-	lp->stats.rx_bytes += skb->len;
+	dev->stats.rx_bytes += skb->len;
 	skb->protocol = eth_type_trans(skb, dev);
 #ifdef CONFIG_PCNET32_NAPI
 	netif_receive_skb(skb);
@@ -1268,7 +1267,7 @@ static void pcnet32_rx_entry(struct net_
 	netif_rx(skb);
 #endif
 	dev->last_rx = jiffies;
-	lp->stats.rx_packets++;
+	dev->stats.rx_packets++;
 	return;
 }
 
@@ -1316,21 +1315,21 @@ static int pcnet32_tx(struct net_device 
 		if (status & 0x4000) {
 			/* There was a major error, log it. */
 			int err_status = le32_to_cpu(lp->tx_ring[entry].misc);
-			lp->stats.tx_errors++;
+			dev->stats.tx_errors++;
 			if (netif_msg_tx_err(lp))
 				printk(KERN_ERR
 				       "%s: Tx error status=%04x err_status=%08x\n",
 				       dev->name, status,
 				       err_status);
 			if (err_status & 0x04000000)
-				lp->stats.tx_aborted_errors++;
+				dev->stats.tx_aborted_errors++;
 			if (err_status & 0x08000000)
-				lp->stats.tx_carrier_errors++;
+				dev->stats.tx_carrier_errors++;
 			if (err_status & 0x10000000)
-				lp->stats.tx_window_errors++;
+				dev->stats.tx_window_errors++;
 #ifndef DO_DXSUFLO
 			if (err_status & 0x40000000) {
-				lp->stats.tx_fifo_errors++;
+				dev->stats.tx_fifo_errors++;
 				/* Ackk!  On FIFO errors the Tx unit is turned off! */
 				/* Remove this verbosity later! */
 				if (netif_msg_tx_err(lp))
@@ -1341,7 +1340,7 @@ static int pcnet32_tx(struct net_device 
 			}
 #else
 			if (err_status & 0x40000000) {
-				lp->stats.tx_fifo_errors++;
+				dev->stats.tx_fifo_errors++;
 				if (!lp->dxsuflo) {	/* If controller doesn't recover ... */
 					/* Ackk!  On FIFO errors the Tx unit is turned off! */
 					/* Remove this verbosity later! */
@@ -1355,8 +1354,8 @@ static int pcnet32_tx(struct net_device 
 #endif
 		} else {
 			if (status & 0x1800)
-				lp->stats.collisions++;
-			lp->stats.tx_packets++;
+				dev->stats.collisions++;
+			dev->stats.tx_packets++;
 		}
 
 		/* We must free the original skb */
@@ -2478,7 +2477,7 @@ static void pcnet32_tx_timeout(struct ne
 		       "%s: transmit timed out, status %4.4x, resetting.\n",
 		       dev->name, lp->a.read_csr(ioaddr, CSR0));
 	lp->a.write_csr(ioaddr, CSR0, CSR0_STOP);
-	lp->stats.tx_errors++;
+	dev->stats.tx_errors++;
 	if (netif_msg_tx_err(lp)) {
 		int i;
 		printk(KERN_DEBUG
@@ -2548,7 +2547,7 @@ static int pcnet32_start_xmit(struct sk_
 	lp->tx_ring[entry].status = cpu_to_le16(status);
 
 	lp->cur_tx++;
-	lp->stats.tx_bytes += skb->len;
+	dev->stats.tx_bytes += skb->len;
 
 	/* Trigger an immediate send poll. */
 	lp->a.write_csr(ioaddr, CSR0, CSR0_INTEN | CSR0_TXPOLL);
@@ -2593,7 +2592,7 @@ pcnet32_interrupt(int irq, void *dev_id)
 
 		/* Log misc errors. */
 		if (csr0 & 0x4000)
-			lp->stats.tx_errors++;	/* Tx babble. */
+			dev->stats.tx_errors++;	/* Tx babble. */
 		if (csr0 & 0x1000) {
 			/*
 			 * This happens when our receive ring is full. This
@@ -2606,7 +2605,7 @@ pcnet32_interrupt(int irq, void *dev_id)
 			 * don't get a rx interrupt, but a missed frame
 			 * interrupt sooner or later.
 			 */
-			lp->stats.rx_errors++;	/* Missed a Rx frame. */
+			dev->stats.rx_errors++;	/* Missed a Rx frame. */
 		}
 		if (csr0 & 0x0800) {
 			if (netif_msg_drv(lp))
@@ -2668,7 +2667,7 @@ static int pcnet32_close(struct net_devi
 
 	spin_lock_irqsave(&lp->lock, flags);
 
-	lp->stats.rx_missed_errors = lp->a.read_csr(ioaddr, 112);
+	dev->stats.rx_missed_errors = lp->a.read_csr(ioaddr, 112);
 
 	if (netif_msg_ifdown(lp))
 		printk(KERN_DEBUG
@@ -2705,10 +2704,10 @@ static struct net_device_stats *pcnet32_
 	unsigned long flags;
 
 	spin_lock_irqsave(&lp->lock, flags);
-	lp->stats.rx_missed_errors = lp->a.read_csr(ioaddr, 112);
+	dev->stats.rx_missed_errors = lp->a.read_csr(ioaddr, 112);
 	spin_unlock_irqrestore(&lp->lock, flags);
 
-	return &lp->stats;
+	return &dev->stats;
 }
 
 /* taken from the sunlance driver, which it took from the depca driver */



^ permalink raw reply

* Re: [PATCH] [net] gianfar: remove orphan struct.
From: Jeff Garzik @ 2007-10-18  0:21 UTC (permalink / raw)
  To: Sebastian Siewior; +Cc: netdev, Li Yang
In-Reply-To: <E1Ii4k6-0000VM-0j@Chamillionaire.breakpoint.cc>

Sebastian Siewior wrote:
> struct net_device_stats is no longer used in driver's private
> struct but in struct net_device.
> 
> Cc: Li Yang <leoli@freescale.com>
> Signed-off-by: Sebastian Siewior <bigeasy@linutronix.de>
> ---
>  drivers/net/gianfar.h |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
> index c16cc8b..46cd773 100644
> --- a/drivers/net/gianfar.h
> +++ b/drivers/net/gianfar.h
> @@ -749,7 +749,6 @@ struct gfar_private {
>  	uint32_t msg_enable;
>  
>  	/* Network Statistics */
> -	struct net_device_stats stats;

applied



^ permalink raw reply

* Re: [PATCH] fs_enet: Update for API changes
From: Jeff Garzik @ 2007-10-18  0:21 UTC (permalink / raw)
  To: Scott Wood; +Cc: netdev, linuxppc-dev
In-Reply-To: <20071017174243.GA4462@loki.buserror.net>

Scott Wood wrote:
> This driver was recently broken by several changes for which this
> driver was not (or was improperly) updated:
> 
> 1. SET_MODULE_OWNER() was removed.
> 2. netif_napi_add() was only being called when building with
> the old CPM binding.
> 3. The received/budget test was backwards.
> 4. to_net_dev() was wrong -- the device struct embedded in
> the net_device struct is not the same as the of_platform
> device in the private struct.
> 5. napi_disable/napi_enable was being called even when napi
> was not being used.
> 
> These changes have been fixed, and napi is now on by default.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>

applied



^ permalink raw reply

* Re: [PATCH][NET] gianfar: fix obviously wrong #ifdef CONFIG_GFAR_NAPI placement
From: Jeff Garzik @ 2007-10-18  0:21 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: leoli, paulus, galak, afleming, apw, linuxppc-dev, netdev,
	linux-kernel
In-Reply-To: <20071017195746.GA15592@localhost.localdomain>

Anton Vorontsov wrote:
> Erroneous #ifdef introduced by 293c8513398657f6263fcdb03c87f2760cf61be4
> causing NAPI-less ethernet malfunctioning.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>

applied

^ permalink raw reply

* Re: [PATCH 1/2] napi_synchronize: waiting for NAPI
From: Jeff Garzik @ 2007-10-18  0:21 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, netdev
In-Reply-To: <20071017202653.266566036@linux-foundation.org>

applied 1-2



^ permalink raw reply

* Re: [PATCH] fix EMAC driver for proper napi_synchronize API
From: Jeff Garzik @ 2007-10-18  0:21 UTC (permalink / raw)
  To: benh
  Cc: Stephen Hemminger, netdev, David S. Miller, Roland Dreier,
	linuxppc-dev list
In-Reply-To: <1192662843.6783.48.camel@pasglop>

Benjamin Herrenschmidt wrote:
> The EMAC driver "fix" was merged by mistake before the dust had settled on
> the new napi synchronize interface (and before it got merged). The final
> version of that function is spelled without underscores.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

applied



^ permalink raw reply

* Re: [PATCH] pcnet32: fix non-napi packet reception
From: Jeff Garzik @ 2007-10-18  0:21 UTC (permalink / raw)
  To: Don Fry; +Cc: netdev
In-Reply-To: <1192661962.474.10.camel@Linux.home>

Don Fry wrote:
> Recent changes to the driver for the new napi API broke the reception of packets when in
> non-napi mode.  The initialization of napi.weight was removed for the non-napi case
> leaving the value zero.
> 
> Tested NAPI and non-NAPI on x86_64.
> 
> Signed-off-by: Don Fry <pcnet32@verizon.net>
> ---
> --- linux-2.6.23-git7/drivers/net/orig.pcnet32.c	2007-10-15 11:19:17.000000000 -0700
> +++ linux-2.6.23-git7/drivers/net/pcnet32.c	2007-10-17 15:56:15.000000000 -0700
> @@ -1849,6 +1849,9 @@ pcnet32_probe1(unsigned long ioaddr, int
>  	lp->mii_if.mdio_read = mdio_read;
>  	lp->mii_if.mdio_write = mdio_write;
>  
> +	/* napi.weight is used in both the napi and non-napi cases */
> +	lp->napi.weight = lp->rx_ring_size / 2;
> +
>  #ifdef CONFIG_PCNET32_NAPI
>  	netif_napi_add(dev, &lp->napi, pcnet32_poll, lp->rx_ring_size / 2);
>  #endif

applied



^ permalink raw reply

* Re: [PATCH] pcnet32:  remove compile warnings in non-napi mode
From: Jeff Garzik @ 2007-10-18  0:22 UTC (permalink / raw)
  To: Don Fry; +Cc: netdev
In-Reply-To: <1192662610.5312.1.camel@Linux.home>

Don Fry wrote:
> Remove compile warning when in non-napi mode.
> 
> Signed-off-by:  Don Fry <pcnet32@verizon.net>
> ---
> --- linux-2.6.23-git7/drivers/net/napi.pcnet32.c	2007-10-17 15:56:15.000000000 -0700
> +++ linux-2.6.23-git7/drivers/net/pcnet32.c	2007-10-17 16:00:44.000000000 -0700
> @@ -442,7 +442,9 @@ static struct pcnet32_access pcnet32_dwi
>  
>  static void pcnet32_netif_stop(struct net_device *dev)
>  {
> +#ifdef CONFIG_PCNET32_NAPI
>  	struct pcnet32_private *lp = netdev_priv(dev);
> +#endif
>  	dev->trans_start = jiffies;
>  #ifdef CONFIG_PCNET32_NAPI
>  	napi_disable(&lp->napi);

applied



^ permalink raw reply

* Re: [PATCH 1/2] napi_synchronize: waiting for NAPI
From: Benjamin Herrenschmidt @ 2007-10-18  0:28 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, Jeff Garzik, netdev
In-Reply-To: <20071017202653.266566036@linux-foundation.org>


On Wed, 2007-10-17 at 13:26 -0700, Stephen Hemminger wrote:
> plain text document attachment (napi-synchronize.patch)
> Some drivers with shared NAPI need a synchronization barrier.
> Also suggested by Benjamin Herrenschmidt for EMAC.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

Just saw this patch, sorry, missed it. Same as you sent me on another
reply, I still think it needs the smp barrier. Can you have a look at
the one I posted today ?

Cheers,
Ben.



^ permalink raw reply

* Re: [PATCH] vortex_up should initialize "err"
From: Jeff Garzik @ 2007-10-18  0:28 UTC (permalink / raw)
  To: Badari Pulavarty; +Cc: netdev, lkml, Steffen Klassert
In-Reply-To: <1192662956.14504.14.camel@dyn9047017100.beaverton.ibm.com>

Badari Pulavarty wrote:
> Simple compile warning fix. (against 2.6.23-git12)
> 
> Thanks,
> Badari
> 
> vortex_up() should initialize 'err' for a successful return.
> 
> drivers/net/3c59x.c: In function `vortex_up':
> drivers/net/3c59x.c:1494: warning: `err' might be used uninitialized in this function

applied as an obvious bug fix (Steffen added to CC)


^ permalink raw reply

* Re: [PATCH] pcnet32:  remove private net_device_stats structure
From: Jeff Garzik @ 2007-10-18  0:28 UTC (permalink / raw)
  To: Don Fry; +Cc: netdev
In-Reply-To: <1192665979.13749.7.camel@Linux.home>

Don Fry wrote:
> Remove the statistics from the private structure.
> Use the net_device_stats in netn_device structure.
> 
> Following Jeff Garzik's massive cleanup Sep 01.
> pcnet32 was not "low-hanging fruit".
> 
> Tested x86_64.
> 
> Signed-off-by:  Don Fry <pcnet32@verizon.net>

applied



^ permalink raw reply

* [PATCH 2/6] Convert balance-rr transmit to new locking
From: Jay Vosburgh @ 2007-10-18  0:37 UTC (permalink / raw)
  To: netdev, jgarzik; +Cc: andy, Jay Vosburgh
In-Reply-To: <11926678763205-git-send-email-fubar@us.ibm.com>

	Change locking in balance-rr transmit processing to use a free
running counter to determine which slave to transmit on.  Instead, a
free-running counter is maintained, and modulo arithmetic used to select
a slave for transmit.

	This removes lock operations from the TX path, and eliminates
a deadlock introduced by the conversion to work queues.

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
 drivers/net/bonding/bond_main.c |   25 ++++++++++++-------------
 drivers/net/bonding/bonding.h   |    1 +
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ed361d6..862ed8e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4057,8 +4057,7 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
 {
 	struct bonding *bond = bond_dev->priv;
 	struct slave *slave, *start_at;
-	int i;
-	int res = 1;
+	int i, slave_no, res = 1;
 
 	read_lock(&bond->lock);
 
@@ -4066,29 +4065,29 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
 		goto out;
 	}
 
-	read_lock(&bond->curr_slave_lock);
-	slave = start_at = bond->curr_active_slave;
-	read_unlock(&bond->curr_slave_lock);
+	/*
+	 * Concurrent TX may collide on rr_tx_counter; we accept that
+	 * as being rare enough not to justify using an atomic op here
+	 */
+	slave_no = bond->rr_tx_counter++ % bond->slave_cnt;
 
-	if (!slave) {
-		goto out;
+	bond_for_each_slave(bond, slave, i) {
+		slave_no--;
+		if (slave_no < 0) {
+			break;
+		}
 	}
 
+	start_at = slave;
 	bond_for_each_slave_from(bond, slave, i, start_at) {
 		if (IS_UP(slave->dev) &&
 		    (slave->link == BOND_LINK_UP) &&
 		    (slave->state == BOND_STATE_ACTIVE)) {
 			res = bond_dev_queue_xmit(bond, skb, slave->dev);
-
-			write_lock(&bond->curr_slave_lock);
-			bond->curr_active_slave = slave->next;
-			write_unlock(&bond->curr_slave_lock);
-
 			break;
 		}
 	}
 
-
 out:
 	if (res) {
 		/* no suitable interface, frame not sent */
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index a8f2384..d1ed14b 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -197,6 +197,7 @@ struct bonding {
 	int      (*xmit_hash_policy)(struct sk_buff *, struct net_device *, int);
 	__be32   master_ip;
 	u16      flags;
+	u16      rr_tx_counter;
 	struct   ad_bond_info ad_info;
 	struct   alb_bond_info alb_info;
 	struct   bond_params params;
-- 
1.5.3.4.206.g58ba4-dirty


^ permalink raw reply related

* [PATCH 0/6] bonding workqueue and lock rework
From: Jay Vosburgh @ 2007-10-18  0:37 UTC (permalink / raw)
  To: netdev, jgarzik; +Cc: andy

	Following are patches to update the locking used in the bonding
driver.  This involves two basic changes: conversion from timers to
workqueues for the various periodic monitor functions, and conversion of
locking.  These patches should resolve the majority of locking and
might sleep related warnings that occur during certain operations.

	The workqueue conversion is fairly straightforward, and
substitutes workqueue driven events for the existing timer driven events.

	The locking changes are of two types: first, changes to resolve
deadlocks created by the conversion to workqueues, and second, changes to
make the locking more correct, which generally involves holding RTNL and
no other locks during specific operations.  Some of the RTNL-related
changes are fairly extensive, and involve either conditional locking or
releasing and reacquiring locks in order to obey lock ordering
constraints.

	These changes were developed and extensively tested by Andy
Gospodarek <andy@greyhouse.net> and myself over the last few months.

	Patches are relative to linux-2.6.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* [PATCH]  Two small fixes to IPoIB support for bonding:
From: Jay Vosburgh @ 2007-10-18  0:37 UTC (permalink / raw)
  To: netdev, jgarzik; +Cc: andy, Jay Vosburgh
In-Reply-To: <1192667875793-git-send-email-fubar@us.ibm.com>

	1- copy header_ops from slave to bonding for IPoIB slaves
	2- move release and destroy logic to UNREGISTER from GOING_DOWN
	   notifier to prevent double release

	Set bonding to version 3.2.1.
---
 drivers/net/bonding/bond_main.c |   11 +++++------
 drivers/net/bonding/bonding.h   |    4 ++--
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index db80f24..6f85cc3 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1263,6 +1263,7 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
 	struct bonding *bond = bond_dev->priv;
 
 	bond_dev->neigh_setup           = slave_dev->neigh_setup;
+	bond_dev->header_ops		= slave_dev->header_ops;
 
 	bond_dev->type		    = slave_dev->type;
 	bond_dev->hard_header_len   = slave_dev->hard_header_len;
@@ -3351,7 +3352,10 @@ static int bond_slave_netdev_event(unsigned long event, struct net_device *slave
 	switch (event) {
 	case NETDEV_UNREGISTER:
 		if (bond_dev) {
-			bond_release(bond_dev, slave_dev);
+			if (bond->setup_by_slave)
+				bond_release_and_destroy(bond_dev, slave_dev);
+			else
+				bond_release(bond_dev, slave_dev);
 		}
 		break;
 	case NETDEV_CHANGE:
@@ -3366,11 +3370,6 @@ static int bond_slave_netdev_event(unsigned long event, struct net_device *slave
 		 * ... Or is it this?
 		 */
 		break;
-	case NETDEV_GOING_DOWN:
-		dprintk("slave %s is going down\n", slave_dev->name);
-		if (bond->setup_by_slave)
-			bond_release_and_destroy(bond_dev, slave_dev);
-		break;
 	case NETDEV_CHANGEMTU:
 		/*
 		 * TODO: Should slaves be allowed to
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index a8bbd56..b818060 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -22,8 +22,8 @@
 #include "bond_3ad.h"
 #include "bond_alb.h"
 
-#define DRV_VERSION	"3.2.0"
-#define DRV_RELDATE	"September 13, 2007"
+#define DRV_VERSION	"3.2.1"
+#define DRV_RELDATE	"October 15, 2007"
 #define DRV_NAME	"bonding"
 #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"
 
-- 
1.5.3.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox