* Re: pull request: wireless-next-2.6 2009-10-28
From: Ivo van Doorn @ 2009-11-03 20:32 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: netdev, Randy Dunlap, Luis Correia, John W. Linville, Ingo Molnar,
Johannes Berg, Jarek Poplawski, Pekka Enberg, David Miller,
linux-wireless, linux-kernel
In-Reply-To: <200911032059.23409.bzolnier@gmail.com>
On Tuesday 03 November 2009, Bartlomiej Zolnierkiewicz wrote:
> On Monday 02 November 2009 19:43:10 Ivo van Doorn wrote:
> > Well and then another rt2x00 developer discovered this nice little
> > fight about rt2x00 on the mailinglists...
> >
> > First for the record, because at the start people where talking about the
> > maintainership of rt2x00, one thing needs to be straight:
> >
> > As mentioned in the MAINTAINERS file, the rt2x00 project is listed as maintainer
> > for the rt2x00 drivers. The rt2x00 drivers include _all_ drivers in the
> > drivers/net/wireless/rt2x00 folder.
> >
> > At this time I am hold the position within the rt2x00 team which is making the
> > decisions about the rt2x00 code and design.
>
> rt2x00 doesn't "own" Linux Ralink support, ditto for rt2800 drivers.
You really have problems reading mails, haven't you? Or at least you
only want to read what you want to read...
Please READ the paragraph again.
I will highlight the thing again:
"about the rt2x00 code and design"
I am not talking about Ralink support I am talking about RT2X00 support.
If you feel that the maintainer of a particular driver has no say about the driver
he wrote himself, then I am not seeing the point of having maintainers. We can
then all just randomly hack in all drivers without any form of structure.
> > I am also the one that is (N)Acks the patches from others when they are send
> > to the rt2x00-users or linux-wireless mailinglist.
>
> If you are not willing to stand behind your own patches, how do you expect
> others to trust your judgment?
Ehm, please explain? I am sending patches upstream, but apparently I don't stand behind them?
Then why the f*** am I sending them?
> > As for my behavior in discussions:
> >
> > I am doing my best to listen to all complains regarding the rt2x00 code and design and
> > improve it if the complainer has a valid point. However, obviously I can disagree with
> > the complainer and in that case I will explain to that person _why_ I disagree. It is up
> > to the complainer to convince me that he is right, agree with my response, or whine.
> >
> > Now as for more specific responses:
> >
> > On Thursday 29 October 2009, Bartlomiej Zolnierkiewicz wrote:
> > > rt2800 drivers have their maintainers and I would like to know what they
> > > are doing besides complaining about users and staging tree..
> >
> > Working for Avanade, Zarafa and as freelancer for Linux Magazine.
> > But I guess you mean rt2x00 specific work?
> > Well that list consists of:
> > - Listening to people complain
> > - Responding to those people, because otherwise they complain that they are being ignored.
> > - Following bug reports, and request testing or additional information if required
> > - Bugfixing
> > - Reviewing patches from contributors
> > - Applying patches from contributors
> > - Discussing improvements over patches from contributors
> >
> > Well nothing of this list should be new to you, but apparently you needed some confirmation.
>
> I meant rt2800 support specifically, sorry if this was not clear.
>
> The work on rt2800 drivers was started in the beginning of 2008 and after
> few months you were already behind deadlines that you had set yourself:
>
> http://markmail.org/message/a753dws6tqytb3a4
>
> During almost two years rt2x00 project didn't manage to produce working
> support for rt2800.. in the meantime the world has moved on and we have
> another chipset generation (rt30x0) to worry about, and also rt33xx one
> on the horizon..
And thank you for your valuable effort in assisting in the driver development
during those 2 years. I am really happy about people looking from the sideline
and complaining things are not moving as fast as they would like it.
If you cared about the slow progress, then you had the past 2 years to help out,
but apparently that is too much to ask. Unfortunately a lot of people think like
you, which means that during those 2 years dozens of people have complained
about the lack of progress without any of them helping out (and I am not talking
about users only, I am also talking about people who have programming skills).
If you look at the Signed-off list of the rt2800pci/usb patches you will see that
only a handfull of people actually helped.
> > On Thursday 29 October 2009, Johannes Berg wrote:
> > > On Thu, 2009-10-29 at 07:20 -0700, David Miller wrote:
> > >
> > > > In case you're concerned, I actually agree with John and others
> > > > on this issue, and disagree with your position.
> > >
> > > In this particular case, I think it makes more sense to duplicate the
> > > code _especially_ because it's not working yet. That frees people
> > > hacking on it of having to worry about breaking other devices.t initialization sequences) and in the review
> >
> > Thank you Johannes, that is exactly what I was trying to tell Bartlomiej
> > in the previous discussion.
>
> Nope, it just adds needles development/maintenance burden since it will
> make people lose fixes already applied to the working code, please see my
> other mail.
As mentioned in my mail, there were differences in the drivers during development,
and I had a working rt2800usb driver as base. But with the same settings the rt2800pci
couldn't get to work.
With the merged drivers you would constantly need to check if it works for rt2800pci,
but also if you haven't broken rt2800usb in the meantime. This way rt2800usb was
able to be merged months before rt2800pci because there was no common code.
Once both drivers are solid working, then there are no problems with merging them,
I wonder how many times I have to repeat that...
> > On Wednesday 28 October 2009, Bartlomiej Zolnierkiewicz wrote:
> > > I find it rather disappointing that all my review comments regarding
> > > rt2800pci support were just completely ignored and then the initial
> > > patch was merged just as it was..
> >
> > Your code review comments were commented upon with my reasons
> > why this code duplication exists. I even admitted that when the time is
> > ready I will remove the code duplication.
>
> The right time was two years ago when you were starting working on those
> drivers. IOW It shouldn't have happened in the first place.
Please see my other mail, please read my above comment, and thank you again
for participating in the rt2800 development of those 2 years.
> I also have serious questions about transparency of the process and cannot
> understand why it takes so long for the code to be pushed upstream.
So you start looking now, and complain you can't see the process of the last 2 years?
There have been dozens of status reports regarding rt2800, and the number of responses
in general have always been limited.
But now I reread that last sentence, you are complaining that rt2800pci wasn't pushed
upstream earlier? While this whole discussion is about that merging rt2800pci was a bad
idea?
> commit 1761631 -- obvious bugfix and I'm pretty sure I've seen it in rt2x00
> tree back in September (you're re-basing rt2x00 tree at random moments so it
> is impossible to track the status of patches that you are handling).
I'm missing the point here.
> rt2800pci patch itself has been getting dust in your tree since at least
> April (it hasn't received any bigger updates since then)..
And how many times do I have to repeat the reason?
I think that after dozens of mails, the message should have been clear, especially
to somebody who apparently has been monitoring and worrying about the development
process for the last 2 years....
> > On Thursday 29 October 2009, Bartlomiej Zolnierkiewicz wrote:
> > > Quite the contrary, I'm pretty confident that addressing my review concerns
> > > would result in better RT28x00 / RT30x0 support in the very near future.
> >
> > The review concerns regarding the duplicate code would only reduce the
> > amount of code. It would not magically fix bugs (at least the chance of that
> > would be quite small).
>
> Please see my other mail.
And did your patches make rt2800pci work better?
Does rt2800usb suddenly work for 11n networks?
> > So far rt2800usb performs better then rt2800pci, and the difference gets
> > only bigger when I use the exact same register initialization from rt2800usb
> > in rt2800pci.
> >
> > But Bartjmoiej knows that the register initialization can be exactly the same,
> > from his experience with the staging drivers.
> > So far hasn't been interested in sharing the knowledge in what must be
> > changed in rt2800pci/usb to make them both work with the same register
> > initialization.
>
> The knowledge is all in your local copy of linux-next (I don't memorize
> things like chipset initialization sequences) and in the review comments
> that you have happily dismissed.
Your review comments were regarding _moving_ code to a generic stack
for rt2800pci and rt2800usb while I already indicated that would not improve
the status of rt2800pci (instead it would make it worse, since I already tested
if those settings would work). But apparently merging the code is better then
fixing the code.
Ivo
^ permalink raw reply
* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Ivo van Doorn @ 2009-11-03 21:00 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: <200911031951.05235.bzolnier@gmail.com>
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.
[snip]
> 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...
> 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.
> 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.
> MAINTAINERS: add rt2800 entry
I see you decided to take over the maintainership?
Doesn't that need the current maintainer to move away, or was this part
of the "going over other peoples head" plan?
> rt2800usb: add rt2800_register_[read,write]() wrappers
> rt2800pci: add rt2800_register_[read,write]() wrappers
> rt2800usb: add rt2800_register_multi[read,write]() wrappers
> rt2800pci: add rt2800_register_multi[read,write]() wrappers
> rt2800usb: add rt2800_regbusy_read() wrapper
> rt2800pci: add rt2800_regbusy_read() wrapper
> rt2800usb: add rt2800_bbp_[read,write]() wrappers
> rt2800pci: add rt2800_bbp_[read,write]() wrappers
> rt2800usb: add rt2800_rfcsr_[read,write]() wrappers
> rt2800pci: add rt2800_rfcsr_[read,write]() wrappers
> rt2800usb: add rt2800_rf_[read,write]() wrappers
> rt2800pci: add rt2800_rf_[read,write]() wrappers
> rt2800usb: add rt2800_mcu_request() wrapper
> rt2800pci: add rt2800_mcu_request() wrapper
> rt2x00: add driver private field to struct rt2x00_dev
> rt2800usb: convert to use struct rt2800_ops methods
> rt2800pci: convert to use struct rt2800_ops methods
> rt2x00: fix rt2x00usb_register_multiwrite() arguments
> rt2x00: fix rt2x00usb_regbusy_read() arguments
> rt2x00: fix rt2x00pci_register_multi[read,write]() arguments
> rt2800: add rt2800lib.h
> rt2800: fix duplication in header files
> rt2800: prepare for rt2800lib addition
> rt2800: add rt2800lib (part one)
> rt2800: add rt2800lib (part two)
> rt2x00: move REGISTER_BUSY_* definitions to rt2x00.h
> rt2800: add rt2800lib (part three)
> rt2800: add rt2800lib (part four)
These are too much (and too big) patches for me to review at once, I'll look at
them later.
Ivo
^ permalink raw reply
* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Gertjan van Wingerde @ 2009-11-03 21:01 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Ivo van Doorn, Randy Dunlap,
Luis Correia, John W. Linville, Ingo Molnar, Johannes Berg,
Jarek Poplawski, Pekka Enberg, David Miller
In-Reply-To: <200911031951.05235.bzolnier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi,
On 11/03/09 19:51, 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.
>
This seems to be a misrepresentation of the situation. The issues raised by
you were acknowledged as being valid, however they were not deemed important
enough to stop inclusion in wireless-next and net-next.
However, it is good to see that you have put effort in providing a patch series
for these issues.
Now, since I believe that it is better to work with people, rather than against
them, would it be possible to post this patch series somewhere as a set of
separate patches, so that they can be reviewed as such?
---
Gertjan
rt2x00 project developer
>
> Namely, it:
>
> - Adds abstraction of chipset register access for chipsets connected to
> different buses by using new structure (struct rt2800_ops) which contains
> all needed register access methods.
>
> [ It is a prerequisite for fixing code duplication between rt2800usb.c
> and rt2800pci.c drivers. ]
>
> - Fixes code duplication in rt2800usb.h and rt2800pci.h header files by
> using new shared rt2800.h header (almost 1800 LOC gone).
>
> Updated debugging scripts are located here:
>
> http://www.kernel.org/pub/linux/kernel/people/bart/rt2800/scripts/
>
> (they also work fine with older drivers)
>
> - Adds rt2800 library containing common code for PCI and USB versions
> of rt2800 chipsets. This removes over 1300 LOC and allows us to save
> a lot of maintenance burden in the future.
>
>
> It also fixes two real bugs (one in rt2800pci and one in rt2800usb) found
> as a direct result of the code de-duplication:
>
> - Fix rt2800usb driver to write the rfcsr read request into RF_CSR_CFG
> register and not BBP_CSR_CFG one in rt2800usb_rfcsr_read().
>
> - Use the correct encryption key index for TX frames in rt2800pci (this is
> based on rt2800usb patch from Benoit PAPILLAULT already in Linus' tree,
> unfortunately the fix was not ported over to rt2800pci).
>
> [ There are also some minor code rt2x00 infrastructure fixes and improvements
> here and there... ]
>
>
> All in all over 3100 LOC are gone and rt2800pci specific code is:
>
> 1685 drivers/net/wireless/rt2x00/rt2800pci.c
> 180 drivers/net/wireless/rt2x00/rt2800pci.h
> 1865 total
>
> instead of:
>
> 3323 drivers/net/wireless/rt2x00/rt2800pci.c
> 1960 drivers/net/wireless/rt2x00/rt2800pci.h
> 5283 total
>
> (wireless-next and net-next trees)
>
> which means decrease of the code needed for rt2800pci by 65% (this in turn
> translates to 31% decrease for rt2800 specific code and 9% for whole rt2x00
> infrastructure).
>
> 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).
>
> 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
> rt2800usb: add rt2800_register_[read,write]() wrappers
> rt2800pci: add rt2800_register_[read,write]() wrappers
> rt2800usb: add rt2800_register_multi[read,write]() wrappers
> rt2800pci: add rt2800_register_multi[read,write]() wrappers
> rt2800usb: add rt2800_regbusy_read() wrapper
> rt2800pci: add rt2800_regbusy_read() wrapper
> rt2800usb: add rt2800_bbp_[read,write]() wrappers
> rt2800pci: add rt2800_bbp_[read,write]() wrappers
> rt2800usb: add rt2800_rfcsr_[read,write]() wrappers
> rt2800pci: add rt2800_rfcsr_[read,write]() wrappers
> rt2800usb: add rt2800_rf_[read,write]() wrappers
> rt2800pci: add rt2800_rf_[read,write]() wrappers
> rt2800usb: add rt2800_mcu_request() wrapper
> rt2800pci: add rt2800_mcu_request() wrapper
> rt2x00: add driver private field to struct rt2x00_dev
> rt2800usb: convert to use struct rt2800_ops methods
> rt2800pci: convert to use struct rt2800_ops methods
> rt2x00: fix rt2x00usb_register_multiwrite() arguments
> rt2x00: fix rt2x00usb_regbusy_read() arguments
> rt2x00: fix rt2x00pci_register_multi[read,write]() arguments
> rt2800: add rt2800lib.h
> rt2800usb: fix comments in rt2800usb.h
> rt2800usb: add RXINFO_DESC_SIZE definition
> rt2800: fix duplication in header files
> rt2800: fix comments in rt2800.h
> rt2x00: add support for different chipset interfaces
> rt2800: prepare for rt2800lib addition
> rt2800: add rt2800lib (part one)
> rt2x00: remove needless ifdefs from rt2x00leds.h
> rt2800: add rt2800lib (part two)
> rt2x00: move REGISTER_BUSY_* definitions to rt2x00.h
> rt2800: add rt2800lib (part three)
> rt2800: add rt2800lib (part four)
> MAINTAINERS: add rt2800 entry
>
> MAINTAINERS | 7 +
> drivers/net/wireless/rt2x00/Kconfig | 5 +
> drivers/net/wireless/rt2x00/Makefile | 1 +
> drivers/net/wireless/rt2x00/rt2800.h | 1816 ++++++++++++++++++++++++++++
> drivers/net/wireless/rt2x00/rt2800lib.c | 1817 ++++++++++++++++++++++++++++
> drivers/net/wireless/rt2x00/rt2800lib.h | 134 +++
> drivers/net/wireless/rt2x00/rt2800pci.c | 1908 +++---------------------------
> drivers/net/wireless/rt2x00/rt2800pci.h | 1780 ----------------------------
> drivers/net/wireless/rt2x00/rt2800usb.c | 1828 ++---------------------------
> drivers/net/wireless/rt2x00/rt2800usb.h | 1818 +----------------------------
> drivers/net/wireless/rt2x00/rt2x00.h | 33 +
> drivers/net/wireless/rt2x00/rt2x00leds.h | 4 -
> drivers/net/wireless/rt2x00/rt2x00pci.h | 24 +-
> drivers/net/wireless/rt2x00/rt2x00usb.c | 2 +-
> drivers/net/wireless/rt2x00/rt2x00usb.h | 17 +-
> 15 files changed, 4036 insertions(+), 7158 deletions(-)
> create mode 100644 drivers/net/wireless/rt2x00/rt2800.h
> create mode 100644 drivers/net/wireless/rt2x00/rt2800lib.c
> create mode 100644 drivers/net/wireless/rt2x00/rt2800lib.h
>
> --
> 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
>
--
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: pull request: wireless-next-2.6 2009-10-28
From: Bartlomiej Zolnierkiewicz @ 2009-11-03 21:04 UTC (permalink / raw)
To: Ivo van Doorn
Cc: netdev, Randy Dunlap, Luis Correia, John W. Linville, Ingo Molnar,
Johannes Berg, Jarek Poplawski, Pekka Enberg, David Miller,
linux-wireless, linux-kernel
In-Reply-To: <200911032132.31533.IvDoorn@gmail.com>
On Tuesday 03 November 2009 21:32:30 Ivo van Doorn wrote:
> > rt2800pci patch itself has been getting dust in your tree since at least
> > April (it hasn't received any bigger updates since then)..
>
> And how many times do I have to repeat the reason?
> I think that after dozens of mails, the message should have been clear, especially
> to somebody who apparently has been monitoring and worrying about the development
> process for the last 2 years....
Just for the record:
I got only interested in rt2x00 support around April this year (while I was
doing major staging cleanups everywhere) and I was hearing back then that
rt2800pci will be merged "really soon" and that the project is making "great
progress"..
--
Bartlomiej Zolnierkiewicz
^ permalink raw reply
* Re: [PATCHv7 3/3] vhost_net: a kernel-level virtio server
From: Eric Dumazet @ 2009-11-03 21:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Gregory Haskins, netdev, virtualization, kvm, linux-kernel, mingo,
linux-mm, akpm, hpa, Rusty Russell, s.hetze, Paul E. McKenney
In-Reply-To: <20091103195841.GB6669@redhat.com>
Michael S. Tsirkin a écrit :
>
> Paul, you acked this previously. Should I add you acked-by line so
> people calm down? If you would rather I replace
> rcu_dereference/rcu_assign_pointer with rmb/wmb, I can do this.
> Or maybe patch Documentation to explain this RCU usage?
>
So you believe I am over-reacting to this dubious use of RCU ?
RCU documentation is already very complex, we dont need to add yet another
subtle use, and makes it less readable.
It seems you use 'RCU api' in drivers/vhost/net.c as convenient macros :
#define rcu_dereference(p) ({ \
typeof(p) _________p1 = ACCESS_ONCE(p); \
smp_read_barrier_depends(); \
(_________p1); \
})
#define rcu_assign_pointer(p, v) \
({ \
if (!__builtin_constant_p(v) || \
((v) != NULL)) \
smp_wmb(); \
(p) = (v); \
})
There are plenty regular uses of smp_wmb() in kernel, not related to Read Copy Update,
there is nothing wrong to use barriers with appropriate comments.
(And you already use mb(), wmb(), rmb(), smp_wmb() in your patch)
BTW there is at least one locking bug in vhost_net_set_features()
Apparently, mutex_unlock() doesnt trigger a fault if mutex is not locked
by current thread... even with DEBUG_MUTEXES / DEBUG_LOCK_ALLOC
static void vhost_net_set_features(struct vhost_net *n, u64 features)
{
size_t hdr_size = features & (1 << VHOST_NET_F_VIRTIO_NET_HDR) ?
sizeof(struct virtio_net_hdr) : 0;
int i;
<<!>> mutex_unlock(&n->dev.mutex);
n->dev.acked_features = features;
smp_wmb();
for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
mutex_lock(&n->vqs[i].mutex);
n->vqs[i].hdr_size = hdr_size;
mutex_unlock(&n->vqs[i].mutex);
}
mutex_unlock(&n->dev.mutex);
vhost_net_flush(n);
}
--
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
* Re: [PATCH 1/2] alchemy: add au1000-eth platform device
From: Florian Fainelli @ 2009-11-03 21:14 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Ralf Baechle, linux-mips, Manuel Lauss, David Miller, netdev
In-Reply-To: <4AE1DEFD.4090902@ru.mvista.com>
Hi Sergei,
On Friday 23 October 2009 18:51:09 Sergei Shtylyov wrote:
> Hello.
>
> Florian Fainelli wrote:
> > Please find below and updated version, hopefully addressing most if not
> > all of your comments.
>
> Thanks. I still have some comments on the code testing the NI2 bit. :-)
This version hopefully addresses your last comments. Thanks !
--
From: Florian Fainelli <florian@openwrt.org>
Subject: [PATCH 1/2] alchemy: add au1000-eth platform device (v4)
This patch makes the board code register the au1000-eth
platform device. The au1000-eth platform data can be
overriden with the au1xxx_override_eth_cfg function
like it has to be done for the Bosporus board which uses
a different MAC/PHY setup.
Changes from v3:
- declare a static au1000_eth_platform_data structure for bosporus and
initialize it
- remove parenthis and bit shifting on SYS_PF_NI2
Changes from v2:
- declared the au1000-eth second driver instance platform_data
- made the override function generic and pass it the port number too
Changes from v1:
- remove per-board platform.c file
- add an override function to pass custom eth0 platform_data PHY settings
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
index 3be14b0..3fbe30c 100644
--- a/arch/mips/alchemy/common/platform.c
+++ b/arch/mips/alchemy/common/platform.c
@@ -19,6 +19,7 @@
#include <asm/mach-au1x00/au1xxx.h>
#include <asm/mach-au1x00/au1xxx_dbdma.h>
#include <asm/mach-au1x00/au1100_mmc.h>
+#include <asm/mach-au1x00/au1xxx_eth.h>
#define PORT(_base, _irq) \
{ \
@@ -326,6 +327,88 @@ static struct platform_device pbdb_smbus_device = {
};
#endif
+/* Macro to help defining the Ethernet MAC resources */
+#define MAC_RES(_base, _enable, _irq) \
+ { \
+ .start = CPHYSADDR(_base), \
+ .end = CPHYSADDR(_base + 0xffff), \
+ .flags = IORESOURCE_MEM, \
+ }, \
+ { \
+ .start = CPHYSADDR(_enable), \
+ .end = CPHYSADDR(_enable + 0x3), \
+ .flags = IORESOURCE_MEM, \
+ }, \
+ { \
+ .start = _irq, \
+ .end = _irq, \
+ .flags = IORESOURCE_IRQ \
+ }
+
+static struct resource au1xxx_eth0_resources[] = {
+#if defined(CONFIG_SOC_AU1000)
+ MAC_RES(AU1000_ETH0_BASE, AU1000_MAC0_ENABLE, AU1000_MAC0_DMA_INT),
+#elif defined(CONFIG_SOC_AU1100)
+ MAC_RES(AU1100_ETH0_BASE, AU1100_MAC0_ENABLE, AU1100_MAC0_DMA_INT),
+#elif defined(CONFIG_SOC_AU1550)
+ MAC_RES(AU1550_ETH0_BASE, AU1550_MAC0_ENABLE, AU1550_MAC0_DMA_INT),
+#elif defined(CONFIG_SOC_AU1500)
+ MAC_RES(AU1500_ETH0_BASE, AU1500_MAC0_ENABLE, AU1500_MAC0_DMA_INT),
+#endif
+};
+
+static struct resource au1xxx_eth1_resources[] = {
+#if defined(CONFIG_SOC_AU1000)
+ MAC_RES(AU1000_ETH1_BASE, AU1000_MAC1_ENABLE, AU1000_MAC1_DMA_INT),
+#elif defined(CONFIG_SOC_AU1550)
+ MAC_RES(AU1550_ETH1_BASE, AU1550_MAC1_ENABLE, AU1550_MAC1_DMA_INT),
+#elif defined(CONFIG_SOC_AU1500)
+ MAC_RES(AU1500_ETH1_BASE, AU1500_MAC1_ENABLE, AU1500_MAC1_DMA_INT),
+#endif
+};
+
+static struct au1000_eth_platform_data au1xxx_eth0_platform_data = {
+ .phy1_search_mac0 = 1,
+};
+
+static struct platform_device au1xxx_eth0_device = {
+ .name = "au1000-eth",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(au1xxx_eth0_resources),
+ .resource = au1xxx_eth0_resources,
+ .dev.platform_data = &au1xxx_eth0_platform_data,
+};
+
+#ifndef CONFIG_SOC_AU1100
+static struct au1000_eth_platform_data au1xxx_eth1_platform_data = {
+ .phy1_search_mac0 = 1,
+};
+
+static struct platform_device au1xxx_eth1_device = {
+ .name = "au1000-eth",
+ .id = 1,
+ .num_resources = ARRAY_SIZE(au1xxx_eth1_resources),
+ .resource = au1xxx_eth1_resources,
+ .dev.platform_data = &au1xxx_eth1_platform_data,
+};
+#endif
+
+void __init au1xxx_override_eth_cfg(unsigned int port,
+ struct au1000_eth_platform_data *eth_data)
+{
+ if (!eth_data || port > 1)
+ return;
+
+ if (port == 0)
+ memcpy(&au1xxx_eth0_platform_data, eth_data,
+ sizeof(struct au1000_eth_platform_data));
+#ifndef CONFIG_SOC_AU1100
+ else
+ memcpy(&au1xxx_eth1_platform_data, eth_data,
+ sizeof(struct au1000_eth_platform_data));
+#endif
+}
+
static struct platform_device *au1xxx_platform_devices[] __initdata = {
&au1xx0_uart_device,
&au1xxx_usb_ohci_device,
@@ -345,6 +428,7 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
#ifdef SMBUS_PSC_BASE
&pbdb_smbus_device,
#endif
+ &au1xxx_eth0_device,
};
static int __init au1xxx_platform_init(void)
@@ -356,6 +440,12 @@ static int __init au1xxx_platform_init(void)
for (i = 0; au1x00_uart_data[i].flags; i++)
au1x00_uart_data[i].uartclk = uartclk;
+#ifndef CONFIG_SOC_AU1100
+ /* Register second MAC if enabled in pinfunc */
+ if (!(au_readl(SYS_PINFUNC) & (u32)SYS_PF_NI2))
+ platform_device_register(&au1xxx_eth1_device);
+#endif
+
return platform_add_devices(au1xxx_platform_devices,
ARRAY_SIZE(au1xxx_platform_devices));
}
diff --git a/arch/mips/alchemy/devboards/db1x00/board_setup.c b/arch/mips/alchemy/devboards/db1x00/board_setup.c
index 7aee14d..ad26db2 100644
--- a/arch/mips/alchemy/devboards/db1x00/board_setup.c
+++ b/arch/mips/alchemy/devboards/db1x00/board_setup.c
@@ -32,6 +32,7 @@
#include <linux/interrupt.h>
#include <asm/mach-au1x00/au1000.h>
+#include <asm/mach-au1x00/au1xxx_eth.h>
#include <asm/mach-db1x00/db1x00.h>
#include <asm/mach-db1x00/bcsr.h>
@@ -43,6 +44,18 @@ char irq_tab_alchemy[][5] __initdata = {
[13] = { -1, AU1500_PCI_INTA, AU1500_PCI_INTB, AU1500_PCI_INTC, AU1500_PCI_INTD }, /* IDSEL 13 - PCI slot */
};
#endif
+
+/*
+ * Micrel/Kendin 5 port switch attached to MAC0,
+ * MAC0 is associated with PHY address 5 (== WAN port)
+ * MAC1 is not associated with any PHY, since it's connected directly
+ * to the switch.
+ * no interrupts are used
+ */
+static struct au1000_eth_platform_data eth0_pdata = {
+ .phy_static_config = 1,
+ .phy_addr = 5,
+};
#ifdef CONFIG_MIPS_BOSPORUS
char irq_tab_alchemy[][5] __initdata = {
@@ -50,6 +63,8 @@ char irq_tab_alchemy[][5] __initdata = {
[12] = { -1, AU1500_PCI_INTA, 0xff, 0xff, 0xff }, /* IDSEL 12 - SN1741 */
[13] = { -1, AU1500_PCI_INTA, AU1500_PCI_INTB, AU1500_PCI_INTC, AU1500_PCI_INTD }, /* IDSEL 13 - PCI slot */
};
+
+
#endif
#ifdef CONFIG_MIPS_MIRAGE
@@ -103,6 +118,8 @@ void __init board_setup(void)
printk(KERN_INFO "AMD Alchemy Au1100/Db1100 Board\n");
#endif
#ifdef CONFIG_MIPS_BOSPORUS
+ au1xxx_override_eth_cfg(0, ð0_pdata);
+
printk(KERN_INFO "AMD Alchemy Bosporus Board\n");
#endif
#ifdef CONFIG_MIPS_MIRAGE
diff --git a/arch/mips/include/asm/mach-au1x00/au1xxx_eth.h b/arch/mips/include/asm/mach-au1x00/au1xxx_eth.h
new file mode 100644
index 0000000..f30529e
--- /dev/null
+++ b/arch/mips/include/asm/mach-au1x00/au1xxx_eth.h
@@ -0,0 +1,18 @@
+#ifndef __AU1X00_ETH_DATA_H
+#define __AU1X00_ETH_DATA_H
+
+/* Platform specific PHY configuration passed to the MAC driver */
+struct au1000_eth_platform_data {
+ int phy_static_config;
+ int phy_search_highest_addr;
+ int phy1_search_mac0;
+ int phy_addr;
+ int phy_busid;
+ int phy_irq;
+};
+
+void __init au1xxx_override_eth_cfg(unsigned port,
+ struct au1000_eth_platform_data *eth_data);
+
+#endif /* __AU1X00_ETH_DATA_H */
+
^ permalink raw reply related
* [PATCH torvalds-2.6] cdc_ether: additional Ericsson MBM PID's to the whitelist
From: Torgny Johansson @ 2009-11-03 21:28 UTC (permalink / raw)
To: davem; +Cc: netdev, saurin.shah
Hi!
This is another attempt to submit this patch that adds more Ericsson Mobile Broadband modules to the cdc_ether whitelist.
The last patch was rejected since it produced rejects with the Torvalds tree. I've tried it again after adding
another vid/pid and also rechecked it with the
checkpatch script. It still applies cleanly for me and checkpatch gives no errors or warnings.
I suspect Outlook garbles it when I send it.
Trying with another e-mail client to see if that works better. I've sent it to my own gmail first and if I "view original" in gmail
the patch works and applies cleanly after downloading it so I'm hoping it will work for you guys too.
Devices added to the whitelist:
- Ericsson Mobile Broadband variants (F3607gw, F3307 and c3607w)
- Dell F3607gw variants
- Toshiba F3607gw variants
Thanks!
Regards
Torgny Johansson
---
Signed-off-by: Torgny Johansson <torgny.johansson@gmail.com>
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index c47237c..32d9356 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -174,7 +174,7 @@ config USB_NET_CDCETHER
* Ericsson Mobile Broadband Module (all variants)
* Motorola (DM100 and SB4100)
* Broadcom Cable Modem (reference design)
- * Toshiba (PCX1100U and F3507g)
+ * Toshiba (PCX1100U and F3507g/F3607gw)
* ...
This driver creates an interface named "ethX", where X depends on
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 4a6aff5..21e1ba1 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -544,20 +544,60 @@ static const struct usb_device_id products [] = {
USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
.driver_info = (unsigned long) &cdc_info,
}, {
- /* Ericsson F3307 */
+ /* Ericsson F3607gw ver 2 */
+ USB_DEVICE_AND_INTERFACE_INFO(0x0bdb, 0x1905, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
+}, {
+ /* Ericsson F3607gw ver 3 */
USB_DEVICE_AND_INTERFACE_INFO(0x0bdb, 0x1906, USB_CLASS_COMM,
USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
.driver_info = (unsigned long) &cdc_info,
}, {
+ /* Ericsson F3307 */
+ USB_DEVICE_AND_INTERFACE_INFO(0x0bdb, 0x190a, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
+}, {
+ /* Ericsson F3307 ver 2 */
+ USB_DEVICE_AND_INTERFACE_INFO(0x0bdb, 0x1909, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
+}, {
+ /* Ericsson C3607w */
+ USB_DEVICE_AND_INTERFACE_INFO(0x0bdb, 0x1049, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
+}, {
/* Toshiba F3507g */
USB_DEVICE_AND_INTERFACE_INFO(0x0930, 0x130b, USB_CLASS_COMM,
USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
.driver_info = (unsigned long) &cdc_info,
}, {
+ /* Toshiba F3607gw */
+ USB_DEVICE_AND_INTERFACE_INFO(0x0930, 0x130c, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
+}, {
+ /* Toshiba F3607gw ver 2 */
+ USB_DEVICE_AND_INTERFACE_INFO(0x0930, 0x1311, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
+}, {
/* Dell F3507g */
USB_DEVICE_AND_INTERFACE_INFO(0x413c, 0x8147, USB_CLASS_COMM,
USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
.driver_info = (unsigned long) &cdc_info,
+}, {
+ /* Dell F3607gw */
+ USB_DEVICE_AND_INTERFACE_INFO(0x413c, 0x8183, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
+}, {
+ /* Dell F3607gw ver 2 */
+ USB_DEVICE_AND_INTERFACE_INFO(0x413c, 0x8184, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_MDLM, USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long) &cdc_info,
},
{ }, // END
};
^ permalink raw reply related
* Re: [PATCH 1/3] sysfs directory scaling: rbtree for dirent name lookups
From: Eric W. Biederman @ 2009-11-03 21:32 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: Greg KH, Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu,
linux-kernel
In-Reply-To: <20091103200155.GQ8227@kvack.org>
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.
Eric
^ permalink raw reply
* Re: 2.6.32-rc5-mmotm1101 - kernel BUG at net/ipv4/tcp_input.c:3707!
From: Ilpo JÀrvinen @ 2009-11-03 21:34 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Valdis.Kletnieks, Eric Dumazet, Andrew Morton, linux-kernel,
netdev, Ori Finkelman
In-Reply-To: <4AF08292.7090102@codefidence.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 854 bytes --]
On Tue, 3 Nov 2009, Gilad Ben-Yossef wrote:
> Eric Dumazet wrote:
>
> > Valdis.Kletnieks@vt.edu a écrit :
> >
> > > Seen right after I started 'fetchmail'. Reproducible - 3 out of 3.
> > > I'll bisect this tonight if nobody jumps up and yells they know what it
> > > is...
> > >
> Bah... this is most probably my fault. Sorry about that.
>
> Can you please try the patch in the next email?
>
> But also, can you please send me the route table in effect when this happened
> and the fetchmail command line/config (removing any passwords or account
> details of course)? I want to understand better when this happens.
According to the stacktrace, it came from ipv6 side which doesn't have any
null checking what so ever atm (you only handled ipv4 correctly). ...You
should be a bit more careful next time when adding any BUG_ONs...
--
i.
^ permalink raw reply
* 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox