From a4f64ccb5e3fe5642b5ab1cd96c726cfc1f73c86 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 17 Apr 2003 21:28:56 -0700 Subject: [PATCH] [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 | 2 +- net/bridge/br.c | 17 ++---- net/bridge/br_device.c | 16 +++-- net/bridge/br_if.c | 126 ++++++++++++++++++---------------------- net/bridge/br_private.h | 11 +--- net/bridge/br_stp_if.c | 2 +- 6 files changed, 79 insertions(+), 95 deletions(-) diff --git a/include/linux/if.h b/include/linux/if.h index c4c72e7907006..a9fdca178d243 100644 --- 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 35b3045cf6b03..7f7ccdd495fd9 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -21,7 +21,6 @@ #include #include #include -#include #include #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 94e8312a059c7..39ccdf1e6e050 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #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 f3b70d7fda576..0ca2b56a073b5 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -17,14 +17,13 @@ #include #include #include +#include #include #include +#include #include #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;idev.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 d6c2f0b36eb88..dedf59459c15c 100644 --- 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 fdff4baa3b128..a9fdf21472b62 100644 --- 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 */ -- 2.53.0