[BRIDGE]: Ethernet bridge driver device mangling cleanup

Second try at the bridge driver module handling cleanup...

1) Eliminate keeping a seperate bridge_list and use a bit on
   the priv_flags structure.  This is equivalent to how the VLAN
   code works. Makes code cleaner and correctly handles cases like
   creating a bridge with the same name as an existing ether device etc.

2) Don't do own module ref counting that is inhernently racy.
   Instead set owner field and cleanup debris on unload.

3) Do last state cleanup in destructor

4) Change of bridge state (dev_open/stop) should use write_lock

5) Make sure timer is not running when cleared.

6) Use "const char *" where possible

diff --git a/include/linux/if.h b/include/linux/if.h
index c4c72e7907006d101f57d1f562c391dc04a78780..a9fdca178d243750a7ece3ea9ff7b104cabce951 100644 (file)
--- a/include/linux/if.h
+++ b/include/linux/if.h
@@ -50,7 +50,7 @@
 
 /* Private (from user) interface flags (netdevice->priv_flags). */
 #define IFF_802_1Q_VLAN 0x1             /* 802.1Q VLAN device.          */
-
+#define IFF_EBRIDGE    0x2             /* Ethernet bridging device.    */
 
 #define IF_GET_IFACE   0x0001          /* for querying only */
 #define IF_GET_PROTO   0x0002

diff --git a/net/bridge/br.c b/net/bridge/br.c
index 35b3045cf6b03dce994a774e902304ab7d4dd41b..7f7ccdd495fd9c7a8d60cdbb23ed30475932c3bc 100644 (file)
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -21,7 +21,6 @@
 #include <linux/etherdevice.h>
 #include <linux/init.h>
 #include <linux/if_bridge.h>
-#include <linux/brlock.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
 
@@ -31,16 +30,6 @@
 
 int (*br_should_route_hook) (struct sk_buff **pskb) = NULL;
 
-void br_dec_use_count()
-{
-       module_put(THIS_MODULE);
-}
-
-void br_inc_use_count()
-{
-       try_module_get(THIS_MODULE);
-}
-
 static int __init br_init(void)
 {
        printk(KERN_INFO "NET4: Ethernet Bridge 008 for NET4.0\n");
@@ -67,15 +56,17 @@ static void __exit br_deinit(void)
        br_netfilter_fini();
 #endif
        unregister_netdevice_notifier(&br_device_notifier);
-
        brioctl_set(NULL);
        br_handle_frame_hook = NULL;
 
 #if defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE)
-       /* FIX ME. move into hook structure with ref count */
        br_fdb_get_hook = NULL;
        br_fdb_put_hook = NULL;
 #endif
+
+       br_cleanup_bridges();
+
+       synchronize_net();
 }
 
 EXPORT_SYMBOL(br_should_route_hook);

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 94e8312a059c788e6851a248f82eb5f7c17f511b..39ccdf1e6e05051d0bb5ebb6e3eea4486d28abc7 100644 (file)
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
 #include <linux/if_bridge.h>
+#include <linux/module.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
 
@@ -91,9 +92,9 @@ static int br_dev_open(struct net_device *dev)
        netif_start_queue(dev);
 
        br = dev->priv;
-       read_lock(&br->lock);
+       write_lock(&br->lock);
        br_stp_enable_bridge(br);
-       read_unlock(&br->lock);
+       write_unlock(&br->lock);
 
        return 0;
 }
@@ -107,9 +108,9 @@ static int br_dev_stop(struct net_device *dev)
        struct net_bridge *br;
 
        br = dev->priv;
-       read_lock(&br->lock);
+       write_lock(&br->lock);
        br_stp_disable_bridge(br);
-       read_unlock(&br->lock);
+       write_unlock(&br->lock);
 
        netif_stop_queue(dev);
 
@@ -121,6 +122,11 @@ static int br_dev_accept_fastpath(struct net_device *dev, struct dst_entry *dst)
        return -1;
 }
 
