Re: Strange uses of netif_start_queue

Re: Strange uses of netif_start_queue

[Ralf Baechle]

On Fri, Aug 12, 2005 at 02:35:14PM +0100, Alan Cox wrote:

> Something I noticed doing the tty work. the 6pack driver calls
> netif_start_queue() before it calls register_netdev. I'm curious if this
> is allowed ?

As part of adding support for extended 6pack which is required by the
PR 430 I've recently fixed that.  It was looking suspect enough that I
fixed it though I don't see any way this could do harm.

  Ralf

Re: Strange uses of netif_start_queue

[Patrick McHardy]

Alan Cox wrote:
> Something I noticed doing the tty work. the 6pack driver calls
> netif_start_queue() before it calls register_netdev. I'm curious if this
> is allowed ?

All netif_start_queue does is clear_bit(...), so it is a NOP at that
point. I guess it could be removed.

Strange uses of netif_start_queue

[Alan Cox]

Something I noticed doing the tty work. the 6pack driver calls
netif_start_queue() before it calls register_netdev. I'm curious if this
is allowed ?

Re: Strange uses of netif_start_queue

[Ralf Baechle]

On Fri, Aug 12, 2005 at 02:27:59PM +0100, Ralf Baechle wrote:

> > Something I noticed doing the tty work. the 6pack driver calls
> > netif_start_queue() before it calls register_netdev. I'm curious if this
> > is allowed ?
> 
> As part of adding support for extended 6pack which is required by the
> PR 430 I've recently fixed that.  It was looking suspect enough that I
> fixed it though I don't see any way this could do harm.

To answer the fundamental question, I think netif_start_queue /
netif_stop_queue should be allowed in case the driver for some reason has
the desire to stop queueing of packet immediately after register_netdev.

  Ralf

Re: Strange uses of netif_start_queue

[David S. Miller]

From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Fri, 12 Aug 2005 14:35:14 +0100

> Something I noticed doing the tty work. the 6pack driver calls
> netif_start_queue() before it calls register_netdev. I'm curious if this
> is allowed ?

It's definitely a bug, and when register_netdev() happens it will
just overwrite that change of state.

Since the queue is not initialized yet, this could also cause
a crash or hang. :-)

Re: Strange uses of netif_start_queue

[David S. Miller]

From: Ralf Baechle <ralf@linux-mips.org>
Date: Fri, 12 Aug 2005 14:39:05 +0100

> To answer the fundamental question, I think netif_start_queue /
> netif_stop_queue should be allowed in case the driver for some reason has
> the desire to stop queueing of packet immediately after register_netdev.

I disagree.  register_netdev() does not make packets start getting
queued, you have to up the interface for that to start occuring.
And your ->open() routine has full control over that.

Re: Strange uses of netif_start_queue

[Thomas Graf]

* David S. Miller <20050812.110034.63126875.davem@davemloft.net> 2005-08-12 11:00
> From: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Date: Fri, 12 Aug 2005 14:35:14 +0100
> 
> > Something I noticed doing the tty work. the 6pack driver calls
> > netif_start_queue() before it calls register_netdev. I'm curious if this
> > is allowed ?
> 
> It's definitely a bug, and when register_netdev() happens it will
> just overwrite that change of state.
> 
> Since the queue is not initialized yet, this could also cause
> a crash or hang. :-)

Hmm, maybe I got something wrong but:

As you say correctly, there is no way we can queue packets until
the device has been put up, a device cannot be put up while it is
not present but that doesn't happen before register_netdevice()
so the value of the queue bit doesn't matter at all. Actually the
default state is indeed to have the queue running (bit set to 0)
so the mentioned netif_start_queue() is a simple nop as Patrick
noted already. It will _not_ be overwritten by a call to
register_netdevice() though which is a good thing: It allows the
queue to be stopped from the very beginning without a race between
register_netdevice() and the call to netif_stop_queue().