* Re: [PATCH 1/3] netfilter : 3 patches to boost ip_tables performance
From: Eric Dumazet @ 2005-09-22 13:17 UTC (permalink / raw)
To: Harald Welte; +Cc: netdev, netfilter-devel, linux-kernel, Andi Kleen
In-Reply-To: <20050922125724.GJ26520@sunbeam.de.gnumonks.org>
Harald Welte a écrit :
> On Wed, Sep 21, 2005 at 11:29:13PM +0200, Eric Dumazet wrote:
>
>>Patch 1/3
>>
>>1) No more one rwlock_t protecting the 'curtain'
>
>
> I have no problem with this change "per se", but with the
> implementation.
>
> As of now, we live without any ugly #ifdef CONFIG_SMP / #endif sections
> in the code - and if possible, I would continue this good tradition.
>
> For example the get_counters() function. Wouldn't all the smp specific
> code (for_each_cpu(), ...) be #defined to nothing anyway?
Well... not exactly, but you are right only the first loop (SET_COUNTER) will
really do something. The if (cpu == curcpu) will be true but the compiler wont
know that, cpu and curcpu are still C variables.
>
> And if we really need the #ifdef's, I would appreciate if those
> sectionas are as small as possible. in get_counters() the section can
> definitely be smaller, rather than basically having the whole function
> body separate for smp and non-smp cases.
get_counters() is not critical, so I agree with you we can stick the general
version (not the UP optimized one)
>
> Also, how much would we loose in runtime performance if we were using a
> "rwlock_t *" even in the UP case?. I mean, it's just one more pointer
> dereference of something that is expected to be in cache anyway, isn't
> it? This gets rid of another huge set of #ifdefs that make the code
> unreadable and prone to errors being introduced later on.
>
Well, in UP case, the rwlock_t is a nulldef.
I was inspired by another use of percpu data in include/linux/genhd.h
#ifdef CONFIG_SMP
struct disk_stats *dkstats;
#else
struct disk_stats dkstats;
#endif
But if you dislike this, we can use pointer for all cases.
Eric
^ permalink raw reply
* Re: [PATCH] channel bonding: add support for device-indexed parameters
From: Florin Malita @ 2005-09-22 13:16 UTC (permalink / raw)
To: Jason R. Martin
Cc: akpm, davem, ctindel, fubar, linux-kernel, netdev, bonding-devel
In-Reply-To: <c295378405092123032534d93b@mail.gmail.com>
On Wed, 21 Sep 2005 23:03:53 -0700
"Jason R. Martin" <nsxfreddy@gmail.com> wrote:
> Personally I think working to get the sysfs support finished in
> bonding and stop relying on module parameters to configure bonds would
> be better, since bonds will truly be independent of each other and be
> able to be added and removed on the fly. Having worked with a
> previous attempt to set per-bond values through module parameters
> (http://marc.theaimsgroup.com/?t=110558187800001&r=1&w=2), it's easy
> to get pretty crazy.
Agreed - that would be a better configuration interface, but I don't see
why we couldn't support module parameter arrays too. Especially since
the changes are minimal and don't break the ABI/ifenslave
compatibility/etc.
IMHO the "primary" semantics are completely broken right now and this
is a possible fix for it.
> For example, you can have more than one
> arp_ip_target, and they really should be per bond as well, so how do
> you divvy those up via module parameters?
Yup, arp_ip_target is one parameter which doesn't lend itself to this
scheme and this is exactly why the patch doesn't try to fix it.
Florin
^ permalink raw reply
* Re: [PATCH 2/3] netfilter : 3 patches to boost ip_tables performance
From: Eric Dumazet @ 2005-09-22 13:05 UTC (permalink / raw)
To: Harald Welte; +Cc: netdev, netfilter-devel, linux-kernel, Andi Kleen
In-Reply-To: <20050922124803.GH26520@sunbeam.de.gnumonks.org>
Harald Welte a écrit :
> On Wed, Sep 21, 2005 at 11:32:24PM +0200, Eric Dumazet wrote:
>
>>Patch 2/3 (please apply after Patch 1/3)
>>
>>2) Loop unrolling
>
>
> First of all, thanks for your performance analysis and patches. I'm
> very inclined of merging them.
>
>
>>It seems that with current compilers and CFLAGS, the code from
>>ip_packet_match() is very bad, using lot of mispredicted conditional branches I made some patches
>>and generated code on i386 and x86_64
>>is much better.
>
>
> This only describes your "compare_if_lstrings()" changes, and I'm happy
> to merge them.
>
> However, you also removed the use of the FWINV macro and replaced it by
> explicit code (including the bool1/bool2 variables, which are not really
> named intuitively). Why was this neccessary?
>
It was necessary to get the best code with gcc-3.4.4 on i386 and gcc-4.0.1 on
x86_64
For example :
bool1 = FWINV(ret != 0, IPT_INV_VIA_OUT);
if (bool1) {
gives a better code than :
if (FWINV(ret != 0, IPT_INV_VIA_OUT)) {
(one less conditional branch)
Dont ask me why, it is shocking but true :(
Eric
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: Andi Kleen @ 2005-09-22 13:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Eric Dumazet, Christoph Lameter, David S. Miller, linux-kernel,
netfilter-devel, netdev
In-Reply-To: <20050922125849.GA27413@infradead.org>
On Thursday 22 September 2005 14:58, Christoph Hellwig wrote:
> Umm, no - adding set_fs/get_fs mess for things like that is not right.
I think it's fine. We're using it for various other interfaces too. In fact
sys_set_mempolicy is already used elsewhere in the kernel too.
-Andi
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: Andi Kleen @ 2005-09-22 13:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, netfilter-devel, netdev
In-Reply-To: <4331CFA7.50104@cosmosbay.com>
> 1) No more central rwlock protecting each table (filter, nat, mangle, raw),
> but one lock per CPU. It avoids cache line ping pongs for each packet.
Another useful change would be to not take the lock when there are no
rules. Currently just loading iptables has a large overhead.
-Andi
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: Christoph Hellwig @ 2005-09-22 12:58 UTC (permalink / raw)
To: Andi Kleen
Cc: Christoph Hellwig, Eric Dumazet, Christoph Lameter,
David S. Miller, linux-kernel, netfilter-devel, netdev
In-Reply-To: <200509221454.22923.ak@suse.de>
On Thu, Sep 22, 2005 at 02:54:22PM +0200, Andi Kleen wrote:
> > > +void *vmalloc_node(unsigned long size, int node)
> > > +{
> > > + void *result;
> > > + struct mempolicy *oldpol = current->mempolicy;
> > > + mm_segment_t oldfs = get_fs();
> > > + DECLARE_BITMAP(prefnode, MAX_NUMNODES);
> > > +
> > > + mpol_get(oldpol);
> > > + bitmap_zero(prefnode, MAX_NUMNODES);
> > > + set_bit(node, prefnode);
> > > +
> > > + set_fs(KERNEL_DS);
> > > + sys_set_mempolicy(MPOL_PREFERRED, prefnode, MAX_NUMNODES);
> > > + set_fs(oldfs);
> > > +
> > > + result = vmalloc(size);
> > > +
> > > + mpol_free(current->mempolicy);
> > > + current->mempolicy = oldpol;
> > > + return result;
> > > +}
> >
> > No way, sorry. If you want a vmalloc node do it right.
>
> The implementation looks fine to me, so I think it's already right.
Umm, no - adding set_fs/get_fs mess for things like that is not right.
If we want to go down the mempolicy-based route we need to add a proper
kernel entry point for setting a mempolicy
^ permalink raw reply
* Re: [PATCH 1/3] netfilter : 3 patches to boost ip_tables performance
From: Harald Welte @ 2005-09-22 12:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, netfilter-devel, linux-kernel, Andi Kleen
In-Reply-To: <4331D0A9.3080801@cosmosbay.com>
[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]
On Wed, Sep 21, 2005 at 11:29:13PM +0200, Eric Dumazet wrote:
> Patch 1/3
>
> 1) No more one rwlock_t protecting the 'curtain'
I have no problem with this change "per se", but with the
implementation.
As of now, we live without any ugly #ifdef CONFIG_SMP / #endif sections
in the code - and if possible, I would continue this good tradition.
For example the get_counters() function. Wouldn't all the smp specific
code (for_each_cpu(), ...) be #defined to nothing anyway?
And if we really need the #ifdef's, I would appreciate if those
sectionas are as small as possible. in get_counters() the section can
definitely be smaller, rather than basically having the whole function
body separate for smp and non-smp cases.
Also, how much would we loose in runtime performance if we were using a
"rwlock_t *" even in the UP case?. I mean, it's just one more pointer
dereference of something that is expected to be in cache anyway, isn't
it? This gets rid of another huge set of #ifdefs that make the code
unreadable and prone to errors being introduced later on.
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: Andi Kleen @ 2005-09-22 12:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Eric Dumazet, Christoph Lameter, David S. Miller, linux-kernel,
netfilter-devel, netdev
In-Reply-To: <20050922124941.GA26936@infradead.org>
On Thursday 22 September 2005 14:49, Christoph Hellwig wrote:
> On Thu, Sep 22, 2005 at 02:11:26PM +0200, Eric Dumazet wrote:
> > +#ifdef CONFIG_NUMA
> > +/**
> > + * vmalloc_node - allocate virtually contiguous memory
> > + *
> > + * @size: allocation size
> > + * @node: preferred node
> > + *
> > + * This vmalloc variant try to allocate memory from a preferred node.
> > + */
> > +void *vmalloc_node(unsigned long size, int node)
> > +{
> > + void *result;
> > + struct mempolicy *oldpol = current->mempolicy;
> > + mm_segment_t oldfs = get_fs();
> > + DECLARE_BITMAP(prefnode, MAX_NUMNODES);
> > +
> > + mpol_get(oldpol);
> > + bitmap_zero(prefnode, MAX_NUMNODES);
> > + set_bit(node, prefnode);
> > +
> > + set_fs(KERNEL_DS);
> > + sys_set_mempolicy(MPOL_PREFERRED, prefnode, MAX_NUMNODES);
> > + set_fs(oldfs);
> > +
> > + result = vmalloc(size);
> > +
> > + mpol_free(current->mempolicy);
> > + current->mempolicy = oldpol;
> > + return result;
> > +}
>
> No way, sorry. If you want a vmalloc node do it right.
The implementation looks fine to me, so I think it's already right.
-Andi
^ permalink raw reply
* Re: [PATCH 3/3] netfilter : 3 patches to boost ip_tables performance
From: Harald Welte @ 2005-09-22 12:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, netfilter-devel, linux-kernel, Andi Kleen
In-Reply-To: <4331D290.6070104@cosmosbay.com>
[-- Attachment #1: Type: text/plain, Size: 664 bytes --]
On Wed, Sep 21, 2005 at 11:37:20PM +0200, Eric Dumazet wrote:
> Patch 3/3 (please apply after Patch 2/3)
Thanks for your patches, again.
> 3) NUMA allocation.
I'll wait with applying this patch until a decision has been made on
merging something like vmalloc_node() to mainline.
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: Christoph Hellwig @ 2005-09-22 12:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: Christoph Lameter, David S. Miller, linux-kernel, netfilter-devel,
netdev, ak
In-Reply-To: <43329F6E.3030706@cosmosbay.com>
On Thu, Sep 22, 2005 at 02:11:26PM +0200, Eric Dumazet wrote:
> +#ifdef CONFIG_NUMA
> +/**
> + * vmalloc_node - allocate virtually contiguous memory
> + *
> + * @size: allocation size
> + * @node: preferred node
> + *
> + * This vmalloc variant try to allocate memory from a preferred node.
> + */
> +void *vmalloc_node(unsigned long size, int node)
> +{
> + void *result;
> + struct mempolicy *oldpol = current->mempolicy;
> + mm_segment_t oldfs = get_fs();
> + DECLARE_BITMAP(prefnode, MAX_NUMNODES);
> +
> + mpol_get(oldpol);
> + bitmap_zero(prefnode, MAX_NUMNODES);
> + set_bit(node, prefnode);
> +
> + set_fs(KERNEL_DS);
> + sys_set_mempolicy(MPOL_PREFERRED, prefnode, MAX_NUMNODES);
> + set_fs(oldfs);
> +
> + result = vmalloc(size);
> +
> + mpol_free(current->mempolicy);
> + current->mempolicy = oldpol;
> + return result;
> +}
No way, sorry. If you want a vmalloc node do it right.
^ permalink raw reply
* Re: [PATCH 2/3] netfilter : 3 patches to boost ip_tables performance
From: Harald Welte @ 2005-09-22 12:48 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, netfilter-devel, linux-kernel, Andi Kleen
In-Reply-To: <4331D168.6090604@cosmosbay.com>
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On Wed, Sep 21, 2005 at 11:32:24PM +0200, Eric Dumazet wrote:
> Patch 2/3 (please apply after Patch 1/3)
>
> 2) Loop unrolling
First of all, thanks for your performance analysis and patches. I'm
very inclined of merging them.
> It seems that with current compilers and CFLAGS, the code from
> ip_packet_match() is very bad, using lot of mispredicted conditional branches I made some patches
> and generated code on i386 and x86_64
> is much better.
This only describes your "compare_if_lstrings()" changes, and I'm happy
to merge them.
However, you also removed the use of the FWINV macro and replaced it by
explicit code (including the bool1/bool2 variables, which are not really
named intuitively). Why was this neccessary?
--
- Harald Welte <laforge@netfilter.org> http://netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: Eric Dumazet @ 2005-09-22 12:11 UTC (permalink / raw)
To: Christoph Lameter
Cc: netdev, ak, netfilter-devel, David S. Miller, linux-kernel
In-Reply-To: <Pine.LNX.4.62.0509211843530.13764@schroedinger.engr.sgi.com>
[-- Attachment #1: Type: text/plain, Size: 868 bytes --]
Christoph Lameter a écrit :
> On Wed, 21 Sep 2005, David S. Miller wrote:
>
>
>>From: Christoph Lameter <clameter@engr.sgi.com>
>>Date: Wed, 21 Sep 2005 15:43:29 -0700 (PDT)
>>
>>
>>>Maybe we better introduce vmalloc_node() instead of improvising this for
>>>several subsystems? The e1000 driver has similar issues.
>>
>>I agree.
>
>
> I did an implementation in June.
>
> See http://marc.theaimsgroup.com/?l=linux-mm&m=111766643127530&w=2
>
> Not sure if this will fit the bill. Never really tested it.
Maybe this simpler patch has more chances to be accepted ?
Thank you
[NUMA]
- Adds a vmalloc_node() function : A simple wrapper around vmalloc() to
allocate memory from a preferred node.
This NUMA aware variant will be used by ip_tables and various network drivers.
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
[-- Attachment #2: vmalloc_node --]
[-- Type: text/plain, Size: 2418 bytes --]
--- linux-2.6.14-rc2/include/linux/vmalloc.h 2005-09-20 05:00:41.000000000 +0200
+++ linux-2.6.14-rc2-ed/include/linux/vmalloc.h 2005-09-22 11:34:55.000000000 +0200
@@ -1,6 +1,7 @@
#ifndef _LINUX_VMALLOC_H
#define _LINUX_VMALLOC_H
+#include <linux/config.h> /* vmalloc_node() needs CONFIG_ options */
#include <linux/spinlock.h>
#include <asm/page.h> /* pgprot_t */
@@ -32,6 +33,14 @@
* Highlevel APIs for driver use
*/
extern void *vmalloc(unsigned long size);
+#ifdef CONFIG_NUMA
+extern void *vmalloc_node(unsigned long size, int node);
+#else
+static inline void *vmalloc_node(unsigned long size, int node)
+{
+ return vmalloc(size);
+}
+#endif
extern void *vmalloc_exec(unsigned long size);
extern void *vmalloc_32(unsigned long size);
extern void *__vmalloc(unsigned long size, unsigned int __nocast gfp_mask, pgprot_t prot);
--- linux-2.6.14-rc2/mm/vmalloc.c 2005-09-20 05:00:41.000000000 +0200
+++ linux-2.6.14-rc2-ed/mm/vmalloc.c 2005-09-22 11:55:19.000000000 +0200
@@ -19,6 +19,9 @@
#include <asm/uaccess.h>
#include <asm/tlbflush.h>
+#ifdef CONFIG_NUMA
+#include <linux/mempolicy.h>
+#endif
DEFINE_RWLOCK(vmlist_lock);
struct vm_struct *vmlist;
@@ -471,7 +474,7 @@
* Allocate enough pages to cover @size from the page level
* allocator and map them into contiguous kernel virtual space.
*
- * For tight cotrol over page level allocator and protection flags
+ * For tight control over page level allocator and protection flags
* use __vmalloc() instead.
*/
void *vmalloc(unsigned long size)
@@ -481,6 +484,40 @@
EXPORT_SYMBOL(vmalloc);
+#ifdef CONFIG_NUMA
+/**
+ * vmalloc_node - allocate virtually contiguous memory
+ *
+ * @size: allocation size
+ * @node: preferred node
+ *
+ * This vmalloc variant try to allocate memory from a preferred node.
+ */
+void *vmalloc_node(unsigned long size, int node)
+{
+ void *result;
+ struct mempolicy *oldpol = current->mempolicy;
+ mm_segment_t oldfs = get_fs();
+ DECLARE_BITMAP(prefnode, MAX_NUMNODES);
+
+ mpol_get(oldpol);
+ bitmap_zero(prefnode, MAX_NUMNODES);
+ set_bit(node, prefnode);
+
+ set_fs(KERNEL_DS);
+ sys_set_mempolicy(MPOL_PREFERRED, prefnode, MAX_NUMNODES);
+ set_fs(oldfs);
+
+ result = vmalloc(size);
+
+ mpol_free(current->mempolicy);
+ current->mempolicy = oldpol;
+ return result;
+}
+
+EXPORT_SYMBOL(vmalloc_node);
+#endif
+
#ifndef PAGE_KERNEL_EXEC
# define PAGE_KERNEL_EXEC PAGE_KERNEL
#endif
^ permalink raw reply
* information for you caldwell
From: jamal velazques @ 2005-09-22 11:39 UTC (permalink / raw)
To: Josiah Unruh; +Cc: knight, linux-xfs
Luis:
http://uk.geocities.com/abaskpill/?They_look_fantastic ei
UnderstandThat dreams can come true[CHORUS]
empdata fendrichs datawatchs sz03 cyndal ejuh
We say to the doctore, "No. Please. No!"
^ permalink raw reply
* Re: [PATCH] remove check_region from PnPWakeUp routine of Eepro ISA driver
From: Andrew Morton @ 2005-09-22 6:37 UTC (permalink / raw)
To: Borislav Petkov; +Cc: netdev, linux-kernel
In-Reply-To: <20050922062943.GA31805@gollum.tnic>
Borislav Petkov <petkov@uni-muenster.de> wrote:
>
> > That being said, the code you've modified is disabled due to
> > !defined(PnPWakeup), and the chances of anyone ever changing that are close
> > to zero.
> should I remove it then altogether?
We might as well leave it there in case someone is inspired to reenable the
feature and work on the driver a bit. The chances of that are near zero -
it's ancient. I don't think there's much point in spending time on this
driver.
^ permalink raw reply
* Re: [PATCH] remove check_region from PnPWakeUp routine of Eepro ISA driver
From: Borislav Petkov @ 2005-09-22 6:29 UTC (permalink / raw)
To: Andrew Morton; +Cc: netdev, linux-kernel
In-Reply-To: <20050921230915.364f0ac9.akpm@osdl.org>
On Wed, Sep 21, 2005 at 11:09:15PM -0700, Andrew Morton wrote:
> Borislav Petkov <petkov@uni-muenster.de> wrote:
> >
> > Hi,
> >
> > The following patch removes the check_region call in the PnPWakeUp
> > path in the Eepro /10 ISA driver. Instead, now it calls request_region
> > for the PnP wake up routine and, after succeeding, it calls release_region
> > for the actual reservation of I/O ports takes place in the eepro_probe1()
> > function straight afterwards.
> >
> > Signed-off-by: Borislav Petkov <petkov@uni-muenster.de>
> >
> >
> > --- drivers/net/eepro.c.orig 2005-09-22 06:20:28.000000000 +0200
> > +++ drivers/net/eepro.c 2005-09-22 07:20:16.000000000 +0200
> > @@ -552,7 +552,7 @@ static int __init do_eepro_probe(struct
> > {
> > unsigned short int WS[32]=WakeupSeq;
> >
> > - if (check_region(WakeupPort, 2)==0) {
> > + if (request_region(WakeupPort, 2, DRV_NAME)) {
> >
> > if (net_debug>5)
> > printk(KERN_DEBUG "Waking UP\n");
> > @@ -563,6 +563,8 @@ static int __init do_eepro_probe(struct
> > outb_p(WS[i],WakeupPort);
> > if (net_debug>5) printk(KERN_DEBUG ": %#x ",WS[i]);
> > }
> > + release_region(WakeupPort, 2);
> > +
> > } else printk(KERN_WARNING "Checkregion Failed!\n");
> > }
> > #endif
>
> hm, that's all a bit strange. It would be better to do the
> request_region() just once and if that works, retain the reservation rather
> than releasing and reacquiring it.
I thought so at first too but was having problems with the call to
eepro_probe1() a bit further down where request_region() is called a second time
and this would have resulted in two consecutive calls to request_region for the
same region without freeing it. I guess in the actual __request_region() in the
while loop, during the second call IORESOURCE_BUSY will be returned meaning that
the region is already reserved?
>
> That being said, the code you've modified is disabled due to
> !defined(PnPWakeup), and the chances of anyone ever changing that are close
> to zero.
should I remove it then altogether?
Regards,
Boris.
^ permalink raw reply
* Re: [PATCH] remove check_region from PnPWakeUp routine of Eepro ISA driver
From: Andrew Morton @ 2005-09-22 6:09 UTC (permalink / raw)
To: Borislav Petkov; +Cc: netdev, linux-kernel
In-Reply-To: <20050922060030.GB19049@gollum.tnic>
Borislav Petkov <petkov@uni-muenster.de> wrote:
>
> Hi,
>
> The following patch removes the check_region call in the PnPWakeUp
> path in the Eepro /10 ISA driver. Instead, now it calls request_region
> for the PnP wake up routine and, after succeeding, it calls release_region
> for the actual reservation of I/O ports takes place in the eepro_probe1()
> function straight afterwards.
>
> Signed-off-by: Borislav Petkov <petkov@uni-muenster.de>
>
>
> --- drivers/net/eepro.c.orig 2005-09-22 06:20:28.000000000 +0200
> +++ drivers/net/eepro.c 2005-09-22 07:20:16.000000000 +0200
> @@ -552,7 +552,7 @@ static int __init do_eepro_probe(struct
> {
> unsigned short int WS[32]=WakeupSeq;
>
> - if (check_region(WakeupPort, 2)==0) {
> + if (request_region(WakeupPort, 2, DRV_NAME)) {
>
> if (net_debug>5)
> printk(KERN_DEBUG "Waking UP\n");
> @@ -563,6 +563,8 @@ static int __init do_eepro_probe(struct
> outb_p(WS[i],WakeupPort);
> if (net_debug>5) printk(KERN_DEBUG ": %#x ",WS[i]);
> }
> + release_region(WakeupPort, 2);
> +
> } else printk(KERN_WARNING "Checkregion Failed!\n");
> }
> #endif
hm, that's all a bit strange. It would be better to do the
request_region() just once and if that works, retain the reservation rather
than releasing and reacquiring it.
That being said, the code you've modified is disabled due to
!defined(PnPWakeup), and the chances of anyone ever changing that are close
to zero.
^ permalink raw reply
* Re: [PATCH] channel bonding: add support for device-indexed parameters
From: Jason R. Martin @ 2005-09-22 6:03 UTC (permalink / raw)
To: Florin Malita
Cc: akpm, davem, ctindel, fubar, linux-kernel, netdev, bonding-devel
In-Reply-To: <20050922000444.369c32c2.fmalita@gmail.com>
On 9/21/05, Florin Malita <fmalita@gmail.com> wrote:
> While originally I was interested in being able to set a different
> primary interface for each bond device (same primary for all bond
> devices doesn't make any sense), most parameters deserve the same
> treatement.
>
> This patch adds support for device indexed module parameter
> arrays instead of the old plain scalars. Mostly module_param
> substitutions and parameter parsing logic tweaking.
[snip]
Personally I think working to get the sysfs support finished in
bonding and stop relying on module parameters to configure bonds would
be better, since bonds will truly be independent of each other and be
able to be added and removed on the fly. Having worked with a
previous attempt to set per-bond values through module parameters
(http://marc.theaimsgroup.com/?t=110558187800001&r=1&w=2), it's easy
to get pretty crazy. For example, you can have more than one
arp_ip_target, and they really should be per bond as well, so how do
you divvy those up via module parameters?
Jason
^ permalink raw reply
* [PATCH] remove check_region from PnPWakeUp routine of Eepro ISA driver
From: Borislav Petkov @ 2005-09-22 6:00 UTC (permalink / raw)
To: netdev; +Cc: akpm, linux-kernel
Hi,
The following patch removes the check_region call in the PnPWakeUp
path in the Eepro /10 ISA driver. Instead, now it calls request_region
for the PnP wake up routine and, after succeeding, it calls release_region
for the actual reservation of I/O ports takes place in the eepro_probe1()
function straight afterwards.
Signed-off-by: Borislav Petkov <petkov@uni-muenster.de>
--- drivers/net/eepro.c.orig 2005-09-22 06:20:28.000000000 +0200
+++ drivers/net/eepro.c 2005-09-22 07:20:16.000000000 +0200
@@ -552,7 +552,7 @@ static int __init do_eepro_probe(struct
{
unsigned short int WS[32]=WakeupSeq;
- if (check_region(WakeupPort, 2)==0) {
+ if (request_region(WakeupPort, 2, DRV_NAME)) {
if (net_debug>5)
printk(KERN_DEBUG "Waking UP\n");
@@ -563,6 +563,8 @@ static int __init do_eepro_probe(struct
outb_p(WS[i],WakeupPort);
if (net_debug>5) printk(KERN_DEBUG ": %#x ",WS[i]);
}
+ release_region(WakeupPort, 2);
+
} else printk(KERN_WARNING "Checkregion Failed!\n");
}
#endif
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: Eric Dumazet @ 2005-09-22 5:07 UTC (permalink / raw)
To: James Morris; +Cc: linux-kernel, netfilter-devel, netdev, Andi Kleen
In-Reply-To: <Pine.LNX.4.63.0509220017430.6397@excalibur.intercode>
James Morris a écrit :
>
> Do you have any performance measurements?
Yes, as I said in the first mail :
>In oprofile results, ipt_do_table() was at the first position.
>It is now at 6th position, using 1/3 of the CPU it was using before.
>(Tests done on a dual Xeon i386 and a dual Opteron x86_64)
On the dual opteron machine, with 40.000 packets coming per second, and 35.000
sent per second, the numbers were : 12.8 % before the patches, 4.4 % after the
patches.
I dont have separate perf measurements for each patch.
Considering the fact that I inlined the read_lock_bh() call (not displayed in
oprofile results, probably because of the special .spinlock.text section) that
should have increased the profile of ipt_do_table(), thats a lot of CPU cycles
and mem bandwitdh that are available for other jobs.
Eric
^ permalink raw reply
* Re: [PATCH 0/3] netfilter : 3 patches to boost ip_tables performance
From: James Morris @ 2005-09-22 4:18 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, netfilter-devel, linux-kernel, Andi Kleen
In-Reply-To: <4331CFA7.50104@cosmosbay.com>
On Wed, 21 Sep 2005, Eric Dumazet wrote:
> I have reworked net/ipv4/ip_tables.c to boost its performance, and post three
> patches.
Do you have any performance measurements?
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [git patches] net driver fixes
From: Jeff Garzik @ 2005-09-22 4:11 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds; +Cc: netdev, linux-kernel
Please pull from 'upstream-fixes' branch of
rsync://rsync.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git
to obtain the following fixes:
drivers/net/bonding/bond_main.c | 3
drivers/net/r8169.c | 4
drivers/net/skge.c | 194 ++++++++++++++++++----------------------
drivers/net/skge.h | 2
4 files changed, 97 insertions(+), 106 deletions(-)
nsxfreddy@gmail.com:
bonding: Fix link monitor capability check (was skge: set mac address oops with bonding)
Stephen Hemminger:
skge: expand ethtool debug register dump
skge: check length from PHY
Tommy Christensen:
r8169: call proper VLAN receive function
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1653,7 +1653,8 @@ static int bond_enslave(struct net_devic
int old_features = bond_dev->features;
int res = 0;
- if (slave_dev->do_ioctl == NULL) {
+ if (!bond->params.use_carrier && slave_dev->ethtool_ops == NULL &&
+ slave_dev->do_ioctl == NULL) {
printk(KERN_WARNING DRV_NAME
": Warning : no link monitoring support for %s\n",
slave_dev->name);
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -100,11 +100,11 @@ VERSION 2.2LK <2005/01/25>
#ifdef CONFIG_R8169_NAPI
#define rtl8169_rx_skb netif_receive_skb
-#define rtl8169_rx_hwaccel_skb vlan_hwaccel_rx
+#define rtl8169_rx_hwaccel_skb vlan_hwaccel_receive_skb
#define rtl8169_rx_quota(count, quota) min(count, quota)
#else
#define rtl8169_rx_skb netif_rx
-#define rtl8169_rx_hwaccel_skb vlan_hwaccel_receive_skb
+#define rtl8169_rx_hwaccel_skb vlan_hwaccel_rx
#define rtl8169_rx_quota(count, quota) count
#endif
diff --git a/drivers/net/skge.c b/drivers/net/skge.c
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -42,7 +42,7 @@
#include "skge.h"
#define DRV_NAME "skge"
-#define DRV_VERSION "1.0"
+#define DRV_VERSION "1.1"
#define PFX DRV_NAME " "
#define DEFAULT_TX_RING_SIZE 128
@@ -105,41 +105,28 @@ static const u32 rxirqmask[] = { IS_R1_F
static const u32 txirqmask[] = { IS_XA1_F, IS_XA2_F };
static const u32 portirqmask[] = { IS_PORT_1, IS_PORT_2 };
-/* Don't need to look at whole 16K.
- * last interesting register is descriptor poll timer.
- */
-#define SKGE_REGS_LEN (29*128)
-
static int skge_get_regs_len(struct net_device *dev)
{
- return SKGE_REGS_LEN;
+ return 0x4000;
}
/*
- * Returns copy of control register region
- * I/O region is divided into banks and certain regions are unreadable
+ * Returns copy of whole control register region
+ * Note: skip RAM address register because accessing it will
+ * cause bus hangs!
*/
static void skge_get_regs(struct net_device *dev, struct ethtool_regs *regs,
void *p)
{
const struct skge_port *skge = netdev_priv(dev);
- unsigned long offs;
const void __iomem *io = skge->hw->regs;
- static const unsigned long bankmap
- = (1<<0) | (1<<2) | (1<<8) | (1<<9)
- | (1<<12) | (1<<13) | (1<<14) | (1<<15) | (1<<16)
- | (1<<17) | (1<<20) | (1<<21) | (1<<22) | (1<<23)
- | (1<<24) | (1<<25) | (1<<26) | (1<<27) | (1<<28);
regs->version = 1;
- for (offs = 0; offs < regs->len; offs += 128) {
- u32 len = min_t(u32, 128, regs->len - offs);
+ memset(p, 0, regs->len);
+ memcpy_fromio(p, io, B3_RAM_ADDR);
- if (bankmap & (1<<(offs/128)))
- memcpy_fromio(p + offs, io + offs, len);
- else
- memset(p + offs, 0, len);
- }
+ memcpy_fromio(p + B3_RI_WTO_R1, io + B3_RI_WTO_R1,
+ regs->len - B3_RI_WTO_R1);
}
/* Wake on Lan only supported on Yukon chps with rev 1 or above */
@@ -775,17 +762,6 @@ static int skge_ring_alloc(struct skge_r
return 0;
}
-static struct sk_buff *skge_rx_alloc(struct net_device *dev, unsigned int size)
-{
- struct sk_buff *skb = dev_alloc_skb(size);
-
- if (likely(skb)) {
- skb->dev = dev;
- skb_reserve(skb, NET_IP_ALIGN);
- }
- return skb;
-}
-
/* Allocate and setup a new buffer for receiving */
static void skge_rx_setup(struct skge_port *skge, struct skge_element *e,
struct sk_buff *skb, unsigned int bufsize)
@@ -858,16 +834,17 @@ static int skge_rx_fill(struct skge_port
{
struct skge_ring *ring = &skge->rx_ring;
struct skge_element *e;
- unsigned int bufsize = skge->rx_buf_size;
e = ring->start;
do {
- struct sk_buff *skb = skge_rx_alloc(skge->netdev, bufsize);
+ struct sk_buff *skb;
+ skb = dev_alloc_skb(skge->rx_buf_size + NET_IP_ALIGN);
if (!skb)
return -ENOMEM;
- skge_rx_setup(skge, e, skb, bufsize);
+ skb_reserve(skb, NET_IP_ALIGN);
+ skge_rx_setup(skge, e, skb, skge->rx_buf_size);
} while ( (e = e->next) != ring->start);
ring->to_clean = ring->start;
@@ -2442,6 +2419,14 @@ static void yukon_set_multicast(struct n
gma_write16(hw, port, GM_RX_CTRL, reg);
}
+static inline u16 phy_length(const struct skge_hw *hw, u32 status)
+{
+ if (hw->chip_id == CHIP_ID_GENESIS)
+ return status >> XMR_FS_LEN_SHIFT;
+ else
+ return status >> GMR_FS_LEN_SHIFT;
+}
+
static inline int bad_phy_status(const struct skge_hw *hw, u32 status)
{
if (hw->chip_id == CHIP_ID_GENESIS)
@@ -2451,80 +2436,99 @@ static inline int bad_phy_status(const s
(status & GMR_FS_RX_OK) == 0;
}
-static void skge_rx_error(struct skge_port *skge, int slot,
- u32 control, u32 status)
-{
- if (netif_msg_rx_err(skge))
- printk(KERN_DEBUG PFX "%s: rx err, slot %d control 0x%x status 0x%x\n",
- skge->netdev->name, slot, control, status);
-
- if ((control & (BMU_EOF|BMU_STF)) != (BMU_STF|BMU_EOF))
- skge->net_stats.rx_length_errors++;
- else if (skge->hw->chip_id == CHIP_ID_GENESIS) {
- if (status & (XMR_FS_RUNT|XMR_FS_LNG_ERR))
- skge->net_stats.rx_length_errors++;
- if (status & XMR_FS_FRA_ERR)
- skge->net_stats.rx_frame_errors++;
- if (status & XMR_FS_FCS_ERR)
- skge->net_stats.rx_crc_errors++;
- } else {
- if (status & (GMR_FS_LONG_ERR|GMR_FS_UN_SIZE))
- skge->net_stats.rx_length_errors++;
- if (status & GMR_FS_FRAGMENT)
- skge->net_stats.rx_frame_errors++;
- if (status & GMR_FS_CRC_ERR)
- skge->net_stats.rx_crc_errors++;
- }
-}
/* Get receive buffer from descriptor.
* Handles copy of small buffers and reallocation failures
*/
static inline struct sk_buff *skge_rx_get(struct skge_port *skge,
struct skge_element *e,
- unsigned int len)
+ u32 control, u32 status, u16 csum)
{
- struct sk_buff *nskb, *skb;
+ struct sk_buff *skb;
+ u16 len = control & BMU_BBC;
+
+ if (unlikely(netif_msg_rx_status(skge)))
+ printk(KERN_DEBUG PFX "%s: rx slot %td status 0x%x len %d\n",
+ skge->netdev->name, e - skge->rx_ring.start,
+ status, len);
+
+ if (len > skge->rx_buf_size)
+ goto error;
+
+ if ((control & (BMU_EOF|BMU_STF)) != (BMU_STF|BMU_EOF))
+ goto error;
+
+ if (bad_phy_status(skge->hw, status))
+ goto error;
+
+ if (phy_length(skge->hw, status) != len)
+ goto error;
if (len < RX_COPY_THRESHOLD) {
- nskb = skge_rx_alloc(skge->netdev, len + NET_IP_ALIGN);
- if (unlikely(!nskb))
- return NULL;
+ skb = dev_alloc_skb(len + 2);
+ if (!skb)
+ goto resubmit;
+ skb_reserve(skb, 2);
pci_dma_sync_single_for_cpu(skge->hw->pdev,
pci_unmap_addr(e, mapaddr),
len, PCI_DMA_FROMDEVICE);
- memcpy(nskb->data, e->skb->data, len);
+ memcpy(skb->data, e->skb->data, len);
pci_dma_sync_single_for_device(skge->hw->pdev,
pci_unmap_addr(e, mapaddr),
len, PCI_DMA_FROMDEVICE);
-
- if (skge->rx_csum) {
- struct skge_rx_desc *rd = e->desc;
- nskb->csum = le16_to_cpu(rd->csum2);
- nskb->ip_summed = CHECKSUM_HW;
- }
skge_rx_reuse(e, skge->rx_buf_size);
- return nskb;
} else {
- nskb = skge_rx_alloc(skge->netdev, skge->rx_buf_size);
- if (unlikely(!nskb))
- return NULL;
+ struct sk_buff *nskb;
+ nskb = dev_alloc_skb(skge->rx_buf_size + NET_IP_ALIGN);
+ if (!nskb)
+ goto resubmit;
pci_unmap_single(skge->hw->pdev,
pci_unmap_addr(e, mapaddr),
pci_unmap_len(e, maplen),
PCI_DMA_FROMDEVICE);
skb = e->skb;
- if (skge->rx_csum) {
- struct skge_rx_desc *rd = e->desc;
- skb->csum = le16_to_cpu(rd->csum2);
- skb->ip_summed = CHECKSUM_HW;
- }
-
+ prefetch(skb->data);
skge_rx_setup(skge, e, nskb, skge->rx_buf_size);
- return skb;
}
+
+ skb_put(skb, len);
+ skb->dev = skge->netdev;
+ if (skge->rx_csum) {
+ skb->csum = csum;
+ skb->ip_summed = CHECKSUM_HW;
+ }
+
+ skb->protocol = eth_type_trans(skb, skge->netdev);
+
+ return skb;
+error:
+
+ if (netif_msg_rx_err(skge))
+ printk(KERN_DEBUG PFX "%s: rx err, slot %td control 0x%x status 0x%x\n",
+ skge->netdev->name, e - skge->rx_ring.start,
+ control, status);
+
+ if (skge->hw->chip_id == CHIP_ID_GENESIS) {
+ if (status & (XMR_FS_RUNT|XMR_FS_LNG_ERR))
+ skge->net_stats.rx_length_errors++;
+ if (status & XMR_FS_FRA_ERR)
+ skge->net_stats.rx_frame_errors++;
+ if (status & XMR_FS_FCS_ERR)
+ skge->net_stats.rx_crc_errors++;
+ } else {
+ if (status & (GMR_FS_LONG_ERR|GMR_FS_UN_SIZE))
+ skge->net_stats.rx_length_errors++;
+ if (status & GMR_FS_FRAGMENT)
+ skge->net_stats.rx_frame_errors++;
+ if (status & GMR_FS_CRC_ERR)
+ skge->net_stats.rx_crc_errors++;
+ }
+
+resubmit:
+ skge_rx_reuse(e, skge->rx_buf_size);
+ return NULL;
}
@@ -2540,32 +2544,16 @@ static int skge_poll(struct net_device *
for (e = ring->to_clean; work_done < to_do; e = e->next) {
struct skge_rx_desc *rd = e->desc;
struct sk_buff *skb;
- u32 control, len, status;
+ u32 control;
rmb();
control = rd->control;
if (control & BMU_OWN)
break;
- len = control & BMU_BBC;
- status = rd->status;
-
- if (unlikely((control & (BMU_EOF|BMU_STF)) != (BMU_STF|BMU_EOF)
- || bad_phy_status(hw, status))) {
- skge_rx_error(skge, e - ring->start, control, status);
- skge_rx_reuse(e, skge->rx_buf_size);
- continue;
- }
-
- if (netif_msg_rx_status(skge))
- printk(KERN_DEBUG PFX "%s: rx slot %td status 0x%x len %d\n",
- dev->name, e - ring->start, rd->status, len);
-
- skb = skge_rx_get(skge, e, len);
+ skb = skge_rx_get(skge, e, control, rd->status,
+ le16_to_cpu(rd->csum2));
if (likely(skb)) {
- skb_put(skb, len);
- skb->protocol = eth_type_trans(skb, dev);
-
dev->last_rx = jiffies;
netif_receive_skb(skb);
diff --git a/drivers/net/skge.h b/drivers/net/skge.h
--- a/drivers/net/skge.h
+++ b/drivers/net/skge.h
@@ -953,6 +953,7 @@ enum {
*/
enum {
XMR_FS_LEN = 0x3fff<<18, /* Bit 31..18: Rx Frame Length */
+ XMR_FS_LEN_SHIFT = 18,
XMR_FS_2L_VLAN = 1<<17, /* Bit 17: tagged wh 2Lev VLAN ID*/
XMR_FS_1_VLAN = 1<<16, /* Bit 16: tagged wh 1ev VLAN ID*/
XMR_FS_BC = 1<<15, /* Bit 15: Broadcast Frame */
@@ -1868,6 +1869,7 @@ enum {
/* Receive Frame Status Encoding */
enum {
GMR_FS_LEN = 0xffff<<16, /* Bit 31..16: Rx Frame Length */
+ GMR_FS_LEN_SHIFT = 16,
GMR_FS_VLAN = 1<<13, /* Bit 13: VLAN Packet */
GMR_FS_JABBER = 1<<12, /* Bit 12: Jabber Packet */
GMR_FS_UN_SIZE = 1<<11, /* Bit 11: Undersize Packet */
^ permalink raw reply
* [PATCH] channel bonding: add support for device-indexed parameters
From: Florin Malita @ 2005-09-22 4:04 UTC (permalink / raw)
To: akpm, davem; +Cc: ctindel, fubar, linux-kernel, netdev
While originally I was interested in being able to set a different
primary interface for each bond device (same primary for all bond
devices doesn't make any sense), most parameters deserve the same
treatement.
This patch adds support for device indexed module parameter
arrays instead of the old plain scalars. Mostly module_param
substitutions and parameter parsing logic tweaking.
Signed-off-by: Florin Malita <fmalita@gmail.com>
---
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -487,6 +487,8 @@
* * Added xmit_hash_policy_layer34()
* - Modified by Jay Vosburgh <fubar@us.ibm.com> to also support mode 4.
* Set version to 2.6.3.
+ * 2005/09/20 - Florin Malita <fmalita at gmail dot com>
+ * - Added support for device-indexed module parameters.
*/
//#define BONDING_DEBUG 1
@@ -545,38 +547,47 @@
#define BOND_LINK_ARP_INTERV 0
static int max_bonds = BOND_DEFAULT_MAX_BONDS;
-static int miimon = BOND_LINK_MON_INTERV;
-static int updelay = 0;
-static int downdelay = 0;
-static int use_carrier = 1;
-static char *mode = NULL;
-static char *primary = NULL;
-static char *lacp_rate = NULL;
-static char *xmit_hash_policy = NULL;
-static int arp_interval = BOND_LINK_ARP_INTERV;
+static int miimon[BOND_MAX_PARMS];
+static int num_miimon;
+static int updelay[BOND_MAX_PARMS];
+static int num_updelay;
+static int downdelay[BOND_MAX_PARMS];
+static int num_downdelay;
+static int use_carrier[BOND_MAX_PARMS];
+static int num_use_carrier;
+static char *mode[BOND_MAX_PARMS];
+static int num_mode;
+static char *primary[BOND_MAX_PARMS];
+static int num_primary;
+static char *lacp_rate[BOND_MAX_PARMS];
+static int num_lacp_rate;
+static char *xmit_hash_policy[BOND_MAX_PARMS];
+static int num_xmit_hash_policy;
+static int arp_interval[BOND_MAX_PARMS];
+static int num_arp_interval;
static char *arp_ip_target[BOND_MAX_ARP_TARGETS] = { NULL, };
-module_param(max_bonds, int, 0);
+module_param(max_bonds, int, S_IRUGO);
MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
-module_param(miimon, int, 0);
+module_param_array(miimon, int, &num_miimon, S_IRUGO);
MODULE_PARM_DESC(miimon, "Link check interval in milliseconds");
-module_param(updelay, int, 0);
+module_param_array(updelay, int, &num_updelay, S_IRUGO);
MODULE_PARM_DESC(updelay, "Delay before considering link up, in milliseconds");
-module_param(downdelay, int, 0);
+module_param_array(downdelay, int, &num_downdelay, S_IRUGO);
MODULE_PARM_DESC(downdelay, "Delay before considering link down, in milliseconds");
-module_param(use_carrier, int, 0);
+module_param_array(use_carrier, int, &num_use_carrier, S_IRUGO);
MODULE_PARM_DESC(use_carrier, "Use netif_carrier_ok (vs MII ioctls) in miimon; 0 for off, 1 for on (default)");
-module_param(mode, charp, 0);
+module_param_array(mode, charp, &num_mode, S_IRUGO);
MODULE_PARM_DESC(mode, "Mode of operation : 0 for round robin, 1 for active-backup, 2 for xor");
-module_param(primary, charp, 0);
+module_param_array(primary, charp, &num_primary, S_IRUGO);
MODULE_PARM_DESC(primary, "Primary network device to use");
-module_param(lacp_rate, charp, 0);
+module_param_array(lacp_rate, charp, &num_lacp_rate, S_IRUGO);
MODULE_PARM_DESC(lacp_rate, "LACPDU tx rate to request from 802.3ad partner (slow/fast)");
-module_param(xmit_hash_policy, charp, 0);
+module_param_array(xmit_hash_policy, charp, &num_xmit_hash_policy, S_IRUGO);
MODULE_PARM_DESC(xmit_hash_policy, "XOR hashing method : 0 for layer 2 (default), 1 for layer 3+4");
-module_param(arp_interval, int, 0);
+module_param_array(arp_interval, int, &num_arp_interval, S_IRUGO);
MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
-module_param_array(arp_ip_target, charp, NULL, 0);
+module_param_array(arp_ip_target, charp, NULL, S_IRUGO);
MODULE_PARM_DESC(arp_ip_target, "arp targets in n.n.n.n form");
/*----------------------------- Global variables ----------------------------*/
@@ -592,9 +603,6 @@ static struct proc_dir_entry *bond_proc_
static u32 arp_target[BOND_MAX_ARP_TARGETS] = { 0, } ;
static int arp_ip_count = 0;
-static int bond_mode = BOND_MODE_ROUNDROBIN;
-static int xmit_hashtype= BOND_XMIT_POLICY_LAYER2;
-static int lacp_fast = 0;
static int app_abi_ver = 0;
static int orig_app_abi_ver = -1; /* This is used to save the first ABI version
* we receive from the application. Once set,
@@ -4714,51 +4722,62 @@ static inline int bond_parse_parm(char *
return -1;
}
-static int bond_check_params(struct bond_params *params)
+static int bond_check_params(struct bond_params *params, int bond)
{
+ params->miimon = (bond < num_miimon) ? miimon[bond] : BOND_LINK_MON_INTERV;
+ params->updelay = (bond < num_updelay) ? updelay[bond] : 0;
+ params->downdelay = (bond < num_downdelay) ? downdelay[bond] : 0;
+ params->use_carrier = (bond < num_use_carrier) ? use_carrier[bond] : 1;
+ params->arp_interval = (bond < num_arp_interval) ? arp_interval[bond] : BOND_LINK_ARP_INTERV;
+
+ params->mode = BOND_MODE_ROUNDROBIN;
+ params->primary[0] = '\0';
+ params->lacp_fast = 0;
+ params->xmit_policy = BOND_XMIT_POLICY_LAYER2;
+
+
/*
* Convert string parameters.
*/
- if (mode) {
- bond_mode = bond_parse_parm(mode, bond_mode_tbl);
- if (bond_mode == -1) {
+ if (bond < num_mode) {
+ params->mode = bond_parse_parm(mode[bond], bond_mode_tbl);
+ if (params->mode == -1) {
printk(KERN_ERR DRV_NAME
": Error: Invalid bonding mode \"%s\"\n",
- mode == NULL ? "NULL" : mode);
+ mode[bond]);
return -EINVAL;
}
}
- if (xmit_hash_policy) {
- if ((bond_mode != BOND_MODE_XOR) &&
- (bond_mode != BOND_MODE_8023AD)) {
+ if (bond < num_xmit_hash_policy) {
+ if ((params->mode != BOND_MODE_XOR) &&
+ (params->mode != BOND_MODE_8023AD)) {
printk(KERN_INFO DRV_NAME
": xor_mode param is irrelevant in mode %s\n",
- bond_mode_name(bond_mode));
+ bond_mode_name(params->mode));
} else {
- xmit_hashtype = bond_parse_parm(xmit_hash_policy,
+ params->xmit_policy = bond_parse_parm(xmit_hash_policy[bond],
xmit_hashtype_tbl);
- if (xmit_hashtype == -1) {
+ if (params->xmit_policy == -1) {
printk(KERN_ERR DRV_NAME
": Error: Invalid xmit_hash_policy \"%s\"\n",
- xmit_hash_policy == NULL ? "NULL" :
- xmit_hash_policy);
+ xmit_hash_policy[bond]);
return -EINVAL;
}
}
}
- if (lacp_rate) {
- if (bond_mode != BOND_MODE_8023AD) {
+ if (bond < num_lacp_rate) {
+ if (params->mode != BOND_MODE_8023AD) {
printk(KERN_INFO DRV_NAME
": lacp_rate param is irrelevant in mode %s\n",
- bond_mode_name(bond_mode));
+ bond_mode_name(params->mode));
} else {
- lacp_fast = bond_parse_parm(lacp_rate, bond_lacp_tbl);
- if (lacp_fast == -1) {
+ params->lacp_fast = bond_parse_parm(lacp_rate[bond], bond_lacp_tbl);
+ if (params->lacp_fast == -1) {
printk(KERN_ERR DRV_NAME
": Error: Invalid lacp rate \"%s\"\n",
- lacp_rate == NULL ? "NULL" : lacp_rate);
+ lacp_rate[bond]);
return -EINVAL;
}
}
@@ -4772,77 +4791,77 @@ static int bond_check_params(struct bond
max_bonds = BOND_DEFAULT_MAX_BONDS;
}
- if (miimon < 0) {
+ if (params->miimon < 0) {
printk(KERN_WARNING DRV_NAME
": Warning: miimon module parameter (%d), "
"not in range 0-%d, so it was reset to %d\n",
- miimon, INT_MAX, BOND_LINK_MON_INTERV);
- miimon = BOND_LINK_MON_INTERV;
+ params->miimon, INT_MAX, BOND_LINK_MON_INTERV);
+ params->miimon = BOND_LINK_MON_INTERV;
}
- if (updelay < 0) {
+ if (params->updelay < 0) {
printk(KERN_WARNING DRV_NAME
": Warning: updelay module parameter (%d), "
"not in range 0-%d, so it was reset to 0\n",
- updelay, INT_MAX);
- updelay = 0;
+ params->updelay, INT_MAX);
+ params->updelay = 0;
}
- if (downdelay < 0) {
+ if (params->downdelay < 0) {
printk(KERN_WARNING DRV_NAME
": Warning: downdelay module parameter (%d), "
"not in range 0-%d, so it was reset to 0\n",
- downdelay, INT_MAX);
- downdelay = 0;
+ params->downdelay, INT_MAX);
+ params->downdelay = 0;
}
- if ((use_carrier != 0) && (use_carrier != 1)) {
+ if ((params->use_carrier != 0) && (params->use_carrier != 1)) {
printk(KERN_WARNING DRV_NAME
": Warning: use_carrier module parameter (%d), "
"not of valid value (0/1), so it was set to 1\n",
- use_carrier);
- use_carrier = 1;
+ params->use_carrier);
+ params->use_carrier = 1;
}
/* reset values for 802.3ad */
- if (bond_mode == BOND_MODE_8023AD) {
- if (!miimon) {
+ if (params->mode == BOND_MODE_8023AD) {
+ if (!params->miimon) {
printk(KERN_WARNING DRV_NAME
": Warning: miimon must be specified, "
"otherwise bonding will not detect link "
"failure, speed and duplex which are "
"essential for 802.3ad operation\n");
printk(KERN_WARNING "Forcing miimon to 100msec\n");
- miimon = 100;
+ params->miimon = 100;
}
}
/* reset values for TLB/ALB */
- if ((bond_mode == BOND_MODE_TLB) ||
- (bond_mode == BOND_MODE_ALB)) {
- if (!miimon) {
+ if ((params->mode == BOND_MODE_TLB) ||
+ (params->mode == BOND_MODE_ALB)) {
+ if (!params->miimon) {
printk(KERN_WARNING DRV_NAME
": Warning: miimon must be specified, "
"otherwise bonding will not detect link "
"failure and link speed which are essential "
"for TLB/ALB load balancing\n");
printk(KERN_WARNING "Forcing miimon to 100msec\n");
- miimon = 100;
+ params->miimon = 100;
}
}
- if (bond_mode == BOND_MODE_ALB) {
+ if (params->mode == BOND_MODE_ALB) {
printk(KERN_NOTICE DRV_NAME
": In ALB mode you might experience client "
"disconnections upon reconnection of a link if the "
"bonding module updelay parameter (%d msec) is "
"incompatible with the forwarding delay time of the "
"switch\n",
- updelay);
+ params->updelay);
}
- if (!miimon) {
- if (updelay || downdelay) {
+ if (!params->miimon) {
+ if (params->updelay || params->downdelay) {
/* just warn the user the up/down delay will have
* no effect since miimon is zero...
*/
@@ -4851,45 +4870,45 @@ static int bond_check_params(struct bond
"and updelay (%d) or downdelay (%d) module "
"parameter is set; updelay and downdelay have "
"no effect unless miimon is set\n",
- updelay, downdelay);
+ params->updelay, params->downdelay);
}
} else {
/* don't allow arp monitoring */
- if (arp_interval) {
+ if (params->arp_interval) {
printk(KERN_WARNING DRV_NAME
": Warning: miimon (%d) and arp_interval (%d) "
"can't be used simultaneously, disabling ARP "
"monitoring\n",
- miimon, arp_interval);
- arp_interval = 0;
+ params->miimon, params->arp_interval);
+ params->arp_interval = 0;
}
- if ((updelay % miimon) != 0) {
+ if ((params->updelay % params->miimon) != 0) {
printk(KERN_WARNING DRV_NAME
": Warning: updelay (%d) is not a multiple "
"of miimon (%d), updelay rounded to %d ms\n",
- updelay, miimon, (updelay / miimon) * miimon);
+ params->updelay, params->miimon, (params->updelay / params->miimon) * params->miimon);
}
- updelay /= miimon;
+ params->updelay /= params->miimon;
- if ((downdelay % miimon) != 0) {
+ if ((params->downdelay % params->miimon) != 0) {
printk(KERN_WARNING DRV_NAME
": Warning: downdelay (%d) is not a multiple "
"of miimon (%d), downdelay rounded to %d ms\n",
- downdelay, miimon,
- (downdelay / miimon) * miimon);
+ params->downdelay, params->miimon,
+ (params->downdelay / params->miimon) * params->miimon);
}
- downdelay /= miimon;
+ params->downdelay /= params->miimon;
}
- if (arp_interval < 0) {
+ if (params->arp_interval < 0) {
printk(KERN_WARNING DRV_NAME
": Warning: arp_interval module parameter (%d) "
", not in range 0-%d, so it was reset to %d\n",
- arp_interval, INT_MAX, BOND_LINK_ARP_INTERV);
- arp_interval = BOND_LINK_ARP_INTERV;
+ params->arp_interval, INT_MAX, BOND_LINK_ARP_INTERV);
+ params->arp_interval = BOND_LINK_ARP_INTERV;
}
for (arp_ip_count = 0;
@@ -4902,33 +4921,33 @@ static int bond_check_params(struct bond
": Warning: bad arp_ip_target module parameter "
"(%s), ARP monitoring will not be performed\n",
arp_ip_target[arp_ip_count]);
- arp_interval = 0;
+ params->arp_interval = 0;
} else {
u32 ip = in_aton(arp_ip_target[arp_ip_count]);
arp_target[arp_ip_count] = ip;
}
}
- if (arp_interval && !arp_ip_count) {
+ if (params->arp_interval && !arp_ip_count) {
/* don't allow arping if no arp_ip_target given... */
printk(KERN_WARNING DRV_NAME
": Warning: arp_interval module parameter (%d) "
"specified without providing an arp_ip_target "
"parameter, arp_interval was reset to 0\n",
- arp_interval);
- arp_interval = 0;
+ params->arp_interval);
+ params->arp_interval = 0;
}
- if (miimon) {
+ if (params->miimon) {
printk(KERN_INFO DRV_NAME
": MII link monitoring set to %d ms\n",
- miimon);
- } else if (arp_interval) {
+ params->miimon);
+ } else if (params->arp_interval) {
int i;
printk(KERN_INFO DRV_NAME
": ARP monitoring set to %d ms with %d target(s):",
- arp_interval, arp_ip_count);
+ params->arp_interval, arp_ip_count);
for (i = 0; i < arp_ip_count; i++)
printk (" %s", arp_ip_target[i]);
@@ -4945,32 +4964,20 @@ static int bond_check_params(struct bond
"otherwise bonding will not detect link failures! see "
"bonding.txt for details.\n");
}
-
- if (primary && !USES_PRIMARY(bond_mode)) {
+
+ if (bond < num_primary) {
/* currently, using a primary only makes sense
* in active backup, TLB or ALB modes
*/
- printk(KERN_WARNING DRV_NAME
- ": Warning: %s primary device specified but has no "
- "effect in %s mode\n",
- primary, bond_mode_name(bond_mode));
- primary = NULL;
- }
-
- /* fill params struct with the proper values */
- params->mode = bond_mode;
- params->xmit_policy = xmit_hashtype;
- params->miimon = miimon;
- params->arp_interval = arp_interval;
- params->updelay = updelay;
- params->downdelay = downdelay;
- params->use_carrier = use_carrier;
- params->lacp_fast = lacp_fast;
- params->primary[0] = 0;
-
- if (primary) {
- strncpy(params->primary, primary, IFNAMSIZ);
- params->primary[IFNAMSIZ - 1] = 0;
+ if (USES_PRIMARY(params->mode)) {
+ strncpy(params->primary, primary[bond], IFNAMSIZ);
+ params->primary[IFNAMSIZ - 1] = 0;
+ } else {
+ printk(KERN_WARNING DRV_NAME
+ ": Warning: %s primary device specified but has no "
+ "effect in %s mode\n",
+ primary[bond], bond_mode_name(params->mode));
+ }
}
memcpy(params->arp_targets, arp_target, sizeof(arp_target));
@@ -4981,16 +4988,12 @@ static int bond_check_params(struct bond
static int __init bonding_init(void)
{
struct bond_params params;
+ struct net_device *bond_dev;
int i;
int res;
printk(KERN_INFO "%s", version);
- res = bond_check_params(¶ms);
- if (res) {
- return res;
- }
-
rtnl_lock();
#ifdef CONFIG_PROC_FS
@@ -4998,8 +5001,6 @@ static int __init bonding_init(void)
#endif
for (i = 0; i < max_bonds; i++) {
- struct net_device *bond_dev;
-
bond_dev = alloc_netdev(sizeof(struct bonding), "", ether_setup);
if (!bond_dev) {
res = -ENOMEM;
@@ -5008,8 +5009,11 @@ static int __init bonding_init(void)
res = dev_alloc_name(bond_dev, "bond%d");
if (res < 0) {
- free_netdev(bond_dev);
- goto out_err;
+ goto out_dev;
+ }
+
+ if ((res = bond_check_params(¶ms, i)) < 0){
+ goto out_dev;
}
/* bond_init() must be called after dev_alloc_name() (for the
@@ -5018,8 +5022,7 @@ static int __init bonding_init(void)
*/
res = bond_init(bond_dev, ¶ms);
if (res < 0) {
- free_netdev(bond_dev);
- goto out_err;
+ goto out_dev;
}
SET_MODULE_OWNER(bond_dev);
@@ -5027,8 +5030,7 @@ static int __init bonding_init(void)
res = register_netdevice(bond_dev);
if (res < 0) {
bond_deinit(bond_dev);
- free_netdev(bond_dev);
- goto out_err;
+ goto out_dev;
}
}
@@ -5038,6 +5040,8 @@ static int __init bonding_init(void)
return 0;
+out_dev:
+ free_netdev(bond_dev);
out_err:
/*
* rtnl_unlock() will run netdev_run_todo(), putting the
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -46,6 +46,7 @@
#define DRV_DESCRIPTION "Ethernet Channel Bonding Driver"
#define BOND_MAX_ARP_TARGETS 16
+#define BOND_MAX_PARMS 16
#ifdef BONDING_DEBUG
#define dprintk(fmt, args...) \
^ permalink raw reply
* Re: [PATCH] bond_main.c: fix device deregistration in init exception path
From: Al Viro @ 2005-09-22 2:43 UTC (permalink / raw)
To: Jeff Garzik
Cc: Andrew Morton, Florin Malita, linux-kernel, ctindel, fubar,
David S. Miller, netdev
In-Reply-To: <43321922.70707@pobox.com>
On Wed, Sep 21, 2005 at 10:38:26PM -0400, Jeff Garzik wrote:
> >+ rtnl_unlock();
> >+ rtnl_lock();
> >+
>
>
> Don't we want a schedule() or schedule_timeout(1) in between?
No. rtnl_unlock() does the needed calls directly.
^ permalink raw reply
* Re: [PATCH] bond_main.c: fix device deregistration in init exception path
From: Andrew Morton @ 2005-09-22 2:42 UTC (permalink / raw)
To: Jeff Garzik; +Cc: fmalita, linux-kernel, ctindel, fubar, davem, netdev
In-Reply-To: <43321922.70707@pobox.com>
Jeff Garzik <jgarzik@pobox.com> wrote:
>
> Andrew Morton wrote:
> > diff -puN drivers/net/bonding/bond_main.c~bond_mainc-fix-device-deregistration-in-init-exception drivers/net/bonding/bond_main.c
> > --- devel/drivers/net/bonding/bond_main.c~bond_mainc-fix-device-deregistration-in-init-exception 2005-09-17 23:18:38.000000000 -0700
> > +++ devel-akpm/drivers/net/bonding/bond_main.c 2005-09-17 23:31:02.000000000 -0700
> > @@ -5039,6 +5039,14 @@ static int __init bonding_init(void)
> > return 0;
> >
> > out_err:
> > + /*
> > + * rtnl_unlock() will run netdev_run_todo(), putting the
> > + * thus-far-registered bonding devices into a state which
> > + * unregigister_netdevice() will accept
> > + */
> > + rtnl_unlock();
> > + rtnl_lock();
> > +
>
>
> Don't we want a schedule() or schedule_timeout(1) in between?
>
No, it's all synchronous. See the nice comment ;)
^ permalink raw reply
* Re: [PATCH] bond_main.c: fix device deregistration in init exception path
From: Jeff Garzik @ 2005-09-22 2:38 UTC (permalink / raw)
To: Andrew Morton, Florin Malita
Cc: linux-kernel, ctindel, fubar, David S. Miller, netdev
In-Reply-To: <20050917233224.2d4b3652.akpm@osdl.org>
Andrew Morton wrote:
> diff -puN drivers/net/bonding/bond_main.c~bond_mainc-fix-device-deregistration-in-init-exception drivers/net/bonding/bond_main.c
> --- devel/drivers/net/bonding/bond_main.c~bond_mainc-fix-device-deregistration-in-init-exception 2005-09-17 23:18:38.000000000 -0700
> +++ devel-akpm/drivers/net/bonding/bond_main.c 2005-09-17 23:31:02.000000000 -0700
> @@ -5039,6 +5039,14 @@ static int __init bonding_init(void)
> return 0;
>
> out_err:
> + /*
> + * rtnl_unlock() will run netdev_run_todo(), putting the
> + * thus-far-registered bonding devices into a state which
> + * unregigister_netdevice() will accept
> + */
> + rtnl_unlock();
> + rtnl_lock();
> +
Don't we want a schedule() or schedule_timeout(1) in between?
Jeff
^ 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