RE: [PATCH 2.6.23.12] net/bonding: option to specify initial bond interface number

2008-01-03 Thread Jari Takkala
On Thursday, January 03, 2008 12:04, Jay Vosburgh wrote:
>> In our startup scripts we need to be able to ensure that the
>> interface name is consistent across reboots. Sometimes bond1 may be
>> brought up before bond0 and it may have different options (requiring
>> a different instance of the bonding driver).
> 
>   With the sysfs interface to bonding, your last statement is not
> true; any number of bonding interfaces, with arbitrary names, can be
> created and have their options set without loading multiple instances
> of the bonding driver.
> 
>   Does your embedded system have sysfs available?  If it does,
> then it's to your advantage to use the sysfs API; for one thing, the
> single instance of the bonding driver with all interfaces through it
> should utilize fewer resources than loading the driver repeatedly.
> 

We do have sysfs available. I actually did not realize that it is possible to 
have one bonding instance and create multiple interfaces with different 
options. It looks like when I initially wrote this patch  (~2.6.20) either this 
feature was not available, or the bonding documentation may have been out of 
date. I see now in the latest Documentation/networking/bonding.txt that this is 
explained clearly.

Thank you for your help, I will drop the patch and move to using the sysfs 
method. Although I may need to patch it for our own uses to either not create 
bond0 on module load, or I may just run 'echo -bond0 > 
/sys/class/net/bonding_masters' immediately after loading the module.

Jari

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2.6.23.12] net/bonding: option to specify initial bond interface number

2008-01-03 Thread Jari Takkala
On Wednesday, January 02, 2008 16:56, Randy Dunlap wrote:
> You could (should) make  be unsigned int and then use
> module_param(ifnum, uint, 0); and then ...
> 
> then this block is mostly useless since ifnum cannot be < 0.
> And how could it ever be > INT_MAX (when ifnum was an int)?
> 
> If  is unsigned int but you want to limit it to INT_MAX,
> then half of this if-test would be OK.
> 

Thanks, that makes sense. I simply copied some of the max_bonds code which uses 
a signed int. I suppose that could be changed as well. I will post back a new 
patch.

Jari
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2.6.23.12] net/bonding: option to specify initial bond interface number

2008-01-03 Thread Jari Takkala
On Wednesday, January 02, 2008 17:24, Jay Vosburgh wrote:
>   What advantage does this have over:
> 
> # echo +bond5 > /sys/class/net/bonding_masters
> 
>   which will create a new bonding master for the already-loaded driver?
>

The advantage is that you can load multiple instances of the bonding driver and 
control the name of the bond interface that will be created. Normally the bond 
interface name would take the next available number.

In our startup scripts we need to be able to ensure that the interface name is 
consistent across reboots. Sometimes bond1 may be brought up before bond0 and 
it may have different options (requiring a different instance of the bonding 
driver).

I understand that the startup scripts could be modified to account for this, 
however we also have an IOS like interface to an embedded system where the user 
can create new bond interfaces and specify the interface number, they may 
create interfaces out of order and this feature enables them to accomplish that.

Jari
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2.6.23.12] net/bonding: option to specify initial bond interface number

2008-01-02 Thread Jari Takkala
Allow the user to specify an initial interface number when loading the bonding 
driver. This is useful when loading multiple instances of the bonding driver 
and you want to control the interface number assignment. For example, if the 
user wishes to create a bond5 interface they can type 'modprobe -o bond5 
bonding ifnum=5'. It also works with the max_bonds option.

Signed-off-by: Jari Takkala <[EMAIL PROTECTED]>
---
diff -ruN linux-2.6.23.12.orig/drivers/net/bonding/bond_main.c 
linux-2.6.23.12/drivers/net/bonding/bond_main.c
--- linux-2.6.23.12.orig/drivers/net/bonding/bond_main.c2007-12-18 
16:55:57.0 -0500
+++ linux-2.6.23.12/drivers/net/bonding/bond_main.c 2008-01-02 
14:49:37.0 -0500
@@ -85,6 +85,7 @@
 #define BOND_LINK_MON_INTERV   0
 #define BOND_LINK_ARP_INTERV   0
 
+static int ifnum   = BOND_DEFAULT_IFNUM;
 static int max_bonds   = BOND_DEFAULT_MAX_BONDS;
 static int miimon  = BOND_LINK_MON_INTERV;
 static int updelay = 0;
@@ -99,6 +100,8 @@
 static char *arp_validate = NULL;
 struct bond_params bonding_defaults;
 
+module_param(ifnum, int, 0);
+MODULE_PARM_DESC(ifnum, "Initial interface number to assign");
 module_param(max_bonds, int, 0);
 MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
 module_param(miimon, int, 0);
@@ -4388,6 +4391,14 @@
}
}
 
