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

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

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 {
+		return -EINVAL;
+
 	del_nbp(p);
 	br_stp_recalculate_bridge_id(br);
-	}
-	spin_unlock_bh(&br->lock);
-
-	return retval;
+	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();
 

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;

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);
-		}
 		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);
-		}
 		break;
 
 	case NETDEV_UNREGISTER:
 		br_del_if(br, dev);
 		break;
 	}
+	spin_unlock_bh(&br->lock);
 
 	return NOTIFY_DONE;
 }

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

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