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: <1438204245.20182.106.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Wed, 29 Jul 2015 23:10:45 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Phil Sutter <phil@....cc>
Cc:	netdev@...r.kernel.org, Jesper Dangaard Brouer <brouer@...hat.com>,
	Cong Wang <cwang@...pensource.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

On Wed, 2015-07-29 at 22:51 +0200, Phil Sutter 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;
>  
>  	return 0;
>  }

This is still broken for htb : htb_init() would run before this,
and htb_init() does :


q->direct_qlen = qdisc_dev(sch)->tx_queue_len;

Really, I am not sure this can be changed. qdisc scripts are mostly
written by skilled people knowing if txqueuelen needs to be changed or
not...

Change the kernel behavior and scripts will break.


--
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