Re: [PATCH] net: Disable queueing when carrier is lost (take 2)

[Tommy Christensen <tommy.christensen@tpack.net>]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

Herbert Xu wrote:
> Doing the wait when IFF_RUNNING is off isn't necessary though.  If
> IFF_RUNNING isn't set, then either the device has never been activated
> or we've already carried out those waits the last time we were in
> dev_deactivate.

Right. But it still depends on what the synchronization is meant to
protect us from. It isn't clear to me, whether it's
  o packets in flight
  o qdisc references
  o device references
so yes, I tried to play it safe.

Anyway, given the comments from Thomas, I've pulled out the IFF_RUNNING
stuff. We can add this later, when the other uses have been sorted out.

> I understand your preference for defensive programming.  However, in
> cases like this it's better to do the obvious thing because:
> 
> 1) We don't cover up bugs that may come back to bite us elsewhere.
> 2) We don't give people misconceptions.  If they're unfamiliar with
> the system they may infer things from this code that aren't necessarily
> the case.

I totally agree with this.

-Tommy

[-- Attachment #2: carrier-5.patch --]
[-- Type: text/plain, Size: 1293 bytes --]

diff -ru linux-2.6.12-rc3/net/core/link_watch.c linux-2.6.12-work/net/core/link_watch.c
--- linux-2.6.12-rc3/net/core/link_watch.c	2005-03-04 09:55:42.000000000 +0100
+++ linux-2.6.12-work/net/core/link_watch.c	2005-05-02 22:40:59.000000000 +0200
@@ -16,6 +16,7 @@
 #include <linux/netdevice.h>
 #include <linux/if.h>
 #include <net/sock.h>
+#include <net/pkt_sched.h>
 #include <linux/rtnetlink.h>
 #include <linux/jiffies.h>
 #include <linux/spinlock.h>
@@ -74,6 +75,12 @@
 		clear_bit(__LINK_STATE_LINKWATCH_PENDING, &dev->state);
 
 		if (dev->flags & IFF_UP) {
+			if (netif_carrier_ok(dev)) {
+				WARN_ON(dev->qdisc_sleeping == &noop_qdisc);
+				dev_activate(dev);
+			} else
+				dev_deactivate(dev);
+
 			netdev_state_change(dev);
 		}
 
diff -ru linux-2.6.12-rc3/net/sched/sch_generic.c linux-2.6.12-work/net/sched/sch_generic.c
--- linux-2.6.12-rc3/net/sched/sch_generic.c	2005-03-04 09:55:44.000000000 +0100
+++ linux-2.6.12-work/net/sched/sch_generic.c	2005-05-04 00:24:55.558105856 +0200
@@ -539,6 +539,10 @@
 		write_unlock_bh(&qdisc_tree_lock);
 	}
 
+	if (!netif_carrier_ok(dev))
+		/* Delay activation until next carrier-on event */
+		return;
+
 	spin_lock_bh(&dev->queue_lock);
 	rcu_assign_pointer(dev->qdisc, dev->qdisc_sleeping);
 	if (dev->qdisc != &noqueue_qdisc) {