* 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
* [PATCH 1/6] Convert bonding timers to workqueues
From: Jay Vosburgh @ 2007-10-18 0:37 UTC (permalink / raw)
To: netdev, jgarzik; +Cc: andy, Jay Vosburgh
In-Reply-To: <1192667871139-git-send-email-fubar@us.ibm.com>
Convert bonding timers to workqueues. This converts the various
monitor functions to run in periodic work queues instead of timers. This
patch introduces the framework and convers the calls, but does not resolve
various locking issues, and does not stand alone.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
drivers/net/bonding/bond_3ad.c | 6 +-
drivers/net/bonding/bond_3ad.h | 2 +-
drivers/net/bonding/bond_alb.c | 6 +-
drivers/net/bonding/bond_alb.h | 2 +-
drivers/net/bonding/bond_main.c | 147 ++++++++++++++++++++-----------------
drivers/net/bonding/bond_sysfs.c | 63 +++++-----------
drivers/net/bonding/bonding.h | 13 ++--
7 files changed, 117 insertions(+), 122 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 7a045a3..2471d3b 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2074,8 +2074,10 @@ void bond_3ad_unbind_slave(struct slave *slave)
* times out, and it selects an aggregator for the ports that are yet not
* related to any aggregator, and selects the active aggregator for a bond.
*/
-void bond_3ad_state_machine_handler(struct bonding *bond)
+void bond_3ad_state_machine_handler(struct work_struct *work)
{
+ struct bonding *bond = container_of(work, struct bonding,
+ ad_work.work);
struct port *port;
struct aggregator *aggregator;
@@ -2126,7 +2128,7 @@ void bond_3ad_state_machine_handler(struct bonding *bond)
}
re_arm:
- mod_timer(&(BOND_AD_INFO(bond).ad_timer), jiffies + ad_delta_in_ticks);
+ queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
out:
read_unlock(&bond->lock);
}
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 862952f..dde02b0 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -276,7 +276,7 @@ struct ad_slave_info {
void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast);
int bond_3ad_bind_slave(struct slave *slave);
void bond_3ad_unbind_slave(struct slave *slave);
-void bond_3ad_state_machine_handler(struct bonding *bond);
+void bond_3ad_state_machine_handler(struct work_struct *);
void bond_3ad_adapter_speed_changed(struct slave *slave);
void bond_3ad_adapter_duplex_changed(struct slave *slave);
void bond_3ad_handle_link_change(struct slave *slave, char link);
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index aea2217..eb320c3 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1375,8 +1375,10 @@ out:
return 0;
}
-void bond_alb_monitor(struct bonding *bond)
+void bond_alb_monitor(struct work_struct *work)
{
+ struct bonding *bond = container_of(work, struct bonding,
+ alb_work.work);
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
struct slave *slave;
int i;
@@ -1479,7 +1481,7 @@ void bond_alb_monitor(struct bonding *bond)
}
re_arm:
- mod_timer(&(bond_info->alb_timer), jiffies + alb_delta_in_ticks);
+ queue_delayed_work(bond->wq, &bond->alb_work, alb_delta_in_ticks);
out:
read_unlock(&bond->lock);
}
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index fd87264..50968f8 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -125,7 +125,7 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave);
void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char link);
void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave);
int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev);
-void bond_alb_monitor(struct bonding *bond);
+void bond_alb_monitor(struct work_struct *);
int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr);
void bond_alb_clear_vlan(struct bonding *bond, unsigned short vlan_id);
#endif /* __BOND_ALB_H__ */
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6f85cc3..ed361d6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2089,9 +2089,10 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
/*-------------------------------- Monitoring -------------------------------*/
/* this function is called regularly to monitor each slave's link. */
-void bond_mii_monitor(struct net_device *bond_dev)
+void bond_mii_monitor(struct work_struct *work)
{
- struct bonding *bond = bond_dev->priv;
+ struct bonding *bond = container_of(work, struct bonding,
+ mii_work.work);
struct slave *slave, *oldcurrent;
int do_failover = 0;
int delta_in_ticks;
@@ -2156,7 +2157,7 @@ void bond_mii_monitor(struct net_device *bond_dev)
": %s: link status down for %s "
"interface %s, disabling it in "
"%d ms.\n",
- bond_dev->name,
+ bond->dev->name,
IS_UP(slave_dev)
? ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
? ((slave == oldcurrent)
@@ -2189,7 +2190,7 @@ void bond_mii_monitor(struct net_device *bond_dev)
": %s: link status definitely "
"down for interface %s, "
"disabling it\n",
- bond_dev->name,
+ bond->dev->name,
slave_dev->name);
/* notify ad that the link status has changed */
@@ -2215,7 +2216,7 @@ void bond_mii_monitor(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: link status up again after %d "
"ms for interface %s.\n",
- bond_dev->name,
+ bond->dev->name,
(bond->params.downdelay - slave->delay) * bond->params.miimon,
slave_dev->name);
}
@@ -2235,7 +2236,7 @@ void bond_mii_monitor(struct net_device *bond_dev)
": %s: link status up for "
"interface %s, enabling it "
"in %d ms.\n",
- bond_dev->name,
+ bond->dev->name,
slave_dev->name,
bond->params.updelay * bond->params.miimon);
}
@@ -2251,7 +2252,7 @@ void bond_mii_monitor(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: link status down again after %d "
"ms for interface %s.\n",
- bond_dev->name,
+ bond->dev->name,
(bond->params.updelay - slave->delay) * bond->params.miimon,
slave_dev->name);
} else {
@@ -2275,7 +2276,7 @@ void bond_mii_monitor(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: link status definitely "
"up for interface %s.\n",
- bond_dev->name,
+ bond->dev->name,
slave_dev->name);
/* notify ad that the link status has changed */
@@ -2301,7 +2302,7 @@ void bond_mii_monitor(struct net_device *bond_dev)
/* Should not happen */
printk(KERN_ERR DRV_NAME
": %s: Error: %s Illegal value (link=%d)\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name,
slave->link);
goto out;
@@ -2331,9 +2332,8 @@ void bond_mii_monitor(struct net_device *bond_dev)
bond_set_carrier(bond);
re_arm:
- if (bond->params.miimon) {
- mod_timer(&bond->mii_timer, jiffies + delta_in_ticks);
- }
+ if (bond->params.miimon)
+ queue_delayed_work(bond->wq, &bond->mii_work, delta_in_ticks);
out:
read_unlock(&bond->lock);
}
@@ -2636,9 +2636,10 @@ out:
* arp is transmitted to generate traffic. see activebackup_arp_monitor for
* arp monitoring in active backup mode.
*/
-void bond_loadbalance_arp_mon(struct net_device *bond_dev)
+void bond_loadbalance_arp_mon(struct work_struct *work)
{
- struct bonding *bond = bond_dev->priv;
+ struct bonding *bond = container_of(work, struct bonding,
+ arp_work.work);
struct slave *slave, *oldcurrent;
int do_failover = 0;
int delta_in_ticks;
@@ -2685,13 +2686,13 @@ void bond_loadbalance_arp_mon(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: link status definitely "
"up for interface %s, ",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
do_failover = 1;
} else {
printk(KERN_INFO DRV_NAME
": %s: interface %s is now up\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
}
}
@@ -2715,7 +2716,7 @@ void bond_loadbalance_arp_mon(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: interface %s is now down.\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
if (slave == oldcurrent) {
@@ -2745,9 +2746,8 @@ void bond_loadbalance_arp_mon(struct net_device *bond_dev)
}
re_arm:
- if (bond->params.arp_interval) {
- mod_timer(&bond->arp_timer, jiffies + delta_in_ticks);
- }
+ if (bond->params.arp_interval)
+ queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
out:
read_unlock(&bond->lock);
}
@@ -2767,9 +2767,10 @@ out:
* may have received.
* see loadbalance_arp_monitor for arp monitoring in load balancing mode
*/
-void bond_activebackup_arp_mon(struct net_device *bond_dev)
+void bond_activebackup_arp_mon(struct work_struct *work)
{
- struct bonding *bond = bond_dev->priv;
+ struct bonding *bond = container_of(work, struct bonding,
+ arp_work.work);
struct slave *slave;
int delta_in_ticks;
int i;
@@ -2821,14 +2822,14 @@ void bond_activebackup_arp_mon(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: %s is up and now the "
"active interface\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
netif_carrier_on(bond->dev);
} else {
printk(KERN_INFO DRV_NAME
": %s: backup interface %s is "
"now up\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
}
@@ -2864,7 +2865,7 @@ void bond_activebackup_arp_mon(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: backup interface %s is now down\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
} else {
read_unlock(&bond->curr_slave_lock);
@@ -2899,7 +2900,7 @@ void bond_activebackup_arp_mon(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: link status down for active interface "
"%s, disabling it\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
write_lock(&bond->curr_slave_lock);
@@ -2921,7 +2922,7 @@ void bond_activebackup_arp_mon(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: changing from interface %s to primary "
"interface %s\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name,
bond->primary_slave->dev->name);
@@ -2985,7 +2986,7 @@ void bond_activebackup_arp_mon(struct net_device *bond_dev)
printk(KERN_INFO DRV_NAME
": %s: backup interface %s is "
"now down.\n",
- bond_dev->name,
+ bond->dev->name,
slave->dev->name);
}
}
@@ -2994,7 +2995,7 @@ void bond_activebackup_arp_mon(struct net_device *bond_dev)
re_arm:
if (bond->params.arp_interval) {
- mod_timer(&bond->arp_timer, jiffies + delta_in_ticks);
+ queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
}
out:
read_unlock(&bond->lock);
@@ -3582,15 +3583,11 @@ static int bond_xmit_hash_policy_l2(struct sk_buff *skb,
static int bond_open(struct net_device *bond_dev)
{
struct bonding *bond = bond_dev->priv;
- struct timer_list *mii_timer = &bond->mii_timer;
- struct timer_list *arp_timer = &bond->arp_timer;
bond->kill_timers = 0;
if ((bond->params.mode == BOND_MODE_TLB) ||
(bond->params.mode == BOND_MODE_ALB)) {
- struct timer_list *alb_timer = &(BOND_ALB_INFO(bond).alb_timer);
-
/* bond_alb_initialize must be called before the timer
* is started.
*/
@@ -3599,44 +3596,31 @@ static int bond_open(struct net_device *bond_dev)
return -1;
}
- init_timer(alb_timer);
- alb_timer->expires = jiffies + 1;
- alb_timer->data = (unsigned long)bond;
- alb_timer->function = (void *)&bond_alb_monitor;
- add_timer(alb_timer);
+ INIT_DELAYED_WORK(&bond->alb_work, bond_alb_monitor);
+ queue_delayed_work(bond->wq, &bond->alb_work, 0);
}
if (bond->params.miimon) { /* link check interval, in milliseconds. */
- init_timer(mii_timer);
- mii_timer->expires = jiffies + 1;
- mii_timer->data = (unsigned long)bond_dev;
- mii_timer->function = (void *)&bond_mii_monitor;
- add_timer(mii_timer);
+ INIT_DELAYED_WORK(&bond->mii_work, bond_mii_monitor);
+ queue_delayed_work(bond->wq, &bond->mii_work, 0);
}
if (bond->params.arp_interval) { /* arp interval, in milliseconds. */
- init_timer(arp_timer);
- arp_timer->expires = jiffies + 1;
- arp_timer->data = (unsigned long)bond_dev;
- if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
- arp_timer->function = (void *)&bond_activebackup_arp_mon;
- } else {
- arp_timer->function = (void *)&bond_loadbalance_arp_mon;
- }
+ if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+ INIT_DELAYED_WORK(&bond->arp_work,
+ bond_activebackup_arp_mon);
+ else
+ INIT_DELAYED_WORK(&bond->arp_work,
+ bond_loadbalance_arp_mon);
+
+ queue_delayed_work(bond->wq, &bond->arp_work, 0);
if (bond->params.arp_validate)
bond_register_arp(bond);
-
- add_timer(arp_timer);
}
if (bond->params.mode == BOND_MODE_8023AD) {
- struct timer_list *ad_timer = &(BOND_AD_INFO(bond).ad_timer);
- init_timer(ad_timer);
- ad_timer->expires = jiffies + 1;
- ad_timer->data = (unsigned long)bond;
- ad_timer->function = (void *)&bond_3ad_state_machine_handler;
- add_timer(ad_timer);
-
+ INIT_DELAYED_WORK(&bond->ad_work, bond_alb_monitor);
+ queue_delayed_work(bond->wq, &bond->ad_work, 0);
/* register to receive LACPDUs */
bond_register_lacpdu(bond);
}
@@ -3664,25 +3648,21 @@ static int bond_close(struct net_device *bond_dev)
write_unlock_bh(&bond->lock);
- /* del_timer_sync must run without holding the bond->lock
- * because a running timer might be trying to hold it too
- */
-
if (bond->params.miimon) { /* link check interval, in milliseconds. */
- del_timer_sync(&bond->mii_timer);
+ cancel_delayed_work(&bond->mii_work);
}
if (bond->params.arp_interval) { /* arp interval, in milliseconds. */
- del_timer_sync(&bond->arp_timer);
+ cancel_delayed_work(&bond->arp_work);
}
switch (bond->params.mode) {
case BOND_MODE_8023AD:
- del_timer_sync(&(BOND_AD_INFO(bond).ad_timer));
+ cancel_delayed_work(&bond->ad_work);
break;
case BOND_MODE_TLB:
case BOND_MODE_ALB:
- del_timer_sync(&(BOND_ALB_INFO(bond).alb_timer));
+ cancel_delayed_work(&bond->alb_work);
break;
default:
break;
@@ -4340,6 +4320,10 @@ static int bond_init(struct net_device *bond_dev, struct bond_params *params)
bond->params = *params; /* copy params struct */
+ bond->wq = create_singlethread_workqueue(bond_dev->name);
+ if (!bond->wq)
+ return -ENOMEM;
+
/* Initialize pointers */
bond->first_slave = NULL;
bond->curr_active_slave = NULL;
@@ -4826,10 +4810,32 @@ out_rtnl:
return res;
}
+static void bond_work_cancel_all(struct bonding *bond)
+{
+ write_lock_bh(&bond->lock);
+ bond->kill_timers = 1;
+ write_unlock_bh(&bond->lock);
+
+ if (bond->params.miimon && delayed_work_pending(&bond->mii_work))
+ cancel_delayed_work(&bond->mii_work);
+
+ if (bond->params.arp_interval && delayed_work_pending(&bond->arp_work))
+ cancel_delayed_work(&bond->arp_work);
+
+ if (bond->params.mode == BOND_MODE_ALB &&
+ delayed_work_pending(&bond->alb_work))
+ cancel_delayed_work(&bond->alb_work);
+
+ if (bond->params.mode == BOND_MODE_8023AD &&
+ delayed_work_pending(&bond->ad_work))
+ cancel_delayed_work(&bond->ad_work);
+}
+
static int __init bonding_init(void)
{
int i;
int res;
+ struct bonding *bond, *nxt;
printk(KERN_INFO "%s", version);
@@ -4856,6 +4862,11 @@ static int __init bonding_init(void)
goto out;
err:
+ list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) {
+ bond_work_cancel_all(bond);
+ destroy_workqueue(bond->wq);
+ }
+
rtnl_lock();
bond_free_all();
bond_destroy_sysfs();
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 80c0c8c..ffa62a7 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -662,12 +662,9 @@ static ssize_t bonding_store_arp_interval(struct device *d,
"%s Disabling MII monitoring.\n",
bond->dev->name, bond->dev->name);
bond->params.miimon = 0;
- /* Kill MII timer, else it brings bond's link down */
- if (bond->arp_timer.function) {
- printk(KERN_INFO DRV_NAME
- ": %s: Kill MII timer, else it brings bond's link down...\n",
- bond->dev->name);
- del_timer_sync(&bond->mii_timer);
+ if (delayed_work_pending(&bond->mii_work)) {
+ cancel_delayed_work(&bond->mii_work);
+ flush_workqueue(bond->wq);
}
}
if (!bond->params.arp_targets[0]) {
@@ -682,25 +679,15 @@ static ssize_t bonding_store_arp_interval(struct device *d,
* timer will get fired off when the open function
* is called.
*/
- if (bond->arp_timer.function) {
- /* The timer's already set up, so fire it off */
- mod_timer(&bond->arp_timer, jiffies + 1);
- } else {
- /* Set up the timer. */
- init_timer(&bond->arp_timer);
- bond->arp_timer.expires = jiffies + 1;
- bond->arp_timer.data =
- (unsigned long) bond->dev;
- if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
- bond->arp_timer.function =
- (void *)
- &bond_activebackup_arp_mon;
- } else {
- bond->arp_timer.function =
- (void *)
- &bond_loadbalance_arp_mon;
- }
- add_timer(&bond->arp_timer);
+ if (!delayed_work_pending(&bond->arp_work)) {
+ if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+ INIT_DELAYED_WORK(&bond->arp_work,
+ bond_activebackup_arp_mon);
+ else
+ INIT_DELAYED_WORK(&bond->arp_work,
+ bond_loadbalance_arp_mon);
+
+ queue_delayed_work(bond->wq, &bond->arp_work, 0);
}
}
@@ -1056,12 +1043,9 @@ static ssize_t bonding_store_miimon(struct device *d,
bond->params.arp_validate =
BOND_ARP_VALIDATE_NONE;
}
- /* Kill ARP timer, else it brings bond's link down */
- if (bond->mii_timer.function) {
- printk(KERN_INFO DRV_NAME
- ": %s: Kill ARP timer, else it brings bond's link down...\n",
- bond->dev->name);
- del_timer_sync(&bond->arp_timer);
+ if (delayed_work_pending(&bond->arp_work)) {
+ cancel_delayed_work(&bond->arp_work);
+ flush_workqueue(bond->wq);
}
}
@@ -1071,18 +1055,11 @@ static ssize_t bonding_store_miimon(struct device *d,
* timer will get fired off when the open function
* is called.
*/
- if (bond->mii_timer.function) {
- /* The timer's already set up, so fire it off */
- mod_timer(&bond->mii_timer, jiffies + 1);
- } else {
- /* Set up the timer. */
- init_timer(&bond->mii_timer);
- bond->mii_timer.expires = jiffies + 1;
- bond->mii_timer.data =
- (unsigned long) bond->dev;
- bond->mii_timer.function =
- (void *) &bond_mii_monitor;
- add_timer(&bond->mii_timer);
+ if (!delayed_work_pending(&bond->mii_work)) {
+ INIT_DELAYED_WORK(&bond->mii_work,
+ bond_mii_monitor);
+ queue_delayed_work(bond->wq,
+ &bond->mii_work, 0);
}
}
}
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index b818060..a8f2384 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -184,8 +184,6 @@ struct bonding {
s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
rwlock_t lock;
rwlock_t curr_slave_lock;
- struct timer_list mii_timer;
- struct timer_list arp_timer;
s8 kill_timers;
s8 send_grat_arp;
s8 setup_by_slave;
@@ -205,6 +203,11 @@ struct bonding {
struct list_head vlan_list;
struct vlan_group *vlgrp;
struct packet_type arp_mon_pt;
+ struct workqueue_struct *wq;
+ struct delayed_work mii_work;
+ struct delayed_work arp_work;
+ struct delayed_work alb_work;
+ struct delayed_work ad_work;
};
/**
@@ -307,9 +310,9 @@ int bond_create_slave_symlinks(struct net_device *master, struct net_device *sla
void bond_destroy_slave_symlinks(struct net_device *master, struct net_device *slave);
int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
-void bond_mii_monitor(struct net_device *bond_dev);
-void bond_loadbalance_arp_mon(struct net_device *bond_dev);
-void bond_activebackup_arp_mon(struct net_device *bond_dev);
+void bond_mii_monitor(struct work_struct *);
+void bond_loadbalance_arp_mon(struct work_struct *);
+void bond_activebackup_arp_mon(struct work_struct *);
void bond_set_mode_ops(struct bonding *bond, int mode);
int bond_parse_parm(char *mode_arg, struct bond_parm_tbl *tbl);
void bond_select_active_slave(struct bonding *bond);
--
1.5.3.4.206.g58ba4-dirty
^ permalink raw reply related
* [PATCH 3/6] Convert miimon to new locking
From: Jay Vosburgh @ 2007-10-18 0:37 UTC (permalink / raw)
To: netdev, jgarzik; +Cc: andy, Jay Vosburgh
In-Reply-To: <11926678762641-git-send-email-fubar@us.ibm.com>
Convert mii (link state) monitor to acquire correct locks for
failover events. In particular, failovers generally require RTNL at a low
level (when manipulating device MAC addresses, for example) and no other
locks. The high level monitor is responsible for acquiring a known set
of locks, RTNL, the bond->lock for read and the slave_lock for write, and
the low level failover processing can then release appropriate locks as
needed. This patch provides the high level portion.
As it is undesirable to acquire RTNL for every monitor pass (which
may occur as often as every 10 ms), the miimon has been converted to
do conditional locking. A first pass inspects all slaves to determine
if any action is required, and if so, a second pass (after acquring RTNL)
is done to perform any actions (doing a complete rescan, as the situation
may have changed when all locks were released).
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
drivers/net/bonding/bond_main.c | 70 ++++++++++++++++++++++++++++----------
1 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 862ed8e..a357727 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2088,27 +2088,25 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
/*-------------------------------- Monitoring -------------------------------*/
-/* this function is called regularly to monitor each slave's link. */
-void bond_mii_monitor(struct work_struct *work)
+/*
+ * if !have_locks, return nonzero if a failover is necessary. if
+ * have_locks, do whatever failover activities are needed.
+ *
+ * This is to separate the inspection and failover steps for locking
+ * purposes; failover requires rtnl, but acquiring it for every
+ * inspection is undesirable, so a wrapper first does inspection, and
+ * the acquires the necessary locks and calls again to perform
+ * failover if needed. Since all locks are dropped, a complete
+ * restart is needed between calls.
+ */
+static int __bond_mii_monitor(struct bonding *bond, int have_locks)
{
- struct bonding *bond = container_of(work, struct bonding,
- mii_work.work);
struct slave *slave, *oldcurrent;
int do_failover = 0;
- int delta_in_ticks;
int i;
- read_lock(&bond->lock);
-
- delta_in_ticks = (bond->params.miimon * HZ) / 1000;
-
- if (bond->kill_timers) {
+ if (bond->slave_cnt == 0)
goto out;
- }
-
- if (bond->slave_cnt == 0) {
- goto re_arm;
- }
/* we will try to read the link status of each of our slaves, and
* set their IFF_RUNNING flag appropriately. For each slave not
@@ -2175,6 +2173,9 @@ void bond_mii_monitor(struct work_struct *work)
if (link_state != BMSR_LSTATUS) {
/* link stays down */
if (slave->delay <= 0) {
+ if (!have_locks)
+ return 1;
+
/* link down for too long time */
slave->link = BOND_LINK_DOWN;
@@ -2258,6 +2259,9 @@ void bond_mii_monitor(struct work_struct *work)
} else {
/* link stays up */
if (slave->delay == 0) {
+ if (!have_locks)
+ return 1;
+
/* now the link has been up for long time enough */
slave->link = BOND_LINK_UP;
slave->jiffies = jiffies;
@@ -2331,13 +2335,41 @@ void bond_mii_monitor(struct work_struct *work)
} else
bond_set_carrier(bond);
-re_arm:
- if (bond->params.miimon)
- queue_delayed_work(bond->wq, &bond->mii_work, delta_in_ticks);
out:
- read_unlock(&bond->lock);
+ return 0;
}
+/*
+ * bond_mii_monitor
+ *
+ * Really a wrapper that splits the mii monitor into two phases: an
+ * inspection, then (if inspection indicates something needs to be
+ * done) an acquisition of appropriate locks followed by another pass
+ * to implement whatever link state changes are indicated.
+ */
+void bond_mii_monitor(struct work_struct *work)
+{
+ struct bonding *bond = container_of(work, struct bonding,
+ mii_work.work);
+ unsigned long delay;
+
+ read_lock(&bond->lock);
+ if (bond->kill_timers) {
+ read_unlock(&bond->lock);
+ return;
+ }
+ if (__bond_mii_monitor(bond, 0)) {
+ read_unlock(&bond->lock);
+ rtnl_lock();
+ read_lock(&bond->lock);
+ __bond_mii_monitor(bond, 1);
+ rtnl_unlock();
+ }
+
+ delay = ((bond->params.miimon * HZ) / 1000) ? : 1;
+ read_unlock(&bond->lock);
+ queue_delayed_work(bond->wq, &bond->mii_work, delay);
+}
static __be32 bond_glean_dev_ip(struct net_device *dev)
{
--
1.5.3.4.206.g58ba4-dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox