* Herbert Xu <20050503100306.GB29788@xxxxxxxxxxxxxxxxxxx> 2005-05-03 20:03
> 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 do like the patch, no question but IFF_RUNNING is still abused
by drivers and some subsystems. So I'm not sure how reliable the
above code will be without those cases fixed. I submitted a
patchset once to fix some of them, not sure about the status. Also,
what about those drivers that do not support or do not use
netif_carrier_(on|off)?
|