Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/4] Diet struct sk_buff a bit
From: Pavel Emelyanov @ 2007-10-19  8:59 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel

The __u16 queue_mapping field only makes sense in the
CONFIG_NETDEVICES_MULTIQUEUE=y case only.

Despite this field may be set explicitly to some non-zero 
value (in net/core/pktgen.c), the exact value affects 
nothing in case the config option in question is N (mainly 
it is used in netif_subqueue_stopped(), which will always
return 0 in this case).

So cleanup the code a bit and move this field under the
config option.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

^ permalink raw reply

* Re: [PATCH 1/2] Interface group: core (netlink) part
From: Laszlo Attila Toth @ 2007-10-19  8:57 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netdev, netfilter-devel
In-Reply-To: <471733D5.4000500@trash.net>

Patrick McHardy írta:
> Laszlo Attila Toth wrote:
>> @@ -846,6 +850,12 @@ static int do_setlink(struct net_device *dev, 
>> struct ifinfomsg *ifm,
>>          write_unlock_bh(&dev_base_lock);
>>      }
>>  
>> +    if (tb[IFLA_IFGROUP]) {
>> +        write_lock_bh(&dev_base_lock);
>> +        dev->ifgroup = nla_get_u32(tb[IFLA_IFGROUP]);
>> +        write_unlock_bh(&dev_base_lock);
>> +    }
> 
> 
> So no notifications at all? Mhh .. I guess its OK for now, this stuff
> needs to be fixed to notify once for all changes anyway.
> 

I'll resend the following patch with the new ifgroup patches. The 
current changes: changes are now atomic and a notification will be send 
later at the end of the function. If an address was changed, a 
notification will be send and after that another which is always sent 
when anyi modification happened.

This patch may not be complete.


diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 4756d58..87ab3ff 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -819,6 +819,7 @@ static int do_setlink(struct net_device *dev, struct 
ifinfomsg *ifm,
         if (tb[IFLA_BROADCAST]) {
                 nla_memcpy(dev->broadcast, tb[IFLA_BROADCAST], 
dev->addr_len);
                 send_addr_notify = 1;
+               modified = 1;
         }

         if (ifm->ifi_flags || ifm->ifi_change) {
@@ -829,21 +830,35 @@ static int do_setlink(struct net_device *dev, 
struct ifinfomsg *ifm,
                         flags = (flags & ifm->ifi_change) |
                                 (dev->flags & ~ifm->ifi_change);
                 dev_change_flags(dev, flags);
+               modified = 1;
         }

-       if (tb[IFLA_TXQLEN])
+       if (tb[IFLA_TXQLEN]) {
+               write_lock_bh(&dev_base_lock);
                 dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
+               write_unlock_bh(&dev_base_lock);
+               modified = 1;
+       }

-       if (tb[IFLA_WEIGHT])
+       if (tb[IFLA_WEIGHT]) {
+               write_lock_bh(&dev_base_lock);
                 dev->weight = nla_get_u32(tb[IFLA_WEIGHT]);
+               write_unlock_bh(&dev_base_lock);
+               modified = 1;
+       }

-       if (tb[IFLA_OPERSTATE])
+       if (tb[IFLA_OPERSTATE]) {
+               write_lock_bh(&dev_base_lock);
                 set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
+               write_unlock_bh(&dev_base_lock);
+               modified = 1;
+       }

         if (tb[IFLA_LINKMODE]) {
                 write_lock_bh(&dev_base_lock);
                 dev->link_mode = nla_get_u8(tb[IFLA_LINKMODE]);
                 write_unlock_bh(&dev_base_lock);
+               modified = 1;
         }

         err = 0;
@@ -857,6 +872,8 @@ errout:

         if (send_addr_notify)
                 call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+       if (modified)
+               rtmsg_ifinfo(RTM_NEWLINK, dev, 0)
         return err;
  }

-
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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 related

* Re: [PATCH 1/2] net: Let inet_diag and friends autoload
From: David Miller @ 2007-10-19  8:34 UTC (permalink / raw)
  To: herbert; +Cc: jdelvare, netdev, kuznet
In-Reply-To: <E1Iin6h-000367-00@gondolin.me.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 19 Oct 2007 16:21:23 +0800

> David Miller <davem@davemloft.net> wrote:
> >
> > I'm concerned it might be dangerous to load the module in
> > this context, the RTNL semaphore is held and the module we
> > are requesting could easily try to take it and deadlock.
> 
> We shouldn't be holding the RTNL here.  AFAICS we only hold
> inet_diag_mutex which is used in just one spot.

You're right.

I'll reconsider these patches.

^ permalink raw reply

* Re: [PATCH 2/8] [MV643XX_ETH] Move ethernet register definitions into private header
From: Christoph Hellwig @ 2007-10-19  8:30 UTC (permalink / raw)
  To: Lennert Buytenhek
  Cc: Dale Farnsworth, Nicolas Pitre, Tzachi Perelstein, Manas Saksena,
	netdev
In-Reply-To: <20071019021010.GD16037@xi.wantstofly.org>

On Fri, Oct 19, 2007 at 04:10:10AM +0200, Lennert Buytenhek wrote:
> Move the mv643xx's ethernet-related register definitions from
> include/linux/mv643xx.h into drivers/net/mv643xx_eth.h, since
> they aren't of any use outside the ethernet driver.
> 
> Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
> Acked-by: Tzachi Perelstein <tzachi@marvell.com>
> 
> Index: linux-2.6/drivers/net/mv643xx_eth.h
> ===================================================================
> --- linux-2.6.orig/drivers/net/mv643xx_eth.h
> +++ linux-2.6/drivers/net/mv643xx_eth.h
> @@ -7,7 +7,7 @@
>  #include <linux/workqueue.h>
>  #include <linux/mii.h>
>  
> -#include <linux/mv643xx.h>
> +#include <linux/mv643xx_eth.h>

Isn't it a little too confusing to have two headers with the same name,
one in drivers/net and one in include/linux?


^ permalink raw reply

* Re: [PATCH 1/2] net: Let inet_diag and friends autoload
From: Herbert Xu @ 2007-10-19  8:21 UTC (permalink / raw)
  To: David Miller; +Cc: jdelvare, netdev, kuznet
In-Reply-To: <20071017.195125.15264659.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote:
>
> I'm concerned it might be dangerous to load the module in
> this context, the RTNL semaphore is held and the module we
> are requesting could easily try to take it and deadlock.

We shouldn't be holding the RTNL here.  AFAICS we only hold
inet_diag_mutex which is used in just one spot.

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

* Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes
From: Jarek Poplawski @ 2007-10-19  8:17 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Andy Fleming, Andrew Morton, Jeff Garzik, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.64N.0710171221510.28993@blysk.ds.pg.gda.pl>

On Thu, Oct 18, 2007 at 12:30:35PM +0100, Maciej W. Rozycki wrote:
> On Wed, 17 Oct 2007, Jarek Poplawski wrote:
...
> > 2) phy_change() doesn't reenable irq line after it sees returns
> > with errors; IMHO it should at least write some warning, but maybe
> > try some safety plan, so enable_irq() and try to disable interrupts
> > and free_irq() on the next call (if it happens). (But, I can be very
> > wrong with this - maybe it's OK and official way.)
> 
>  No way to do this safely -- at this point the device probably still has 
> its interrupt output asserted and the register to clear it is 
> inaccessible, so enabling the line will enter an infinite loop.  At this 
> point the system is no longer stable, so it is better to keep at least 
> some functionality, so that it may be attempted to be shut down cleanly, 
> rather than make it completely irresponsive.  The alternative is panic().

But then... your patch seems to make it possible, because it enables
irq to the initial state of the counter. Of course, this could happen
on closing only.

Jarek P.

^ permalink raw reply

* Re: [PATCH 04/13] r8169: MSI support
From: Rolf Eike Beer @ 2007-10-19  7:59 UTC (permalink / raw)
  To: Francois Romieu; +Cc: jgarzik, netdev, Edward Hsu, Will Trives
In-Reply-To: <20071018210106.GE31724@electric-eye.fr.zoreil.com>

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

Francois Romieu wrote:
> It is currently limited to the tested 0x8136 and 0x8168. 8169sb/8110sb
> ought to handle it as well where they support MSI.
>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Edward Hsu <edward_hsu@realtek.com.tw>
> Tester-Cc: Rolf Eike Beer <eike-kernel@sf-tec.de>

Tested-by: Rolf Eike Beer <eike-kernel@sf-tec.de>

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

^ permalink raw reply

* Re: [PATCH] flush_work_sync vs. flush_scheduled_work Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes
From: Johannes Berg @ 2007-10-19  8:00 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Jarek Poplawski, Maciej W. Rozycki, Andy Fleming, Andrew Morton,
	Jeff Garzik, netdev, linux-kernel
In-Reply-To: <20071018154819.GA425@tv-sign.ru>

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

On Thu, 2007-10-18 at 19:48 +0400, Oleg Nesterov wrote:

> > +void flush_work_sync(struct work_struct *work)

> If we really the new helper, perhaps we can make it a bit better?
> 
> 1. Modify insert_work() to take the "struct list_head *at" parameter instead
>    of "int tail". I think this patch will also cleanup the code a bit, and
>    shrink a couple of bytes from .text
> 
> 2. flush_work_sync() inserts a barrier right after this work and blocks.
>    We still need some retry logic to handle the queueing is in progress
>    of course, but we won't spin waiting for the other works.

3. Add lockdep annotation like the other API. :) Andrew just sent my
patch (used to be two patches by somebody's request but that's fine)
titled "workqueue: debug flushing deadlocks with lockdep" to Linus.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [PATCH] flush_work_sync vs. flush_scheduled_work Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes
From: Jarek Poplawski @ 2007-10-19  8:01 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Maciej W. Rozycki, Andy Fleming, Andrew Morton, Jeff Garzik,
	netdev, linux-kernel
