| Message ID | 200906091406.45463.arnd@arndb.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 9 Jun 2009 14:06:44 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 09 June 2009, Stephen Hemminger wrote: > > In order to create multiple bonding dynamically, it is common practice to > > load the bonding module multiple times. This got broken in recent kernels > > 2.6.29 and later. > > > > Doing the following will OOPS: > > modprobe -o bond0 bonding > > modprobe -o bond1 bonding > > > > 2.6.29 actually OOPS on error handling, but that is fixed in 2.6.30. > > But 2.6.30 still has the regression (caused by sysfs). > > > > This regression was introduced by changes to sysfs and proc that > > made duplicate insertion a problem. > > Well, I guess it's more like the changes just made it obvious that > it's wrong to do this. Registering the same entries in procfs or sysfs > means that the user will only be able to talk to one of the two > bonding drivers through these interfaces. > > The log messages you quoted are not actually Oops but rather WARNING, > which is (in this case) not fatal at all, just an indication that the > root user did something he should not have: > > > [ 134.012578] WARNING: at fs/proc/generic.c:590 proc_register+0x154/0x191() > > [ 134.012583] proc_dir_entry 'net/bonding' already registered > > > [ 134.014516] WARNING: at fs/sysfs/dir.c:487 sysfs_add_one+0xcc/0xe4() > > [ 134.014521] sysfs: cannot create duplicate filename '/class/net/bonding_masters' > > The bonding driver could work around this by checking if the directories > already exist before registering them. One can also add rtnl_link_ops to > the driver for dynamically adding more interfaces. > > If you combine the two, you can even print a helpful message like 'please > use "ip link add type bonding" instead of "modprobe -o bond0 bonding"'. > > In the mean time, you could probably work around this by ignoring the error > condition (see below), but I would suspect that there may be more problems > with the concept of just loading the module again. The best advice to > users is probably to configure the maximum number of bonding devices they > might need with the max_bonds= module parameter (if I understand that > parameter correctly. > > Arnd <>< > > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -5203,7 +5203,7 @@ static int __init bonding_init(void) > > res = bond_create_sysfs(); > if (res) > - goto err; > + pr_info("Loading bonding module without sysfs interface\n"); > > register_netdevice_notifier(&bond_netdev_notifier); > register_inetaddr_notifier(&bond_inetaddr_notifier); That only makes it limp along, and there still are warnings. The point is that who ever added the WARN() in proc and sysfs, effectively broke a bonding usage model.
Stephen Hemminger wrote: >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -5203,7 +5203,7 @@ static int __init bonding_init(void) >> >> res = bond_create_sysfs(); >> if (res) >> - goto err; >> + pr_info("Loading bonding module without sysfs interface\n"); >> >> register_netdevice_notifier(&bond_netdev_notifier); >> register_inetaddr_notifier(&bond_inetaddr_notifier); > > That only makes it limp along, and there still are warnings. > The point is that who ever added the WARN() in proc and sysfs, effectively > broke a bonding usage model. It did already dump the stack before that change, didn't it? In any case, this is not the first time this has been broken and the fundamental reason is in my opinion that the bonding interface is broken to begin with. The module aliasing thing is complete crap and should have been phased out long ago. At this point its probably not worth anymore to migrate people to the sysfs interface though, the best thing would be to add an rtnl_link interface and phase out both. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patrick McHardy <kaber@trash.net> wrote: >Stephen Hemminger wrote: >>> --- a/drivers/net/bonding/bond_main.c >>> +++ b/drivers/net/bonding/bond_main.c >>> @@ -5203,7 +5203,7 @@ static int __init bonding_init(void) >>> res = bond_create_sysfs(); >>> if (res) >>> - goto err; >>> + pr_info("Loading bonding module without sysfs interface\n"); >>> register_netdevice_notifier(&bond_netdev_notifier); >>> register_inetaddr_notifier(&bond_inetaddr_notifier); >> >> That only makes it limp along, and there still are warnings. >> The point is that who ever added the WARN() in proc and sysfs, effectively >> broke a bonding usage model. > >It did already dump the stack before that change, didn't it? > >In any case, this is not the first time this has been broken and the >fundamental reason is in my opinion that the bonding interface is >broken to begin with. The module aliasing thing is complete crap >and should have been phased out long ago. At this point its probably >not worth anymore to migrate people to the sysfs interface though, >the best thing would be to add an rtnl_link interface and phase out >both. The "load bonding multiple times" stuff is only there now for backwards compatibility with old distro initscripts / sysconfig packages that don't configure bonding through sysfs (a sysfs API was added to bonding three or four years ago). All of the current distro releases I'm aware of use sysfs to configure bonding, and have done so for at least a year or two. I haven't done an exhaustive survey, but it seems unlikely that users are running a current up to date kernel with a two or three year old initscripts / sysconfig package. Anybody have information to the contrary? If nobody has any heartburn at dropping support for multiple bonding instances on old distros, I'm as happy as anybody to remove all of the multiple load logic from bonding. There's been plenty of time for transitioning from "multiple load" to sysfs. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jay Vosburgh wrote: > Patrick McHardy <kaber@trash.net> wrote: > >> In any case, this is not the first time this has been broken and the >> fundamental reason is in my opinion that the bonding interface is >> broken to begin with. The module aliasing thing is complete crap >> and should have been phased out long ago. At this point its probably >> not worth anymore to migrate people to the sysfs interface though, >> the best thing would be to add an rtnl_link interface and phase out >> both. > > The "load bonding multiple times" stuff is only there now for > backwards compatibility with old distro initscripts / sysconfig packages > that don't configure bonding through sysfs (a sysfs API was added to > bonding three or four years ago). > > All of the current distro releases I'm aware of use sysfs to > configure bonding, and have done so for at least a year or two. I > haven't done an exhaustive survey, but it seems unlikely that users are > running a current up to date kernel with a two or three year old > initscripts / sysconfig package. Anybody have information to the > contrary? I'd expect its not the distros, but rather the applicances which might still be using this. I know a vendor I used to work for a couple of years ago just recently made the switch from 2.6.16 to a current kernel, and I'd expect that they are still using this (I can find out tommorrow if you want to know for sure). Vyatta likewise, I guess. > If nobody has any heartburn at dropping support for multiple > bonding instances on old distros, I'm as happy as anybody to remove all > of the multiple load logic from bonding. There's been plenty of time > for transitioning from "multiple load" to sysfs. In my opinion it would need a feature-removal-schedule announcement. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patrick McHardy <kaber@trash.net> wrote: >Jay Vosburgh wrote: >> Patrick McHardy <kaber@trash.net> wrote: >> >>> In any case, this is not the first time this has been broken and the >>> fundamental reason is in my opinion that the bonding interface is >>> broken to begin with. The module aliasing thing is complete crap >>> and should have been phased out long ago. At this point its probably >>> not worth anymore to migrate people to the sysfs interface though, >>> the best thing would be to add an rtnl_link interface and phase out >>> both. >> >> The "load bonding multiple times" stuff is only there now for >> backwards compatibility with old distro initscripts / sysconfig packages >> that don't configure bonding through sysfs (a sysfs API was added to >> bonding three or four years ago). >> >> All of the current distro releases I'm aware of use sysfs to >> configure bonding, and have done so for at least a year or two. I >> haven't done an exhaustive survey, but it seems unlikely that users are >> running a current up to date kernel with a two or three year old >> initscripts / sysconfig package. Anybody have information to the >> contrary? > >I'd expect its not the distros, but rather the applicances which might >still be using this. I know a vendor I used to work for a couple of >years ago just recently made the switch from 2.6.16 to a current kernel, >and I'd expect that they are still using this (I can find out tommorrow >if you want to know for sure). Vyatta likewise, I guess. Yes, I'd like to know for sure; thanks. >> If nobody has any heartburn at dropping support for multiple >> bonding instances on old distros, I'm as happy as anybody to remove all >> of the multiple load logic from bonding. There's been plenty of time >> for transitioning from "multiple load" to sysfs. > >In my opinion it would need a feature-removal-schedule announcement. Yah, probably. The multiple load stuff was working fine as of, oh, a year or two ago, so I wasn't worried so much about getting rid of it. If it's causing problems, though, it's time for it to go (or be scheduled to go in the not too distant future). Can't keep driving that Ford Pinto forever. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jay Vosburgh wrote: > Patrick McHardy <kaber@trash.net> wrote: > >> I'd expect its not the distros, but rather the applicances which might >> still be using this. I know a vendor I used to work for a couple of >> years ago just recently made the switch from 2.6.16 to a current kernel, >> and I'd expect that they are still using this (I can find out tommorrow >> if you want to know for sure). Vyatta likewise, I guess. > > Yes, I'd like to know for sure; thanks. I'll let you know. >>> If nobody has any heartburn at dropping support for multiple >>> bonding instances on old distros, I'm as happy as anybody to remove all >>> of the multiple load logic from bonding. There's been plenty of time >>> for transitioning from "multiple load" to sysfs. >> In my opinion it would need a feature-removal-schedule announcement. > > Yah, probably. The multiple load stuff was working fine as of, > oh, a year or two ago, so I wasn't worried so much about getting rid of > it. If it's causing problems, though, it's time for it to go (or be > scheduled to go in the not too distant future). Can't keep driving that > Ford Pinto forever. Well, if it has been broken for long enough, maybe we don't need a feature-removal-schedule warning. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patrick McHardy wrote: > Jay Vosburgh wrote: >> Patrick McHardy <kaber@trash.net> wrote: >> >>> I'd expect its not the distros, but rather the applicances which might >>> still be using this. I know a vendor I used to work for a couple of >>> years ago just recently made the switch from 2.6.16 to a current kernel, >>> and I'd expect that they are still using this (I can find out tommorrow >>> if you want to know for sure). Vyatta likewise, I guess. >> >> Yes, I'd like to know for sure; thanks. > > I'll let you know. They are still using the module alias method, but on a kernel slightly older than the patch which broke this (2.6.25). -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -5203,7 +5203,7 @@ static int __init bonding_init(void) res = bond_create_sysfs(); if (res) - goto err; + pr_info("Loading bonding module without sysfs interface\n"); register_netdevice_notifier(&bond_netdev_notifier); register_inetaddr_notifier(&bond_inetaddr_notifier);