Re: [PATCH]:Ethernet Bonding Driver-2.4.17

[Ken Brownfield <brownfld@irridia.com>]

What about something like this?  MAX_BONDS appears to be the default
max_bonds value, so max_bonds>MAX_BONDS is fine as long as
max_bonds<=INT_MAX, AFAICT.

I'm thinking a bogus value should reset to the default rather than
<=0 --> 1 or >INT_MAX --> INT_MAX.  Compiled but not tested.

-- 
Ken.
brownfld@irridia.com

--- linux/drivers/net/bonding.c.orig	Fri Feb 22 00:49:57 2002
+++ linux/drivers/net/bonding.c	Fri Feb 22 01:19:36 2002
@@ -234,7 +234,7 @@
 static struct bonding *these_bonds =  NULL;
 static struct net_device *dev_bonds = NULL;
 
-MODULE_PARM(max_bonds, "1-" __MODULE_STRING(INT_MAX) "i");
+MODULE_PARM(max_bonds, "i");
 MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
 MODULE_PARM(miimon, "i");
 MODULE_PARM_DESC(miimon, "Link check interval in milliseconds");
@@ -2037,6 +2037,14 @@
 
 	/* Find a name for this unit */
 	static struct net_device *dev_bond = NULL;
+
+	if ( max_bonds < 1 || max_bonds > INT_MAX ) {
+		printk( KERN_WARNING
+			"binding_init(): max_bonds (%d) not in range %d-%d, "
+			"was reset to MAX_BONDS (%d)",
+			max_bonds, 1, INT_MAX, MAX_BONDS );
+		max_bonds = MAX_BONDS ;
+	}
 
 	dev_bond = dev_bonds = kmalloc(max_bonds*sizeof(struct net_device), 
 					GFP_KERNEL);

On Sat, Feb 16, 2002 at 01:28:03PM +0100, Pozsar Balazs wrote:
| 
| On Sat, 16 Feb 2002, Rajasekhar Inguva wrote:
| 
| > Hi all,
| >
| > The patch is WRT a problem with the Ethernet Bonding Driver when
| > compiled as a module. Tested on 2.4.17
| > and the patch is also against 2.4.17.
| >
| > # insmod bonding max_bonds=2
| > Using /lib/modules/2.4.17/kernel/drivers/net/bonding.o
| > Warning: /lib/modules/2.4.17/kernel/drivers/net/bonding.o parameter
| > max_bonds
| > has max < min!
| > /lib/modules/2.4.17/kernel/drivers/net/bonding.o: unknown parameter type
| > '(' for
| > max_bonds
| >
| > and the module fails to load.
| >
| > The problem seems to be with the way MODULE_PARM was written for
| > max_bonds.
| >
| > MODULE_PARM(max_bonds,"1-" __MODULE_STRING(INT_MAX) "i");
| >
| > INT_MAX is defined to be ((int)(~0U>>1)) and 'insmod' gets the string
| > "1-((int)(~0U>>1))i" which it is failing to understand.
| >
| > As max_bonds is an integer and not an array, i feel omitting the min-max
| > range would be a better option.
| >
| > And if a negative or zero value is supplied to max_bonds while loading,
| > it can be taken care of in bonding_init() by setting it back to
| > MAX_BONDS.
| >
| > Thx,
| > Rajasekhar Inguva
| >
| >
| > --- /usr/src/linux/drivers/net/bonding.c	Fri Dec 21 23:11:54 2001
| > +++ /usr/src/linux/drivers/fixed/bonding.c	Sat Feb 16 17:03:48 2002
| > @@ -226,7 +226,7 @@
| >  static struct bonding *these_bonds =  NULL;
| >  static struct net_device *dev_bonds = NULL;
| >
| > -MODULE_PARM(max_bonds, "1-" __MODULE_STRING(INT_MAX) "i");
| > +MODULE_PARM(max_bonds,"i");
| >  MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
| >  MODULE_PARM(miimon, "i");
| >  MODULE_PARM_DESC(miimon, "Link check interval in milliseconds");
| > @@ -1981,6 +1981,10 @@
| >
| >  	/* Find a name for this unit */
| >  	static struct net_device *dev_bond = NULL;
| > +
| > +	/* If max_bonds <=0, set it to MAX_BONDS */
| > +	if(max_bonds <=0)
| > +		max_bonds = MAX_BONDS;
| 
| 
| Imho you would need a check for the upper limit too. like:
| 	if(max_bonds > MAX_BONDS)
| 		max_bonds = MAX_BONDS;
| 
| 
| >
| >  	dev_bond = dev_bonds = kmalloc(max_bonds*sizeof(struct net_device),
| >  					GFP_KERNEL);
| 
| -- 
| pozsy