[net PATCH] net: sched: Fix for duplicate class dump

[net PATCH] net: sched: Fix for duplicate class dump

[Phil Sutter]

When dumping classes by parent, kernel would return classes twice:

| # tc qdisc add dev lo root prio
| # tc class show dev lo
| class prio 8001:1 parent 8001:
| class prio 8001:2 parent 8001:
| class prio 8001:3 parent 8001:
| # tc class show dev lo parent 8001:
| class prio 8001:1 parent 8001:
| class prio 8001:2 parent 8001:
| class prio 8001:3 parent 8001:
| class prio 8001:1 parent 8001:
| class prio 8001:2 parent 8001:
| class prio 8001:3 parent 8001:

This comes from qdisc_match_from_root() potentially returning the root
qdisc itself if its handle matched. Though in that case, root's classes
were already dumped a few lines above.

Fixes: cb395b2010879 ("net: sched: optimize class dumps")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/sched/sch_api.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 6684641ea3448..3dc0acf542454 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -2059,7 +2059,8 @@ static int tc_dump_tclass_root(struct Qdisc *root, struct sk_buff *skb,
 
 	if (tcm->tcm_parent) {
 		q = qdisc_match_from_root(root, TC_H_MAJ(tcm->tcm_parent));
-		if (q && tc_dump_tclass_qdisc(q, skb, tcm, cb, t_p, s_t) < 0)
+		if (q && q != root &&
+		    tc_dump_tclass_qdisc(q, skb, tcm, cb, t_p, s_t) < 0)
 			return -1;
 		return 0;
 	}
-- 
2.19.0

Re: [net PATCH] net: sched: Fix for duplicate class dump

[Jiri Pirko]

Thu, Oct 18, 2018 at 10:34:26AM CEST, phil@nwl.cc wrote:
>When dumping classes by parent, kernel would return classes twice:
>
>| # tc qdisc add dev lo root prio
>| # tc class show dev lo
>| class prio 8001:1 parent 8001:
>| class prio 8001:2 parent 8001:
>| class prio 8001:3 parent 8001:
>| # tc class show dev lo parent 8001:
>| class prio 8001:1 parent 8001:
>| class prio 8001:2 parent 8001:
>| class prio 8001:3 parent 8001:
>| class prio 8001:1 parent 8001:
>| class prio 8001:2 parent 8001:
>| class prio 8001:3 parent 8001:
>
>This comes from qdisc_match_from_root() potentially returning the root
>qdisc itself if its handle matched. Though in that case, root's classes
>were already dumped a few lines above.
>
>Fixes: cb395b2010879 ("net: sched: optimize class dumps")
>Signed-off-by: Phil Sutter <phil@nwl.cc>

Reviewed-by: Jiri Pirko <jiri@mellanox.com>

Re: [net PATCH] net: sched: Fix for duplicate class dump

[Eric Dumazet]

On Thu, Oct 18, 2018 at 6:03 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Thu, Oct 18, 2018 at 10:34:26AM CEST, phil@nwl.cc wrote:
> >When dumping classes by parent, kernel would return classes twice:
> >
> >| # tc qdisc add dev lo root prio
> >| # tc class show dev lo
> >| class prio 8001:1 parent 8001:
> >| class prio 8001:2 parent 8001:
> >| class prio 8001:3 parent 8001:
> >| # tc class show dev lo parent 8001:
> >| class prio 8001:1 parent 8001:
> >| class prio 8001:2 parent 8001:
> >| class prio 8001:3 parent 8001:
> >| class prio 8001:1 parent 8001:
> >| class prio 8001:2 parent 8001:
> >| class prio 8001:3 parent 8001:
> >
> >This comes from qdisc_match_from_root() potentially returning the root
> >qdisc itself if its handle matched. Though in that case, root's classes
> >were already dumped a few lines above.
> >
> >Fixes: cb395b2010879 ("net: sched: optimize class dumps")
> >Signed-off-by: Phil Sutter <phil@nwl.cc>
>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>

Good catch, thanks for the fix !

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [net PATCH] net: sched: Fix for duplicate class dump

[David Miller]

From: Phil Sutter <phil@nwl.cc>
Date: Thu, 18 Oct 2018 10:34:26 +0200

> When dumping classes by parent, kernel would return classes twice:
> 
> | # tc qdisc add dev lo root prio
> | # tc class show dev lo
> | class prio 8001:1 parent 8001:
> | class prio 8001:2 parent 8001:
> | class prio 8001:3 parent 8001:
> | # tc class show dev lo parent 8001:
> | class prio 8001:1 parent 8001:
> | class prio 8001:2 parent 8001:
> | class prio 8001:3 parent 8001:
> | class prio 8001:1 parent 8001:
> | class prio 8001:2 parent 8001:
> | class prio 8001:3 parent 8001:
> 
> This comes from qdisc_match_from_root() potentially returning the root
> qdisc itself if its handle matched. Though in that case, root's classes
> were already dumped a few lines above.
> 
> Fixes: cb395b2010879 ("net: sched: optimize class dumps")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Applied and queued up for -stable, thanks Phil.