Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] e1000: the power down when running ifdown command
From: Jeff Kirsher @ 2009-11-03 21:37 UTC (permalink / raw)
  To: Naohiro Ooiwa
  Cc: jesse.brandeburg, peter.p.waskiewicz.jr, john.ronciak, davem,
	Andrew Morton, netdev, svaidy, e1000-devel
In-Reply-To: <4AF00097.4040708@miraclelinux.com>

2009/11/3 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
> Jeff Kirsher wrote:
>> 2009/10/31 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
>>
>> I have added this patch to my tree for testing.  This patch requires a
>> fair amount of regression testing, so once its passed testing I will
>> push the patch to David/netdev.
>
> I appreciate the marge your tree.
> If there is anything I can do, please let me know.
>
> And I know this patch is good for e100 driver too.
> I would really like to create patch for it.
> How do you think about e100 driver.
>
>
> Thanks,
> Naohiro Ooiwa
>

Patches are always welcome (referring to a e100 patch).

As far as the e1000 patch goes, it has a number of issues which were
found in testing.  Here are just a few problems we saw:
1. ethtool -t - crashes the system
2. ethtool eth0 - always shows link/speed as 1000/Full even when there
is no cable
3. ethtool -s eth0 autoneg on/off - system hang. Sometimes a copper
interface will show up as fiber after this.
4. ethtool -d/-S/-g etc - will corrupt the stats of the interface
while doing ifup/down

So it appears that more needs to be done to the driver to get this to
work as expected.

NAK

-- 
Cheers,
Jeff

^ permalink raw reply

* [PATCH net-next-2.6] em_meta: avoid one dev_put()
From: Eric Dumazet @ 2009-11-03 21:38 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

Another rcu conversion to avoid one dev_hold()/dev_put() pair

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/sched/em_meta.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
index 18d85d2..8e8d836 100644
--- a/net/sched/em_meta.c
+++ b/net/sched/em_meta.c
@@ -303,17 +303,17 @@ META_COLLECTOR(var_sk_bound_if)
 {
 	SKIP_NONLOCAL(skb);
 
-	 if (skb->sk->sk_bound_dev_if == 0) {
+	if (skb->sk->sk_bound_dev_if == 0) {
 		dst->value = (unsigned long) "any";
 		dst->len = 3;
-	 } else  {
+	} else {
 		struct net_device *dev;
 
-		dev = dev_get_by_index(&init_net, skb->sk->sk_bound_dev_if);
+		rcu_read_lock();
+		dev = dev_get_by_index_rcu(&init_net, skb->sk->sk_bound_dev_if);
 		*err = var_dev(dev, dst);
-		if (dev)
-			dev_put(dev);
-	 }
+		rcu_read_unlock();
+	}
 }
 
 META_COLLECTOR(int_sk_refcnt)

^ permalink raw reply related

* Re: [PATCH 1/3] sysfs directory scaling: rbtree for dirent name lookups
From: Eric W. Biederman @ 2009-11-03 21:43 UTC (permalink / raw)
  To: Benjamin LaHaise
  Cc: Greg KH, Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu,
	linux-kernel
In-Reply-To: <m1my338fbi.fsf@fess.ebiederm.org>

ebiederm@xmission.com (Eric W. Biederman) writes:

> Benjamin LaHaise <bcrl@lhnet.ca> writes:
>
>> On Mon, Nov 02, 2009 at 07:50:58PM -0800, Greg KH wrote:
>>> On Sun, Nov 01, 2009 at 11:31:30AM -0500, Benjamin LaHaise wrote:
>>> > Use an rbtree in sysfs_dirent to speed up file lookup times
>>> > 
>>> > Systems with large numbers (tens of thousands and more) of network 
>>> > interfaces stress the sysfs code in ways that make the linear search for 
>>> > a name match take far too long.  Avoid this by using an rbtree.
>>> 
>>> What kind of speedups are you seeing here?  And do these changes cause a
>>> memory increase due to the structure changes which outweigh the
>>> speedups?
>>
>> Depends on the number of interfaces being created.  Without the patch, 
>> interface creation time tends to double or worse for every 5,000-10,000 
>> additional network interfaces.
>>
>>> What kind of test are you doing to reproduce this?
>>
>> I'm creating 30,000+ network interfaces, with the goal being 100,000.  
>> With other hacks in the tree to get around the sysctl and procfs scaling 
>> issues, as well as disabling things like NetworkManager, the results look 
>> as follows:
>>
>> 	Interfaces	no-rb	rbtree	rbtree+list
>> 	0-5,000		13.8s	14.0s	13.0s
>> 	5,000-10,000	20.0s	17.4s	14.4s
>> 	10,000-15,000	27.3s	24.1s	16.9s
>> 	15,000-20,000	36.3s	32.2s	19.7s
>> 	20,000-25,000	45.2s	40.0s	22.9s
>> 	25,000-30,000	54.2s	48.2s	26.6s
>> 	30,000-35,000	63.9s	54.9s	30.7s
>>
>> Thoughts?
>
> Something is very weird.  I just took your no-rb numbers
> and divided by the number of interfaces to see how well we are
> scaling.  I got:
>
>  	Interfaces	per-interface cost
>  	5,000		0.002760s
>  	10,000		0.002000s
>  	15,000		0.001820s
>  	20,000		0.001815s
>  	25,000		0.001808s
>  	30,000		0.001807s
>  	35,000		0.001826s
>
> I ran a variant of this test a long time ago and at that the
> cost per interface grew with additional interfaces added.  This
> jives with the fact that the fundamental cost of adding N
> network interfaces to sysfs is O(N^2).
>
> Are your numbers from your application and are they real world?
> In which case they are interesting, but it would be good if
> we could also have microbenchmark numbers that just measure
> the sysfs costs.   If nothing else I am seeing a big startup
> overhead that isn't being subtracted out that makes it hard
> to see the real costs here.

I guess in particular what I would expect is that if we can do 35000
interfaces in 63s with an O(N^2) algorithm.  Then we should be able to
do 35000 interfaces with an O(NlogN) algorithm in under a second.
Which for your application should make the time essentially flat in
the number of interfaces.

Until we get close to that I figure we need to do more digging.

Eric

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Bartlomiej Zolnierkiewicz @ 2009-11-03 21:44 UTC (permalink / raw)
  To: Ivo van Doorn
  Cc: linux-wireless, linux-kernel, netdev, Randy Dunlap, Luis Correia,
	John W. Linville, Ingo Molnar, Johannes Berg, Jarek Poplawski,
	Pekka Enberg, David Miller
In-Reply-To: <200911032200.04516.IvDoorn@gmail.com>

On Tuesday 03 November 2009 22:00:03 Ivo van Doorn wrote:
> On Tuesday 03 November 2009, Bartlomiej Zolnierkiewicz wrote:
> > 
> > Hi,
> > 
> > The following patch series (against wireless-next) addresses issues raised
> > during code review and subsequently rejected by rt2x00/wireless/networking
> > maintainers.
> 
> Really stop reading only the half of emails, try reading it entirely (or at least don't
> stop at the second word in a sentence). It really starts the bug me to repeat
> myself over and over again because you refuse to read.
> 
> Your comments during code review were ACCEPTED with the only remark that
> it shouldn't be done right here and now.

Please stop this bullshit.  We have some standards for the upstream code
and by being maintainer you have to live up to this standards and make sure
that they are respected instead of watering them down yourself..

You were not interested even in fixing the headers duplication (it turned
out debugging scripts needed only 25 lines of code to be able to work with
fixed headers -- 25 LOC in bash scripts used only for debugging instead
of 1800 LOC of kernel code).

Also: I've mostly heard that I can fix the code myself.  Which I did.

> > The rewrite was quite conservative and there is still a room for improvement
> > but it should serve as a good starting base for all future work on rt2800
> > drivers, and there is a lot to do there (both drivers are still practically
> > non-functional).
> 
> Hence the reason I can use my rt2800usb device as long as I don't connect to
> a 11n AP. But since everybody in the world has 11n devices, the rt2800usb device
> is not capable of doing anything...

I use 11bg AP but mine rt2800usb device is RT3070 (which is quite popular
nowadays) and it simply doesn't even work with rt2800usb currently.

> > Comments and patches are welcomed.
> > 
> > 
> > The following changes since commit fa867e7355a1bdcd9bf7d55ebe9296f5b9c4028a:
> >   Juuso Oikarinen (1):
> >         wl1271: Generalize command response reading
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/bart/misc.git rt2800
> > 
> > Bartlomiej Zolnierkiewicz (40):
> >       rt2800usb: fix rt2800usb_rfcsr_read()
> >       rt2800pci: fix crypto in TX frame
> >       rt2800pci: fix comment about register access
> >       rt2800pci: fix comment about IV/EIV fields
> >       rt2x00: fix rt2x00usb_register_read() comment
> >       rt2800usb: use rt2x00usb_register_multiwrite() to set key entries
> [.. snip..]
> >       rt2800usb: fix comments in rt2800usb.h
> >       rt2800usb: add RXINFO_DESC_SIZE definition
> [..snip..]
> >       rt2800: fix comments in rt2800.h
> [..snip..]
> >       rt2x00: remove needless ifdefs from rt2x00leds.h
> 
> These 10 patches look sane enough. Please send them as patch series
> to linux-wireless.

I'll re-post later whole patch series to linux-wireless to ease the review.

> >       rt2x00: add support for different chipset interfaces
> 
> Not needed, you can determine exactly what chipset you have
> by looking at the other fields. So extending the structure to
> repeat the same information isn't needed.

It is a better to have a single field always indicating this since:
- combining information from other fields is complex and error-prone
- the situation may change in the future

However I would love to be proven wrong with the patch.

> >       MAINTAINERS: add rt2800 entry
> 
> I see you decided to take over the maintainership?

This is my kernel tree after all. :)

> Doesn't that need the current maintainer to move away, or was this part
> of the "going over other peoples head" plan?

I just do what is the best to get working drivers in the foreseeable future.

-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [PATCH 1/3] sysfs directory scaling: rbtree for dirent name lookups
From: Benjamin LaHaise @ 2009-11-03 21:52 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg KH, Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu,
	linux-kernel
In-Reply-To: <m1my338fbi.fsf@fess.ebiederm.org>

On Tue, Nov 03, 2009 at 01:32:33PM -0800, Eric W. Biederman wrote:
> Are your numbers from your application and are they real world?
> In which case they are interesting, but it would be good if
> we could also have microbenchmark numbers that just measure
> the sysfs costs.   If nothing else I am seeing a big startup
> overhead that isn't being subtracted out that makes it hard
> to see the real costs here.

They're application based, so there's a bunch of other overhead included 
that won't show up on a microbenchmark.  Each interface requires a round 
trip between 2 L2TP daemons, so there are lots of syscalls and other cache 
polluting effects that won't show up on a microbenchmark.  One of the L2TP 
daemons is configured not to instantiate any kernel state -- running in 
this mode, it has very little overhead.

The other thing to note is that the costs posted are how long it takes to 
add an additional 5,000 interfaces in the given range, not the total time 
to add say 35,000 interfaces (I didn't feel like waiting that long).

		-ben

^ permalink raw reply

* Re: [PATCH 1/3] sysfs directory scaling: rbtree for dirent name lookups
From: Benjamin LaHaise @ 2009-11-03 21:56 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg KH, Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu,
	linux-kernel
In-Reply-To: <m1y6mn708g.fsf@fess.ebiederm.org>

On Tue, Nov 03, 2009 at 01:43:43PM -0800, Eric W. Biederman wrote:
> I guess in particular what I would expect is that if we can do 35000
> interfaces in 63s with an O(N^2) algorithm.  Then we should be able to
> do 35000 interfaces with an O(NlogN) algorithm in under a second.
> Which for your application should make the time essentially flat in
> the number of interfaces.

That's the wrong way to interprete the numbers.  The 35000 number of 63s is 
the time that it takes 63s to add 5000 more interfaces in the 30,000 to 
35,000 range.  This includes the time required to add a point to point ip 
route on the interface and bring the interface up.

		-ben

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Ivo van Doorn @ 2009-11-03 22:01 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: linux-wireless, linux-kernel, netdev, Randy Dunlap, Luis Correia,
	John W. Linville, Ingo Molnar, Johannes Berg, Jarek Poplawski,
	Pekka Enberg, David Miller
In-Reply-To: <200911032244.11946.bzolnier@gmail.com>

> > > The following patch series (against wireless-next) addresses issues raised
> > > during code review and subsequently rejected by rt2x00/wireless/networking
> > > maintainers.
> > 
> > Really stop reading only the half of emails, try reading it entirely (or at least don't
> > stop at the second word in a sentence). It really starts the bug me to repeat
> > myself over and over again because you refuse to read.
> > 
> > Your comments during code review were ACCEPTED with the only remark that
> > it shouldn't be done right here and now.
> 
> Please stop this bullshit.  We have some standards for the upstream code
> and by being maintainer you have to live up to this standards and make sure
> that they are respected instead of watering them down yourself..
> 
> You were not interested even in fixing the headers duplication (it turned
> out debugging scripts needed only 25 lines of code to be able to work with
> fixed headers -- 25 LOC in bash scripts used only for debugging instead
> of 1800 LOC of kernel code).

Yeah I know that. But like I said, I still needed to get around to do that,
and I am very happy you were interested in fixing it.

> Also: I've mostly heard that I can fix the code myself.  Which I did.

And thats good.

> > > The rewrite was quite conservative and there is still a room for improvement
> > > but it should serve as a good starting base for all future work on rt2800
> > > drivers, and there is a lot to do there (both drivers are still practically
> > > non-functional).
> > 
> > Hence the reason I can use my rt2800usb device as long as I don't connect to
> > a 11n AP. But since everybody in the world has 11n devices, the rt2800usb device
> > is not capable of doing anything...
> 
> I use 11bg AP but mine rt2800usb device is RT3070 (which is quite popular
> nowadays) and it simply doesn't even work with rt2800usb currently.

Mine devices are plain rt2870 chips.

> > > Comments and patches are welcomed.
> > > 
> > > 
> > > The following changes since commit fa867e7355a1bdcd9bf7d55ebe9296f5b9c4028a:
> > >   Juuso Oikarinen (1):
> > >         wl1271: Generalize command response reading
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/bart/misc.git rt2800
> > > 
> > > Bartlomiej Zolnierkiewicz (40):
> > >       rt2800usb: fix rt2800usb_rfcsr_read()
> > >       rt2800pci: fix crypto in TX frame
> > >       rt2800pci: fix comment about register access
> > >       rt2800pci: fix comment about IV/EIV fields
> > >       rt2x00: fix rt2x00usb_register_read() comment
> > >       rt2800usb: use rt2x00usb_register_multiwrite() to set key entries
> > [.. snip..]
> > >       rt2800usb: fix comments in rt2800usb.h
> > >       rt2800usb: add RXINFO_DESC_SIZE definition
> > [..snip..]
> > >       rt2800: fix comments in rt2800.h
> > [..snip..]
> > >       rt2x00: remove needless ifdefs from rt2x00leds.h
> > 
> > These 10 patches look sane enough. Please send them as patch series
> > to linux-wireless.
> 
> I'll re-post later whole patch series to linux-wireless to ease the review.

Make them 2 series, the above can be the real [PATCH] (which I will ack directly),
and the others can be RFC's which can be reviewed/discussed further.

> > >       rt2x00: add support for different chipset interfaces
> > 
> > Not needed, you can determine exactly what chipset you have
> > by looking at the other fields. So extending the structure to
> > repeat the same information isn't needed.
> 
> It is a better to have a single field always indicating this since:
> - combining information from other fields is complex and error-prone
> - the situation may change in the future
> 
> However I would love to be proven wrong with the patch.

Well something that looks like this (as function in rt2800lib.h)

static inline is_rt2800pci(__chip)
{
	return
		(__chip->rt & 0xFF00) == 0x0600 ||
		(__chip->rt & 0xFF00) == 0x0700 ||
		__chip->rt == 0x2880 ||
		__chip->rt == RT3052;
}

You might even go a bit shorter by checking for USB instead:

static inline is_rt2800usb(__chip)
{
	return __chip->rt == RT2870
}

In rt2800lib you already know __chip->rt is part of
the rt2800 family, the is_rt2800usb() (or whatever name
you are going to give that function) is sufficient to know
if you are using PCI or USB.

And when that is not good enough, then please change the
field to only indicate PCI or USB. That way the field could be
used for other things in the future.

> > >       MAINTAINERS: add rt2800 entry
> > 
> > I see you decided to take over the maintainership?
> 
> This is my kernel tree after all. :)

Fine, but please keep this patch in your tree then. ;)

> > Doesn't that need the current maintainer to move away, or was this part
> > of the "going over other peoples head" plan?
> 
> I just do what is the best to get working drivers in the foreseeable future.

Thanks,

Ivo

^ permalink raw reply

* Re: [PATCH 1/3] sysfs directory scaling: rbtree for dirent name lookups
From: Eric Dumazet @ 2009-11-03 22:06 UTC (permalink / raw)
  To: Benjamin LaHaise
  Cc: Eric W. Biederman, Greg KH, Octavian Purdila, netdev,
	Cosmin Ratiu
In-Reply-To: <20091103215650.GT8227@kvack.org>

Benjamin LaHaise a écrit :
> On Tue, Nov 03, 2009 at 01:43:43PM -0800, Eric W. Biederman wrote:
>> I guess in particular what I would expect is that if we can do 35000
>> interfaces in 63s with an O(N^2) algorithm.  Then we should be able to
>> do 35000 interfaces with an O(NlogN) algorithm in under a second.
>> Which for your application should make the time essentially flat in
>> the number of interfaces.
> 
> That's the wrong way to interprete the numbers.  The 35000 number of 63s is 
> the time that it takes 63s to add 5000 more interfaces in the 30,000 to 
> 35,000 range.  This includes the time required to add a point to point ip 
> route on the interface and bring the interface up.

Speaking of pppol2tp, it seems /proc/net/pppol2tp is not safe,
with a strange two phases locking...

(We keep in struct pppol2tp_seq_data pointers to structures
that might have been freed between to read() syscalls)

^ permalink raw reply

* Re: [PATCH 1/3] sysfs directory scaling: rbtree for dirent name lookups
From: Eric W. Biederman @ 2009-11-03 22:18 UTC (permalink / raw)
  To: Benjamin LaHaise
  Cc: Greg KH, Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu,
	linux-kernel
