* Re: [PATCH linux-2.6] bonding: two small fixes for IPoIB support
From: Moni Shoua @ 2007-10-16 7:56 UTC (permalink / raw)
To: Jay Vosburgh, Jeff Garzik; +Cc: rdreier, netdev, general, Or Gerlitz, Moni Levy
In-Reply-To: <9245.1192491867@death>
Jay Vosburgh wrote:
> Two small fixes to IPoIB support for bonding:
>
> 1- copy header_ops from slave to bonding for IPoIB slaves
> 2- move release and destroy logic to UNREGISTER from GOING_DOWN
> notifier to avoid double release
>
> Set bonding to version 3.2.1.
>
> Signed-off-by: Moni Shoua <monis at voltaire.com>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
>
> ---
> 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"
>
Jay,
Thanks for this work.
Jeff,
Thanks for applying.
I noticed that this patch isn't applied. It includes important fixes.
Can you please apply it also?
MoniS
^ permalink raw reply
* Re: [PATCH/RFC] net: Add __napi_sycnhronize() to sync with napi poll
From: David Miller @ 2007-10-16 7:44 UTC (permalink / raw)
To: benh; +Cc: herbert, shemminger, netdev, rdreier, linuxppc-dev
In-Reply-To: <1192520223.7205.5.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 16 Oct 2007 17:37:03 +1000
>
> On Tue, 2007-10-16 at 14:06 +0800, Herbert Xu wrote:
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> > >
> > > Note: I use msleep_interruptible(1); just like napi_disable(). However
> > > I'm not too happy that the "hot" loop that results of a pending signal
> > > here will spin without even a cpu_relax ... what do you guys think would
> > > be the best way to handle this ?
> >
> > Well since the loop does not check signals at all, it should
> > just use msleep.
> >
> > Granted the process will end up in the D state and contribute
> > to the load average. But if this loop executes long enough
> > for that to be noticed then we've got bigger problems to worry
> > about.
>
> If Dave & Stephen agree, I'll send a patch changing napi_disable() too
> then.
I agree with the msleep() change.
^ permalink raw reply
* Re: new NAPI interface broken
From: David Miller @ 2007-10-16 7:44 UTC (permalink / raw)
To: benh; +Cc: ossthema, shemminger, netdev, themann, raisch
In-Reply-To: <1192519787.7205.3.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 16 Oct 2007 17:29:47 +1000
> Do you have any pointer to how that is done on x86 or sparc64 ?
Sparc64 does it statically in the kernel.
For x86, see http://irqbalance.org/
^ permalink raw reply
* Re: new NAPI interface broken
From: Benjamin Herrenschmidt @ 2007-10-16 7:42 UTC (permalink / raw)
To: David Miller; +Cc: ossthema, shemminger, netdev, themann, raisch
In-Reply-To: <1192519787.7205.3.camel@pasglop>
> So the powerpc platform just honors the affinity mask, and depending on
> the PIC does things that range from nothing to spreading interrupts to
> CPUs in the affinity mask.
>
> All interrupts by defaults are spread to all CPUs (full balancing).
>
> At this stage, it's afaik userland business to enforce different
> policies by changing the affinities via /proc/irq/*.
>
> Do you have any pointer to how that is done on x86 or sparc64 ? On my
> x86 laptop using ubuntu gutsy, I definitely see the IRQ on which the
> network card is connected (e1000) happily spread between the 2 cores
> just like powerpc would do.
More specifically, IRQF_NOBALANCING doesn't seem to be set anywhere
except a few arch specific timer interrupts etc... nowhere I can see in
network drivers or the network stack (the stack wouldn't know what IRQ
anyway since not all drivers set netdev->irq).
We currently don't have a balance kthread like x86 has, though I wonder
if we should move this one out of x86 and make it generic (hell, it's
even hidden in the IO_APIC code :-) But at this stage, HW balancing by
the PIC is the norm and seems to be happening.
Ben.
^ permalink raw reply
* Re: [PATCH/RFC] net: Add __napi_sycnhronize() to sync with napi poll
From: Benjamin Herrenschmidt @ 2007-10-16 7:37 UTC (permalink / raw)
To: Herbert Xu; +Cc: shemminger, netdev, davem, rdreier, linuxppc-dev
In-Reply-To: <E1IhfZr-0002FE-00@gondolin.me.apana.org.au>
On Tue, 2007-10-16 at 14:06 +0800, Herbert Xu wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> > Note: I use msleep_interruptible(1); just like napi_disable(). However
> > I'm not too happy that the "hot" loop that results of a pending signal
> > here will spin without even a cpu_relax ... what do you guys think would
> > be the best way to handle this ?
>
> Well since the loop does not check signals at all, it should
> just use msleep.
>
> Granted the process will end up in the D state and contribute
> to the load average. But if this loop executes long enough
> for that to be noticed then we've got bigger problems to worry
> about.
If Dave & Stephen agree, I'll send a patch changing napi_disable() too
then.
Cheers,
Ben.
^ permalink raw reply
* Re: new NAPI interface broken
From: Benjamin Herrenschmidt @ 2007-10-16 7:29 UTC (permalink / raw)
To: David Miller; +Cc: ossthema, shemminger, netdev, themann, raisch
In-Reply-To: <20070912.055004.88490155.davem@davemloft.net>
Jumping on an old train ...
On Wed, 2007-09-12 at 05:50 -0700, David Miller wrote:
>
> > This would mean we have a problem on all SMP machines right now.
>
> This is not a correct statement.
>
> Only on your platform do network device interrupts get moved
> around, no other platform does this.
>
> Sparc64 doesn't, all interrupts stay in one location after
> the cpu is initially choosen.
>
> x86 and x86_64 specifically do not move around network
> device interrupts, even though other device types do
> get dynamic IRQ cpu distribution.
>
> That's why you are the only person seeing this problem.
>
> I agree that it should be fixed, but we should also fix the IRQ
> distribution scheme used on powerpc platforms which is totally
> broken in these cases.
So the powerpc platform just honors the affinity mask, and depending on
the PIC does things that range from nothing to spreading interrupts to
CPUs in the affinity mask.
All interrupts by defaults are spread to all CPUs (full balancing).
At this stage, it's afaik userland business to enforce different
policies by changing the affinities via /proc/irq/*.
Do you have any pointer to how that is done on x86 or sparc64 ? On my
x86 laptop using ubuntu gutsy, I definitely see the IRQ on which the
network card is connected (e1000) happily spread between the 2 cores
just like powerpc would do.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes
From: Jarek Poplawski @ 2007-10-16 6:21 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Andy Fleming, Andrew Morton, Jeff Garzik, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.64N.0710151737250.16262@blysk.ds.pg.gda.pl>
On Mon, Oct 15, 2007 at 06:03:20PM +0100, Maciej W. Rozycki wrote:
> On Mon, 15 Oct 2007, Jarek Poplawski wrote:
>
> > Could you explain why cancel_work_sync() is better here than
> > flush_scheduled_work() wrt. rtnl_lock()?
>
> Well, this is actually the bit that made cancel_work_sync() be written in
> the first place. The short story is the netlink lock is most probably
> held at this point (depending on the usage of phy_disconnect()) and there
> is also an event waiting in the queue that requires the lock, so if
> flush_scheduled_work() is called here a deadlock will happen.
>
> Let me find a reference for a longer story...:
>
> http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=Pine.LNX.4.64N.0610031509380.4642%40blysk.ds.pg.gda.pl
>
> and then discussed again:
>
> http://www.uwsg.indiana.edu/hypermail/linux/kernel/0612.0/0593.html
>
Yes, it's all right here. Sorry for bothering - I should've found this
by myself.
I've still some doubts about this possible enable_irq() after
free_irq(). If it's the only handler the status would be changed again
and at least some of this code in check_irq_resend() would be run, but
I can miss something again or/and this doesn't matter, as well.
Thanks,
Jarek P.
^ permalink raw reply
* Re: [PATCH/RFC] net: Add __napi_sycnhronize() to sync with napi poll
From: Herbert Xu @ 2007-10-16 6:06 UTC (permalink / raw)
To: benh; +Cc: shemminger, netdev, davem, rdreier, linuxppc-dev
In-Reply-To: <1192513792.19073.23.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> Note: I use msleep_interruptible(1); just like napi_disable(). However
> I'm not too happy that the "hot" loop that results of a pending signal
> here will spin without even a cpu_relax ... what do you guys think would
> be the best way to handle this ?
Well since the loop does not check signals at all, it should
just use msleep.
Granted the process will end up in the D state and contribute
to the load average. But if this loop executes long enough
for that to be noticed then we've got bigger problems to worry
about.
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
^ permalink raw reply
* [PATCH/RFC] net: Add __napi_sycnhronize() to sync with napi poll
From: Benjamin Herrenschmidt @ 2007-10-16 5:49 UTC (permalink / raw)
To: shemminger; +Cc: netdev, David S. Miller, Roland Dreier, linuxppc-dev list
net: Add __napi_sycnhronize() 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 :-)
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
(Use correct address for Stephen this time)
If the approach is accepted, I would like to have this merged now
so the EMAC patch to make it work again can follow :-)
Note: I use msleep_interruptible(1); just like napi_disable(). However
I'm not too happy that the "hot" loop that results of a pending signal
here will spin without even a cpu_relax ... what do you guys think would
be the best way to handle this ?
Index: linux-work/include/linux/netdevice.h
===================================================================
--- linux-work.orig/include/linux/netdevice.h 2007-10-16 15:27:27.000000000 +1000
+++ linux-work/include/linux/netdevice.h 2007-10-16 15:27:38.000000000 +1000
@@ -394,6 +394,21 @@ static inline void napi_disable(struct n
}
/**
+ * __napi_synchronize - synchronize with a concurrent poll
+ * @n: napi context
+ *
+ * Synchronizes with a concurrent poll. Not to be used in normal
+ * drivers, mostly useful if you end up with multiple interfaces
+ * on one NAPI instance.
+ */
+static inline void __napi_synchronize(struct napi_struct *n)
+{
+ smp_mb();
+ while (test_bit(NAPI_STATE_SCHED, &n->state))
+ msleep_interruptible(1);
+}
+
+/**
* napi_enable - enable NAPI scheduling
* @n: napi context
*
^ permalink raw reply
* Re: [PATCH] gianfar: Fix compile regression caused by 09f75cd7
From: Jeff Garzik @ 2007-10-16 5:48 UTC (permalink / raw)
To: Kumar Gala; +Cc: Li Yang, netdev
In-Reply-To: <Pine.LNX.4.64.0710160040120.21722@gate.crashing.org>
Kumar Gala wrote:
>
> On Fri, 12 Oct 2007, Li Yang wrote:
>
>> Signed-off-by: Li Yang <leoli@freescale.com>
>> ---
>> drivers/net/gianfar.c | 7 +++----
>> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> this patch got lost.
can someone resend, then?
^ permalink raw reply
* netif_napi_add vs. multiple netdev's
From: Benjamin Herrenschmidt @ 2007-10-16 5:47 UTC (permalink / raw)
To: shemminger; +Cc: netdev
Hi Stehphen !
The new netif_napi_add() function takes a netdev argument. In the EMAC
case, there is one NAPI instance working on behalf of multiple netdev's,
so that isn't very useful. For my EMAC patch (just posted to you & the
list), I'm not passing NULL, but I'm wondering what would be a good way
to handle netpoll here...
The way it's currently implemented, there's a list of NAPI's attached to
the netdev, so obviously, that won't work for my usage scenario.
I'm not sure what's the best data structure that would be suitable for
both N ndev's for 1 NAPI and 1 ndev for N NAPI's though... I could
allocate "stub" list heads and queue those up, but that's a bit gross...
Any better idea ?
Cheers,
Ben.
^ permalink raw reply
* [PATCH] net: Fix new EMAC driver for NAPI changes
From: Benjamin Herrenschmidt @ 2007-10-16 5:40 UTC (permalink / raw)
To: shemminger; +Cc: netdev, David S. Miller, Roland Dreier, linuxppc-dev list
net: Fix new EMAC driver for NAPI changes
This fixes the new EMAC driver for the NAPI updates. The previous patch
by Roland Dreier (already applied) to do that doesn't actually work. This
applies on top of it makes it work on my test Ebony machine.
This patch depends on "net: Add __napi_sycnhronize() to sync with napi poll"
posted previously.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
The old EMAC driver does things a bit differently (doesn't do useful
locking :-) and seems to work with Roland patch. So I'm not going to
touch it unless somebody reports me that it has problems
Index: linux-work/drivers/net/ibm_newemac/mal.c
===================================================================
--- linux-work.orig/drivers/net/ibm_newemac/mal.c 2007-10-16 14:51:11.000000000 +1000
+++ linux-work/drivers/net/ibm_newemac/mal.c 2007-10-16 14:59:23.000000000 +1000
@@ -45,6 +45,8 @@ int __devinit mal_register_commac(struct
return -EBUSY;
}
+ if (list_empty(&mal->list))
+ napi_enable(&mal->napi);
mal->tx_chan_mask |= commac->tx_chan_mask;
mal->rx_chan_mask |= commac->rx_chan_mask;
list_add(&commac->list, &mal->list);
@@ -67,6 +69,8 @@ void __devexit mal_unregister_commac(str
mal->tx_chan_mask &= ~commac->tx_chan_mask;
mal->rx_chan_mask &= ~commac->rx_chan_mask;
list_del_init(&commac->list);
+ if (list_empty(&mal->list))
+ napi_disable(&mal->napi);
spin_unlock_irqrestore(&mal->lock, flags);
}
@@ -182,7 +186,7 @@ static inline void mal_enable_eob_irq(st
set_mal_dcrn(mal, MAL_CFG, get_mal_dcrn(mal, MAL_CFG) | MAL_CFG_EOPIE);
}
-/* synchronized by __LINK_STATE_RX_SCHED bit in ndev->state */
+/* synchronized by NAPI state */
static inline void mal_disable_eob_irq(struct mal_instance *mal)
{
// XXX might want to cache MAL_CFG as the DCR read can be slooooow
@@ -317,8 +321,8 @@ void mal_poll_disable(struct mal_instanc
while (test_and_set_bit(MAL_COMMAC_POLL_DISABLED, &commac->flags))
msleep(1);
- /* Synchronize with the MAL NAPI poller. */
- napi_disable(&mal->napi);
+ /* Synchronize with the MAL NAPI poller */
+ __napi_synchronize(&mal->napi);
}
void mal_poll_enable(struct mal_instance *mal, struct mal_commac *commac)
@@ -326,7 +330,12 @@ void mal_poll_enable(struct mal_instance
smp_wmb();
clear_bit(MAL_COMMAC_POLL_DISABLED, &commac->flags);
- // XXX might want to kick a poll now...
+ /* Feels better to trigger a poll here to catch up with events that
+ * may have happened on this channel while disabled. It will most
+ * probably be delayed until the next interrupt but that's mostly a
+ * non-issue in the context where this is called.
+ */
+ napi_schedule(&mal->napi);
}
static int mal_poll(struct napi_struct *napi, int budget)
@@ -336,8 +345,7 @@ static int mal_poll(struct napi_struct *
int received = 0;
unsigned long flags;
- MAL_DBG2(mal, "poll(%d) %d ->" NL, *budget,
- rx_work_limit);
+ MAL_DBG2(mal, "poll(%d)" NL, budget);
again:
/* Process TX skbs */
list_for_each(l, &mal->poll_list) {
@@ -528,11 +536,12 @@ static int __devinit mal_probe(struct of
}
INIT_LIST_HEAD(&mal->poll_list);
- mal->napi.weight = CONFIG_IBM_NEW_EMAC_POLL_WEIGHT;
- mal->napi.poll = mal_poll;
INIT_LIST_HEAD(&mal->list);
spin_lock_init(&mal->lock);
+ netif_napi_add(NULL, &mal->napi, mal_poll,
+ CONFIG_IBM_NEW_EMAC_POLL_WEIGHT);
+
/* Load power-on reset defaults */
mal_reset(mal);
^ permalink raw reply
* Re: [PATCH] gianfar: Fix compile regression caused by 09f75cd7
From: Kumar Gala @ 2007-10-16 5:41 UTC (permalink / raw)
To: jgarzik; +Cc: Li Yang, netdev
In-Reply-To: <1192197233-24958-2-git-send-email-leoli@freescale.com>
On Fri, 12 Oct 2007, Li Yang wrote:
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> drivers/net/gianfar.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
this patch got lost.
- k
>
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index 2b758fa..6d1456a 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -1228,8 +1228,6 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
> * starting over will fix the problem. */
> static void gfar_timeout(struct net_device *dev)
> {
> - struct gfar_private *priv = netdev_priv(dev);
> -
> dev->stats.tx_errors++;
>
> if (dev->flags & IFF_UP) {
> @@ -1335,8 +1333,9 @@ struct sk_buff * gfar_new_skb(struct net_device *dev, struct rxbd8 *bdp)
> return skb;
> }
>
> -static inline void count_errors(unsigned short status, struct gfar_private *priv)
> +static inline void count_errors(unsigned short status, struct net_device *dev)
> {
> + struct gfar_private *priv = netdev_priv(dev);
> struct net_device_stats *stats = &dev->stats;
> struct gfar_extra_stats *estats = &priv->extra_stats;
>
> @@ -1530,7 +1529,7 @@ int gfar_clean_rx_ring(struct net_device *dev, int rx_work_limit)
>
> dev->stats.rx_bytes += pkt_len;
> } else {
> - count_errors(bdp->status, priv);
> + count_errors(bdp->status, dev);
>
> if (skb)
> dev_kfree_skb_any(skb);
> --
> 1.5.3.2.104.g41ef
>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* [PATCH/RFC] net: Add __napi_sycnhronize() to sync with napi poll
From: Benjamin Herrenschmidt @ 2007-10-16 5:40 UTC (permalink / raw)
To: shemminger; +Cc: netdev, David S. Miller, Roland Dreier, linuxppc-dev list
net: Add __napi_sycnhronize() 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 :-)
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
If the approach is accepted, I would like to have this merged now
so the EMAC patch to make it work again can follow :-)
Note: I use msleep_interruptible(1); just like napi_disable(). However
I'm not too happy that the "hot" loop that results of a pending signal
here will spin without even a cpu_relax ... what do you guys think would
be the best way to handle this ?
Index: linux-work/include/linux/netdevice.h
===================================================================
--- linux-work.orig/include/linux/netdevice.h 2007-10-16 15:27:27.000000000 +1000
+++ linux-work/include/linux/netdevice.h 2007-10-16 15:27:38.000000000 +1000
@@ -394,6 +394,21 @@ static inline void napi_disable(struct n
}
/**
+ * __napi_synchronize - synchronize with a concurrent poll
+ * @n: napi context
+ *
+ * Synchronizes with a concurrent poll. Not to be used in normal
+ * drivers, mostly useful if you end up with multiple interfaces
+ * on one NAPI instance.
+ */
+static inline void __napi_synchronize(struct napi_struct *n)
+{
+ smp_mb();
+ while (test_bit(NAPI_STATE_SCHED, &n->state))
+ msleep_interruptible(1);
+}
+
+/**
* napi_enable - enable NAPI scheduling
* @n: napi context
*
^ permalink raw reply
* Re: [PATCH] gianfar: Cleanup compile warning caused by 0795af57
From: Kumar Gala @ 2007-10-16 5:39 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Li Yang, netdev
In-Reply-To: <4713B46A.70004@pobox.com>
On Mon, 15 Oct 2007, Jeff Garzik wrote:
> Li Yang wrote:
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > drivers/net/gianfar.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
>
> applied all three gianfar patches
>
Jeff,
You seem to have lost one of the patches:
[PATCH] gianfar: Fix compile regression caused by 09f75cd7
http://marc.info/?l=linux-netdev&m=119219683128258&w=2
- k
^ permalink raw reply
* Re: question on sky2 driver panic
From: David Miller @ 2007-10-16 5:20 UTC (permalink / raw)
To: cfriesen; +Cc: shemminger, netdev
In-Reply-To: <47144920.4010402@nortel.com>
From: "Chris Friesen" <cfriesen@nortel.com>
Date: Mon, 15 Oct 2007 23:16:16 -0600
> Our version of the e1000 passes 200 bytes in the initial chunk, and the
> rest in fragments. tipc currently handles that without any difficulty.
There is no requirement for any bytes to be in the initial skb->data
chunk, in fact the Neptune NIU driver only pulls the ethernet header
into there for example.
net/tipc/eth_media.c:recv_msg() seems to dereference buf->data
directly without making any pskb_may_pull() checks, and that is
a bug.
^ permalink raw reply
* Re: question on sky2 driver panic
From: Chris Friesen @ 2007-10-16 5:16 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20071015150408.22f31d77@freepuppy.rosehill>
Stephen Hemminger wrote:
> Maybe TIPC can't handle fragmented receive buffers. The sky2 driver
> generates skb's with header and multiple pages if MTU is big enough.
> For 9K MTU that would be 1K of data + 2 4K pages. The protocols are
> supposed to be smart enough to handle this, but TIPC is rarely used.
We already had to modify tipc to handle fragmented receive buffers when
we had memory allocation errors on the e1000 and moved to fragmented
skbs in that driver.
Our version of the e1000 passes 200 bytes in the initial chunk, and the
rest in fragments. tipc currently handles that without any difficulty.
I was just checking more to see if there were any known issues in this
area that have been fixed in more recent driver versions.
Chris
^ permalink raw reply
* Re: Will RFC1146 (tcp alternative checksum options) be implemented in Linux tcp stack ?
From: Stephen Hemminger @ 2007-10-16 3:38 UTC (permalink / raw)
To: Yanping Du; +Cc: netdev
In-Reply-To: <887901.30138.qm@web55205.mail.re4.yahoo.com>
Yanping Du wrote:
> Hi,
>
> We found the standard 16-bit tcp checksum is not
> strong enough in some cases. Is there any roadmap on
> implementing RFC1146 (tcp alternative checksum
> options) in Linux tcp stack ? If yes, how soon will
> that be in ?
>
> Please kindly copy reply to my email address as I've
> not subscribed the netdev@ mailing list at present.
>
> http://www.faqs.org/rfcs/rfc1146.html
>
>
You would of course lose all the hardware offload of checksumming since
no hardware supports alternative checksums.
^ permalink raw reply
* Re: Will RFC1146 (tcp alternative checksum options) be implemented in Linux tcp stack ?
From: Ian McDonald @ 2007-10-16 1:57 UTC (permalink / raw)
To: Yanping Du; +Cc: netdev
In-Reply-To: <887901.30138.qm@web55205.mail.re4.yahoo.com>
On 10/16/07, Yanping Du <ypdu2001@yahoo.com> wrote:
> Hi,
>
> We found the standard 16-bit tcp checksum is not
> strong enough in some cases. Is there any roadmap on
> implementing RFC1146 (tcp alternative checksum
> options) in Linux tcp stack ? If yes, how soon will
> that be in ?
>
> Please kindly copy reply to my email address as I've
> not subscribed the netdev@ mailing list at present.
>
> http://www.faqs.org/rfcs/rfc1146.html
>
> Thanks!
> -Yanping
>
>
Yanping,
The way that features get added to Linux is that someone interested
writes it. You can't just say - is this on the roadmap, as there is no
roadmap really!
I have been interested in network features from an academic point of
view and so I wrote what I needed (along with others) and that was
added to the Linux kernel.
So have a go at implementing it if you consider it important and come
back here with some patches. Then others will help review it until the
patches are good.
I will let others comment on whether the checksums are a good idea or not.
Ian
--
Web1: http://wand.net.nz/~iam4/
Web2: http://www.jandi.co.nz
Blog: http://iansblog.jandi.co.nz
^ permalink raw reply
* Will RFC1146 (tcp alternative checksum options) be implemented in Linux tcp stack ?
From: Yanping Du @ 2007-10-16 1:15 UTC (permalink / raw)
To: netdev
Hi,
We found the standard 16-bit tcp checksum is not
strong enough in some cases. Is there any roadmap on
implementing RFC1146 (tcp alternative checksum
options) in Linux tcp stack ? If yes, how soon will
that be in ?
Please kindly copy reply to my email address as I've
not subscribed the netdev@ mailing list at present.
http://www.faqs.org/rfcs/rfc1146.html
Thanks!
-Yanping
____________________________________________________________________________________
Building a website is a piece of cake. Yahoo! Small Business gives you all the tools to get online.
http://smallbusiness.yahoo.com/webhosting
^ permalink raw reply
* [PATCH linux-2.6] bonding: two small fixes for IPoIB support
From: Jay Vosburgh @ 2007-10-15 23:44 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Moni Shoua, rdreier, netdev, general
In-Reply-To: <4713E20F.9080305@pobox.com>
Two small fixes to IPoIB support for bonding:
1- copy header_ops from slave to bonding for IPoIB slaves
2- move release and destroy logic to UNREGISTER from GOING_DOWN
notifier to avoid double release
Set bonding to version 3.2.1.
Signed-off-by: Moni Shoua <monis at voltaire.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
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
* Re: [PATCH] PowerPC: Add BCM5248 and Marvell 88E1111 PHY support to NEW EMAC.
From: Josh Boyer @ 2007-10-15 22:47 UTC (permalink / raw)
To: benh; +Cc: Jeff Garzik, netdev, linuxppc-dev
In-Reply-To: <1192482323.11795.38.camel@pasglop>
On Tue, 2007-10-16 at 07:05 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2007-10-15 at 15:04 -0400, Jeff Garzik wrote:
> > I would ideally like a single active patch generator (even if they
> > are
> > merely reviewed others work sometimes).
> >
> > Outside of that, I'm hoping you and the other people listed making
> > changes will self-organize without my help :)
>
> Josh, do you want to be the central point / maintainer for it or do you
> want me to do it ? There's a lot of code from me in there and I did this
> fork in the first place so I have a pretty good idea of what's going on
> in this driver and what still needs to be done :-)
As always, you're welcome to it. You probably don't want to own it long
term, but I'd appreciate the help for the time being.
josh
^ permalink raw reply
* Re: MSI interrupts and disable_irq
From: Jeff Garzik @ 2007-10-15 22:17 UTC (permalink / raw)
To: Manfred Spraul
Cc: Ayaz Abdulla, nedev, Linux Kernel Mailing List, David Miller,
Andrew Morton
In-Reply-To: <4710901F.8010206@colorfullife.com>
Manfred Spraul wrote:
> Jeff Garzik wrote:
>>
>> I think the scenario you outline is an illustration of the approach's
>> fragility: disable_irq() is a heavy hammer that originated with INTx,
>> and it relies on a chip-specific disable method (kernel/irq/manage.c)
>> that practically guarantees behavior will vary across MSI/INTx/etc.
>>
> I checked the code: IRQ_DISABLE is implemented in software, i.e.
> handle_level_irq() only calls handle_IRQ_event() [and then the nic irq
> handler] if IRQ_DISABLE is not set.
> OTHO: The last trace looks as if nv_do_nic_poll() is interrupted by an irq.
>
> Perhaps something corrupts dev->irq? The irq is requested with
> request_irq(np->pci_dev->irq, handler, IRQF_SHARED, dev->name, dev)
> and disabled with
> disable_irq_lockdep(dev->irq);
>
> Someone around with a MSI capable board? The forcedeth driver does
> dev->irq = pci_dev->irq
> in nv_probe(), especially before pci_enable_msi().
> Does pci_enable_msi() change pci_dev->irq? Then we would disable the
> wrong interrupt....
Remember, fundamentally MSI-X is a one-to-many relationship, when you
consider a single PCI device might have multiple vectors.
Jeff
^ permalink raw reply
* Re: [PATCH][BNX2X] round three
From: Stephen Hemminger @ 2007-10-15 22:06 UTC (permalink / raw)
To: David Miller; +Cc: eliezert, andi, netdev, jeff, mchan
In-Reply-To: <20071015.150022.30183587.davem@davemloft.net>
On Mon, 15 Oct 2007 15:00:22 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Mon, 15 Oct 2007 14:58:29 -0700
>
> > What about using loadable firmware rather than building it into the driver?
>
> Stephen, do me a huge favor, and don't start down that path.
>
> Please...
I know the history, loadable firmware is a maintenance nuisance/nightmare.
And it often leads to stupid license crap.
---
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: question on sky2 driver panic
From: Stephen Hemminger @ 2007-10-15 22:04 UTC (permalink / raw)
To: Chris Friesen; +Cc: netdev
In-Reply-To: <4713CAEE.6010106@nortel.com>
On Mon, 15 Oct 2007 14:17:50 -0600
"Chris Friesen" <cfriesen@nortel.com> wrote:
>
> Hi,
>
> We're using Yukon-XL (0xb3) rev 3 hardware with a vendor-supplied
> 2.6.14. BAsed on suggestions here, I backported the sky2 driver (v1.10
> from 2.6.20.6) to 2.6.14.
>
> Unfortunately, when I booted this I got the following:
>
>
> skb_over_panic: text:d0000000000d4e14 len:60 put:60
> head:c000000264920770 data:c000000264920720 tail:c000000264920720
> end:c0000002649207a0 dev:<NULL>
> kernel BUG in skb_over_panic at
> /usr/local/src/2.6.14/gd/linux/net/core/skbuff.c:94!
> Oops: Exception in kernel mode, sig: 5 [#1]
> SMP NR_CPUS=2
> Modules linked in: tipc bond1 bond0 ipmi_devintf ipmi_msghandler
> NIP: C00000000020D7E8 XER: 00000000 LR: C00000000020D7E4 CTR:
> C0000000001C210C
> REGS: c00000025c2aefe0 TRAP: 0700 Not tainted (2.6.14-pne)
> MSR: 9000000000029032 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11 CR: 28008022
> DAR: 0000000000000000 DSISR: c00000025c2af1c0
> TASK: c00000000fec2940[2107] 'insmod' THREAD: c00000025c2ac000 CPU: 0
> GPR00: C00000000020D7E4 C00000025C2AF300 C00000000041DA08 000000000000009C
> GPR04: 9000000000009032 FFFFFFFFFFFFFFFF 0000000000000030 C00000000037C428
> GPR08: 0000000000000000 C00000000037EEF0 C00000000043AD68 C00000000043AC88
> GPR12: 0000000000000010 C000000000374000 0000000000000000 00000000100D47E8
> GPR16: 00000000100D55A0 00000000FFFFFFFF 00000000FFFFFFFF 0000000000000000
> GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000001
> GPR24: 0000000000000000 00000000B6AA7FFF 0000000000000000 0000000000000000
> GPR28: 000000000000003C C00000000FEF5B10 C0000000003BB778 000000000000003C
> NIP [c00000000020d7e8] .skb_over_panic+0x50/0x68
> LR [c00000000020d7e4] .skb_over_panic+0x4c/0x68
> Call Trace:
> [c00000025c2af300] [c00000000020d7e4] .skb_over_panic+0x4c/0x68 (unreliable)
> [c00000025c2af390] [d0000000000d4e20] .named_prepare_buf+0x298/0x2a8 [tipc]
> [c00000025c2af450] [d0000000000d4e90] .named_publish+0x60/0xe4 [tipc]
> [c00000025c2af4e0] [d0000000000d80a8] .nametbl_publish+0x128/0x198 [tipc]
> [c00000025c2af590] [d0000000000de7dc] .tipc_publish+0xe8/0x188 [tipc]
> [c00000025c2af650] [d0000000000d7f4c] .nametbl_publish_rsv+0x30/0x64 [tipc]
> [c00000025c2af6e0] [d0000000000d2600] .cfg_init+0x120/0x150 [tipc]
> [c00000025c2af7a0] [d0000000000e31ac] .process_signal_queue+0xa4/0x100
> [tipc]
> [c00000025c2af8/0x1ec [tipc]
> [c00000025c2afcf0] [c0000000000685ec] .sys_init_module+0x28c/0x510
> [c00000025c2afd90] [c000000000009b9c] syscall_exit+0x0/0x18
>
>
>
> Now granted it looks like this was triggered by tipc, but is there
> anything that you can think of in the sky2 driver that may have been
> related? Maybe due to the fragmented buffer handling? The link would
> have been using an mtu of 9KB.
>
> Thanks,
>
> Chris
Maybe TIPC can't handle fragmented receive buffers. The sky2 driver
generates skb's with header and multiple pages if MTU is big enough.
For 9K MTU that would be 1K of data + 2 4K pages. The protocols are
supposed to be smart enough to handle this, but TIPC is rarely used.
--
Stephen Hemminger <shemminger@linux-foundation.org>
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
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