[BRIDGE]: Fix several startup/shutdown timer races.

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index b4e92cb875c68a371fa1b8821639874f2c94dfdd..36d02b08989d6b181147b187161943454ccb8098 100644 (file)
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -110,23 +110,38 @@ static int br_dev_accept_fastpath(struct net_device *dev, struct dst_entry *dst)
        return -1;
 }
 
+/* convert later to direct kfree */
+static void br_dev_free(struct net_device *dev)
+{
+       struct net_bridge *br = dev->priv;
+
+       WARN_ON(!list_empty(&br->port_list));
+       WARN_ON(!list_empty(&br->age_list));
+
+       BUG_ON(timer_pending(&br->hello_timer));
+       BUG_ON(timer_pending(&br->tcn_timer));
+       BUG_ON(timer_pending(&br->topology_change_timer));
+       BUG_ON(timer_pending(&br->gc_timer));
+
+       kfree(dev);
+}
 
 void br_dev_setup(struct net_device *dev)
 {
        memset(dev->dev_addr, 0, ETH_ALEN);
 
+       ether_setup(dev);
+
        dev->do_ioctl = br_dev_do_ioctl;
        dev->get_stats = br_dev_get_stats;
        dev->hard_start_xmit = br_dev_xmit;
        dev->open = br_dev_open;
        dev->set_multicast_list = br_dev_set_multicast_list;
-       dev->destructor = (void (*)(struct net_device *))kfree;
+       dev->destructor = br_dev_free;
        SET_MODULE_OWNER(dev);
        dev->stop = br_dev_stop;
        dev->accept_fastpath = br_dev_accept_fastpath;
        dev->tx_queue_len = 0;
        dev->set_mac_address = NULL;
        dev->priv_flags = IFF_EBRIDGE;
-
-       ether_setup(dev);
 }

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index cbfa8653f21076c59e8a8842aad18ac08b9359e2..220b97a09921a2862056076dbf0ea73b2cc43248 100644 (file)
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -41,6 +41,13 @@ static int br_initial_port_cost(struct net_device *dev)
 static void destroy_nbp(void *arg)
 {
        struct net_bridge_port *p = arg;
+
+       p->dev->br_port = NULL;
+
+       BUG_ON(timer_pending(&p->message_age_timer));
+       BUG_ON(timer_pending(&p->forward_delay_timer));
+       BUG_ON(timer_pending(&p->hold_timer));
+
        dev_put(p->dev);
        kfree(p);
 }
@@ -53,16 +60,19 @@ static void del_nbp(struct net_bridge_port *p)
        br_stp_disable_port(p);
 
        dev_set_promiscuity(dev, -1);
-       dev->br_port = NULL;
 
        list_del_rcu(&p->list);
 
        br_fdb_delete_by_port(p->br, p);
 
+       del_timer(&p->message_age_timer);
+       del_timer(&p->forward_delay_timer);
+       del_timer(&p->hold_timer);
+       
        call_rcu(&p->rcu, destroy_nbp, p);
 }
 
-static void del_ifs(struct net_bridge *br)
+static void del_br(struct net_bridge *br)
 {
        struct list_head *p, *n;
 
@@ -71,6 +81,10 @@ static void del_ifs(struct net_bridge *br)
                del_nbp(list_entry(p, struct net_bridge_port, list));
        }
        spin_unlock_bh(&br->lock);
+
+       del_timer_sync(&br->gc_timer);
+
+       unregister_netdevice(br->dev);
 }
 
 static struct net_bridge *new_nb(const char *name)
@@ -182,15 +196,14 @@ int br_del_bridge(const char *name)
                ret = -EBUSY;
        } 
 
-       else {
-               del_ifs((struct net_bridge *) dev->priv);
-               unregister_netdevice(dev);
-       }
+       else 
+               del_br(dev->priv);
 
        rtnl_unlock();
        return ret;
 }
 