In-Reply-To: <20091103215251.GS8227@kvack.org>

Benjamin LaHaise <bcrl@lhnet.ca> writes:

> On Tue, Nov 03, 2009 at 01:32:33PM -0800, Eric W. Biederman wrote:
>> Are your numbers from your application and are they real world?
>> In which case they are interesting, but it would be good if
>> we could also have microbenchmark numbers that just measure
>> the sysfs costs.   If nothing else I am seeing a big startup
>> overhead that isn't being subtracted out that makes it hard
>> to see the real costs here.
>
> They're application based, so there's a bunch of other overhead included 
> that won't show up on a microbenchmark.  Each interface requires a round 
> trip between 2 L2TP daemons, so there are lots of syscalls and other cache 
> polluting effects that won't show up on a microbenchmark.  One of the L2TP 
> daemons is configured not to instantiate any kernel state -- running in 
> this mode, it has very little overhead.
>
> The other thing to note is that the costs posted are how long it takes to 
> add an additional 5,000 interfaces in the given range, not the total time 
> to add say 35,000 interfaces (I didn't feel like waiting that long).

Ok.  That makes a lot more sense.  The times you posted ideally would be flat
but they go up from 12s to 60s.

Eric

^ permalink raw reply

* Re: [PATCH 1/3] sysfs directory scaling: rbtree for dirent name lookups
From: Eric W. Biederman @ 2009-11-03 22:28 UTC (permalink / raw)
  To: Greg KH
  Cc: Eric Dumazet, Benjamin LaHaise, Octavian Purdila, netdev,
	Cosmin Ratiu, linux-kernel
In-Reply-To: <20091103160715.GD23857@kroah.com>

Greg KH <greg@kroah.com> writes:

> On Tue, Nov 03, 2009 at 07:14:33AM +0100, Eric Dumazet wrote:
>> Greg KH a ?crit :
>> > On Sun, Nov 01, 2009 at 11:31:30AM -0500, Benjamin LaHaise wrote:
>> >> Use an rbtree in sysfs_dirent to speed up file lookup times
>> >>
>> >> Systems with large numbers (tens of thousands and more) of network 
>> >> interfaces stress the sysfs code in ways that make the linear search for 
>> >> a name match take far too long.  Avoid this by using an rbtree.
>> > 
>> > What kind of speedups are you seeing here?  And do these changes cause a
>> > memory increase due to the structure changes which outweigh the
>> > speedups?
>> > 
>> > What kind of test are you doing to reproduce this?
>> > 
>> 
>> Its curious because in my tests the biggest problems come from
>> kernel/sysctl.c (__register_sysctl_paths) consuming 80% of cpu
>> in following attempt to create 20.000 devices
>> 
>> (disable hotplug before trying this, and ipv6 too !)
>> modprobe dummy numdummies=20000
>> 
>> I believe we should address __register_sysctl_paths() scalability
>> problems too.
>
> But registering 20000 devices is a far different problem from using
> those 20000 devices :)
>
> I think the "use the device" path should be the one we care the most
> about fixing up, as that is much more common than the register path for
> all users.

Definitely.  Of the three proc sysctl and sysfs.  sysctl tends to have
the worst costs across the board.  They are all rarely used so a lot
of what gets hit when scaling are rare path events that even the most
horrible code works fine on small systems.

Usually slow registration times indicate an O(N^2) or worse data
structure for filename lookup.

Eric

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Bartlomiej Zolnierkiewicz @ 2009-11-03 22:34 UTC (permalink / raw)
  To: Ivo van Doorn
  Cc: linux-wireless, linux-kernel, netdev, Randy Dunlap, Luis Correia,
	John W. Linville, Ingo Molnar, Johannes Berg, Jarek Poplawski,
	Pekka Enberg, David Miller
In-Reply-To: <200911032301.32907.IvDoorn@gmail.com>

On Tuesday 03 November 2009 23:01:32 Ivo van Doorn wrote:
> > > > The following patch series (against wireless-next) addresses issues raised
> > > > during code review and subsequently rejected by rt2x00/wireless/networking
> > > > maintainers.
> > > 
> > > Really stop reading only the half of emails, try reading it entirely (or at least don't
> > > stop at the second word in a sentence). It really starts the bug me to repeat
> > > myself over and over again because you refuse to read.
> > > 
> > > Your comments during code review were ACCEPTED with the only remark that
> > > it shouldn't be done right here and now.
> > 
> > Please stop this bullshit.  We have some standards for the upstream code
> > and by being maintainer you have to live up to this standards and make sure
> > that they are respected instead of watering them down yourself..
> > 
> > You were not interested even in fixing the headers duplication (it turned
> > out debugging scripts needed only 25 lines of code to be able to work with
> > fixed headers -- 25 LOC in bash scripts used only for debugging instead
> > of 1800 LOC of kernel code).
> 
> Yeah I know that. But like I said, I still needed to get around to do that,
> and I am very happy you were interested in fixing it.

Lets make one thing clear: YOU SHOULD BE THE ONE FIXING IT.

I'm not in slightest interested in wasting my time on such
things and educating some maintainers about basics.

[ Code duplication is bad, mmm'okay?  Just say no, mmm'okay? ]

> > Also: I've mostly heard that I can fix the code myself.  Which I did.
> 
> And thats good.
> 
> > > > The rewrite was quite conservative and there is still a room for improvement
> > > > but it should serve as a good starting base for all future work on rt2800
> > > > drivers, and there is a lot to do there (both drivers are still practically
> > > > non-functional).
> > > 
> > > Hence the reason I can use my rt2800usb device as long as I don't connect to
> > > a 11n AP. But since everybody in the world has 11n devices, the rt2800usb device
> > > is not capable of doing anything...
> > 
> > I use 11bg AP but mine rt2800usb device is RT3070 (which is quite popular
> > nowadays) and it simply doesn't even work with rt2800usb currently.
> 
> Mine devices are plain rt2870 chips.
> 
> > > > Comments and patches are welcomed.
> > > > 
> > > > 
> > > > The following changes since commit fa867e7355a1bdcd9bf7d55ebe9296f5b9c4028a:
> > > >   Juuso Oikarinen (1):
> > > >         wl1271: Generalize command response reading
> > > > 
> > > > are available in the git repository at:
> > > > 
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/bart/misc.git rt2800
> > > > 
> > > > Bartlomiej Zolnierkiewicz (40):
> > > >       rt2800usb: fix rt2800usb_rfcsr_read()
> > > >       rt2800pci: fix crypto in TX frame
> > > >       rt2800pci: fix comment about register access
> > > >       rt2800pci: fix comment about IV/EIV fields
> > > >       rt2x00: fix rt2x00usb_register_read() comment
> > > >       rt2800usb: use rt2x00usb_register_multiwrite() to set key entries
> > > [.. snip..]
> > > >       rt2800usb: fix comments in rt2800usb.h
> > > >       rt2800usb: add RXINFO_DESC_SIZE definition
> > > [..snip..]
> > > >       rt2800: fix comments in rt2800.h
> > > [..snip..]
> > > >       rt2x00: remove needless ifdefs from rt2x00leds.h
> > > 
> > > These 10 patches look sane enough. Please send them as patch series
> > > to linux-wireless.
> 
> > I'll re-post later whole patch series to linux-wireless to ease the review.
> 
> Make them 2 series, the above can be the real [PATCH] (which I will ack directly),
> and the others can be RFC's which can be reviewed/discussed further.

Please at least read all patches before even starting making such comments:

i.e. "rt2800: fix comments in rt2800.h" depends on
"rt2800: fix duplication in header files"

and redoing it would be plain waste of time.

Besides for now I'm more interested in working on improving drivers further
than making artificial patch splits.

> > > >       rt2x00: add support for different chipset interfaces
> > > 
> > > Not needed, you can determine exactly what chipset you have
> > > by looking at the other fields. So extending the structure to
> > > repeat the same information isn't needed.
> > 
> > It is a better to have a single field always indicating this since:
> > - combining information from other fields is complex and error-prone
> > - the situation may change in the future
> > 
> > However I would love to be proven wrong with the patch.
> 
> Well something that looks like this (as function in rt2800lib.h)
> 
> static inline is_rt2800pci(__chip)
> {
> 	return
> 		(__chip->rt & 0xFF00) == 0x0600 ||
> 		(__chip->rt & 0xFF00) == 0x0700 ||
> 		__chip->rt == 0x2880 ||
> 		__chip->rt == RT3052;
> }
> 
> You might even go a bit shorter by checking for USB instead:
> 
> static inline is_rt2800usb(__chip)
> {
> 	return __chip->rt == RT2870
> }

Which can turn into maintenance nightmare as might need update
each time new chipset version is added and is error-prone.

Setting chipset interface from the driver itself is much more
maintainer friendly.

> In rt2800lib you already know __chip->rt is part of
> the rt2800 family, the is_rt2800usb() (or whatever name
> you are going to give that function) is sufficient to know
> if you are using PCI or USB.
> 
> And when that is not good enough, then please change the
> field to only indicate PCI or USB. That way the field could be
> used for other things in the future.

Please explain what do you mean by that.  The field in question is enum:

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] rt2x00: add support for different chipset interfaces

