Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id ; Fri, 22 Feb 2002 05:14:01 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id ; Fri, 22 Feb 2002 05:13:53 -0500 Received: from asooo.flowerfire.com ([63.254.226.247]:56488 "EHLO asooo.flowerfire.com") by vger.kernel.org with ESMTP id ; Fri, 22 Feb 2002 05:13:38 -0500 Date: Fri, 22 Feb 2002 04:13:08 -0600 From: Ken Brownfield To: Pozsar Balazs Cc: Rajasekhar Inguva , girouard@us.ibm.com, ctindel@ieee.org, willy@meta-x.org, linux kernel Subject: Re: [PATCH]:Ethernet Bonding Driver-2.4.17 Message-ID: <20020222041308.A12791@asooo.flowerfire.com> In-Reply-To: <3C6E462D.39039598@in.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: ; from pozsy@sch.bme.hu on Sat, Feb 16, 2002 at 01:28:03PM +0100 Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/