netdev
[Top] [All Lists]

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

To: Tommy Christensen <tommy.christensen@xxxxxxxxx>
Subject: Re: [PATCH] net: Disable queueing when carrier is lost (take 2)
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date: Tue, 3 May 2005 20:03:06 +1000
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>, netdev@xxxxxxxxxxx
In-reply-to: <4276B13F.2040103@xxxxxxxxx>
References: <4276B13F.2040103@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.6+20040907i
Hi Tommy:

On Tue, May 03, 2005 at 01:01:19AM +0200, Tommy Christensen wrote:
> Some network drivers call netif_stop_queue() when detecting loss of
> carrier. This leads to packets being queued up at the qdisc level for
> an unbound period of time. In order to prevent this effect, the core
> networking stack will now seize to queue packets for any device, that
> is operationally down (i.e. the queue is flushed and disabled).

This looks great.

> @@ -552,15 +560,18 @@
>  {
>       struct Qdisc *qdisc;
>  
> -     spin_lock_bh(&dev->queue_lock);
> -     qdisc = dev->qdisc;
> -     dev->qdisc = &noop_qdisc;
> +     if (dev->flags & IFF_RUNNING) {
> +             spin_lock_bh(&dev->queue_lock);
> +             qdisc = dev->qdisc;
> +             dev->qdisc = &noop_qdisc;
>  
> -     qdisc_reset(qdisc);
> +             qdisc_reset(qdisc);
>  
> -     spin_unlock_bh(&dev->queue_lock);
> +             spin_unlock_bh(&dev->queue_lock);
>  
> -     dev_watchdog_down(dev);
> +             dev_watchdog_down(dev);
> +     }
> +     dev->flags &= ~IFF_RUNNING;
>  
>       while (test_bit(__LINK_STATE_SCHED, &dev->state))
>               yield();

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.

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.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

<Prev in Thread] Current Thread [Next in Thread>