[ Impact: rt2x00 infrastructure enhancement ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00.h |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Index: b/drivers/net/wireless/rt2x00/rt2x00.h
===================================================================
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -144,6 +144,11 @@ struct avg_val {
 	int avg_weight;
 };
 
+enum rt2x00_chip_intf {
+	RT2X00_CHIP_INTF_PCI,
+	RT2X00_CHIP_INTF_USB,
+};
+
 /*
  * Chipset identification
  * The chipset on the device is composed of a RT and RF chip.
@@ -169,6 +174,8 @@ struct rt2x00_chip {
 
 	u16 rf;
 	u32 rev;
+
+	enum rt2x00_chip_intf intf;
 };
 
 /*
@@ -937,6 +944,18 @@ static inline bool rt2x00_check_rev(cons
 	return ((chipset->rev & mask) == rev);
 }
 
+static inline void rt2x00_set_chip_intf(struct rt2x00_dev *rt2x00dev,
+					enum rt2x00_chip_intf intf)
+{
+	rt2x00dev->chip.intf = intf;
+}
+
+static inline bool rt2x00_intf(const struct rt2x00_chip *chipset,
+			       enum rt2x00_chip_intf intf)
+{
+	return (chipset->intf == intf);
+}
+
 /**
  * rt2x00queue_map_txskb - Map a skb into DMA for TX purposes.
  * @rt2x00dev: Pointer to &struct rt2x00_dev.


-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: William Allen Simpson @ 2009-11-03 22:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Linux Kernel Developers, Linux Kernel Network Developers
In-Reply-To: <4AEDCD7C.2010403@gmail.com>

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

Eric Dumazet wrote:
> This patch looks fine, but I dont see how this new function is used.
> 
> Some points :
> 
> 1) We are working hard to remove rwlocks from network stack, so please dont
> add a new one. You probably can use a seqlock or RCU, or a server handling 
> 10.000 connections request per second on many NIC will hit this rwlock.
> 
This is my attempt at using RCU, as seqlock didn't seem to apply (and is
missing any Documentation.)

After the discussion about context, one question that I have is the need
for the _bh suffix?

+		rcu_read_lock_bh();
+		memcpy(&xvp->cookie_bakery[0],
+		       &rcu_dereference(tcp_secret_generating)->secrets[0],
+		       sizeof(tcp_secret_generating->secrets));
+		rcu_read_unlock_bh();


Documentation/RCU/checklist.txt #7 says:

   One exception to this rule: rcu_read_lock() and rcu_read_unlock()
   may be substituted for rcu_read_lock_bh() and rcu_read_unlock_bh()
   in cases where local bottom halves are already known to be
   disabled, for example, in irq or softirq context.  Commenting
   such cases is a must, of course!  And the jury is still out on
   whether the increased speed is worth it.

[-- Attachment #2: TCPCT+1d1.patch --]
[-- Type: text/plain, Size: 7145 bytes --]

diff --git a/include/linux/cryptohash.h b/include/linux/cryptohash.h
index c118b2a..ec78a4b 100644
--- a/include/linux/cryptohash.h
+++ b/include/linux/cryptohash.h
@@ -2,6 +2,7 @@
 #define __CRYPTOHASH_H
 
 #define SHA_DIGEST_WORDS 5
+#define SHA_MESSAGE_BYTES (512 /*bits*/ / 8)
 #define SHA_WORKSPACE_WORDS 80
 
 void sha_init(__u32 *buf);
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 51b7426..f669c43 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1526,12 +1526,18 @@ static inline int tcp_s_data_size(const struct tcp_sock *tp)
 		: 0;
 }
 
+/* Using SHA1 for now, define some constants.
+ */
+#define COOKIE_DIGEST_WORDS (SHA_DIGEST_WORDS)
+#define COOKIE_MESSAGE_WORDS (SHA_MESSAGE_BYTES / 4)
+#define COOKIE_WORKSPACE_WORDS (COOKIE_DIGEST_WORDS + COOKIE_MESSAGE_WORDS)
+
 /* As tcp_request_sock has already been extended in other places, the
  * only remaining method is to pass stack values along as function
  * parameters.  These parameters are not needed after sending SYNACK.
  */
 struct tcp_extend_values {
-	u8				cookie_bakery[TCP_COOKIE_MAX];
+	u32				cookie_bakery[COOKIE_WORKSPACE_WORDS];
 	u8				cookie_plus;
 	u8				cookie_in_always:1,
 					cookie_out_never:1;
@@ -1542,6 +1548,8 @@ static inline struct tcp_extend_values *tcp_xv(const struct request_values *rvp)
 	return (struct tcp_extend_values *)rvp;
 }
 
+extern int tcp_cookie_generator(struct tcp_extend_values *xvp);
+
 extern void tcp_v4_init(void);
 extern void tcp_init(void);
 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 12409df..a8c5d99 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -264,6 +264,7 @@
 #include <linux/cache.h>
 #include <linux/err.h>
 #include <linux/crypto.h>
+#include <linux/time.h>
 
 #include <net/icmp.h>
 #include <net/tcp.h>
@@ -2933,6 +2934,126 @@ EXPORT_SYMBOL(tcp_md5_hash_key);
 
 #endif
 
+/**
+ * Each Responder maintains up to two secret values concurrently for
+ * efficient secret rollover.  Each secret value has 4 states:
+ *
+ * Generating.  (tcp_secret_generating != tcp_secret_primary)
+ *    Generates new Responder-Cookies, but not yet used for primary
+ *    verification.  This is a short-term state, typically lasting only
+ *    one round trip time (RTT).
+ *
+ * Primary.  (tcp_secret_generating == tcp_secret_primary)
+ *    Used both for generation and primary verification.
+ *
+ * Retiring.  (tcp_secret_retiring != tcp_secret_secondary)
+ *    Used for verification, until the first failure that can be
+ *    verified by the newer Generating secret.  At that time, this
+ *    cookie's state is changed to Secondary, and the Generating
+ *    cookie's state is changed to Primary.  This is a short-term state,
+ *    typically lasting only one round trip time (RTT).
+ *
+ * Secondary.  (tcp_secret_retiring == tcp_secret_secondary)
+ *    Used for secondary verification, after primary verification
+ *    failures.  This state lasts no more than twice the Maximum Segment
+ *    Lifetime (2MSL).  Then, the secret is discarded.
+ */
+struct tcp_cookie_secret {
+	/* The secret is divided into two parts.  The digest part is the
+	 * equivalent of previously hashing a secret and saving the state,
+	 * and serves as an initialization vector (IV).  The message part
+	 * serves as the trailing secret.
+	 */
+	u32				secrets[COOKIE_WORKSPACE_WORDS];
+	unsigned long			expires;
+};
+
+#define TCP_SECRET_1MSL (HZ * TCP_PAWS_MSL)
+#define TCP_SECRET_2MSL (HZ * TCP_PAWS_MSL * 2)
+#define TCP_SECRET_LIFE (HZ * 600)
+
+static struct tcp_cookie_secret tcp_secret_one;
+static struct tcp_cookie_secret tcp_secret_two;
+
+/* Essentially a circular list, without dynamic allocation. */
+static struct tcp_cookie_secret *tcp_secret_generating;
+static struct tcp_cookie_secret *tcp_secret_primary;
+static struct tcp_cookie_secret *tcp_secret_retiring;
+static struct tcp_cookie_secret *tcp_secret_secondary;
+
+static DEFINE_SPINLOCK(tcp_secret_locker);
+
+/* Fill cookie_bakery with current generator, updating as needed.
+ * Only called in softirq context.
+ * Returns: 0 for success.
+ */
+int tcp_cookie_generator(struct tcp_extend_values *xvp)
+{
+	unsigned long jiffy = jiffies;
+
+	if (unlikely(time_after(jiffy, tcp_secret_generating->expires))) {
+		u32 secrets[COOKIE_WORKSPACE_WORDS];
+
+		spin_lock(&tcp_secret_locker);
+		if (!time_after(jiffy, tcp_secret_generating->expires)) {
+			/* refreshed by another */
+			spin_unlock(&tcp_secret_locker);
+			memcpy(&xvp->cookie_bakery[0],
+			       &tcp_secret_generating->secrets[0],
+			       sizeof(tcp_secret_generating->secrets));
+		} else {
+			/* still needs refreshing */
+			get_random_bytes(secrets, sizeof(secrets));
+
+			/* The first time, paranoia assumes that the
+			 * randomization function isn't as strong.  But,
+			 * this secret initialization is delayed until
+			 * the last possible moment (packet arrival).
+			 * Although that time is observable, it is
+			 * unpredictably variable.  Mash in the most
+			 * volatile clock bits available, and expire the
+			 * secret extra quickly.
+			 */
+			if (unlikely(tcp_secret_primary->expires ==
+				     tcp_secret_secondary->expires)) {
+				struct timespec tv;
+	
+				getnstimeofday(&tv);
+				secrets[COOKIE_DIGEST_WORDS+0] ^= (u32)tv.tv_nsec;
+				tcp_secret_secondary->expires = jiffy
+							      + TCP_SECRET_1MSL;
+			} else {
+				tcp_secret_secondary->expires = jiffy
+							      + TCP_SECRET_LIFE;
+				tcp_secret_primary->expires = jiffy
+							    + TCP_SECRET_2MSL;
+			}
+			memcpy(&tcp_secret_secondary->secrets[0],
+			       &secrets[0],
+			       sizeof(secrets));
+
+			rcu_assign_pointer(tcp_secret_generating,
+					   tcp_secret_secondary);
+			rcu_assign_pointer(tcp_secret_retiring,
+					   tcp_secret_primary);
+			spin_unlock(&tcp_secret_locker);
+			/* call_rcu() or synchronize_rcu() not needed. */
+
+			memcpy(&xvp->cookie_bakery[0],
+			       &secrets[0],
+			       sizeof(secrets));
+		}
+	} else {
+		rcu_read_lock_bh();
+		memcpy(&xvp->cookie_bakery[0],
+		       &rcu_dereference(tcp_secret_generating)->secrets[0],
+		       sizeof(tcp_secret_generating->secrets));
+		rcu_read_unlock_bh();
+	}
+	return 0;
+}
+EXPORT_SYMBOL(tcp_cookie_generator);
+
 void tcp_done(struct sock *sk)
 {
 	if (sk->sk_state == TCP_SYN_SENT || sk->sk_state == TCP_SYN_RECV)
@@ -2967,6 +3088,7 @@ void __init tcp_init(void)
 	struct sk_buff *skb = NULL;
 	unsigned long nr_pages, limit;
 	int order, i, max_share;
+	unsigned long jiffy = jiffies;
 
 	BUILD_BUG_ON(sizeof(struct tcp_skb_cb) > sizeof(skb->cb));
 
@@ -3060,6 +3182,15 @@ void __init tcp_init(void)
 	       tcp_hashinfo.ehash_mask + 1, tcp_hashinfo.bhash_size);
 
 	tcp_register_congestion_control(&tcp_reno);
