Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1336458

Re: [PATCH] Revert "net: sched: drop all special handling of tx_queue_len == 0"

From Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Newsgroups linux.kernel
Subject Re: [PATCH] Revert "net: sched: drop all special handling of tx_queue_len == 0"
Date 2016-02-17 15:00 +0100
Message-ID <r3aO7-7aS-35@gated-at.bofh.it> (permalink)
References <r2YDh-73D-15@gated-at.bofh.it> <r39S2-6yA-25@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


----- On Feb 17, 2016, at 7:47 AM, Phil Sutter phil@nwl.cc wrote:

> Hi,
> 
> On Tue, Feb 16, 2016 at 07:56:23PM -0500, Mathieu Desnoyers wrote:
>> This reverts commit 348e3435cbefa815bd56a5205c1412b5afe7b92e.
>> It breaks HTB classful qdiscs on the loopback interface.
>> 
>> It has been broken since kernel v4.2. The offending commit has
>> been identified by bissection of the issue with the following
>> test-case. It appears that the loopback interface does indeed
>> still have tx_queue_len == 0.
>> 
>> Reverting the commit on a v4.4.1 kernel fixes the issue.
> 
> Indeed, this is ugly. Affected are all drivers not calling ether_setup()
> in their setup callback, and therefore not initializing tx_queue_len.
> 
> As the commit to be reverted shows, there is no common fallback value
> for tx_queue_len - most qdisc implementations used 1, but HTB fell back
> to 2 and PLUG to 100. But as the removed comment in sch_plug.c stated:
> 
>| /* We will set a default limit of 100 pkts (~150kB)
>|  * in case tx_queue_len is not available. The
>|  * default value is completely arbitrary.
>|  */
> 
> It seems not to be overly important to fallback to the exact value each
> qdisc had before. Therefore I guess the following change should
> appropriately fix the issue at hand:
> 
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7440,8 +7440,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv,
> const char *name,
>        dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
>        setup(dev);
> 
> -       if (!dev->tx_queue_len)
> +       if (!dev->tx_queue_len) {
>                dev->priv_flags |= IFF_NO_QUEUE;
> +               dev->tx_queue_len = 1;
> +       }
> 
>        dev->num_tx_queues = txqs;
>        dev->real_num_tx_queues = txqs;
> 
> Unless there is concern, I will formally submit this later.

Works for me!

Tested-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

Thanks,

Mathieu

> 
> Thanks, Phil

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] Revert "net: sched: drop all special handling of tx_queue_len == 0" Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-02-17 02:00 +0100
  Re: [PATCH] Revert "net: sched: drop all special handling of  tx_queue_len == 0" Phil Sutter <phil@nwl.cc> - 2016-02-17 14:00 +0100
    Re: [PATCH] Revert "net: sched: drop all special handling of  tx_queue_len == 0" Phil Sutter <phil@nwl.cc> - 2016-02-17 15:00 +0100
    Re: [PATCH] Revert "net: sched: drop all special handling of  tx_queue_len == 0" Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-02-17 15:00 +0100
      [net PATCH] IFF_NO_QUEUE: Fix for drivers not calling ether_setup() Phil Sutter <phil@nwl.cc> - 2016-02-17 15:40 +0100
        Re: [net PATCH] IFF_NO_QUEUE: Fix for drivers not calling  ether_setup() David Miller <davem@davemloft.net> - 2016-02-18 21:00 +0100

csiph-web