[PATCH] Module use count must be updated as bridges are created/destroyed

Previous thread: Man page for revised timerfd API by Michael Kerrisk on Wednesday, September 26, 2007 - 3:12 am. (10 messages)

Next thread: NinjaSCSI PCMCIA card by Pavel Fedin on Wednesday, September 26, 2007 - 3:39 am. (3 messages)
To: <shemminger@...>
Cc: <bridge@...>, <linux-kernel@...>
Date: Wednesday, September 26, 2007 - 3:53 am

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>

net/bridge/br_if.c | 9 +++++++++
1 file changed, 9 insertions(+)

--- linux-2.6.23-rc8/net/bridge/br_if.c 2007-09-26 09:23:54.000000000 +0200
+++ 2.6.23-rc8-bridge-module-get-put/net/bridge/br_if.c 2007-09-25 14:31:01.000000000 +0200
@@ -276,6 +276,11 @@ int br_add_bridge(const char *name)
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);
@@ -294,6 +299,8 @@ int br_add_bridge(const char *name)
unregister_netdevice(dev);
out:
rtnl_unlock();
+ if (ret)
+ module_put(THIS_MODULE);
return ret;
}

@@ -321,6 +328,8 @@ int br_del_bridge(const char *name)
del_br(netdev_priv(dev));

rtnl_unlock();
+ if (ret == 0)
+ module_put(THIS_MODULE);
return ret;
}

-

To: Jan Beulich <jbeulich@...>
Cc: <bridge@...>, <linux-kernel@...>
Date: Wednesday, September 26, 2007 - 11:37 am

On Wed, 26 Sep 2007 08:53:27 +0100

No, network devices don't do reference counting.
What is the dependency? Where is the source of the module interacting
with the bridge?
-

To: Stephen Hemminger <shemminger@...>
Cc: Jan Beulich <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Wednesday, September 26, 2007 - 5:06 pm

* Wed, 26 Sep 2007 08:37:05 -0700

Could you explain why, please?

After `udevd` on boot loads lots of unused crap, i surrendered, and use
$(rmmod `lsmod | just first column`). Networing bravely wipes away. OK,
there are lots of configs: udev, hotplug, modprobe, that somebody might
like to fix. But it came to the end with me. I just don't care. So,
please answer :)
____
-

To: Oleg Verych <olecom@...>
Cc: Jan Beulich <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Wednesday, September 26, 2007 - 5:06 pm

On Wed, 26 Sep 2007 23:06:53 +0200

For hotplug and other reasons, the network developers decided that being
able to remove a network module at any time was a good thing. It works.

--
Stephen Hemminger <shemminger@linux-foundation.org>
-

To: Stephen Hemminger <shemminger@...>
Cc: Oleg Verych <olecom@...>, Jan Beulich <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Wednesday, September 26, 2007 - 6:18 pm

Except that for ipv6.ko, it's all opposite. After modprobe,
it already got a refcount like 8 and you're wondering how
to get rid of that.

-

To: Jan Engelhardt <jengelh@...>
Cc: Oleg Verych <olecom@...>, Jan Beulich <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Wednesday, September 26, 2007 - 6:33 pm

On Thu, 27 Sep 2007 00:18:55 +0200 (CEST)

ipv6 is not a network driver, it is a protocol. You might be able to remove it if you zap
all the routes and applications, ...

--
Stephen Hemminger <shemminger@linux-foundation.org>
-

To: Stephen Hemminger <shemminger@...>
Cc: Jan Engelhardt <jengelh@...>, Oleg Verych <olecom@...>, Jan Beulich <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Thursday, September 27, 2007 - 7:54 am

Wouldn't it be enough to down all the interfaces and close all the sockets?
No need to bring down every app.

Helge Hafting
-

To: <helge.hafting@...>
Cc: <shemminger@...>, <jengelh@...>, <olecom@...>, <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Thursday, September 27, 2007 - 3:01 pm

From: Helge Hafting <helge.hafting@aitel.hist.no>

And there are routes, and neighbour cache entries, and all sorts
of external references to the stack. For example, if a packet
gets stuck in a device because the link just went down, that
can hold references to the ipv6 module from several angles.