+
+	memset(&tcp_secret_one.secrets[0], 0, sizeof(tcp_secret_one.secrets));
+	memset(&tcp_secret_two.secrets[0], 0, sizeof(tcp_secret_two.secrets));
+	tcp_secret_one.expires = jiffy; /* past due */
+	tcp_secret_two.expires = jiffy; /* past due */
+	tcp_secret_generating = &tcp_secret_one;
+	tcp_secret_primary = &tcp_secret_one;
+	tcp_secret_retiring = &tcp_secret_two;
+	tcp_secret_secondary = &tcp_secret_two;
 }
 
 EXPORT_SYMBOL(tcp_close);

^ permalink raw reply related

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Jarek Poplawski @ 2009-11-03 23:00 UTC (permalink / raw)
  To: Changli Gao; +Cc: Jamal Hadi Salim, devik, netdev
In-Reply-To: <412e6f7f0911030518w7a5f02a4ue8a4b6539496dd8f@mail.gmail.com>

On Tue, Nov 03, 2009 at 09:18:49PM +0800, Changli Gao wrote:
> On Tue, Nov 3, 2009 at 6:05 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> >
> > The ceil specification is controlled only by ctokens, which are always
> > updated, so no such risk.
> >
> Nevertheless, updating tokens is necessary too.

If it's really necessary you should present some test case fixed by
your patch, I guess.

In the meantime let's consider what could be broken:
class 1:1 (parent) rate 10 packets/sec
class 1:2 rate 5 packets/sec ceil 10 packets/sec
class 1:3 rate 5 packets/sec ceil 10 packets/sec

class 1:2 doesn't use all its rate, and sends every other second
	(in even seconds)
class 1:3 sends 10 packets during the first second, so with your
	patch it will use its tokens for 2 seconds
class 1:2 uses its rate in the second second..., so class 1:1
	can't lend anything
class 1:3 can only borrow, so it won't be able to send during
	this second anything

So, the effect would be class 1:3 sending every odd second 10 packets
while every even second - nothing...

Of course the example is a simplification of the htb algorithm. There
should be also considered priority of borrowing, which should add
similar effects even with regular traffic. But generally the
guaranteed rates would be simply...  less guaranteed.

Regards,
Jarek P.

^ permalink raw reply

* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: Eric Dumazet @ 2009-11-03 23:03 UTC (permalink / raw)
  To: William Allen Simpson
  Cc: Linux Kernel Developers, Linux Kernel Network Developers
In-Reply-To: <4AF0B0D2.4030905@gmail.com>

William Allen Simpson a écrit :
> Eric Dumazet wrote:
>> This patch looks fine, but I dont see how this new function is used.
>>
>> Some points :
>>
>> 1) We are working hard to remove rwlocks from network stack, so please
>> dont
>> add a new one. You probably can use a seqlock or RCU, or a server
>> handling 10.000 connections request per second on many NIC will hit
>> this rwlock.
>>
> This is my attempt at using RCU, as seqlock didn't seem to apply (and is
> missing any Documentation.)
> 

That seems very good, thanks, we can sort out details later, when full picture
is available.

> After the discussion about context, one question that I have is the need
> for the _bh suffix?
> 
> +        rcu_read_lock_bh();
> +        memcpy(&xvp->cookie_bakery[0],
> +               &rcu_dereference(tcp_secret_generating)->secrets[0],
> +               sizeof(tcp_secret_generating->secrets));
> +        rcu_read_unlock_bh();
> 

Well, you dont need to disable BH in this code running in softirq context only.

Just use rcu_read_lock() (like you use spin_lock() in same function/context)


> 
> Documentation/RCU/checklist.txt #7 says:
> 
>   One exception to this rule: rcu_read_lock() and rcu_read_unlock()
>   may be substituted for rcu_read_lock_bh() and rcu_read_unlock_bh()
>   in cases where local bottom halves are already known to be
>   disabled, for example, in irq or softirq context.  Commenting
>   such cases is a must, of course!  And the jury is still out on
>   whether the increased speed is worth it.
> 

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Gertjan van Wingerde @ 2009-11-03 23:09 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Ivo van Doorn, linux-wireless, linux-kernel, netdev, Randy Dunlap,
	Luis Correia, John W. Linville, Ingo Molnar, Johannes Berg,
	Jarek Poplawski, Pekka Enberg, David Miller
In-Reply-To: <200911032334.40547.bzolnier@gmail.com>

On Tue, Nov 3, 2009 at 11:34 PM, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:
> On Tuesday 03 November 2009 23:01:32 Ivo van Doorn wrote:
>> > > > The following patch series (against wireless-next) addresses issues raised
>> > > > during code review and subsequently rejected by rt2x00/wireless/networking
>> > > > maintainers.
>> > >
>> > > Really stop reading only the half of emails, try reading it entirely (or at least don't
>> > > stop at the second word in a sentence). It really starts the bug me to repeat
>> > > myself over and over again because you refuse to read.
>> > >
>> > > Your comments during code review were ACCEPTED with the only remark that
>> > > it shouldn't be done right here and now.
>> >
>> > Please stop this bullshit.  We have some standards for the upstream code
>> > and by being maintainer you have to live up to this standards and make sure
>> > that they are respected instead of watering them down yourself..
>> >
>> > You were not interested even in fixing the headers duplication (it turned
>> > out debugging scripts needed only 25 lines of code to be able to work with
>> > fixed headers -- 25 LOC in bash scripts used only for debugging instead
>> > of 1800 LOC of kernel code).
>>
>> Yeah I know that. But like I said, I still needed to get around to do that,
>> and I am very happy you were interested in fixing it.
>
> Lets make one thing clear: YOU SHOULD BE THE ONE FIXING IT.
>
> I'm not in slightest interested in wasting my time on such
> things and educating some maintainers about basics.
>
> [ Code duplication is bad, mmm'okay?  Just say no, mmm'okay? ]
>

Bart,

Are you really interested in working with us (the rt2x00 project) in
getting the rt2800{pci,usb}
drivers in a better shape, or do you just want to continue your
ramblings on how bad you
think the rt2x00 maintainers, wireless maintainer, and networking
maintainer are in your view?

Just continuing these discussions doesn't help a bit as Ivo, John, and
David said they disagreed
with you on this topic.

If you just want to continue with a hostile take-over of the rt2800
maintainership, then please
let us know that, so that we stop spending time on useless
discussions, and let John Linville
decide how he wants to handle this situation. It would be a shame of
the good patches and work
you did, but if that's the case, than that's it.

Otherwise, please focus on the technical contents of the patches and
work with us to get
these drivers in a better shape.

---
Gertjan.


,

^ permalink raw reply

* Re: [Bugme-new] [Bug 14450] New: [2.6.31] Network interfaces are dead with 2.6.31 (iwlagn and sky2)
From: Andrew Morton @ 2009-11-03 23:21 UTC (permalink / raw)
  To: orion.linbug
  Cc: bugzilla-daemon, bugme-daemon, netdev, linux-acpi,
	Rafael J. Wysocki, linux-wireless
In-Reply-To: <bug-14450-10286@http.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Wed, 21 Oct 2009 07:30:12 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=14450
> 
>            Summary: [2.6.31] Network interfaces are dead with 2.6.31
>                     (iwlagn and sky2)
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.31

Rafael, this is a 2.6.30->2.6.31 regression.