+static void br_dev_destruct(struct net_device *dev) 
+{
+       kfree(dev->priv);
+}
+
 void br_dev_setup(struct net_device *dev)
 {
        memset(dev->dev_addr, 0, ETH_ALEN);
@@ -130,6 +136,8 @@ void br_dev_setup(struct net_device *dev)
        dev->hard_start_xmit = br_dev_xmit;
        dev->open = br_dev_open;
        dev->set_multicast_list = br_dev_set_multicast_list;
+       dev->destructor = br_dev_destruct;
+       dev->owner = THIS_MODULE;
        dev->stop = br_dev_stop;
        dev->accept_fastpath = br_dev_accept_fastpath;
        dev->tx_queue_len = 0;

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index f3b70d7fda5766b1ac4999b9b7d55412b1574bc9..0ca2b56a073b5217c91f49c4bcc6655b6eb546f7 100644 (file)
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -17,14 +17,13 @@
 #include <linux/if_arp.h>
 #include <linux/if_bridge.h>
 #include <linux/inetdevice.h>
+#include <linux/module.h>
 #include <linux/rtnetlink.h>
 #include <linux/brlock.h>
+#include <net/sock.h>
 #include <asm/uaccess.h>
 #include "br_private.h"
 
-static struct net_bridge *bridge_list;
-static spinlock_t bridge_lock = SPIN_LOCK_UNLOCKED;
-
 static int br_initial_port_cost(struct net_device *dev)
 {
        if (!strncmp(dev->name, "lec", 3))
@@ -70,23 +69,6 @@ static int __br_del_if(struct net_bridge *br, struct net_device *dev)
        return 0;
 }
 
-/* called with bridge_lock */
-static struct net_bridge **__find_br(char *name)
-{
-       struct net_bridge **b;
-       struct net_bridge *br;
-
-       b = &bridge_list;
-       while ((br = *b) != NULL) {
-               if (!strncmp(br->dev.name, name, IFNAMSIZ))
-                       return b;
-
-               b = &(br->next);
-       }
-
-       return NULL;
-}
-
 static void del_ifs(struct net_bridge *br)
 {
        br_write_lock_bh(BR_NETPROTO_LOCK);
@@ -97,7 +79,7 @@ static void del_ifs(struct net_bridge *br)
        br_write_unlock_bh(BR_NETPROTO_LOCK);
 }
 
-static struct net_bridge *new_nb(char *name)
+static struct net_bridge *new_nb(const char *name)
 {
        struct net_bridge *br;
        struct net_device *dev;
@@ -112,6 +94,7 @@ static struct net_bridge *new_nb(char *name)
 
        strncpy(dev->name, name, IFNAMSIZ);
        dev->priv = br;
+       dev->priv_flags = IFF_EBRIDGE;
        ether_setup(dev);
        br_dev_setup(dev);
 
@@ -178,56 +161,47 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br, struct net_device
        return p;
 }
 
-int br_add_bridge(char *name)
+int br_add_bridge(const char *name)
 {
        struct net_bridge *br;
+       int ret;
 
-       if ((br = new_nb(name)) == NULL)
+       if ((br = new_nb(name)) == NULL) 
                return -ENOMEM;
 
-       if (__dev_get_by_name(name) != NULL) {
+       ret = register_netdev(&br->dev);
+       if (ret)
                kfree(br);
-               return -EEXIST;
-       }
-
-       spin_lock(&bridge_lock);
-       br->next = bridge_list;
-       bridge_list = br;
-       spin_unlock(&bridge_lock);
-
-       br_inc_use_count();
-       register_netdev(&br->dev);
-
-       return 0;
+       return ret;
 }
 
-int br_del_bridge(char *name)
+int br_del_bridge(const char *name)
 {
-       struct net_bridge **b;
-       struct net_bridge *br;
+       struct net_device *dev;
+       int ret = 0;
 
-       spin_lock(&bridge_lock);
-       if ((b = __find_br(name)) == NULL) {
-               spin_unlock(&bridge_lock);
-               return -ENXIO;
-       }
+       dev = dev_get_by_name(name);
+       if (dev == NULL) 
+               return -ENXIO;  /* Could not find device */
 
-       br = *b;
-       if (br->dev.flags & IFF_UP) {
-               spin_unlock(&bridge_lock);
-               return -EBUSY;
+       if (!(dev->priv_flags & IFF_EBRIDGE)) {
+               /* Attempt to delete non bridge device! */
+               ret = -EPERM;
        }
 
-       *b = br->next;
-       spin_unlock(&bridge_lock);
-
-       del_ifs(br);
+       else if (dev->flags & IFF_UP) {
+               /* Not shutdown yet. */
+               ret = -EBUSY;
+       } 
 
-       unregister_netdev(&br->dev);
-       kfree(br);
-       br_dec_use_count();
+       else {
+               del_ifs((struct net_bridge *) dev->priv);
+       
+               unregister_netdev(dev);
+       }
 
-       return 0;
+       dev_put(dev);
+       return ret;
 }
 
 int br_add_if(struct net_bridge *br, struct net_device *dev)
@@ -278,24 +252,19 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 
 int br_get_bridge_ifindices(int *indices, int num)
 {
-       struct net_bridge *br;
-       int i;
-
-       spin_lock(&bridge_lock);
-       br = bridge_list;
-       for (i=0;i<num;i++) {
-               if (br == NULL)
-                       break;
+       struct net_device *dev;
+       int i = 0;
 
-               indices[i] = br->dev.ifindex;
-               br = br->next;
+       rtnl_shlock();
+       for (dev = dev_base; dev && i < num; dev = dev->next) {
+               if (dev->priv_flags & IFF_EBRIDGE) 
+                       indices[i++] = dev->ifindex;
        }
-       spin_unlock(&bridge_lock);
+       rtnl_shunlock();
 
        return i;
 }
 
-/* called under ioctl_lock */
 void br_get_port_ifindices(struct net_bridge *br, int *ifindices)
 {
        struct net_bridge_port *p;
@@ -308,3 +277,24 @@ void br_get_port_ifindices(struct net_bridge *br, int *ifindices)
        }
        read_unlock(&br->lock);
 }
+
+
+void __exit br_cleanup_bridges(void)
+{
+       struct net_device *dev, *nxt;
+
+       rtnl_lock();
+       for (dev = dev_base; dev; dev = nxt) {
+               nxt = dev->next;
+               if ((dev->priv_flags & IFF_EBRIDGE)
+                   && dev->owner == THIS_MODULE) {
+                       pr_debug("cleanup %s\n", dev->name);
+
+                       del_ifs((struct net_bridge *) dev->priv);
+                       
+                       unregister_netdevice(dev);
+               }
+       }
+       rtnl_unlock();
+
+}

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index d6c2f0b36eb88038143a8bc74b73083a07f3130d..dedf59459c15c4d1ce2c78f4e0eb0ae8d1c0470d 100644 (file)
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -79,7 +79,6 @@ struct net_bridge_port
 
 struct net_bridge
 {
-       struct net_bridge               *next;
        rwlock_t                        lock;
        struct net_bridge_port          *port_list;
        struct net_device               dev;
@@ -115,10 +114,6 @@ struct net_bridge
 extern struct notifier_block br_device_notifier;
 extern unsigned char bridge_ula[6];
 
-/* br.c */
-extern void br_dec_use_count(void);
-extern void br_inc_use_count(void);
-
 /* br_device.c */
 extern void br_dev_setup(struct net_device *dev);
 extern int br_dev_xmit(struct sk_buff *skb, struct net_device *dev);
@@ -156,8 +151,9 @@ extern void br_flood_forward(struct net_bridge *br,
                      int clone);
 
 /* br_if.c */
-extern int br_add_bridge(char *name);
-extern int br_del_bridge(char *name);
+extern int br_add_bridge(const char *name);
+extern int br_del_bridge(const char *name);
+extern void br_cleanup_bridges(void);
 extern int br_add_if(struct net_bridge *br,
              struct net_device *dev);
 extern int br_del_if(struct net_bridge *br,
@@ -172,7 +168,6 @@ extern int br_handle_frame_finish(struct sk_buff *skb);
 extern int br_handle_frame(struct sk_buff *skb);
 
 /* br_ioctl.c */
-extern void br_call_ioctl_atomic(void (*fn)(void));
 extern int br_ioctl(struct net_bridge *br,
             unsigned int cmd,
             unsigned long arg0,

diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index fdff4baa3b128491cfe055b5aa743e32f30a76f5..a9fdf21472b62b420e04c079da4f0f305d63483b 100644 (file)
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -85,7 +85,7 @@ void br_stp_disable_bridge(struct net_bridge *br)
                p = p->next;
        }
 
-       del_timer(&br->tick);
+       del_timer_sync(&br->tick);
 }
 
 /* called under bridge lock */