+/* called under bridge lock */
 int br_add_if(struct net_bridge *br, struct net_device *dev)
 {
        struct net_bridge_port *p;
@@ -205,7 +218,6 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
                return -ELOOP;
 
        dev_hold(dev);
-       spin_lock_bh(&br->lock);
        if ((p = new_nbp(br, dev)) == NULL) {
                spin_unlock_bh(&br->lock);
                dev_put(dev);
@@ -218,26 +230,21 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
        br_fdb_insert(br, p, dev->dev_addr, 1);
        if ((br->dev->flags & IFF_UP) && (dev->flags & IFF_UP))
                br_stp_enable_port(p);
-       spin_unlock_bh(&br->lock);
 
        return 0;
 }
 
+/* called under bridge lock */
 int br_del_if(struct net_bridge *br, struct net_device *dev)
 {
        struct net_bridge_port *p;
-       int retval = 0;
 
-       spin_lock_bh(&br->lock);
        if ((p = dev->br_port) == NULL || p->br != br)
-               retval = -EINVAL;
-       else {
-               del_nbp(p);
-               br_stp_recalculate_bridge_id(br);
-       }
-       spin_unlock_bh(&br->lock);
+               return -EINVAL;
 
-       return retval;
+       del_nbp(p);
+       br_stp_recalculate_bridge_id(br);
+       return 0;
 }
 
 int br_get_bridge_ifindices(int *indices, int num)
@@ -274,13 +281,8 @@ void __exit br_cleanup_bridges(void)
        rtnl_lock();
        for (dev = dev_base; dev; dev = nxt) {
                nxt = dev->next;
-               if (dev->priv_flags & IFF_EBRIDGE) {
-                       pr_debug("cleanup %s\n", dev->name);
-
-                       del_ifs((struct net_bridge *) dev->priv);
-                       
-                       unregister_netdevice(dev);
-               }
+               if (dev->priv_flags & IFF_EBRIDGE)
+                       del_br(dev->priv);
        }
        rtnl_unlock();
 

diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index 939663847ca496926228076f6f529a811a3c3857..3ee2e2e8361dec22a59b63116d011f4fb96e1a3d 100644 (file)
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -59,10 +59,12 @@ static int br_ioctl_device(struct net_bridge *br,
                if (dev == NULL)
                        return -EINVAL;
 
+               spin_lock_bh(&br->lock);
                if (cmd == BRCTL_ADD_IF)
                        ret = br_add_if(br, dev);
                else
                        ret = br_del_if(br, dev);
+               spin_unlock_bh(&br->lock);
 
                dev_put(dev);
                return ret;

diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 72cc91e87523379563ce2e7a6b7a7bd64d244582..9993944c56cfd73ccd23c8fe21f0ad923b8bb970 100644 (file)
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -38,39 +38,27 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 
        br = p->br;
 
+       spin_lock_bh(&br->lock);
        switch (event) 
        {
        case NETDEV_CHANGEADDR:
-               spin_lock_bh(&br->lock);
                br_fdb_changeaddr(p, dev->dev_addr);
                br_stp_recalculate_bridge_id(br);
-               spin_unlock_bh(&br->lock);
-               break;
-
-       case NETDEV_GOING_DOWN:
-               /* extend the protocol to send some kind of notification? */
                break;
 
        case NETDEV_DOWN:
-               if (br->dev->flags & IFF_UP) {
-                       spin_lock_bh(&br->lock);
-                       br_stp_disable_port(p);
-                       spin_unlock_bh(&br->lock);
-               }
+               br_stp_disable_port(p);
                break;
 
        case NETDEV_UP:
-               if (!(br->dev->flags & IFF_UP)) {
-                       spin_lock_bh(&br->lock);
-                       br_stp_enable_port(p);
-                       spin_unlock_bh(&br->lock);
-               }
+               br_stp_enable_port(p);
                break;
 
        case NETDEV_UNREGISTER:
                br_del_if(br, dev);
                break;
        }
+       spin_unlock_bh(&br->lock);
 
        return NOTIFY_DONE;
 }

diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index f644f2164a5ef03a6a0437317fa84e50a8884e49..c3a906a1159c63c7631d4ac19e828a78b8bd19cc 100644 (file)
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -43,8 +43,7 @@ void br_stp_enable_bridge(struct net_bridge *br)
        struct net_bridge_port *p;
 
        spin_lock_bh(&br->lock);
-       br->hello_timer.expires = jiffies + br->hello_time;
-       add_timer(&br->hello_timer);
+       mod_timer(&br->hello_timer, jiffies + br->hello_time);
        br_config_bpdu_generation(br);
 
        list_for_each_entry(p, &br->port_list, list) {
@@ -74,8 +73,6 @@ void br_stp_disable_bridge(struct net_bridge *br)
        del_timer_sync(&br->hello_timer);
        del_timer_sync(&br->topology_change_timer);
        del_timer_sync(&br->tcn_timer);
-       del_timer_sync(&br->gc_timer);
-
 }
 
 /* called under bridge lock */

diff --git a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
index a5a75cede729d05b3087d33a4d35efad5cb03b29..eb106a4042fde65876bad56493c23247f91398f9 100644 (file)
--- a/net/bridge/br_stp_timer.c
+++ b/net/bridge/br_stp_timer.c
@@ -43,8 +43,7 @@ static void br_hello_timer_expired(unsigned long arg)
        if (br->dev->flags & IFF_UP) {
                br_config_bpdu_generation(br);
 
-               br->hello_timer.expires = jiffies + br->hello_time;
-               add_timer(&br->hello_timer);
+               mod_timer(&br->hello_timer, jiffies + br->hello_time);
        }
        spin_unlock_bh(&br->lock);
 }
@@ -73,6 +72,8 @@ static void br_message_age_timer_expired(unsigned long arg)
         * check is redundant. I'm leaving it in for now, though.
         */
        spin_lock_bh(&br->lock);
+       if (p->state == BR_STATE_DISABLED)
+               goto unlock;
        was_root = br_is_root_bridge(br);
 
        br_become_designated_port(p);
@@ -80,6 +81,7 @@ static void br_message_age_timer_expired(unsigned long arg)
        br_port_state_selection(br);
        if (br_is_root_bridge(br) && !was_root)
                br_become_root_bridge(br);
+ unlock:
        spin_unlock_bh(&br->lock);
 }
 
@@ -93,8 +95,8 @@ static void br_forward_delay_timer_expired(unsigned long arg)
        spin_lock_bh(&br->lock);
        if (p->state == BR_STATE_LISTENING) {
                p->state = BR_STATE_LEARNING;
-               p->forward_delay_timer.expires = jiffies + br->forward_delay;
-               add_timer(&p->forward_delay_timer);
+               mod_timer(&p->forward_delay_timer,
+                         jiffies + br->forward_delay);
        } else if (p->state == BR_STATE_LEARNING) {
                p->state = BR_STATE_FORWARDING;
                if (br_is_designated_for_some_port(br))
@@ -113,8 +115,7 @@ static void br_tcn_timer_expired(unsigned long arg)
        if (br->dev->flags & IFF_UP) {
                br_transmit_tcn(br);
        
-               br->tcn_timer.expires = jiffies + br->bridge_hello_time;
-               add_timer(&br->tcn_timer);
+               mod_timer(&br->tcn_timer,jiffies + br->bridge_hello_time);
        }
        spin_unlock_bh(&br->lock);
 }