In-Reply-To: <20071019075014.GA1765@ff.dom.local>

On Fri, Oct 19, 2007 at 09:50:14AM +0200, Jarek Poplawski wrote:
...
> sched_work_sync() with rtnl_lock(). It's only less probable to lockup
> with this than with flush_schedule_work().

...But, not much less...

Jarek P.

^ permalink raw reply

* Re: [PATCH] sparc/xen/cxgb3: use irq_handler_t where appropriate
From: Jeremy Fitzhardinge @ 2007-10-19  7:54 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: David Miller, jeremy, netdev, LKML, Andrew Morton
In-Reply-To: <20071019073303.GA5728@havoc.gtf.org>

Jeff Garzik wrote:
> commit 21b1f26bf54a2ba1e4072db6dd01da128b1f66ef
> Author: Jeff Garzik <jeff@garzik.org>
> Date:   Fri Oct 19 03:12:20 2007 -0400
>
>     [SPARC, XEN, NET/CXGB3] use irq_handler_t where appropriate
>     
>     Rather than hand-rolling our own prototype, make the code more
>     future-proof by using the standard irq_handler_t typedef.
>     
>     Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
>   

Acked-by: Jeremy Fitzhardinge <jeremy@xensource.com>

>  arch/sparc/kernel/irq.c     |    4 ++--
>  arch/x86/xen/events.c       |    4 ++--
>  drivers/net/cxgb3/adapter.h |    4 +---
>  drivers/net/cxgb3/sge.c     |    2 +-
>  4 files changed, 6 insertions(+), 8 deletions(-)
>
> 21b1f26bf54a2ba1e4072db6dd01da128b1f66ef
> diff --git a/arch/sparc/kernel/irq.c b/arch/sparc/kernel/irq.c
> index 722d67d..e1e24f3 100644
> --- a/arch/sparc/kernel/irq.c
> +++ b/arch/sparc/kernel/irq.c
> @@ -479,7 +479,7 @@ EXPORT_SYMBOL(pdma_areasize);
>  
>  extern void floppy_hardint(void);
>  
> -static irqreturn_t (*floppy_irq_handler)(int irq, void *dev_id);
> +static irq_handler_t floppy_irq_handler;
>  
>  void sparc_floppy_irq(int irq, void *dev_id, struct pt_regs *regs)
>  {
> @@ -500,7 +500,7 @@ void sparc_floppy_irq(int irq, void *dev_id, struct pt_regs *regs)
>  }
>  
>  int sparc_floppy_request_irq(int irq, unsigned long flags,
> -			     irqreturn_t (*irq_handler)(int irq, void *))
> +			     irq_handler_t irq_handler)
>  {
>  	floppy_irq_handler = irq_handler;
>  	return request_fast_irq(irq, floppy_hardint, flags, "floppy");
> diff --git a/arch/x86/xen/events.c b/arch/x86/xen/events.c
> index da1b173..6d1da58 100644
> --- a/arch/x86/xen/events.c
> +++ b/arch/x86/xen/events.c
> @@ -383,7 +383,7 @@ static void unbind_from_irq(unsigned int irq)
>  }
>  
>  int bind_evtchn_to_irqhandler(unsigned int evtchn,
> -			      irqreturn_t (*handler)(int, void *),
> +			      irq_handler_t handler,
>  			      unsigned long irqflags,
>  			      const char *devname, void *dev_id)
>  {
> @@ -402,7 +402,7 @@ int bind_evtchn_to_irqhandler(unsigned int evtchn,
>  EXPORT_SYMBOL_GPL(bind_evtchn_to_irqhandler);
>  
>  int bind_virq_to_irqhandler(unsigned int virq, unsigned int cpu,
> -			    irqreturn_t (*handler)(int, void *),
> +			    irq_handler_t handler,
>  			    unsigned long irqflags, const char *devname, void *dev_id)
>  {
>  	unsigned int irq;
> diff --git a/drivers/net/cxgb3/adapter.h b/drivers/net/cxgb3/adapter.h
> index 0442617..3814cc6 100644
> --- a/drivers/net/cxgb3/adapter.h
> +++ b/drivers/net/cxgb3/adapter.h
> @@ -46,8 +46,6 @@
>  #include <asm/bitops.h>
>  #include <asm/io.h>
>  
> -typedef irqreturn_t(*intr_handler_t) (int, void *);
> -
>  struct vlan_group;
>  struct adapter;
>  struct sge_qset;
> @@ -270,7 +268,7 @@ void t3_sge_start(struct adapter *adap);
>  void t3_sge_stop(struct adapter *adap);
>  void t3_free_sge_resources(struct adapter *adap);
>  void t3_sge_err_intr_handler(struct adapter *adapter);
> -intr_handler_t t3_intr_handler(struct adapter *adap, int polling);
> +irq_handler_t t3_intr_handler(struct adapter *adap, int polling);
>  int t3_eth_xmit(struct sk_buff *skb, struct net_device *dev);
>  int t3_mgmt_tx(struct adapter *adap, struct sk_buff *skb);
>  void t3_update_qset_coalesce(struct sge_qset *qs, const struct qset_params *p);
> diff --git a/drivers/net/cxgb3/sge.c b/drivers/net/cxgb3/sge.c
> index 994b5d6..c15e43a 100644
> --- a/drivers/net/cxgb3/sge.c
> +++ b/drivers/net/cxgb3/sge.c
> @@ -2431,7 +2431,7 @@ static irqreturn_t t3b_intr_napi(int irq, void *cookie)
>   *	(MSI-X, MSI, or legacy) and whether NAPI will be used to service the
>   *	response queues.
>   */
> -intr_handler_t t3_intr_handler(struct adapter *adap, int polling)
> +irq_handler_t t3_intr_handler(struct adapter *adap, int polling)
>  {
>  	if (adap->flags & USING_MSIX)
>  		return polling ? t3_sge_intr_msix_napi : t3_sge_intr_msix;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>   


^ permalink raw reply

* Re: [PATCH 0/4] Fix race between sk_filter reassign and sk_clone()
From: David Miller @ 2007-10-19  7:52 UTC (permalink / raw)
  To: xemul; +Cc: olof, netdev, devel, linux-kernel
In-Reply-To: <47185E9E.1030102@openvz.org>

From: Pavel Emelyanov <xemul@openvz.org>
Date: Fri, 19 Oct 2007 11:37:02 +0400

> David Miller wrote:
> > From: Olof Johansson <olof@lixom.net>
> > Date: Thu, 18 Oct 2007 21:29:47 -0500
> > 
> >> So, looks like rcu_dereference() returned NULL. I don't know the
> >> filter code at all, but it seems like it might be a valid case?
> >> sk_detach_filter() seems to handle a NULL sk_filter, at least.
> >>
> >>
> >> So, this needs review by someone who knows the filter, but it fixes the
> >> problem for me:
> 
> Yes. The NULL filter is a valid case, when there are no
> filters attached at all. So this fix is correct.
> 
> Thanks, Olof. Sorry, Dave :(

No worries, thanks for reviewing.

^ permalink raw reply

* Re: [PATCH] flush_work_sync vs. flush_scheduled_work Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes
From: Jarek Poplawski @ 2007-10-19  7:50 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Maciej W. Rozycki, Andy Fleming, Andrew Morton, Jeff Garzik,
	netdev, linux-kernel
In-Reply-To: <20071018154819.GA425@tv-sign.ru>

On Thu, Oct 18, 2007 at 07:48:19PM +0400, Oleg Nesterov wrote:
> On 10/18, Jarek Poplawski wrote:
> >
> > +/**
> > + * flush_work_sync - block until a work_struct's callback has terminated
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Hmm...
> 
> > + * Similar to cancel_work_sync() but will only busy wait (without cancel)
> > + * if the work is queued.
> 
> Yes, it won't block, but will spin in busy-wait loop until all other works
> scheduled before this work are finished. Not good. After that it really
> blocks waiting for this work to complete.
> 
> And I am a bit confused. We can't use flush_workqueue() because some of the
> queued work_structs may take rtnl_lock, yes? But in that case we can't use
> the new flush_work_sync() helper as well, no?

OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOPS!

Of course, we can't!!! I remembered there was this issue long time
ago, but then I've had some break in tracking net & workqueue. So,
while reading this patch I was alarmed at first, and self-misled
later. I think, there is definitely needed some warning about
locking (or unlocking) during these flush_ & cancel_ functions.
(Btw, I've very much wondered now, why I didn't notice at that 'old'
time, that you added such a great feature (wrt. locking) and I even
didn't notice this...). 

So, Maciej (and other readers of this thread) - I withdraw my false
opinion from my second message here: it's very wrong to call this
sched_work_sync() with rtnl_lock(). It's only less probable to lockup
with this than with flush_schedule_work().

> 
> If we can't just cancel the work, can't we do something like
> 
> 	if (cancel_work_sync(w))
> 		w->func(w);
> 
> instead?
> 
> > +void flush_work_sync(struct work_struct *work)
> > +{
> > +	int ret;
> > +
> > +	do {
> > +		ret = work_pending(work);
> > +		wait_on_work(work);
> > +		if (ret)
> > +			cpu_relax();
> > +	} while (ret);
> > +}
> 
> If we really the new helper, perhaps we can make it a bit better?
> 
> 1. Modify insert_work() to take the "struct list_head *at" parameter instead
>    of "int tail". I think this patch will also cleanup the code a bit, and
>    shrink a couple of bytes from .text

Looks like a very good idea, but I need more time to rethink this.
Probably some code example should be helpful.

> 
> 2. flush_work_sync() inserts a barrier right after this work and blocks.
>    We still need some retry logic to handle the queueing is in progress
>    of course, but we won't spin waiting for the other works.

Until monday I should have an opinion on that (today a bit under
fire...).

> 
> What do you think?

Since there is no gain wrt. locking with my current proposal, I
withdraw this patch of course.

It looks like my wrong patch was great idea because we got this very
precious Oleg's opinion! (I know I'm a genius sometimes...)

Thanks very much,
Jarek P.

^ permalink raw reply

* Re: [PATCH 0/4] Fix race between sk_filter reassign and sk_clone()
From: Pavel Emelyanov @ 2007-10-19  7:37 UTC (permalink / raw)
  To: David Miller, olof; +Cc: netdev, devel, linux-kernel
In-Reply-To: <20071018.215521.28810637.davem@davemloft.net>

David Miller wrote:
> From: Olof Johansson <olof@lixom.net>
> Date: Thu, 18 Oct 2007 21:29:47 -0500
> 
>> So, looks like rcu_dereference() returned NULL. I don't know the
>> filter code at all, but it seems like it might be a valid case?
>> sk_detach_filter() seems to handle a NULL sk_filter, at least.
>>
>>
>> So, this needs review by someone who knows the filter, but it fixes the
>> problem for me:

Yes. The NULL filter is a valid case, when there are no
filters attached at all. So this fix is correct.

Thanks, Olof. Sorry, Dave :(

>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>

Acked-by: Pavel Emelyanov <xemul@openvz.org>

> I've applied this for now to my net-2.6 tree, thanks Olof
> for tracking this down.
> 
> Pavel please take a look at this and let me know if it should
> fixed in some other way.
> 
> Thanks!
> 


^ permalink raw reply

* Re: [NET]: Fix possible dev_deactivate race condition
From: Peter Zijlstra @ 2007-10-19  7:35 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Linux Kernel Mailing List, David S. Miller, linuxppc-dev,
	Thomas Gleixner, netdev, akpm, Linus Torvalds, Ingo Molnar
In-Reply-To: <20071019053624.GA10560@gondor.apana.org.au>

On Fri, 2007-10-19 at 13:36 +0800, Herbert Xu wrote:
> On Fri, Oct 19, 2007 at 12:20:25PM +0800, Herbert Xu wrote:
> >
> > In fact this bug exists elsewhere too.  For example, the network
> > stack does this in net/sched/sch_generic.c:
> > 
> >         /* Wait for outstanding qdisc_run calls. */
> >       while (test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state))
> >               yield();
> > 
> > This has the same problem as the current synchronize_irq code.
> 

> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index e01d576..b3b7420 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -556,6 +556,7 @@ void dev_deactivate(struct net_device *dev)
>  {
>         struct Qdisc *qdisc;
>         struct sk_buff *skb;
> +       int running;
>  
>         spin_lock_bh(&dev->queue_lock);
>         qdisc = dev->qdisc;
> @@ -571,12 +572,31 @@ void dev_deactivate(struct net_device *dev)
>  
>         dev_watchdog_down(dev);
>  
> -       /* Wait for outstanding dev_queue_xmit calls. */
> +       /* Wait for outstanding qdisc-less dev_queue_xmit calls. */
>         synchronize_rcu();
>  
>         /* Wait for outstanding qdisc_run calls. */
> -       while (test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state))
> -               yield();
> +       do {
> +               while (test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state))
> +                       yield();
> +

Ouch!, is there really no sane locking alternative? Hashed waitqueues
like for the page lock come to mind.

> +               /*
> +                * Double-check inside queue lock to ensure that all effects
> +                * of the queue run are visible when we return.
> +                */
> +               spin_lock_bh(&dev->queue_lock);
> +               running = test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state);
> +               spin_unlock_bh(&dev->queue_lock);
> +
> +               /*
> +                * The running flag should never be set at this point because
> +                * we've already set dev->qdisc to noop_qdisc *inside* the same
> +                * pair of spin locks.  That is, if any qdisc_run starts after
> +                * our initial test it should see the noop_qdisc and then
> +                * clear the RUNNING bit before dropping the queue lock.  So
> +                * if it is set here then we've found a bug.
> +                */
> +       } while (WARN_ON_ONCE(running));
>  }
>  
>  void dev_init_scheduler(struct net_device *dev) 

^ permalink raw reply

* [PATCH] atm/stallion/ucb1400_ts: remove needless use of irq handler first arg
From: Jeff Garzik @ 2007-10-19  7:34 UTC (permalink / raw)
  To: LKML, netdev, dmitry.torokhov; +Cc: Andrew Morton


commit aeb16d7836f97576218fae6f3959c415b0fd09f0
Author: Jeff Garzik <jeff@garzik.org>
Date:   Fri Oct 19 03:19:08 2007 -0400

    [ATM, CHAR, TOUCHSCREEN] remove needless use of irq handler first arg
    
    Like the vast majority of other drivers, these drivers do not need to
    reference their 'irq' function argument at all.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/atm/ambassador.c               |    2 +-
 drivers/char/stallion.c                |    2 +-
 drivers/input/touchscreen/ucb1400_ts.c |   11 ++++-------
 3 files changed, 6 insertions(+), 9 deletions(-)

aeb16d7836f97576218fae6f3959c415b0fd09f0
diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c
index b34b382..c2b9464 100644
--- a/drivers/atm/ambassador.c
+++ b/drivers/atm/ambassador.c
@@ -872,7 +872,7 @@ static irqreturn_t interrupt_handler(int irq, void *dev_id) {
   
     // for us or someone else sharing the same interrupt
     if (!interrupt) {
-      PRINTD (DBG_IRQ, "irq not for me: %d", irq);
+      PRINTD (DBG_IRQ, "irq not for me");
       return IRQ_NONE;
     }
     
diff --git a/drivers/char/stallion.c b/drivers/char/stallion.c
index 45758d5..bf5a134 100644
--- a/drivers/char/stallion.c
+++ b/drivers/char/stallion.c
@@ -1629,7 +1629,7 @@ static irqreturn_t stl_intr(int irq, void *dev_id)
 {
 	struct stlbrd *brdp = dev_id;
 
-	pr_debug("stl_intr(brdp=%p,irq=%d)\n", brdp, irq);
+	pr_debug("stl_intr(brdp=%p,irq=%d)\n", brdp, brdp->irq);
 
 	return IRQ_RETVAL((* brdp->isr)(brdp));
 }
diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index 89373b0..68dac18 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -360,13 +360,10 @@ static irqreturn_t ucb1400_hard_irq(int irqnr, void *devid)
 {
 	struct ucb1400 *ucb = devid;
 
-	if (irqnr == ucb->irq) {
-		disable_irq(ucb->irq);
-		ucb->irq_pending = 1;
-		wake_up(&ucb->ts_wait);
-		return IRQ_HANDLED;
-	}
-	return IRQ_NONE;
+	disable_irq(ucb->irq);
+	ucb->irq_pending = 1;
+	wake_up(&ucb->ts_wait);
+	return IRQ_HANDLED;
 }
 
 static int ucb1400_ts_open(struct input_dev *idev)

^ permalink raw reply related

* [PATCH] lib82596, netxen: delete pointless tests from irq handler
From: Jeff Garzik @ 2007-10-19  7:33 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Andrew Morton


commit e96888518af94d9f607b996f8b90873330dbfc32
Author: Jeff Garzik <jeff@garzik.org>
Date:   Fri Oct 19 03:14:03 2007 -0400

    [NETDRVR] lib82596, netxen: delete pointless tests from irq handler
    
    Remove always-false tests in irq handler.
    
    Also a few other minor cleanups.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/net/lib82596.c               |    8 +-------
 drivers/net/netxen/netxen_nic_main.c |   11 ++---------
 2 files changed, 3 insertions(+), 16 deletions(-)

e96888518af94d9f607b996f8b90873330dbfc32
diff --git a/drivers/net/lib82596.c b/drivers/net/lib82596.c
index ffaa14f..9a855e5 100644
--- a/drivers/net/lib82596.c
+++ b/drivers/net/lib82596.c
@@ -1124,12 +1124,6 @@ static irqreturn_t i596_interrupt(int irq, void *dev_id)
 	struct i596_dma *dma;
 	unsigned short status, ack_cmd = 0;
 
-	if (dev == NULL) {
-		printk(KERN_WARNING "%s: irq %d for unknown device.\n",
-		       __FUNCTION__, irq);
-		return IRQ_NONE;
-	}
-
 	lp = netdev_priv(dev);
 	dma = lp->dma;
 
@@ -1140,7 +1134,7 @@ static irqreturn_t i596_interrupt(int irq, void *dev_id)
 
 	DEB(DEB_INTS, printk(KERN_DEBUG
 			     "%s: i596 interrupt, IRQ %d, status %4.4x.\n",
-			dev->name, irq, status));
+			dev->name, dev->irq, status));
 
 	ack_cmd = status & 0xf000;
 
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 601051c..a80f0cd 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -1268,17 +1268,10 @@ netxen_handle_int(struct netxen_adapter *adapter, struct net_device *netdev)
  */
 irqreturn_t netxen_intr(int irq, void *data)
 {
-	struct netxen_adapter *adapter;
-	struct net_device *netdev;
+	struct netxen_adapter *adapter = data;
+	struct net_device *netdev = adapter->netdev;
 	u32 our_int = 0;
 
-	if (unlikely(!irq)) {
-		return IRQ_NONE;	/* Not our interrupt */
-	}
-
-	adapter = (struct netxen_adapter *)data;
-	netdev  = adapter->netdev;
-
 	if (!(adapter->flags & NETXEN_NIC_MSI_ENABLED)) {
 		our_int = readl(NETXEN_CRB_NORMALIZE(adapter, CRB_INT_VECTOR));
 		/* not our interrupt */

^ permalink raw reply related

* [PATCH] sparc/xen/cxgb3: use irq_handler_t where appropriate
From: Jeff Garzik @ 2007-10-19  7:33 UTC (permalink / raw)
  To: David Miller, jeremy, netdev; +Cc: LKML, Andrew Morton


commit 21b1f26bf54a2ba1e4072db6dd01da128b1f66ef
Author: Jeff Garzik <jeff@garzik.org>
Date:   Fri Oct 19 03:12:20 2007 -0400

    [SPARC, XEN, NET/CXGB3] use irq_handler_t where appropriate
    
    Rather than hand-rolling our own prototype, make the code more
    future-proof by using the standard irq_handler_t typedef.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 arch/sparc/kernel/irq.c     |    4 ++--
 arch/x86/xen/events.c       |    4 ++--
 drivers/net/cxgb3/adapter.h |    4 +---
 drivers/net/cxgb3/sge.c     |    2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

21b1f26bf54a2ba1e4072db6dd01da128b1f66ef
diff --git a/arch/sparc/kernel/irq.c b/arch/sparc/kernel/irq.c
index 722d67d..e1e24f3 100644
--- a/arch/sparc/kernel/irq.c
+++ b/arch/sparc/kernel/irq.c
@@ -479,7 +479,7 @@ EXPORT_SYMBOL(pdma_areasize);
 
 extern void floppy_hardint(void);
 
-static irqreturn_t (*floppy_irq_handler)(int irq, void *dev_id);
+static irq_handler_t floppy_irq_handler;
 
 void sparc_floppy_irq(int irq, void *dev_id, struct pt_regs *regs)
 {
@@ -500,7 +500,7 @@ void sparc_floppy_irq(int irq, void *dev_id, struct pt_regs *regs)
 }
 
 int sparc_floppy_request_irq(int irq, unsigned long flags,
-			     irqreturn_t (*irq_handler)(int irq, void *))
+			     irq_handler_t irq_handler)
 {
 	floppy_irq_handler = irq_handler;
 	return request_fast_irq(irq, floppy_hardint, flags, "floppy");
diff --git a/arch/x86/xen/events.c b/arch/x86/xen/events.c
index da1b173..6d1da58 100644
--- a/arch/x86/xen/events.c
+++ b/arch/x86/xen/events.c
@@ -383,7 +383,7 @@ static void unbind_from_irq(unsigned int irq)
 }
 
 int bind_evtchn_to_irqhandler(unsigned int evtchn,
-			      irqreturn_t (*handler)(int, void *),
+			      irq_handler_t handler,
 			      unsigned long irqflags,
 			      const char *devname, void *dev_id)
 {
@@ -402,7 +402,7 @@ int bind_evtchn_to_irqhandler(unsigned int evtchn,
 EXPORT_SYMBOL_GPL(bind_evtchn_to_irqhandler);
 
 int bind_virq_to_irqhandler(unsigned int virq, unsigned int cpu,
-			    irqreturn_t (*handler)(int, void *),
+			    irq_handler_t handler,
 			    unsigned long irqflags, const char *devname, void *dev_id)
 {
 	unsigned int irq;
diff --git a/drivers/net/cxgb3/adapter.h b/drivers/net/cxgb3/adapter.h
index 0442617..3814cc6 100644
--- a/drivers/net/cxgb3/adapter.h
+++ b/drivers/net/cxgb3/adapter.h
@@ -46,8 +46,6 @@
 #include <asm/bitops.h>
 #include <asm/io.h>
 
-typedef irqreturn_t(*intr_handler_t) (int, void *);
-
 struct vlan_group;
 struct adapter;
 struct sge_qset;
@@ -270,7 +268,7 @@ void t3_sge_start(struct adapter *adap);
 void t3_sge_stop(struct adapter *adap);
 void t3_free_sge_resources(struct adapter *adap);
 void t3_sge_err_intr_handler(struct adapter *adapter);
-intr_handler_t t3_intr_handler(struct adapter *adap, int polling);
+irq_handler_t t3_intr_handler(struct adapter *adap, int polling);
 int t3_eth_xmit(struct sk_buff *skb, struct net_device *dev);
 int t3_mgmt_tx(struct adapter *adap, struct sk_buff *skb);
 void t3_update_qset_coalesce(struct sge_qset *qs, const struct qset_params *p);
diff --git a/drivers/net/cxgb3/sge.c b/drivers/net/cxgb3/sge.c
index 994b5d6..c15e43a 100644
--- a/drivers/net/cxgb3/sge.c
+++ b/drivers/net/cxgb3/sge.c
@@ -2431,7 +2431,7 @@ static irqreturn_t t3b_intr_napi(int irq, void *cookie)
  *	(MSI-X, MSI, or legacy) and whether NAPI will be used to service the
  *	response queues.
  */
-intr_handler_t t3_intr_handler(struct adapter *adap, int polling)
+irq_handler_t t3_intr_handler(struct adapter *adap, int polling)
 {
 	if (adap->flags & USING_MSIX)
 		return polling ? t3_sge_intr_msix_napi : t3_sge_intr_msix;

^ permalink raw reply related

* [PATCH] Eliminate pointless casts from void* in a few driver irq handlers.
From: Jeff Garzik @ 2007-10-19  7:31 UTC (permalink / raw)
  To: netdev, LKML; +Cc: Andrew Morton


commit 9739eb5090cc136ab50f2b323b83894c38d1ecb9
Author: Jeff Garzik <jeff@garzik.org>
Date:   Fri Oct 19 03:10:11 2007 -0400

    Eliminate pointless casts from void* in a few driver irq handlers.
    
    Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

 drivers/atm/horizon.c              |    5 +++--
 drivers/char/tpm/tpm_tis.c         |    4 ++--
 drivers/mtd/onenand/onenand_base.c |    2 +-
 drivers/net/typhoon.c              |    2 +-
 drivers/net/ucc_geth.c             |    2 +-
 drivers/net/wan/sbni.c             |    4 ++--
 6 files changed, 10 insertions(+), 9 deletions(-)

9739eb5090cc136ab50f2b323b83894c38d1ecb9
diff --git a/drivers/atm/horizon.c b/drivers/atm/horizon.c
index f96446c..9b2cf25 100644
--- a/drivers/atm/horizon.c
+++ b/drivers/atm/horizon.c
@@ -1382,8 +1382,9 @@ static inline void rx_data_av_handler (hrz_dev * dev) {
 
 /********** interrupt handler **********/
 
-static irqreturn_t interrupt_handler(int irq, void *dev_id) {
-  hrz_dev * dev = (hrz_dev *) dev_id;
+static irqreturn_t interrupt_handler(int irq, void *dev_id)
+{
+  hrz_dev *dev = dev_id;
   u32 int_source;
   unsigned int irq_ok;
   
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index a8e8084..fd771a4 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -381,7 +381,7 @@ static struct tpm_vendor_specific tpm_tis = {
 
 static irqreturn_t tis_int_probe(int irq, void *dev_id)
 {
-	struct tpm_chip *chip = (struct tpm_chip *) dev_id;
+	struct tpm_chip *chip = dev_id;
 	u32 interrupt;
 
 	interrupt = ioread32(chip->vendor.iobase +
@@ -401,7 +401,7 @@ static irqreturn_t tis_int_probe(int irq, void *dev_id)
 
 static irqreturn_t tis_int_handler(int irq, void *dev_id)
 {
-	struct tpm_chip *chip = (struct tpm_chip *) dev_id;
+	struct tpm_chip *chip = dev_id;
 	u32 interrupt;
 	int i;
 
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index dd28355..1b0b320 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -359,7 +359,7 @@ static int onenand_wait(struct mtd_info *mtd, int state)
  */
 static irqreturn_t onenand_interrupt(int irq, void *data)
 {
-	struct onenand_chip *this = (struct onenand_chip *) data;
+	struct onenand_chip *this = data;
 
 	/* To handle shared interrupt */
 	if (!this->complete.done)
diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c
index 72e5e9b..94ac586 100644
--- a/drivers/net/typhoon.c
+++ b/drivers/net/typhoon.c
@@ -1801,7 +1801,7 @@ typhoon_poll(struct napi_struct *napi, int budget)
 static irqreturn_t
 typhoon_interrupt(int irq, void *dev_instance)
 {
-	struct net_device *dev = (struct net_device *) dev_instance;
+	struct net_device *dev = dev_instance;
 	struct typhoon *tp = dev->priv;
 	void __iomem *ioaddr = tp->ioaddr;
 	u32 intr_status;
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index bec413b..9741d61 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3607,7 +3607,7 @@ static int ucc_geth_poll(struct napi_struct *napi, int budget)
 
 static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
 {
-	struct net_device *dev = (struct net_device *)info;
+	struct net_device *dev = info;
 	struct ucc_geth_private *ugeth = netdev_priv(dev);
 	struct ucc_fast_private *uccf;
 	struct ucc_geth_info *ug_info;
diff --git a/drivers/net/wan/sbni.c b/drivers/net/wan/sbni.c
index 76db40d..2e8b5c2 100644
--- a/drivers/net/wan/sbni.c
+++ b/drivers/net/wan/sbni.c
@@ -502,8 +502,8 @@ sbni_start_xmit( struct sk_buff  *skb,  struct net_device  *dev )
 static irqreturn_t
 sbni_interrupt( int  irq,  void  *dev_id )
 {
-	struct net_device	  *dev = (struct net_device *) dev_id;
-	struct net_local  *nl  = (struct net_local *) dev->priv;
+	struct net_device	  *dev = dev_id;
+	struct net_local  *nl  = dev->priv;
 	int	repeat;
 
 	spin_lock( &nl->lock );

^ permalink raw reply related

* Re: networking crash in current mainline: sk_filter_delayed_uncharge()
From: Andrew Morton @ 2007-10-19  6:48 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, paulus, benh
In-Reply-To: <20071018.231127.00953748.davem@davemloft.net>

On Thu, 18 Oct 2007 23:11:27 -0700 (PDT) David Miller <davem@davemloft.net> wrote:

> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Thu, 18 Oct 2007 23:09:48 -0700
> 
> > 
> > powerpc mac G5
> > config: http://userweb.kernel.org/~akpm/config-g5.txt
> > screenshot: http://userweb.kernel.org/~akpm/dsc00005.jpg
> > 
> > It does this shortly after bringing up eth0 (tg3), in dhclient.
> 
> Try this:

Now it says "login:".  Let me know if you want a photo ;)

^ permalink raw reply

* Re: networking crash in current mainline: sk_filter_delayed_uncharge()
From: Andrew Morton @ 2007-10-19  6:12 UTC (permalink / raw)
  To: netdev, Paul Mackerras, Benjamin Herrenschmidt; +Cc: Pavel Emelyanov
In-Reply-To: <20071018230948.959e89d1.akpm@linux-foundation.org>

On Thu, 18 Oct 2007 23:09:48 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> 
> powerpc mac G5
> config: http://userweb.kernel.org/~akpm/config-g5.txt
> screenshot: http://userweb.kernel.org/~akpm/dsc00005.jpg
> 
> It does this shortly after bringing up eth0 (tg3), in dhclient.
> 

<looks in the git tree, adds cc ;)>

^ permalink raw reply

* [PATCH 2.6.24 1/1]S2io: Support for add/delete/store/restore ethernet addresses
From: Sreenivasa Honnur @ 2007-10-19  5:52 UTC (permalink / raw)
  To: jeff, netdev; +Cc: support

- Support to add/delete/store/restore 64 and 128 Ethernet addresses for Xframe I and Xframe II respectively.

Signed-off-by: Sreenivasa Honnur <sreenivasa.honnur@neterion.com>
---
diff -urpN org/drivers/net/s2io.c patch_1/drivers/net/s2io.c
--- org/drivers/net/s2io.c	2007-09-26 00:01:14.000000000 +0530
+++ patch_1/drivers/net/s2io.c	2007-09-26 22:42:11.000000000 +0530
@@ -84,7 +84,7 @@
 #include "s2io.h"
 #include "s2io-regs.h"
 
-#define DRV_VERSION "2.0.26.5"
+#define DRV_VERSION "2.0.26.6"
 
 /* S2io Driver name & version. */
 static char s2io_driver_name[] = "Neterion";
@@ -3363,6 +3363,9 @@ static void s2io_reset(struct s2io_nic *
 	/* Set swapper to enable I/O register access */
 	s2io_set_swapper(sp);
 
+	/* restore mac address entries */
+	do_s2io_restore_unicast_mc(sp);
+
 	/* Restore the MSIX table entries from local variables */
 	restore_xmsi_data(sp);
 
@@ -3421,9 +3424,6 @@ static void s2io_reset(struct s2io_nic *
 		writeq(val64, &bar0->pcc_err_reg);
 	}
 
-	/* restore the previously assigned mac address */
-	do_s2io_prog_unicast(sp->dev, (u8 *)&sp->def_mac_addr[0].mac_addr);
-
 	sp->device_enabled_once = FALSE;
 }
 
@@ -3896,8 +3896,17 @@ hw_init_failed:
 static int s2io_close(struct net_device *dev)
 {
 	struct s2io_nic *sp = dev->priv;
+	struct config_param *config = &sp->config;
+	u64 tmp64;
+	int off;
 
 	netif_stop_queue(dev);
+	/* delete all populated mac entries */
+	for(off =1; off < config->max_mc_addr; off++) {
+		tmp64 = do_s2io_read_unicast_mc(sp,off);
+		if(tmp64 != S2IO_DISABLE_MAC_ENTRY)
+			do_s2io_delete_unicast_mc(sp, tmp64);
+	}
 	napi_disable(&sp->napi);
 	/* Reset card, kill tasklet and free Tx and Rx buffers. */
 	s2io_card_down(sp);
@@ -4699,8 +4708,9 @@ static void s2io_set_multicast(struct ne
 	struct XENA_dev_config __iomem *bar0 = sp->bar0;
 	u64 val64 = 0, multi_mac = 0x010203040506ULL, mask =
 	    0xfeffffffffffULL;
-	u64 dis_addr = 0xffffffffffffULL, mac_addr = 0;
+	u64 dis_addr = S2IO_DISABLE_MAC_ENTRY, mac_addr = 0;
 	void __iomem *add;
+	struct config_param *config = &sp->config;
 
 	if ((dev->flags & IFF_ALLMULTI) && (!sp->m_cast_flg)) {
 		/*  Enable all Multicast addresses */
@@ -4710,7 +4720,7 @@ static void s2io_set_multicast(struct ne
 		       &bar0->rmac_addr_data1_mem);
 		val64 = RMAC_ADDR_CMD_MEM_WE |
 		    RMAC_ADDR_CMD_MEM_STROBE_NEW_CMD |
-		    RMAC_ADDR_CMD_MEM_OFFSET(MAC_MC_ALL_MC_ADDR_OFFSET);
+			RMAC_ADDR_CMD_MEM_OFFSET(config->max_mc_addr - 1);
 		writeq(val64, &bar0->rmac_addr_cmd_mem);
 		/* Wait till command completes */
 		wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
@@ -4718,7 +4728,7 @@ static void s2io_set_multicast(struct ne
 					S2IO_BIT_RESET);
 
 		sp->m_cast_flg = 1;
-		sp->all_multi_pos = MAC_MC_ALL_MC_ADDR_OFFSET;
+		sp->all_multi_pos = config->max_mc_addr - 1;
 	} else if ((dev->flags & IFF_ALLMULTI) && (sp->m_cast_flg)) {
 		/*  Disable all Multicast addresses */
 		writeq(RMAC_ADDR_DATA0_MEM_ADDR(dis_addr),
@@ -4787,7 +4797,7 @@ static void s2io_set_multicast(struct ne
 	/*  Update individual M_CAST address list */
 	if ((!sp->m_cast_flg) && dev->mc_count) {
 		if (dev->mc_count >
-		    (MAX_ADDRS_SUPPORTED - MAC_MC_ADDR_START_OFFSET - 1)) {
+			(config->max_mc_addr - config->max_mac_addr)) {
 			DBG_PRINT(ERR_DBG, "%s: No more Rx filters ",
 				  dev->name);
 			DBG_PRINT(ERR_DBG, "can be added, please enable ");
@@ -4807,7 +4817,7 @@ static void s2io_set_multicast(struct ne
 			val64 = RMAC_ADDR_CMD_MEM_WE |
 			    RMAC_ADDR_CMD_MEM_STROBE_NEW_CMD |
 			    RMAC_ADDR_CMD_MEM_OFFSET
-			    (MAC_MC_ADDR_START_OFFSET + i);
+				(config->mc_start_offset + i);
 			writeq(val64, &bar0->rmac_addr_cmd_mem);
 
 			/* Wait for command completes */
@@ -4839,7 +4849,7 @@ static void s2io_set_multicast(struct ne
 			val64 = RMAC_ADDR_CMD_MEM_WE |
 			    RMAC_ADDR_CMD_MEM_STROBE_NEW_CMD |
 			    RMAC_ADDR_CMD_MEM_OFFSET
-			    (i + MAC_MC_ADDR_START_OFFSET);
+				(i + config->mc_start_offset);
 			writeq(val64, &bar0->rmac_addr_cmd_mem);
 
 			/* Wait for command completes */
@@ -4855,8 +4865,76 @@ static void s2io_set_multicast(struct ne
 	}
 }
 
-/* add unicast MAC address to CAM */
-static int do_s2io_add_unicast(struct s2io_nic *sp, u64 addr, int off)
+/* read from CAM unicast & multicast addresses and store it in def_mac_addr structure
++ **/
+void do_s2io_store_unicast_mc(struct s2io_nic *sp)
+{
+	int offset;
+	u64 mac_addr=0x0;
+	struct config_param *config = &sp->config;
+
+	/* store unicast & multicast mac addresses */
+	for(offset = 0; offset < config->max_mc_addr; offset++) {
+		mac_addr = do_s2io_read_unicast_mc(sp,offset);
+		/* if read fails disable the entry */
+		if(mac_addr == FAILURE)
+			mac_addr = S2IO_DISABLE_MAC_ENTRY;
+		do_s2io_copy_mac_addr(sp, offset, mac_addr);
+	}
+}
+
+/* restore unicast addresses & multicast to CAM from def_mac_addr structure */
+static void do_s2io_restore_unicast_mc(struct s2io_nic *sp)
+{
+	int offset;
+	struct config_param *config = &sp->config;
+
+	/* restore unicast mac address */
+	for(offset = 0; offset < config->max_mac_addr; offset++)
+		do_s2io_prog_unicast(sp->dev,sp->def_mac_addr[offset].mac_addr);
+
+	/* restore multicast mac address */
+	for(offset = config->mc_start_offset;
+		offset < config->max_mc_addr; offset++)
+	do_s2io_add_mc(sp, sp->def_mac_addr[offset].mac_addr);
+}
+
+/* add a multicast MAC address to CAM */
+static int do_s2io_add_mc(struct s2io_nic *sp, u8* addr)
+{
+	int i;
+	u64 mac_addr=0;
+	struct config_param *config = &sp->config;
+
+	for (i = 0; i < ETH_ALEN; i++) {
+		mac_addr <<= 8;
+		mac_addr |= addr[i];
+	}
+	if((0ULL == mac_addr) || (mac_addr == S2IO_DISABLE_MAC_ENTRY))
+		return SUCCESS;
+
+	/* check if the multicast mac already preset in CAM */
+	for(i = config->mc_start_offset; i < config->max_mc_addr; i++) {
+		u64 tmp64;
+		tmp64 = do_s2io_read_unicast_mc(sp,i);
+		if(tmp64 == S2IO_DISABLE_MAC_ENTRY) /* CAM entry is empty */
+			break;
+
+		if(tmp64 == mac_addr)
+			return SUCCESS;
+	}
+	if(i == config->max_mc_addr) {
+		DBG_PRINT(ERR_DBG,"CAM full no space left for multicast MAC\n");
+		return FAILURE;
+	}
+	/* Update the internal structure with this new mac address */
+	do_s2io_copy_mac_addr(sp, i, mac_addr);
+
+	return (do_s2io_add_mac(sp,mac_addr,i));
+}
+
+/* add MAC address to CAM memory */
+static int do_s2io_add_mac(struct s2io_nic *sp, u64 addr, int off)
 {
 	u64 val64;
 	struct XENA_dev_config __iomem *bar0 = sp->bar0;
@@ -4879,6 +4957,53 @@ static int do_s2io_add_unicast(struct s2
 	return SUCCESS;
 }
 
+/* deletes a specified unicast/multicast mac entry from CAM */
+static int do_s2io_delete_unicast_mc(struct s2io_nic *sp, u64 addr)
+{
+	int off;
+	u64 dis_addr = S2IO_DISABLE_MAC_ENTRY,tmp64;
+	struct config_param *config = &sp->config;
+
+	for(off = 1;
+		off < config->max_mc_addr; off++) {
+		tmp64 = do_s2io_read_unicast_mc(sp,off);
+		if(tmp64 == addr) {
+			/* disable the entry by writing  0xffffffffffffULL */
+			if(do_s2io_add_mac(sp,dis_addr,off) ==  FAILURE)
+				return FAILURE;
+			/* store the new mac list from CAM */
+			do_s2io_store_unicast_mc(sp);
+			return SUCCESS;
+		}
+	}
+	DBG_PRINT(ERR_DBG,"MAC address 0x%llx not found in CAM\n",
+		(unsigned long long)addr);
+	return FAILURE;
+}
+
+/* read mac entries from CAM */
+static u64 do_s2io_read_unicast_mc(struct s2io_nic *sp, int offset)
+{
+	u64 tmp64=0xffffffffffff0000ULL, val64;
+	struct XENA_dev_config __iomem *bar0 = sp->bar0;
+
+	/* read mac addr */
+	val64 =
+		RMAC_ADDR_CMD_MEM_RD | RMAC_ADDR_CMD_MEM_STROBE_NEW_CMD |
+		RMAC_ADDR_CMD_MEM_OFFSET(offset);
+	writeq(val64, &bar0->rmac_addr_cmd_mem);
+
+	/* Wait till command completes */
+	if (wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
+		RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING,
+		S2IO_BIT_RESET)) {
+		DBG_PRINT(INFO_DBG, "do_s2io_read_unicast_mc failed\n");
+		return FAILURE;
+	}
+	tmp64 = readq(&bar0->rmac_addr_data0_mem);
+	return (tmp64 >> 16);
+}
+
 /**
  * s2io_set_mac_addr driver entry point
  */
@@ -4909,6 +5034,8 @@ static int do_s2io_prog_unicast(struct n
 	struct s2io_nic *sp = dev->priv;
 	register u64 mac_addr = 0, perm_addr = 0;
 	int i;
+	u64 tmp64;
+	struct config_param *config = &sp->config;
 
 	/*
 	* Set the new MAC address as the new unicast filter and reflect this
@@ -4926,9 +5053,27 @@ static int do_s2io_prog_unicast(struct n
 	if (mac_addr == perm_addr)
 		return SUCCESS;
 
+	/* check if the mac already preset in CAM */
+	for(i = 1; i < config->max_mac_addr; i++) {
+		tmp64 = do_s2io_read_unicast_mc(sp,i);
+		if(tmp64 == S2IO_DISABLE_MAC_ENTRY) /* CAM entry is empty */
+			break;
+
+		if(tmp64 == mac_addr) {
+			DBG_PRINT(INFO_DBG,
+				"MAC addr:0x%llx already present in CAM\n",
+				(unsigned long long)mac_addr);
+			return SUCCESS;
+		}
+	}
+	if(i == config->max_mac_addr) {
+		DBG_PRINT(ERR_DBG,"CAM full no space left for Unicast MAC\n");
+		return FAILURE;
+	}
+
 	/* Update the internal structure with this new mac address */
-	do_s2io_copy_mac_addr(sp, 0, mac_addr);
-	return (do_s2io_add_unicast(sp, mac_addr, 0));
+	do_s2io_copy_mac_addr(sp, i, mac_addr);
+	return (do_s2io_add_mac(sp,mac_addr,i));
 }
 
 /**
@@ -7625,7 +7770,7 @@ s2io_init_nic(struct pci_dev *pdev, cons
 	 */
 	bar0 = sp->bar0;
 	val64 = RMAC_ADDR_CMD_MEM_RD | RMAC_ADDR_CMD_MEM_STROBE_NEW_CMD |
-	    RMAC_ADDR_CMD_MEM_OFFSET(0 + MAC_MAC_ADDR_START_OFFSET);
+		RMAC_ADDR_CMD_MEM_OFFSET(0 + S2IO_MAC_ADDR_START_OFFSET);
 	writeq(val64, &bar0->rmac_addr_cmd_mem);
 	wait_for_cmd_complete(&bar0->rmac_addr_cmd_mem,
 		      RMAC_ADDR_CMD_MEM_STROBE_CMD_EXECUTING, S2IO_BIT_RESET);
@@ -7645,7 +7790,22 @@ s2io_init_nic(struct pci_dev *pdev, cons
 	memcpy(dev->dev_addr, sp->def_mac_addr, ETH_ALEN);
 	memcpy(dev->perm_addr, dev->dev_addr, ETH_ALEN);
 
-	 /* Store the values of the MSIX table in the s2io_nic structure */
+	/* initialize number of multicast & unicast MAC entries variables */
+	if (sp->device_type == XFRAME_I_DEVICE) {
+		config->max_mc_addr = S2IO_XENA_MAX_MC_ADDRESSES;
+		config->max_mac_addr = S2IO_XENA_MAX_MAC_ADDRESSES;
+		config->mc_start_offset = S2IO_XENA_MC_ADDR_START_OFFSET;
+	}
+	else  if (sp->device_type == XFRAME_II_DEVICE) {
+		config->max_mc_addr = S2IO_HERC_MAX_MC_ADDRESSES;
+		config->max_mac_addr = S2IO_HERC_MAX_MAC_ADDRESSES;
+		config->mc_start_offset = S2IO_HERC_MC_ADDR_START_OFFSET;
+	}
+
+	/* store mac addresses from CAM to s2io_nic structure */
+	do_s2io_store_unicast_mc(sp);
+
+	/* Store the values of the MSIX table in the s2io_nic structure */
 	store_xmsi_data(sp);
 	/* reset Nic and bring it to known state */
 	s2io_reset(sp);
diff -urpN org/drivers/net/s2io.h patch_1/drivers/net/s2io.h
--- org/drivers/net/s2io.h	2007-09-26 00:01:14.000000000 +0530
+++ patch_1/drivers/net/s2io.h	2007-09-26 02:34:11.000000000 +0530
@@ -31,6 +31,7 @@
 #define SUCCESS 0
 #define FAILURE -1
 #define S2IO_MINUS_ONE 0xFFFFFFFFFFFFFFFFULL
+#define S2IO_DISABLE_MAC_ENTRY 0xFFFFFFFFFFFFULL
 #define S2IO_MAX_PCI_CONFIG_SPACE_REINIT 100
 #define S2IO_BIT_RESET 1
 #define S2IO_BIT_SET 2
@@ -458,6 +459,9 @@ struct config_param {
 #define MAX_MTU_JUMBO               (MAX_PYLD_JUMBO+18)
 #define MAX_MTU_JUMBO_VLAN          (MAX_PYLD_JUMBO+22)
 	u16 bus_speed;
+	int max_mc_addr;    /* xena=64 herc=256 */
+	int max_mac_addr;   /* xena=16 herc=64 */
+	int mc_start_offset;    /* xena=16 herc=64 */
 };
 
 /* Structure representing MAC Addrs */
@@ -824,9 +828,8 @@ struct s2io_nic {
 	void __iomem *bar0;
 	void __iomem *bar1;
 #define MAX_MAC_SUPPORTED   16
-#define MAX_SUPPORTED_MULTICASTS MAX_MAC_SUPPORTED
 
-	struct mac_addr def_mac_addr[MAX_MAC_SUPPORTED];
+	struct mac_addr def_mac_addr[256];
 
 	struct net_device_stats stats;
 	int high_dma_flag;
@@ -850,10 +853,9 @@ struct s2io_nic {
 #define PROMISC     1
 #define ALL_MULTI   2
 
-#define MAX_ADDRS_SUPPORTED 64
 	u16 usr_addr_count;
 	u16 mc_addr_count;
-	struct usr_addr usr_addrs[MAX_ADDRS_SUPPORTED];
+	struct usr_addr usr_addrs[256];
 
 	u16 m_cast_flg;
 	u16 all_multi_pos;
@@ -1066,6 +1068,12 @@ static int s2io_add_isr(struct s2io_nic 
 static void s2io_rem_isr(struct s2io_nic * sp);
 
 static void restore_xmsi_data(struct s2io_nic *nic);
+static void do_s2io_store_unicast_mc(struct s2io_nic *sp);
+static void do_s2io_restore_unicast_mc(struct s2io_nic *sp);
+static u64 do_s2io_read_unicast_mc(struct s2io_nic *sp, int offset);
+static int do_s2io_add_mc(struct s2io_nic *sp, u8 *addr);
+static int do_s2io_add_mac(struct s2io_nic *sp, u64 addr, int offset);
+static int do_s2io_delete_unicast_mc(struct s2io_nic *sp, u64 addr);
 
 static int
 s2io_club_tcp_session(u8 *buffer, u8 **tcp, u32 *tcp_len, struct lro **lro,
diff -urpN org/drivers/net/s2io-regs.h patch_1/drivers/net/s2io-regs.h
--- org/drivers/net/s2io-regs.h	2007-09-26 00:01:14.000000000 +0530
+++ patch_1/drivers/net/s2io-regs.h	2007-09-26 02:37:53.000000000 +0530
@@ -721,12 +721,16 @@ struct XENA_dev_config {
 
 	u64 rmac_cfg_key;
 #define RMAC_CFG_KEY(val)               vBIT(val,0,16)
+#define S2IO_MAC_ADDR_START_OFFSET		0
 
-#define MAX_MAC_ADDRESSES           16
-#define MAX_MC_ADDRESSES            32	/* Multicast addresses */
-#define MAC_MAC_ADDR_START_OFFSET   0
-#define MAC_MC_ADDR_START_OFFSET    16
-#define MAC_MC_ALL_MC_ADDR_OFFSET   63	/* enables all multicast pkts */
+#define S2IO_XENA_MAX_MC_ADDRESSES		64  /* multicast addresses */
+#define S2IO_HERC_MAX_MC_ADDRESSES		256
+
+#define S2IO_XENA_MAX_MAC_ADDRESSES    16
+#define S2IO_HERC_MAX_MAC_ADDRESSES    64
+
+#define S2IO_XENA_MC_ADDR_START_OFFSET 16
+#define S2IO_HERC_MC_ADDR_START_OFFSET 64
 	u64 rmac_addr_cmd_mem;
 #define RMAC_ADDR_CMD_MEM_WE                    BIT(7)
 #define RMAC_ADDR_CMD_MEM_RD                    0


^ permalink raw reply

* Re: networking crash in current mainline: sk_filter_delayed_uncharge()
From: David Miller @ 2007-10-19  6:11 UTC (permalink / raw)
  To: akpm; +Cc: netdev, paulus, benh
In-Reply-To: <20071018230948.959e89d1.akpm@linux-foundation.org>

From: Andrew Morton <akpm@linux-foundation.org>
Date: Thu, 18 Oct 2007 23:09:48 -0700

> 
> powerpc mac G5
> config: http://userweb.kernel.org/~akpm/config-g5.txt
> screenshot: http://userweb.kernel.org/~akpm/dsc00005.jpg
> 
> It does this shortly after bringing up eth0 (tg3), in dhclient.

Try this:

>From 9b013e05e0289c190a53d78ca029e2f21c0e4485 Mon Sep 17 00:00:00 2001
From: Olof Johansson <olof@lixom.net>
Date: Thu, 18 Oct 2007 21:48:39 -0700
Subject: [PATCH] [NET]: Fix bug in sk_filter race cures.

Looks like this might be causing problems, at least for me on ppc. This
happened during a normal boot, right around first interface config/dhcp
run..

cpu 0x0: Vector: 300 (Data Access) at [c00000000147b820]
    pc: c000000000435e5c: .sk_filter_delayed_uncharge+0x1c/0x60
    lr: c0000000004360d0: .sk_attach_filter+0x170/0x180
    sp: c00000000147baa0
   msr: 9000000000009032
   dar: 4
 dsisr: 40000000
  current = 0xc000000004780fa0
  paca    = 0xc000000000650480
    pid   = 1295, comm = dhclient3
0:mon> t
[c00000000147bb20] c0000000004360d0 .sk_attach_filter+0x170/0x180
[c00000000147bbd0] c000000000418988 .sock_setsockopt+0x788/0x7f0
[c00000000147bcb0] c000000000438a74 .compat_sys_setsockopt+0x4e4/0x5a0
[c00000000147bd90] c00000000043955c .compat_sys_socketcall+0x25c/0x2b0
[c00000000147be30] c000000000007508 syscall_exit+0x0/0x40
--- Exception: c01 (System Call) at 000000000ff618d8
SP (fffdf040) is in userspace
0:mon>

I.e. null pointer deref at sk_filter_delayed_uncharge+0x1c:

0:mon> di $.sk_filter_delayed_uncharge
c000000000435e40  7c0802a6      mflr    r0
c000000000435e44  fbc1fff0      std     r30,-16(r1)
c000000000435e48  7c8b2378      mr      r11,r4
c000000000435e4c  ebc2cdd0      ld      r30,-12848(r2)
c000000000435e50  f8010010      std     r0,16(r1)
c000000000435e54  f821ff81      stdu    r1,-128(r1)
c000000000435e58  380300a4      addi    r0,r3,164
c000000000435e5c  81240004      lwz     r9,4(r4)

That's the deref of fp:

static void sk_filter_delayed_uncharge(struct sock *sk, struct sk_filter *fp)
{
        unsigned int size = sk_filter_len(fp);
...

That is called from sk_attach_filter():

...
        rcu_read_lock_bh();
        old_fp = rcu_dereference(sk->sk_filter);
        rcu_assign_pointer(sk->sk_filter, fp);
        rcu_read_unlock_bh();

        sk_filter_delayed_uncharge(sk, old_fp);
        return 0;
...

So, looks like rcu_dereference() returned NULL. I don't know the
filter code at all, but it seems like it might be a valid case?
sk_detach_filter() seems to handle a NULL sk_filter, at least.

So, this needs review by someone who knows the filter, but it fixes the
problem for me:

Signed-off-by: Olof Johansson <olof@lixom.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/core/filter.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 1f0068e..e0a0694 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -447,7 +447,8 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
 	rcu_assign_pointer(sk->sk_filter, fp);
 	rcu_read_unlock_bh();
 
-	sk_filter_delayed_uncharge(sk, old_fp);
+	if (old_fp)
+		sk_filter_delayed_uncharge(sk, old_fp);
 	return 0;
 }
 
-- 
1.5.3.4


^ permalink raw reply related

* networking crash in current mainline: sk_filter_delayed_uncharge()
From: Andrew Morton @ 2007-10-19  6:09 UTC (permalink / raw)
  To: netdev; +Cc: Paul Mackerras, Benjamin Herrenschmidt


powerpc mac G5
config: http://userweb.kernel.org/~akpm/config-g5.txt
screenshot: http://userweb.kernel.org/~akpm/dsc00005.jpg

It does this shortly after bringing up eth0 (tg3), in dhclient.


^ permalink raw reply

* Re: [PATCH] [Bug 9187] ir-functions.c:(.text+0xbce18): undefined reference to `input_event'
From: Randy Dunlap @ 2007-10-19  5:57 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, jeff
In-Reply-To: <20071018.220222.35036834.davem@davemloft.net>

On Thu, 18 Oct 2007 22:02:22 -0700 (PDT) David Miller wrote:

> From: Randy Dunlap <randy.dunlap@oracle.com>
> Date: Thu, 18 Oct 2007 21:53:50 -0700
> 
> > [bugme-daemon@bugzilla.kernel.org wrote:]
> > 
> > 
> > 
> > From: Randy Dunlap <randy.dunlap@oracle.com>
> > 
> > Drivers that use lro functions should depend on INET, otherwise they
> > may not link correctly.  Let's not select INET.  Select should be used
> > only for library-like code, not to enable subsystems.
> > 
> > ERROR: "lro_flush_all" [drivers/net/myri10ge/myri10ge.ko] undefined!
> > ERROR: "lro_receive_frags" [drivers/net/myri10ge/myri10ge.ko] undefined!
> > 
> > Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> 
> Acked-by: David S. Miller <davem@davemloft.net>
> 
> Jeff, please pick this one up too, thanks!


This patch is for kernel bugzilla #9186, not #9187.
Clerical error, sorry about that.

---
~Randy

^ permalink raw reply

* Re: Throughput Bug?
From: Bill Fink @ 2007-10-19  5:44 UTC (permalink / raw)
  To: Matthew Faulkner; +Cc: netdev
In-Reply-To: <c565abbb0710180854j6f2f756sdd390161bafd1c4a@mail.gmail.com>

On Thu, 18 Oct 2007, Matthew Faulkner wrote:

> Hey all
> 
> I'm using netperf to perform TCP throughput tests via the localhost
> interface. This is being done on a SMP machine. I'm forcing the
> netperf server and client to run on the same core. However, for any
> packet sizes below 523 the throughput is much lower compared to the
> throughput when the packet sizes are greater than 524.
> 
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    MBytes  /s  % S      % S      us/KB   us/KB
>  65536  65536    523    30.01        81.49   50.00    50.00    11.984  11.984
>  65536  65536    524    30.01       460.61   49.99    49.99    2.120   2.120
> 
> The chances are i'm being stupid and there is an obvious reason for
> this, but when i put  the server and client on different cores i don't
> see this effect.
> 
> Any help explaining this will be greatly appreciated.
> 
> Machine details:
> 
> Linux 2.6.22-2-amd64 #1 SMP Thu Aug 30 23:43:59 UTC 2007 x86_64 GNU/Linux
> 
> sched_affinity is used by netperf internally to set the core affinity.

I don't know if it's relevant, but note that 524 bytes + 52 bytes
of IP(20)/TCP(20)/TimeStamp(12) overhead gives a 576 byte packet,
which is the specified size that all IP routers must handle (and
the smallest value possible during PMTU discovery I believe).  A
message size of 523 bytes would be 1 less than that.  Could this
possibly have to do with ABC (possibly try disabling it if set)?

						-Bill

^ permalink raw reply


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