Re: [PATCH] net: Disable queueing when carrier is lost

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

Herbert Xu wrote:
> On Sat, Apr 30, 2005 at 02:57:12PM +0200, Tommy Christensen wrote:
>>So, I am reluctant to drop this check for netif_queue_stopped. Doing so
>>would needlessly impact the drivers that work fine already (or at least
>>should be handling this situation).
> 
> I don't see how those drivers are worse off without this check.  When
> the carrier is off it doesn't really matter whether we're sending them
> packets or not.  Besides, you didn't put this check in the link_watch
> code path which would affect them a lot more than this path.

My reasoning is reversed. You're saying:
   "What could possibly happen, if we do this?",
whereas I'm saying:
   "This has no benefit, so why risk it?".

I'm just not confident that some odd driver won't choke on this change.

As I'm probably just being too cautious (and your approach does seem
more consistent), I'm sending a new patch following your ideas.
And then Dave has something to choose from ;-)

>>Ideas, anyone?
> 
> You mean how we can avoid the double call? Here is one way.  Move
> the setting of IF_RUNNING out of dev_get_flags and into dev_activate.
> This would guarantee that IF_RUNNING is set iff the device has a qdisc.
> Assuming that you remove the aformentioned netif_queue_stopped check,
> the device has a qdisc iff the carrier is on.  So this preserves the
> meaning of IF_RUNNING.
> 
> Now you can avoid the double call by returning early in dev_activate if
> IF_RUNNING is set, and in dev_deactivate if IF_RUNNING is not set.

Implemented in the new patch, thanks.


-Tommy