[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

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

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);

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;

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();
+
+}

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,

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 */