But you have to add code to actually keep track of all of these
references and there is no such code in the ipv6 module at all
and it's a nontrivial time consuming job to implement it.
-

To: Helge Hafting <helge.hafting@...>
Cc: Jan Engelhardt <jengelh@...>, Oleg Verych <olecom@...>, Jan Beulich <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Thursday, September 27, 2007 - 10:51 am

On Thu, 27 Sep 2007 13:54:23 +0200

You need every socket to close and all routes to go away including the routes through
loopback device, and still there probably are control sockets buried inside ipv6
that hold ref count.

IMHO the kernel should just admit that IPV6 can't be removed.

--- a/net/ipv6/af_inet6.c 2007-09-26 16:28:01.000000000 -0700
+++ b/net/ipv6/af_inet6.c 2007-09-26 17:38:23.000000000 -0700
@@ -914,6 +914,9 @@ out_unregister_tcp_proto:
}
module_init(inet6_init);

+/* Disabled at present because it is impossible to remove all references */
+#ifdef IPV6_UNLOAD
+
static void __exit inet6_exit(void)
{
/* First of all disallow new sockets creation. */
@@ -952,5 +955,6 @@ static void __exit inet6_exit(void)
proto_unregister(&tcpv6_prot);
}
module_exit(inet6_exit);
+#endif

MODULE_ALIAS_NETPROTO(PF_INET6);
--- a/net/ipv6/addrconf.c 2007-09-26 15:07:35.000000000 -0700
+++ b/net/ipv6/addrconf.c 2007-09-26 17:36:52.000000000 -0700
@@ -4255,6 +4255,7 @@ errout:
return err;
}

+#ifdef IPV6_UNLOAD
void __exit addrconf_cleanup(void)
{
struct net_device *dev;
@@ -4308,3 +4309,4 @@ void __exit addrconf_cleanup(void)
proc_net_remove(&init_net, "if_inet6");
#endif
}
+#endif
--- a/net/ipv6/ipv6_sockglue.c 2007-09-26 15:07:35.000000000 -0700
+++ b/net/ipv6/ipv6_sockglue.c 2007-09-26 17:36:17.000000000 -0700
@@ -1132,7 +1132,9 @@ void __init ipv6_packet_init(void)
dev_add_pack(&ipv6_packet_type);
}

-void ipv6_packet_cleanup(void)
+#ifdef IPV6_UNLOAD
+void __exit ipv6_packet_cleanup(void)
{
dev_remove_pack(&ipv6_packet_type);
}
+#endif
-

To: Stephen Hemminger <shemminger@...>
Cc: Helge Hafting <helge.hafting@...>, Oleg Verych <olecom@...>, Jan Beulich <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Thursday, September 27, 2007 - 10:55 am

I cannot accept that. If ipv6.ko has a way to tack ipv6 structs onto all
sockets, interfaces and addresses, it should also be able to untack it
again.
-

To: <jengelh@...>
Cc: <shemminger@...>, <helge.hafting@...>, <olecom@...>, <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Thursday, September 27, 2007 - 3:15 pm

From: Jan Engelhardt <jengelh@computergmbh.de>

Then you do the work and see how incredibly difficult the
implementation is, others have tried. It's not trivial
and at best it's a very time consuming piece of work to
embark on.

Until then it's un-removable, plain as that :-)

I don't know why we're wasting our fingers discussing this
at all to be honest with you.
-

To: <shemminger@...>
Cc: <jengelh@...>, <olecom@...>, <jbeulich@...>, <bridge@...>, <linux-kernel@...>
Date: Wednesday, September 26, 2007 - 6:57 pm

From: Stephen Hemminger <shemminger@linux-foundation.org>

It is purposefully set to have a permanent elevated reference
count because it is not designed to be unloaded safely.

It has been unloadable forever.
-

Previous thread: Man page for revised timerfd API by Michael Kerrisk on Wednesday, September 26, 2007 - 3:12 am. (10 messages)

Next thread: NinjaSCSI PCMCIA card by Pavel Fedin on Wednesday, September 26, 2007 - 3:39 am. (3 messages)