From: Krister Johansen <kjlx@templeofstupid•com>
To: Jesper Dangaard Brouer <brouer@redhat•com>
Cc: netdev@vger•kernel.org, Phil Sutter <phil@nwl•cc>,
Robert Olsson <robert@herjulf•se>,
Jamal Hadi Salim <jhs@mojatatu•com>
Subject: Re: [net-next PATCH 2/3] net/qdisc: IFF_NO_QUEUE drivers should use consistent TX queue len
Date: Thu, 3 Nov 2016 13:54:40 -0700 [thread overview]
Message-ID: <20161103205440.GB2940@templeofstupid.com> (raw)
In-Reply-To: <20161103135606.28737.67383.stgit@firesoul>
On Thu, Nov 03, 2016 at 02:56:06PM +0100, Jesper Dangaard Brouer wrote:
> The flag IFF_NO_QUEUE marks virtual device drivers that doesn't need a
> default qdisc attached, given they will be backed by physical device,
> that already have a qdisc attached for pushback.
>
> It is still supported to attach a qdisc to a IFF_NO_QUEUE device, as
> this can be useful for difference policy reasons (e.g. bandwidth
> limiting containers). For this to work, the tx_queue_len need to have
> a sane value, because some qdiscs inherit/copy the tx_queue_len
> (namely, pfifo, bfifo, gred, htb, plug and sfb).
>
> Commit a813104d9233 ("IFF_NO_QUEUE: Fix for drivers not calling
> ether_setup()") caught situations where some drivers didn't initialize
> tx_queue_len. The problem with the commit was choosing 1 as the
> fallback value.
>
> A qdisc queue length of 1 causes more harm than good, because it
> creates hard to debug situations for userspace. It gives userspace a
> false sense of a working config after attaching a qdisc. As low
> volume traffic (that doesn't activate the qdisc policy) works,
> like ping, while traffic that e.g. needs shaping cannot reach the
> configured policy levels, given the queue length is too small.
Thanks for fixing this. I've run into this in the exact scenario you
describe -- bandwith limiting containers. I'm pretty sure my vote
doesn't count, but I'm in favor of this change.
-K
next prev parent reply other threads:[~2016-11-03 20:54 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-03 13:55 [net-next PATCH 0/3] qdisc and tx_queue_len cleanups for IFF_NO_QUEUE devices Jesper Dangaard Brouer
2016-11-03 13:56 ` [net-next PATCH 1/3] net: make default TX queue length a defined constant Jesper Dangaard Brouer
2016-11-03 13:56 ` [net-next PATCH 2/3] net/qdisc: IFF_NO_QUEUE drivers should use consistent TX queue len Jesper Dangaard Brouer
2016-11-03 20:54 ` Krister Johansen [this message]
2016-11-04 10:56 ` Jesper Dangaard Brouer
2016-11-03 13:56 ` [net-next PATCH 3/3] qdisc: catch misconfig of attaching qdisc to tx_queue_len zero device Jesper Dangaard Brouer
2016-11-04 9:35 ` Phil Sutter
2016-11-04 10:10 ` Jesper Dangaard Brouer
2016-11-04 10:59 ` Phil Sutter
2016-11-04 12:09 ` Jesper Dangaard Brouer
2016-11-04 12:53 ` Sergei Shtylyov
2016-11-08 6:14 ` Maciej Żenczykowski
2016-11-08 7:46 ` Jesper Dangaard Brouer
2016-11-07 18:13 ` [net-next PATCH 0/3] qdisc and tx_queue_len cleanups for IFF_NO_QUEUE devices David Miller
2016-11-07 20:11 ` Jesper Dangaard Brouer
2016-11-08 1:16 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161103205440.GB2940@templeofstupid.com \
--to=kjlx@templeofstupid$(echo .)com \
--cc=brouer@redhat$(echo .)com \
--cc=jhs@mojatatu$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=phil@nwl$(echo .)cc \
--cc=robert@herjulf$(echo .)se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox