| Message ID | 4DBA830A020000780003ED5D@vpn.id2.novell.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: "Jan Beulich" <JBeulich@novell.com> Date: Fri, 29 Apr 2011 08:21:14 +0100 > Otherwise 'modprobe -r' on a module having a dependency on bridge will > implicitly unload bridge, bringing down all connectivity that was using > bridges. > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > Cc: Jeff Mahoney <jeffm@suse.com> All network device drivers behave exactly the same way, when you rmmod the thing we unconfigure all the routes, addresses, etc. going through that device and let you unload it. And this behavior is very much intentional. Don't add an exception here. -- 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
>>> On 29.04.11 at 09:25, David Miller <davem@davemloft.net> wrote: > From: "Jan Beulich" <JBeulich@novell.com> > Date: Fri, 29 Apr 2011 08:21:14 +0100 > >> Otherwise 'modprobe -r' on a module having a dependency on bridge will >> implicitly unload bridge, bringing down all connectivity that was using >> bridges. >> >> Signed-off-by: Jan Beulich <jbeulich@novell.com> >> Cc: Jeff Mahoney <jeffm@suse.com> > > All network device drivers behave exactly the same way, when you rmmod > the thing we unconfigure all the routes, addresses, etc. going through > that device and let you unload it. > > And this behavior is very much intentional. > > Don't add an exception here. You talk of rmmod on the very module, but the issue is about modprobe -r on a dependent module. I cannot believe you consider it correct that *implicit* unloading of bridge.ko should happen when bridges are configured. If the solution proposed isn't satisfactory, can you suggest a better alternative still serving the purpose? Jan -- 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
From: "Jan Beulich" <JBeulich@novell.com> Date: Fri, 29 Apr 2011 08:41:10 +0100 > You talk of rmmod on the very module, but the issue is about > modprobe -r on a dependent module. I cannot believe you consider > it correct that *implicit* unloading of bridge.ko should happen when > bridges are configured. Which module in particular depends upon bridge and causes the problem? -- 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
>>> On 29.04.11 at 10:10, David Miller <davem@davemloft.net> wrote: > From: "Jan Beulich" <JBeulich@novell.com> > Date: Fri, 29 Apr 2011 08:41:10 +0100 > >> You talk of rmmod on the very module, but the issue is about >> modprobe -r on a dependent module. I cannot believe you consider >> it correct that *implicit* unloading of bridge.ko should happen when >> bridges are configured. > > Which module in particular depends upon bridge and causes the > problem? The problem was observed (a long time ago) with ebtable_broute, and I cannot see how this would have changed meanwhile. Jan -- 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
From: "Jan Beulich" <JBeulich@novell.com> Date: Fri, 29 Apr 2011 09:31:27 +0100 >>>> On 29.04.11 at 10:10, David Miller <davem@davemloft.net> wrote: >> From: "Jan Beulich" <JBeulich@novell.com> >> Date: Fri, 29 Apr 2011 08:41:10 +0100 >> >>> You talk of rmmod on the very module, but the issue is about >>> modprobe -r on a dependent module. I cannot believe you consider >>> it correct that *implicit* unloading of bridge.ko should happen when >>> bridges are configured. >> >> Which module in particular depends upon bridge and causes the >> problem? > > The problem was observed (a long time ago) with ebtable_broute, > and I cannot see how this would have changed meanwhile. Well your change makes it so that someone who actually _wants_ to unload the bridge module, regardless of configuration, cannot do so. I think that's a worse problem than this ebtables thing. Nothing on the system should be hitting modules with unload requests unless the user explicitly asked for that specific module to be unloaded. At least not by default. So the me the problem is perhaps that "modprobe -r" does this auto dependency unloading thing by default. When we first fixed network device drivers so that they now properly always run with no module refcount at all, people complained because there were some distributions that ran some daemon that periodically looked for "unreferenced" modules and "helped" the user by automatically unloaded them. We killed that foolish daemon, and we can fix "modprobe -r" too. Does "rmmod" have this behavior too? If not, and it does the right thing by only unloaded what the user asked for, then people should use that. I really don't in any way want to block people from being able to cleanly unload the bridge module, regardless of configuration, if that's what they want so your patch as written is not going to be considered for inclusion. -- 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
>>> On 29.04.11 at 10:44, David Miller <davem@davemloft.net> wrote: > From: "Jan Beulich" <JBeulich@novell.com> > Date: Fri, 29 Apr 2011 09:31:27 +0100 > >>>>> On 29.04.11 at 10:10, David Miller <davem@davemloft.net> wrote: >>> From: "Jan Beulich" <JBeulich@novell.com> >>> Date: Fri, 29 Apr 2011 08:41:10 +0100 >>> >>>> You talk of rmmod on the very module, but the issue is about >>>> modprobe -r on a dependent module. I cannot believe you consider >>>> it correct that *implicit* unloading of bridge.ko should happen when >>>> bridges are configured. >>> >>> Which module in particular depends upon bridge and causes the >>> problem? >> >> The problem was observed (a long time ago) with ebtable_broute, >> and I cannot see how this would have changed meanwhile. > > Well your change makes it so that someone who actually _wants_ to > unload the bridge module, regardless of configuration, cannot do so. > > I think that's a worse problem than this ebtables thing. > > Nothing on the system should be hitting modules with unload requests > unless the user explicitly asked for that specific module to be > unloaded. At least not by default. > > So the me the problem is perhaps that "modprobe -r" does this auto > dependency unloading thing by default. > > When we first fixed network device drivers so that they now properly > always run with no module refcount at all, people complained because > there were some distributions that ran some daemon that periodically > looked for "unreferenced" modules and "helped" the user by > automatically unloaded them. > > We killed that foolish daemon, and we can fix "modprobe -r" too. Michal - aren't you the modutils maintainer? What are your thoughts here? (The original report we got is https://bugzilla.novell.com/show_bug.cgi?id=267651.) > Does "rmmod" have this behavior too? If not, and it does the right > thing by only unloaded what the user asked for, then people should > use that. No, it doesn't. Other than modprobe, rmmod deals only with the module specified. > I really don't in any way want to block people from being able to > cleanly unload the bridge module, regardless of configuration, if > that's what they want so your patch as written is not going to be > considered for inclusion. I understood that meanwhile, yet fail to see an alternative solution (imo this auto-unloading is quite desirable in other cases). Jan -- 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
On 29.4.2011 11:09, Jan Beulich wrote: >>>> On 29.04.11 at 10:44, David Miller<davem@davemloft.net> wrote: >> From: "Jan Beulich"<JBeulich@novell.com> >> Date: Fri, 29 Apr 2011 09:31:27 +0100 >> >>>>>> On 29.04.11 at 10:10, David Miller<davem@davemloft.net> wrote: >>>> From: "Jan Beulich"<JBeulich@novell.com> >>>> Date: Fri, 29 Apr 2011 08:41:10 +0100 >>>> >>>>> You talk of rmmod on the very module, but the issue is about >>>>> modprobe -r on a dependent module. I cannot believe you consider >>>>> it correct that *implicit* unloading of bridge.ko should happen when >>>>> bridges are configured. >>>> >>>> Which module in particular depends upon bridge and causes the >>>> problem? >>> >>> The problem was observed (a long time ago) with ebtable_broute, >>> and I cannot see how this would have changed meanwhile. >> >> Well your change makes it so that someone who actually _wants_ to >> unload the bridge module, regardless of configuration, cannot do so. >> >> I think that's a worse problem than this ebtables thing. >> >> Nothing on the system should be hitting modules with unload requests >> unless the user explicitly asked for that specific module to be >> unloaded. At least not by default. >> >> So the me the problem is perhaps that "modprobe -r" does this auto >> dependency unloading thing by default. >> >> When we first fixed network device drivers so that they now properly >> always run with no module refcount at all, people complained because >> there were some distributions that ran some daemon that periodically >> looked for "unreferenced" modules and "helped" the user by >> automatically unloaded them. >> >> We killed that foolish daemon, and we can fix "modprobe -r" too. > > Michal - aren't you the modutils maintainer? That would be Jon (CC added). > What are your thoughts > here? (The original report we got is > https://bugzilla.novell.com/show_bug.cgi?id=267651.) I think that defaulting to not removing dependencies would be a good idea. But do not expect that it will help with those artificial tests, they will just proceed a few steps further until they hit the module with broken unloading ;-). Michal > >> Does "rmmod" have this behavior too? If not, and it does the right >> thing by only unloaded what the user asked for, then people should >> use that. > > No, it doesn't. Other than modprobe, rmmod deals only with the > module specified. > >> I really don't in any way want to block people from being able to >> cleanly unload the bridge module, regardless of configuration, if >> that's what they want so your patch as written is not going to be >> considered for inclusion. > > I understood that meanwhile, yet fail to see an alternative solution > (imo this auto-unloading is quite desirable in other cases). > > Jan > -- 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
On Fri, 29 Apr 2011 00:25:30 -0700 (PDT) David Miller <davem@davemloft.net> wrote: > From: "Jan Beulich" <JBeulich@novell.com> > Date: Fri, 29 Apr 2011 08:21:14 +0100 > > > Otherwise 'modprobe -r' on a module having a dependency on bridge will > > implicitly unload bridge, bringing down all connectivity that was using > > bridges. > > > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > > Cc: Jeff Mahoney <jeffm@suse.com> > > All network device drivers behave exactly the same way, when you rmmod > the thing we unconfigure all the routes, addresses, etc. going through > that device and let you unload it. > > And this behavior is very much intentional. > > Don't add an exception here. Agreed. -- 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
On Fri, 2011-04-29 at 13:08 +0200, Michal Marek wrote: > On 29.4.2011 11:09, Jan Beulich wrote: > >>>> On 29.04.11 at 10:44, David Miller<davem@davemloft.net> wrote: > >> Nothing on the system should be hitting modules with unload requests > >> unless the user explicitly asked for that specific module to be > >> unloaded. At least not by default. > >> > >> So the me the problem is perhaps that "modprobe -r" does this auto > >> dependency unloading thing by default. > >> > >> When we first fixed network device drivers so that they now properly > >> always run with no module refcount at all, people complained because > >> there were some distributions that ran some daemon that periodically > >> looked for "unreferenced" modules and "helped" the user by > >> automatically unloaded them. > >> > >> We killed that foolish daemon, and we can fix "modprobe -r" too. > > > > Michal - aren't you the modutils maintainer? > > That would be Jon (CC added). Thanks. So the specific feature you mention was added precisely because some folks wanted to clean up ununsed modules by removing all of their dependencies. Since I've not been on this thread until now, can you let me know what precisely you need, and why? We can make the unloading of unused modules configurable, but it sounds like you're saying even that isn't good enough. What actually happens, what's the bug experience? I realize there isn't a general fondness of module removing, and I for one don't really mind having a few extra modules loaded in my kernel. Jon. -- 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
>>> On 29.04.11 at 18:05, Jon Masters <jonathan@jonmasters.org> wrote: > On Fri, 2011-04-29 at 13:08 +0200, Michal Marek wrote: >> On 29.4.2011 11:09, Jan Beulich wrote: >> >>>> On 29.04.11 at 10:44, David Miller<davem@davemloft.net> wrote: > >> >> Nothing on the system should be hitting modules with unload requests >> >> unless the user explicitly asked for that specific module to be >> >> unloaded. At least not by default. >> >> >> >> So the me the problem is perhaps that "modprobe -r" does this auto >> >> dependency unloading thing by default. >> >> >> >> When we first fixed network device drivers so that they now properly >> >> always run with no module refcount at all, people complained because >> >> there were some distributions that ran some daemon that periodically >> >> looked for "unreferenced" modules and "helped" the user by >> >> automatically unloaded them. >> >> >> >> We killed that foolish daemon, and we can fix "modprobe -r" too. >> > >> > Michal - aren't you the modutils maintainer? >> >> That would be Jon (CC added). > > Thanks. So the specific feature you mention was added precisely because > some folks wanted to clean up ununsed modules by removing all of their > dependencies. Since I've not been on this thread until now, can you let > me know what precisely you need, and why? We can make the unloading of > unused modules configurable, but it sounds like you're saying even that > isn't good enough. What actually happens, what's the bug experience? The problem observed was that unloading (via modprobe -r) ebtable_broute.ko, bridge.ko was also unloaded, causing all bridged networking to stop functioning on a machine. Jan -- 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
On Fri, 2011-04-29 at 17:07 +0100, Jan Beulich wrote: > >>> On 29.04.11 at 18:05, Jon Masters <jonathan@jonmasters.org> wrote: > > On Fri, 2011-04-29 at 13:08 +0200, Michal Marek wrote: > >> On 29.4.2011 11:09, Jan Beulich wrote: > >> >>>> On 29.04.11 at 10:44, David Miller<davem@davemloft.net> wrote: > > > >> >> Nothing on the system should be hitting modules with unload requests > >> >> unless the user explicitly asked for that specific module to be > >> >> unloaded. At least not by default. > >> >> > >> >> So the me the problem is perhaps that "modprobe -r" does this auto > >> >> dependency unloading thing by default. > >> >> > >> >> When we first fixed network device drivers so that they now properly > >> >> always run with no module refcount at all, people complained because > >> >> there were some distributions that ran some daemon that periodically > >> >> looked for "unreferenced" modules and "helped" the user by > >> >> automatically unloaded them. > >> >> > >> >> We killed that foolish daemon, and we can fix "modprobe -r" too. > >> > > >> > Michal - aren't you the modutils maintainer? > >> > >> That would be Jon (CC added). > > > > Thanks. So the specific feature you mention was added precisely because > > some folks wanted to clean up ununsed modules by removing all of their > > dependencies. Since I've not been on this thread until now, can you let > > me know what precisely you need, and why? We can make the unloading of > > unused modules configurable, but it sounds like you're saying even that > > isn't good enough. What actually happens, what's the bug experience? > > The problem observed was that unloading (via modprobe -r) > ebtable_broute.ko, bridge.ko was also unloaded, causing all > bridged networking to stop functioning on a machine. Ah, right...ouch. That would be a "little" problem. Short of having an exclusion list and all that nonsense, probably best to start with either removing the unload logic or making it globally configurable. Thanks. Jon. -- 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
--- 2.6.39-rc5/net/bridge/br_if.c +++ 2.6.39-rc5-bridge-module-get-put/net/bridge/br_if.c @@ -290,6 +290,11 @@ int br_add_bridge(struct net *net, const if (!dev) return -ENOMEM; + if (!try_module_get(THIS_MODULE)) { + free_netdev(dev); + return -ENOENT; + } + rtnl_lock(); if (strchr(dev->name, '%')) { ret = dev_alloc_name(dev, dev->name); @@ -308,6 +313,8 @@ int br_add_bridge(struct net *net, const unregister_netdevice(dev); out: rtnl_unlock(); + if (ret) + module_put(THIS_MODULE); return ret; out_free: @@ -339,6 +346,8 @@ int br_del_bridge(struct net *net, const del_br(netdev_priv(dev), NULL); rtnl_unlock(); + if (ret == 0) + module_put(THIS_MODULE); return ret; }
Otherwise 'modprobe -r' on a module having a dependency on bridge will implicitly unload bridge, bringing down all connectivity that was using bridges. Signed-off-by: Jan Beulich <jbeulich@novell.com> Cc: Jeff Mahoney <jeffm@suse.com> --- net/bridge/br_if.c | 9 +++++++++ 1 file changed, 9 insertions(+) -- 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