+   if (ifnum < 0 || ifnum > INT_MAX) {
+   printk(KERN_WARNING DRV_NAME
+  ": Warning: ifnum (%d) not in range %d-%d, so it "
+  "was reset to BOND_DEFAULT_IFNUM (%d)\n",
+  ifnum, 0, INT_MAX, BOND_DEFAULT_IFNUM);
+   ifnum = BOND_DEFAULT_IFNUM;
+   }
+
if (max_bonds < 1 || max_bonds > INT_MAX) {
printk(KERN_WARNING DRV_NAME
   ": Warning: max_bonds (%d) not in range %d-%d, so it "
@@ -4703,6 +4714,7 @@
 {
int i;
int res;
+   char ifname[16];
 
printk(KERN_INFO "%s", version);
 
@@ -4715,7 +4727,13 @@
bond_create_proc_dir();
 #endif
for (i = 0; i < max_bonds; i++) {
-   res = bond_create(NULL, &bonding_defaults, NULL);
+   if (ifnum != BOND_DEFAULT_IFNUM) {
+   ifnum = ifnum + i;
+   snprintf(ifname, sizeof(ifname) - 1, "bond%d", ifnum);
+   res = bond_create(ifname, &bonding_defaults, NULL);
+   } else {
+   res = bond_create(NULL, &bonding_defaults, NULL);
+   }
if (res)
goto err;
}
diff -ruN linux-2.6.23.12.orig/include/linux/if_bonding.h 
linux-2.6.23.12/include/linux/if_bonding.h
--- linux-2.6.23.12.orig/include/linux/if_bonding.h 2007-12-18 
16:55:57.0 -0500
+++ linux-2.6.23.12/include/linux/if_bonding.h  2008-01-02 13:30:18.0 
-0500
@@ -81,6 +81,7 @@
 #define BOND_STATE_ACTIVE   0   /* link is active */
 #define BOND_STATE_BACKUP   1   /* link is backup */
 
+#define BOND_DEFAULT_IFNUM 0   /* Default interface number */
 #define BOND_DEFAULT_MAX_BONDS  1   /* Default maximum number of devices to 
support */
 
 /* hashing types */

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 06/17] neighbour.c, pneigh_get_next() skips published entry

2006-06-09 Thread Jari Takkala

On Fri, 9 Jun 2006, Herbert Xu wrote:

> Could you post an exact sequence of commands that reproduces the bug?
> That would help us in verifying your fix.
> 

Publish a large number of ARP entries (greater than 10 required on my
system):
'arp -Ds   pub'

View output of /proc/net/arp:

'dd if=/proc/net/arp of=arp-1024.out bs=1024'

The produced output will be missing on average one entry for every ten
entries published. Occasionally, the output will vary and the missing
entry will be displayed.


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 06/17] neighbour.c, pneigh_get_next() skips published entry

2006-06-08 Thread Jari Takkala

On Mon, 5 Jun 2006, David Miller wrote:

> This patch doesn't make any sense, I've been over it a few times.
> 
> The seqfile layer should take care of that user buffering issue
transparently as
> long as we implement the interface callbacks properly.
>
> Even if something needs to be fixed in the pneigh dumper, special
casing *pos==1
> doesn't look right.  Also, if pneigh has this problem, how come the
neigh seqfile
> iterators don't have the same problem or do they?

>From my analysis, the problem is that pneigh_get_next() ends up
assigning pn to one entry ahead of what it should be pointing too. This
only occurs when the user's read buffer fills up, where pneigh_get_idx()
is called, which calls pneigh_get_next() until *pos is not true.

I have not checked neigh seqfile iterators, the problem may exist in
there as well. My patch solves this issue for us, however a more elegant
solution would be most welcome.  Could the root of the problem be that
*pos is off by one when pneigh_get_idx() is called?

Jari

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] neighbour.c, pneigh_get_next() skips published entry

2006-05-11 Thread Jari Takkala
The following patch fixes a problem where output from /proc/net/arp
skips a record when the full output does not fit into the users read()
buffer.

To reproduce: publish a large number of ARP entries (more than 10
required on my system). Run 'dd if=/proc/net/arp of=arp-1024.out
bs=1024'. View the output, one entry will be missing.

Please review and commit if acceptable.

Signed-off-by: Jari Takkala <[EMAIL PROTECTED]>

--- linux-2.6.16.15.orig/net/core/neighbour.c   2006-05-09
15:53:30.0 -0400
+++ linux-2.6.16.15/net/core/neighbour.c2006-05-10
16:06:40.0 -0400
@@ -2120,6 +2120,11 @@
struct neigh_seq_state *state = seq->private;
struct neigh_table *tbl = state->tbl;

+   if (pos != NULL && *pos == 1 && (pn->next ||
tbl->phash_buckets[state->bucket])) {
+   --(*pos);
+   return pn;
+   }
+
pn = pn->next;
while (!pn) {
if (++state->bucket > PNEIGH_HASHMASK)

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html