>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: high
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: orion.linbug@gmail.com
>         Regression: Yes
> 
> 
> My wireless and ethernet networks are dead with the kernel 2.6.31. All work
> very fine with the 2.6.30 and older. I have a Toshiba laptop wich uses the
> iwlagn module for wireless and the sky2 for ethernet. I have tested with
> acpi=off in the grub kernel line but it freezes the system on boot...
> ifconfig gets only the lo interface. ifconfig eth0 or wlan0 up gets something
> like "the interface doesn't exist".
> 
> lspci shows:
> 00:00.0 Host bridge: Intel Corporation Mobile 4 Series Chipset Memory
> Controller Hub (rev 07)                                                         
> 00:01.0 PCI bridge: Intel Corporation Mobile 4 Series Chipset PCI Express
> Graphics Port (rev 07)                                                          
> 00:1a.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
> Controller #4 (rev 03)                                                          
> 00:1a.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
> Controller #5 (rev 03)                                                          
> 00:1a.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
> Controller #6 (rev 03)
> 00:1a.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
> Controller #2 (rev 03)
> 00:1b.0 Audio device: Intel Corporation 82801I (ICH9 Family) HD Audio
> Controller (rev 03)
> 00:1c.0 PCI bridge: Intel Corporation 82801I (ICH9 Family) PCI Express Port 1
> (rev 03)
> 00:1c.1 PCI bridge: Intel Corporation 82801I (ICH9 Family) PCI Express Port 2
> (rev 03)
> 00:1c.3 PCI bridge: Intel Corporation 82801I (ICH9 Family) PCI Express Port 4
> (rev 03)
> 00:1d.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
> Controller #1 (rev 03)
> 00:1d.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
> Controller #2 (rev 03)
> 00:1d.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
> Controller #3 (rev 03)
> 00:1d.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
> Controller #1 (rev 03)
> 00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev 93)
> 00:1f.0 ISA bridge: Intel Corporation ICH9M LPC Interface Controller (rev 03)
> 00:1f.2 SATA controller: Intel Corporation ICH9M/M-E SATA AHCI Controller (rev
> 03)
> 00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 03)
> 01:00.0 VGA compatible controller: ATI Technologies Inc M92 [Mobility Radeon HD
> 4500 Series]
> 01:00.1 Audio device: ATI Technologies Inc R700 Audio Device [Radeon HD 4000
> Series]
> 02:00.0 Network controller: Intel Corporation Wireless WiFi Link 5100
> 03:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8040T PCI-E Fast
> Ethernet Controller (rev 12)
> 0a:01.0 FireWire (IEEE 1394): O2 Micro, Inc. Firewire (IEEE 1394) (rev 02)
> 0a:01.2 SD Host controller: O2 Micro, Inc. Integrated MMC/SD Controller (rev
> 02)
> 0a:01.3 Mass storage controller: O2 Micro, Inc. Integrated MS/xD Controller
> (rev 01)
> 
> Dmesg shows errors on sky2 and iwlagn driver:
> iwlagn: Intel(R) Wireless WiFi Link AGN driver for Linux, 1.3.27ks2009-10-13
> 17:57:36
> iwlagn: Copyright(c) 2003-2009 Intel Corporation2009-10-13 17:57:36
> iwlagn 0000:02:00.0: enabling device (0000 -> 0002)2009-10-13 17:57:36    
> iwlagn 0000:02:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 162009-10-13
> 17:57:36
> iwlagn 0000:02:00.0: Detected Intel Wireless WiFi Link 5100AGN
> REV=0xFFFFFFFF2009-10-13 17:57:36
> Clocksource tsc unstable (delta = -212055894 ns)2009-10-13 17:57:36
> iwlagn 0000:02:00.0: Failed, HW not ready2009-10-13 17:57:36
> iwlagn 0000:02:00.0: PCI INT A disabled2009-10-13 17:57:36
> 
> sky2 driver version 1.232009-10-13 17:50:48
> sky2 0000:03:00.0: enabling device (0000 -> 0003)2009-10-13 17:50:48
> sky2 0000:03:00.0: PCI INT A -> GSI 17 (level, low) -> IRQ 172009-10-13
> 17:50:48
> sky2 0000:03:00.0: PCI INT A disabled2009-10-13 17:50:48
> sky2: probe of 0000:03:00.0 failed with error -952009-10-13 17:50:48

Given that both the wired and wireless interfaces failed, I'd assume
that something went wrong at the bus level.  Perhaps due to an ACPI
change.

Could the sky2, iwlagn and ACPI developers please take a look, offer an
opinion as to where we should be looking?

Thanks.


^ permalink raw reply

* Re: [Bugme-new] [Bug 14451] New: b44 BCM4401 immediate disconnect and power down after ip assignment
From: Andrew Morton @ 2009-11-03 23:23 UTC (permalink / raw)
  To: Gary Zambrano; +Cc: bugzilla-daemon, bugme-daemon, netdev, andreas
In-Reply-To: <bug-14451-10286@http.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Wed, 21 Oct 2009 07:38:13 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=14451
> 
>            Summary: b44 BCM4401 immediate disconnect and power down after
>                     ip assignment
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.31.4
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: high
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: andreas@schipplock.de
>         Regression: No
> 
> 
> Hi,
> I compiled the linux kernel 2.6.31.4 and after "dhcpcd" wanted to assign an ip,
> it worked for 500ms and I then got the following message being repeated a _lot_
> in dmesg:
> 
> b44: eth0: powering down PHY
> b44: eth0: Link is down.
> b44: eth0: Link is up at 100 Mbps, full duplex.
> b44: eth0: Flow control is off for TX and off for RX.
> b44: eth0: powering down PHY
> b44: eth0: Link is down.
> b44: eth0: Link is up at 100 Mbps, full duplex.
> b44: eth0: Flow control is off for TX and off for RX.
> 
> These messages repeat endlessly and I can't establish a connection. 
> 
> The exact name of the ethernet controller is: "08:00.0 Ethernet controller:
> Broadcom Corporation BCM4401-B0 100Base-TX (rev 02)" regarding lspci output.
> 
> When I execute "ifconfig" every 500ms I can see that the device "eth0" gets a
> valid ip but the next time I execute ifconfig the ip is already "gone" and I
> again get the the messages posted above in dmesg. 
> 
> I first thought was that I have a media problem so I replaced all my wires but
> the problem still exists and the problem doesn't appear on the 2.6.28.x kernel
> which I ran before. I even installed linux mint in a hurry to see if it's
> really not a hardware issue and it worked there without a problem (using kernel
> 2.6.28-11, that's how they call it).
> 
> I've got no further idea. I created three different config files from scratch
> using menuconfig. In the end I even thought other modules are conflicting and I
> adapted the config so it only fits for my specific hardware which gave me some
> good boot time but no success regarding the bcm4401 issue. 
> 
> Any help is appreciated. 
> 


^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Bartlomiej Zolnierkiewicz @ 2009-11-03 23:46 UTC (permalink / raw)
  To: Gertjan van Wingerde
  Cc: Ivo van Doorn, linux-wireless, linux-kernel, netdev, Randy Dunlap,
	Luis Correia, John W. Linville, Ingo Molnar, Johannes Berg,
	Jarek Poplawski, Pekka Enberg, David Miller
In-Reply-To: <14add3d10911031509l328e7674kb625b70250ff30fb@mail.gmail.com>

On Wednesday 04 November 2009 00:09:02 Gertjan van Wingerde wrote:
> On Tue, Nov 3, 2009 at 11:34 PM, Bartlomiej Zolnierkiewicz
> <bzolnier@gmail.com> wrote:
> > On Tuesday 03 November 2009 23:01:32 Ivo van Doorn wrote:
> >> > > > The following patch series (against wireless-next) addresses issues raised
> >> > > > during code review and subsequently rejected by rt2x00/wireless/networking
> >> > > > maintainers.
> >> > >
> >> > > Really stop reading only the half of emails, try reading it entirely (or at least don't
> >> > > stop at the second word in a sentence). It really starts the bug me to repeat
> >> > > myself over and over again because you refuse to read.
> >> > >
> >> > > Your comments during code review were ACCEPTED with the only remark that
> >> > > it shouldn't be done right here and now.
> >> >
> >> > Please stop this bullshit.  We have some standards for the upstream code
> >> > and by being maintainer you have to live up to this standards and make sure
> >> > that they are respected instead of watering them down yourself..
> >> >
> >> > You were not interested even in fixing the headers duplication (it turned
> >> > out debugging scripts needed only 25 lines of code to be able to work with
> >> > fixed headers -- 25 LOC in bash scripts used only for debugging instead
> >> > of 1800 LOC of kernel code).
> >>
> >> Yeah I know that. But like I said, I still needed to get around to do that,
> >> and I am very happy you were interested in fixing it.
> >
> > Lets make one thing clear: YOU SHOULD BE THE ONE FIXING IT.
> >
> > I'm not in slightest interested in wasting my time on such
> > things and educating some maintainers about basics.
> >
> > [ Code duplication is bad, mmm'okay?  Just say no, mmm'okay? ]
> >
> 
> Bart,
> 
> Are you really interested in working with us (the rt2x00 project) in
> getting the rt2800{pci,usb}
> drivers in a better shape, or do you just want to continue your
> ramblings on how bad you
> think the rt2x00 maintainers, wireless maintainer, and networking
> maintainer are in your view?
> 
> Just continuing these discussions doesn't help a bit as Ivo, John, and
> David said they disagreed
> with you on this topic.

I tried explain many times that it is not about what is in MAINTAINERS
file or what somebody says.

> If you just want to continue with a hostile take-over of the rt2800
> maintainership, then please
> let us know that, so that we stop spending time on useless

I fail to see why you see it as a hostile takeover.

I just did what should have been done in the first place (+ I'm going
to push drivers further in this direction in my tree) and I was always
pretty clear that once staging drivers become sufficiently cleaned up
I would start re-basing my efforts on in kernel drivers.

I will be glad to cooperate with you or anyone else from rt2x00 project.
However I will not spin in some stupid bureaucracy when I see that things
can be done more effectively.

> discussions, and let John Linville
> decide how he wants to handle this situation. It would be a shame of
> the good patches and work
> you did, but if that's the case, than that's it.

John can just pull my tree in right now since it is based on his tree
and it would be an immediate improvement over what its in his tree.

It is up to him, or Ivo can also pull my patches into his tree.

You can also decide to throw up my patches completely or re-do them
for some silly reasons.  I won't be making much noise about it since
I'll be already on some next patches..

> Otherwise, please focus on the technical contents of the patches and
> work with us to get
> these drivers in a better shape.

This is what I'm focused on, if you have any technical arguments w.r.t.
my patches I'm willing to listen and address them in sensible time
(if they are valid).  I would also be happy to work with people with any
patches that they are working on currently.

Thanks.
-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Alan Cox @ 2009-11-03 23:48 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Ivo van Doorn, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap, Luis Correia,
	John W. Linville, Ingo Molnar, Johannes Berg, Jarek Poplawski,
	Pekka Enberg, David Miller
In-Reply-To: <200911032334.40547.bzolnier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> > Yeah I know that. But like I said, I still needed to get around to do that,
> > and I am very happy you were interested in fixing it.
> 
> Lets make one thing clear: YOU SHOULD BE THE ONE FIXING IT.

Really - you have a service and support contract with Ivo.. no i thought
not.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Bartlomiej Zolnierkiewicz @ 2009-11-03 23:52 UTC (permalink / raw)
  To: Alan Cox
  Cc: Ivo van Doorn, linux-wireless, linux-kernel, netdev, Randy Dunlap,
	Luis Correia, John W. Linville, Ingo Molnar, Johannes Berg,
	Jarek Poplawski, Pekka Enberg, David Miller
In-Reply-To: <20091103234835.3042f7b5@lxorguk.ukuu.org.uk>

On Wednesday 04 November 2009 00:48:35 Alan Cox wrote:
> > > Yeah I know that. But like I said, I still needed to get around to do that,
> > > and I am very happy you were interested in fixing it.
> > 
> > Lets make one thing clear: YOU SHOULD BE THE ONE FIXING IT.
> 
> Really - you have a service and support contract with Ivo.. no i thought
> not.

Just today I got PM from somebody complaining to me about non-working
pata_pdc2026x_old (pdc202xx_old works fine for him) simply because
I fixed some bug there some time ago....

Problems are publicly know -- do you want bz# from Debian, SuSE or Red Hat?

Wait.. unfortunately I also don't have support contract with you..

-- 
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* [net-next PATCH 0/3] qlge: Clean up and firmware subcommands.
From: Ron Mercer @ 2009-11-03 23:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer

Hello Dave,

Patch #1 is a respin per your request from yesterday.
Patch #2 adds handling for more firmware events.
Patch #3 cleans up netdev statistics handling.

Regards,
Ron Mercer



^ permalink raw reply

* [net-next PATCH 1/3] qlge: Fix indentations.
From: Ron Mercer @ 2009-11-03 23:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1257292171-11683-1-git-send-email-ron.mercer@qlogic.com>

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index fa53009..4a07548 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -2102,12 +2102,12 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
 	 */
 	var = ql_read32(qdev, ISR1);
 	if (var & intr_context->irq_mask) {
-				QPRINTK(qdev, INTR, INFO,
+		QPRINTK(qdev, INTR, INFO,
 			"Waking handler for rx_ring[0].\n");
 		ql_disable_completion_interrupt(qdev, intr_context->intr);
-					napi_schedule(&rx_ring->napi);
-				work_done++;
-			}
+		napi_schedule(&rx_ring->napi);
+		work_done++;
+	}
 	ql_enable_completion_interrupt(qdev, intr_context->intr);
 	return work_done ? IRQ_HANDLED : IRQ_NONE;
 }
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 3/3] qlge: Clean up netdev->stats usage.
From: Ron Mercer @ 2009-11-03 23:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1257292171-11683-1-git-send-email-ron.mercer@qlogic.com>

