RE: [PATCH 2.6.23.12] net/bonding: option to specify initial bond interface number
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
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
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
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
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
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
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