Skip to content

Commit ea32746

Browse files
Jiri Kosinadavem330
authored andcommitted
net: sched: avoid duplicates in qdisc dump
tc_dump_qdisc() performs dumping of the per-device qdiscs in two phases; first, the "standard" dev->qdisc is being dumped. Second, if there is/are ingress queue(s), they are being dumped as well. After conversion of netdevice's qdisc linked-list into hashtable, these two sets are not in two disjunctive sets/lists any more, but are both "reachable" directly from netdevice's hashtable. As a consequence, the "full-depth" dump of the ingress qdiscs results in immediately hitting the netdevice hashtable again, and duplicating the dump that has already been performed for dev->qdisc. What in fact needs to be dumped in case of ingress queue is "just" the top-level ingress qdisc, as everything else has been dumped already. Fix this by extending tc_dump_qdisc_root() in a way that it can be instructed whether it should (while performing the "full" per-netdev qdisc dump) perform the whole recursion, or just dump "additional" top-level (ingress) qdiscs without performing any kind of recursion. This fixes duplicate dumps such as qdisc mq 0: root qdisc pfifo_fast 0: parent :4 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 qdisc pfifo_fast 0: parent :3 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 qdisc pfifo_fast 0: parent :2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 qdisc pfifo_fast 0: parent :1 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 qdisc clsact ffff: parent ffff:fff1 qdisc pfifo_fast 0: parent :4 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 qdisc pfifo_fast 0: parent :3 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 qdisc pfifo_fast 0: parent :2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 qdisc pfifo_fast 0: parent :1 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 Fixes: 59cc1f6 ("net: sched: convert qdisc linked list to hashtable") Reported-by: Daniel Borkmann <daniel@iogearbox.net> Tested-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Jiri Kosina <jkosina@suse.cz> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 69012ae commit ea32746

File tree

1 file changed

+10
-4
lines changed

1 file changed

+10
-4
lines changed

net/sched/sch_api.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,7 +1435,7 @@ static int qdisc_notify(struct net *net, struct sk_buff *oskb,
14351435

14361436
static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
14371437
struct netlink_callback *cb,
1438-
int *q_idx_p, int s_q_idx)
1438+
int *q_idx_p, int s_q_idx, bool recur)
14391439
{
14401440
int ret = 0, q_idx = *q_idx_p;
14411441
struct Qdisc *q;
@@ -1455,7 +1455,13 @@ static int tc_dump_qdisc_root(struct Qdisc *root, struct sk_buff *skb,
14551455
q_idx++;
14561456
}
14571457

1458-
if (!qdisc_dev(root))
1458+
/* If dumping singletons, there is no qdisc_dev(root) and the singleton
1459+
* itself has already been dumped.
1460+
*
1461+
* If we've already dumped the top-level (ingress) qdisc above and the global
1462+
* qdisc hashtable, we don't want to hit it again
1463+
*/
1464+
if (!qdisc_dev(root) || !recur)
14591465
goto out;
14601466

14611467
hash_for_each(qdisc_dev(root)->qdisc_hash, b, q, hash) {
@@ -1499,13 +1505,13 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
14991505
s_q_idx = 0;
15001506
q_idx = 0;
15011507

1502-
if (tc_dump_qdisc_root(dev->qdisc, skb, cb, &q_idx, s_q_idx) < 0)
1508+
if (tc_dump_qdisc_root(dev->qdisc, skb, cb, &q_idx, s_q_idx, true) < 0)
15031509
goto done;
15041510

15051511
dev_queue = dev_ingress_queue(dev);
15061512
if (dev_queue &&
15071513
tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb,
1508-
&q_idx, s_q_idx) < 0)
1514+
&q_idx, s_q_idx, false) < 0)
15091515
goto done;
15101516

15111517
cont:

0 commit comments

Comments
 (0)