Don't access netdev->stats in IO path.  Save them in tx_ring/rx_rings
and add them up when get_stats API is called.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |    8 +++++++
 drivers/net/qlge/qlge_main.c |   46 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 5e4d343..1f59f05 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -1263,6 +1263,9 @@ struct tx_ring {
 	atomic_t queue_stopped;	/* Turns queue off when full. */
 	struct delayed_work tx_work;
 	struct ql_adapter *qdev;
+	u64 tx_packets;
+	u64 tx_bytes;
+	u64 tx_errors;
 };
 
 /*
@@ -1329,6 +1332,11 @@ struct rx_ring {
 	struct napi_struct napi;
 	u8 reserved;
 	struct ql_adapter *qdev;
+	u64 rx_packets;
+	u64 rx_multicast;
+	u64 rx_bytes;
+	u64 rx_dropped;
+	u64 rx_errors;
 };
 
 /*
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 4a07548..0de596a 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1661,6 +1661,7 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 	if (unlikely(!skb)) {
 		QPRINTK(qdev, RX_STATUS, DEBUG,
 			"No skb available, drop packet.\n");
+		rx_ring->rx_dropped++;
 		return;
 	}
 
@@ -1669,6 +1670,7 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 		QPRINTK(qdev, DRV, ERR, "Receive error, flags2 = 0x%x\n",
 					ib_mac_rsp->flags2);
 		dev_kfree_skb_any(skb);
+		rx_ring->rx_errors++;
 		return;
 	}
 
@@ -1677,6 +1679,7 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 	 */
 	if (skb->len > ndev->mtu + ETH_HLEN) {
 		dev_kfree_skb_any(skb);
+		rx_ring->rx_dropped++;
 		return;
 	}
 
@@ -1697,6 +1700,7 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 			IB_MAC_IOCB_RSP_M_REG ? "Registered" : "",
 			(ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
 			IB_MAC_IOCB_RSP_M_PROM ? "Promiscuous" : "");
+		rx_ring->rx_multicast++;
 	}
 	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_P) {
 		QPRINTK(qdev, RX_STATUS, DEBUG, "Promiscuous Packet.\n");
@@ -1728,8 +1732,8 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 		}
 	}
 
-	ndev->stats.rx_packets++;
-	ndev->stats.rx_bytes += skb->len;
+	rx_ring->rx_packets++;
+	rx_ring->rx_bytes += skb->len;
 	skb_record_rx_queue(skb, rx_ring->cq_id);
 	if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
 		if (qdev->vlgrp &&
@@ -1753,7 +1757,6 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 static void ql_process_mac_tx_intr(struct ql_adapter *qdev,
 				   struct ob_mac_iocb_rsp *mac_rsp)
 {
-	struct net_device *ndev = qdev->ndev;
 	struct tx_ring *tx_ring;
 	struct tx_ring_desc *tx_ring_desc;
 
@@ -1761,8 +1764,8 @@ static void ql_process_mac_tx_intr(struct ql_adapter *qdev,
 	tx_ring = &qdev->tx_ring[mac_rsp->txq_idx];
 	tx_ring_desc = &tx_ring->q[mac_rsp->tid];
 	ql_unmap_send(qdev, tx_ring_desc, tx_ring_desc->map_cnt);
-	ndev->stats.tx_bytes += (tx_ring_desc->skb)->len;
-	ndev->stats.tx_packets++;
+	tx_ring->tx_bytes += (tx_ring_desc->skb)->len;
+	tx_ring->tx_packets++;
 	dev_kfree_skb(tx_ring_desc->skb);
 	tx_ring_desc->skb = NULL;
 
@@ -2205,6 +2208,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 			__func__, tx_ring_idx);
 		netif_stop_subqueue(ndev, tx_ring->wq_id);
 		atomic_inc(&tx_ring->queue_stopped);
+		tx_ring->tx_errors++;
 		return NETDEV_TX_BUSY;
 	}
 	tx_ring_desc = &tx_ring->q[tx_ring->prod_idx];
@@ -2239,6 +2243,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 			NETDEV_TX_OK) {
 		QPRINTK(qdev, TX_QUEUED, ERR,
 				"Could not map the segments.\n");
+		tx_ring->tx_errors++;
 		return NETDEV_TX_BUSY;
 	}
 	QL_DUMP_OB_MAC_IOCB(mac_iocb_ptr);
