Re: [PATCH 20/21] net: warn if drivers set tx_queue_len = 0
On Wed, 2015-08-19 at 13:31 -0700, Eric Dumazet wrote: lpaa5:~# tc qd sh dev eth1 qdisc mq 0: root qdisc fq 0: parent :4 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :3 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :2 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :1 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 Well, it seems I just leaked fact that we use 3-bands in our fq implementation ;) -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/21] net: warn if drivers set tx_queue_len = 0
Hi, On Tue, Aug 18, 2015 at 07:47:28AM -0700, Eric Dumazet wrote: On Tue, 2015-08-18 at 10:30 +0200, Phil Sutter wrote: Due to the introduction of IFF_NO_QUEUE, there is a better way for drivers to indicate that no qdisc should be attached by default. Though, the old convention can't be dropped since ignoring that setting would break drivers still using it. Instead, add a warning so out-of-tree driver maintainers get a chance to adjust their code before we finally get rid of any special handling of tx_queue_len == 0. How an admin can remove qdisc on a regular ethernet device with this schem ? I was doing : tc qdisc replace dev eth0 root pfifo ifconfig eth0 txqueuelen 0 tc qdisc del dev eth0 root I have a hack wich exports the noqueue qdisc like any other standard one, so it can be attached to a device. Semantics is a bit odd, though: | # tc qd show dev eth0 | qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 | # tc qd add dev eth0 root pfifo | # tc qd show dev eth0 | qdisc pfifo 8003: root refcnt 2 limit 1000p | # tc qd del dev eth0 root | # tc qd show dev eth0 | qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 | # tc qd add dev eth0 root noqueue | # tc qd show dev eth0 | # tc qd del dev eth0 root | RTNETLINK answers: No such file or directory So whenn I add the noqueue qdisc, it actually leads to no qdisc attached, and none can be removed afterwards. To recover from this situation, I have to temporarily attach a different qdisc: | # tc qd add dev eth0 root pfifo | # tc qd show dev eth0 | qdisc pfifo 8005: root refcnt 2 limit 1000p | # tc qd del dev eth0 root | # tc qd show dev eth0 | qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 This was the quick'n'dirty approach, and I am not satisfied with the outcome. Instead, I would suggest to have the following: - noqueue is really the no queue like it says, so 'tc qd del root' attaches noqueue, no matter if virtual or physical interface - physical interfaces get pfifo_fast by default, which can be removed to get noqueue This has a few implications: - the default qdisc (pfifo_fast or noqueue) becomes the initial qdisc - pfifo_fast needs to be exported, so users can go back to the initial qdisc of physical devices after having removed it I'll start implementing the above immediately, but would appreciate to hear your comments on it meanwhile. I wonder especially what makes the difference between pfifo and pfifo_fast and why the latter can't be selected explicitly by a user yet. Are there any good reasons for it aside from it being the default and therefore selecting it can be done by having tx_queue_len 0 and removing the root qdisc? Thanks, Phil -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/21] net: warn if drivers set tx_queue_len = 0
On Wed, Aug 19, 2015 at 08:39:24PM +0200, Phil Sutter wrote: [...] I'll start implementing the above immediately, but would appreciate to hear your comments on it meanwhile. I wonder especially what makes the difference between pfifo and pfifo_fast and why the latter can't be selected explicitly by a user yet. Are there any good reasons for it aside from it being the default and therefore selecting it can be done by having tx_queue_len 0 and removing the root qdisc? Please ignore most of my questions regarding pfifo_fast here. I based my assumptions upon 'tc' utility of RHEL7, which is buggy in that it does not allow to add qdiscs which do not support extra options. Using mainline tc, pfifo_fast can be attached just like any other qdisc. Sorry for the noise, Phil -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/21] net: warn if drivers set tx_queue_len = 0
On Wed, 2015-08-19 at 22:21 +0200, Phil Sutter wrote: On Wed, Aug 19, 2015 at 08:39:24PM +0200, Phil Sutter wrote: [...] I'll start implementing the above immediately, but would appreciate to hear your comments on it meanwhile. I wonder especially what makes the difference between pfifo and pfifo_fast and why the latter can't be selected explicitly by a user yet. Are there any good reasons for it aside from it being the default and therefore selecting it can be done by having tx_queue_len 0 and removing the root qdisc? Please ignore most of my questions regarding pfifo_fast here. I based my assumptions upon 'tc' utility of RHEL7, which is buggy in that it does not allow to add qdiscs which do not support extra options. Using mainline tc, pfifo_fast can be attached just like any other qdisc. Sorry for the noise, Yes, I was about to send you : lpaa5:~# tc qd sh dev eth1 qdisc mq 0: root qdisc fq 0: parent :4 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :3 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :2 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :1 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 lpaa5:~# tc qd replace dev eth1 root pfifo_fast lpaa5:~# tc -s -d qd sh dev eth1 qdisc pfifo_fast 8003: root refcnt 45 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 Sent 2662 bytes 22 pkt (dropped 0, overlimits 0 requeues 0) backlog 0b 0p requeues 0 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/21] net: warn if drivers set tx_queue_len = 0
On Wed, Aug 19, 2015 at 01:33:40PM -0700, Eric Dumazet wrote: On Wed, 2015-08-19 at 13:31 -0700, Eric Dumazet wrote: lpaa5:~# tc qd sh dev eth1 qdisc mq 0: root qdisc fq 0: parent :4 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :3 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :2 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 qdisc fq 0: parent :1 limit 1p flow_limit 1000p buckets 1024 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3028 initial_quantum 15140 Well, it seems I just leaked fact that we use 3-bands in our fq implementation ;) The problem with subtle details is they cease to be if pointed out. In another episode I was about to ask who the ominous we are, but then I noticed your mail's message ID. :) Cheers, Phil -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 20/21] net: warn if drivers set tx_queue_len = 0
Due to the introduction of IFF_NO_QUEUE, there is a better way for drivers to indicate that no qdisc should be attached by default. Though, the old convention can't be dropped since ignoring that setting would break drivers still using it. Instead, add a warning so out-of-tree driver maintainers get a chance to adjust their code before we finally get rid of any special handling of tx_queue_len == 0. Signed-off-by: Phil Sutter p...@nwl.cc --- net/core/dev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/core/dev.c b/net/core/dev.c index 4870c35..b1f3f48 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6997,6 +6997,9 @@ 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) + printk(KERN_WARNING %s uses DEPRECATED zero tx_queue_len - convert driver to use IFF_NO_QUEUE instead.\n, name); + dev-num_tx_queues = txqs; dev-real_num_tx_queues = txqs; if (netif_alloc_netdev_queues(dev)) -- 2.1.2 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/21] net: warn if drivers set tx_queue_len = 0
On Tue, 2015-08-18 at 10:30 +0200, Phil Sutter wrote: Due to the introduction of IFF_NO_QUEUE, there is a better way for drivers to indicate that no qdisc should be attached by default. Though, the old convention can't be dropped since ignoring that setting would break drivers still using it. Instead, add a warning so out-of-tree driver maintainers get a chance to adjust their code before we finally get rid of any special handling of tx_queue_len == 0. How an admin can remove qdisc on a regular ethernet device with this schem ? I was doing : tc qdisc replace dev eth0 root pfifo ifconfig eth0 txqueuelen 0 tc qdisc del dev eth0 root I see nothing in your patches to avoid the DEPRECATED warning. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
