Weird behavior (bug?) with mq qdisc

Weird behavior (bug?) with mq qdisc

[Maxim Mikityanskiy]

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

Re: Weird behavior (bug?) with mq qdisc

[Cong Wang]

On Wed, Apr 8, 2020 at 8:13 AM Maxim Mikityanskiy <maximmi@mellanox.com> wrote:
> 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...

For 1) we don't update qdisc's when changing tx queues, it should not
be hard to call ->attach() in netif_set_real_num_tx_queues(). But it is
not easy either, because mq_attach() merely attaches default qdisc's,
let's say if you already have 4 non-default qdisc's in your case, you
probably want to just duplicate 4 more when growing to 8. IMHO, this
is not a bug, it is just inconvenient.

For 2), I do not think it is a bug, as you can think noop as a deleted
qdisc.

Thanks.