@@ -3817,6 +3822,37 @@ static int qlge_change_mtu(struct net_device *ndev, int new_mtu)
 static struct net_device_stats *qlge_get_stats(struct net_device
 					       *ndev)
 {
+	struct ql_adapter *qdev = netdev_priv(ndev);
+	struct rx_ring *rx_ring = &qdev->rx_ring[0];
+	struct tx_ring *tx_ring = &qdev->tx_ring[0];
+	unsigned long pkts, mcast, dropped, errors, bytes;
+	int i;
+
+	/* Get RX stats. */
+	pkts = mcast = dropped = errors = bytes = 0;
+	for (i = 0; i < qdev->rss_ring_count; i++, rx_ring++) {
+			pkts += rx_ring->rx_packets;
+			bytes += rx_ring->rx_bytes;
+			dropped += rx_ring->rx_dropped;
+			errors += rx_ring->rx_errors;
+			mcast += rx_ring->rx_multicast;
+	}
+	ndev->stats.rx_packets = pkts;
+	ndev->stats.rx_bytes = bytes;
+	ndev->stats.rx_dropped = dropped;
+	ndev->stats.rx_errors = errors;
+	ndev->stats.multicast = mcast;
+
+	/* Get TX stats. */
+	pkts = errors = bytes = 0;
+	for (i = 0; i < qdev->tx_ring_count; i++, tx_ring++) {
+			pkts += tx_ring->tx_packets;
+			bytes += tx_ring->tx_bytes;
+			errors += tx_ring->tx_errors;
+	}
+	ndev->stats.tx_packets = pkts;
+	ndev->stats.tx_bytes = bytes;
+	ndev->stats.tx_errors = errors;
 	return &ndev->stats;
 }
 
-- 
1.6.0.2


^ permalink raw reply related

* [net-next PATCH 2/3] qlge: Add firmware/driver sub-command support.
From: Ron Mercer @ 2009-11-03 23:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1257292171-11683-1-git-send-email-ron.mercer@qlogic.com>

These sub-commands are issued by another (FCoE) function requesting
an operation on a shared resource.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h     |   12 +++++++++
 drivers/net/qlge/qlge_mpi.c |   56 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 872e95e..5e4d343 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -816,6 +816,18 @@ enum {
 	MB_CMD_GET_MGMNT_TFK_CTL = 0x00000161, /* Get Mgmnt Traffic Control */
 	MB_GET_MPI_TFK_STOPPED = (1 << 0),
 	MB_GET_MPI_TFK_FIFO_EMPTY = (1 << 1),
+	/* Sub-commands for IDC request.
+	 * This describes the reason for the
+	 * IDC request.
+	 */
+	MB_CMD_IOP_NONE = 0x0000,
+	MB_CMD_IOP_PREP_UPDATE_MPI	= 0x0001,
+	MB_CMD_IOP_COMP_UPDATE_MPI	= 0x0002,
+	MB_CMD_IOP_PREP_LINK_DOWN	= 0x0010,
+	MB_CMD_IOP_DVR_START	 = 0x0100,
+	MB_CMD_IOP_FLASH_ACC	 = 0x0101,
+	MB_CMD_IOP_RESTART_MPI	= 0x0102,
+	MB_CMD_IOP_CORE_DUMP_MPI	= 0x0103,
 
 	/* Mailbox Command Status. */
 	MB_CMD_STS_GOOD = 0x00004000,	/* Success. */
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index bac7b86..f5619fe 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -1038,8 +1038,11 @@ void ql_mpi_idc_work(struct work_struct *work)
 	int status;
 	struct mbox_params *mbcp = &qdev->idc_mbc;
 	u32 aen;
+	int timeout;
 
+	rtnl_lock();
 	aen = mbcp->mbox_out[1] >> 16;
+	timeout = (mbcp->mbox_out[1] >> 8) & 0xf;
 
 	switch (aen) {
 	default:
@@ -1047,22 +1050,61 @@ void ql_mpi_idc_work(struct work_struct *work)
 			"Bug: Unhandled IDC action.\n");
 		break;
 	case MB_CMD_PORT_RESET:
-	case MB_CMD_SET_PORT_CFG:
 	case MB_CMD_STOP_FW:
 		ql_link_off(qdev);
+	case MB_CMD_SET_PORT_CFG:
 		/* Signal the resulting link up AEN
 		 * that the frame routing and mac addr
 		 * needs to be set.
 		 * */
 		set_bit(QL_CAM_RT_SET, &qdev->flags);
-		rtnl_lock();
-		status = ql_mb_idc_ack(qdev);
-		rtnl_unlock();
-		if (status) {
-			QPRINTK(qdev, DRV, ERR,
-			"Bug: No pending IDC!\n");
+		/* Do ACK if required */
+		if (timeout) {
+			status = ql_mb_idc_ack(qdev);
+			if (status)
+				QPRINTK(qdev, DRV, ERR,
+					"Bug: No pending IDC!\n");
+		} else {
+			QPRINTK(qdev, DRV, DEBUG,
+				    "IDC ACK not required\n");
+			status = 0; /* success */
+		}
+		break;
+
+	/* These sub-commands issued by another (FCoE)
+	 * function are requesting to do an operation
+	 * on the shared resource (MPI environment).
+	 * We currently don't issue these so we just
+	 * ACK the request.
+	 */
+	case MB_CMD_IOP_RESTART_MPI:
+	case MB_CMD_IOP_PREP_LINK_DOWN:
+		/* Drop the link, reload the routing
+		 * table when link comes up.
+		 */
+		ql_link_off(qdev);
+		set_bit(QL_CAM_RT_SET, &qdev->flags);
+		/* Fall through. */
+	case MB_CMD_IOP_DVR_START:
+	case MB_CMD_IOP_FLASH_ACC:
+	case MB_CMD_IOP_CORE_DUMP_MPI:
+	case MB_CMD_IOP_PREP_UPDATE_MPI:
+	case MB_CMD_IOP_COMP_UPDATE_MPI:
+	case MB_CMD_IOP_NONE:	/*  an IDC without params */
+		/* Do ACK if required */
+		if (timeout) {
+			status = ql_mb_idc_ack(qdev);
+			if (status)
+				QPRINTK(qdev, DRV, ERR,
+				    "Bug: No pending IDC!\n");
+		} else {
+			QPRINTK(qdev, DRV, DEBUG,
+			    "IDC ACK not required\n");
+			status = 0; /* success */
 		}
+		break;
 	}
+	rtnl_unlock();
 }
 
 void ql_mpi_work(struct work_struct *work)
-- 
1.6.0.2


^ permalink raw reply related

* Re: [PATCHv7 3/3] vhost_net: a kernel-level virtio server
From: Paul E. McKenney @ 2009-11-03 23:57 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Eric Dumazet, Michael S. Tsirkin, netdev, virtualization, kvm,
	linux-kernel, mingo, linux-mm, akpm, hpa, Rusty Russell, s.hetze
In-Reply-To: <4AF072EE.9020202@gmail.com>

On Tue, Nov 03, 2009 at 01:14:06PM -0500, Gregory Haskins wrote:
> Gregory Haskins wrote:
> > Eric Dumazet wrote:
> >> Michael S. Tsirkin a écrit :
> >>> +static void handle_tx(struct vhost_net *net)
> >>> +{
> >>> +	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> >>> +	unsigned head, out, in, s;
> >>> +	struct msghdr msg = {
> >>> +		.msg_name = NULL,
> >>> +		.msg_namelen = 0,
> >>> +		.msg_control = NULL,
> >>> +		.msg_controllen = 0,
> >>> +		.msg_iov = vq->iov,
> >>> +		.msg_flags = MSG_DONTWAIT,
> >>> +	};
> >>> +	size_t len, total_len = 0;
> >>> +	int err, wmem;
> >>> +	size_t hdr_size;
> >>> +	struct socket *sock = rcu_dereference(vq->private_data);
> >>> +	if (!sock)
> >>> +		return;
> >>> +
> >>> +	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >>> +	if (wmem >= sock->sk->sk_sndbuf)
> >>> +		return;
> >>> +
> >>> +	use_mm(net->dev.mm);
> >>> +	mutex_lock(&vq->mutex);
> >>> +	vhost_no_notify(vq);
> >>> +
> >> using rcu_dereference() and mutex_lock() at the same time seems wrong, I suspect
> >> that your use of RCU is not correct.
> >>
> >> 1) rcu_dereference() should be done inside a read_rcu_lock() section, and
> >>    we are not allowed to sleep in such a section.
> >>    (Quoting Documentation/RCU/whatisRCU.txt :
> >>      It is illegal to block while in an RCU read-side critical section, )
> >>
> >> 2) mutex_lock() can sleep (ie block)
> >>
> > 
> > 
> > Michael,
> >   I warned you that this needed better documentation ;)
> > 
> > Eric,
> >   I think I flagged this once before, but Michael convinced me that it
> > was indeed "ok", if but perhaps a bit unconventional.  I will try to
> > find the thread.
> > 
> > Kind Regards,
> > -Greg
> > 
> 
> Here it is:
> 
> http://lkml.org/lkml/2009/8/12/173

What was happening in that case was that the rcu_dereference()
was being used in a workqueue item.  The role of rcu_read_lock()
was taken on be the start of execution of the workqueue item, of
rcu_read_unlock() by the end of execution of the workqueue item, and
of synchronize_rcu() by flush_workqueue().  This does work, at least
assuming that flush_workqueue() operates as advertised, which it appears
to at first glance.

The above code looks somewhat different, however -- I don't see
handle_tx() being executed in the context of a work queue.  Instead
it appears to be in an interrupt handler.

So what is the story?  Using synchronize_irq() or some such?

							Thanx, Paul

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ 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