netdev - Weird behavior (bug?) with mq 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-next>] [day] [month] [year] [list]

Date:   Wed, 8 Apr 2020 14:06:27 +0000
From:   Maxim Mikityanskiy <maximmi@...lanox.com>
To:     Eric Dumazet <edumazet@...gle.com>,
        "David S. Miller" <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Yossi Kuperman <yossiku@...lanox.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: Weird behavior (bug?) with mq qdisc

Hi,

Commit 95dc19299f74 ("pkt_sched: give visibility to mq slave qdiscs") by
Eric exposes mq's child qdiscs. It uses real_num_tx_queues to limit the
amount of per-queue qdiscs exposed, but it only queries that value on
attach. However, this value may be changed afterwards by ethtool -L, but
tc qdisc show will continue to show the old amount of per-queue qdiscs:

1. 8 channels, `tc qdisc show dev eth1` shows mq and 8 pfifo_fast
qdiscs.
2. Run `ethtool -L eth1 combined 4`.
3. `tc qdisc show dev eth1` shows the same.
4. Run `tc qdisc replace dev eth1 root mq`.
5. `tc qdisc show dev eth1` shows mq and 4 pfifo_fast qdiscs.
6. Run `ethtool -L eth1 combined 8`.
7. `tc qdisc show dev eth1` still shows mq and 4 pfifo_fast qdiscs.

As I understand, the purpose of the aforementioned commit is to expose
stats along with the per-queue qdiscs, and after some trivial
configuration changes we end up without stats on half of queues.

Moreover, it can be continued:

8. Run `tc qdisc replace dev eth1 parent 8001:1 pfifo`.
9. Run `tc qdisc del dev eth1 parent 8001:1`.
10. Now the qdisc for queue 0 is deleted completely.

Such behavior looks like a bug to me. When I delete the root qdisc, it
gets replaced by a sane default. I would expect that when I delete a
qdisc of a netdev queue, it would be restored to the default too.

Now, if we look at both issues at the same time, in the first case
qdiscs were missing from tc qdisc show output, but they were actually
there, and in the second case they are deleted for real, but these cases
can't be distinguished by tc qdisc show.

I would like to hear more opinions on these two issues (1. qdiscs are
not shown when the number of queues grows, 2. tc qdisc del for a queue
reverts to noop, rather than to some sane default). Any ideas about
fixing them, especially issue 1? Some kind of notification mechanism
from netif_set_real_num_tx_queues to mq or even complete reattachment of
mq when the number of queues change...

Thanks,
Max