netdev - Re: [net-next PATCH 2/3] net: sched: set tx_queue_len to default when changing noqueue device's qdisc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150729211022.GG32705@breakpoint.cc>
Date:	Wed, 29 Jul 2015 23:10:22 +0200
From:	Florian Westphal <fw@...len.de>
To:	Phil Sutter <phil@....cc>
Cc:	netdev@...r.kernel.org, Jesper Dangaard Brouer <brouer@...hat.com>,
	Cong Wang <cwang@...pensource.com>,
	Eric Dumazet <eric.dumazet@...il.com>,
	David Miller <davem@...emloft.net>
Subject: Re: [net-next PATCH 2/3] net: sched: set tx_queue_len to default
 when changing noqueue device's qdisc

Phil Sutter <phil@....cc> wrote:
> Virtual interfaces don't necessarily need a qdisc attached to them. This
> is signalled by setting dev->tx_queue_len to zero upon initialisation. The
> problems begin when a user still adds a qdisc, as then the special value
> is used as a regular one causing massive packet drops as soon as the
> device sees a bit of traffic load.
> 
> This patch solves the issue by setting tx_queue_len to the global
> default for physical devices if it is zero and a qdisc is added. Another
> benefit of this is that veth devices can default to noqueue while still
> preserving the expected behaviour when adding a qdisc.
> 
> The drawback of this patch is that the noqueue state won't be restored
> after a user has removed the custom qdisc, as the information about
> whether this is legitimate for the given device is lost.
> 
> Signed-off-by: Phil Sutter <phil@....cc>
> ---
>  net/sched/sch_api.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index f06aa01..79b8900 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1315,6 +1315,10 @@ graft:
>  			qdisc_destroy(q);
>  		return err;
>  	}
> +	/* circumvent noqueue hack for virtual interfaces if
> +	 * user desires to use a qdisc on it */
> +	if (!dev->tx_queue_len)
> +		dev->tx_queue_len = CONFIG_DEFAULT_TX_QUEUE_LEN;

The comment should tell you that this isn't pretty :-)

How about making tx_queue_len really the tx_queue_len, and some
not some "yeah, if its zero don't assign a queue"?

IOW, why not specifically annotate virtual devices so they do not get a
queue, ever (unless configured?)

That would allow to leave every default setting just the way they are,
and it would not muck with people assigning e.g. htb to veth since
the txqlen would be untouched.

But aside from that, I think your suggested hack would work.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html