[Patch net] net_sched: fix tcm_parent in tc filter dump

[Patch net] net_sched: fix tcm_parent in tc filter dump

[Cong Wang]

When we tell kernel to dump filters from root (ffff:ffff),
those filters on ingress (ffff:0000) are matched, but their
true parents must be dumped as they are. However, kernel
dumps just whatever we tell it, that is either ffff:ffff
or ffff:0000:

 $ nl-cls-list --dev=dummy0 --parent=root
 cls basic dev dummy0 id none parent root prio 49152 protocol ip match-all
 cls basic dev dummy0 id :1 parent root prio 49152 protocol ip match-all
 $ nl-cls-list --dev=dummy0 --parent=ffff:
 cls basic dev dummy0 id none parent ffff: prio 49152 protocol ip match-all
 cls basic dev dummy0 id :1 parent ffff: prio 49152 protocol ip match-all

This is confusing and misleading, more importantly this is
a regression since 4.15, so the old behavior must be restored.

Steps to reproduce this:
 ip li set dev dummy0 up
 tc qd add dev dummy0 ingress
 tc filter add dev dummy0 parent ffff: protocol arp basic action pass
 tc filter show dev dummy0 root

Before this patch:
 filter protocol arp pref 49152 basic
 filter protocol arp pref 49152 basic handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 1 ref 1 bind 1

After this patch:
 filter parent ffff: protocol arp pref 49152 basic
 filter parent ffff: protocol arp pref 49152 basic handle 0x1
 	action order 1: gact action pass
 	 random type none pass val 0
	 index 1 ref 1 bind 1

Fixes: a10fa20101ae ("net: sched: propagate q and parent from caller down to tcf_fill_node")
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/cls_api.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 55bd1429678f..80e93c96d2b2 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2612,12 +2612,10 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 			return skb->len;
 
 		parent = tcm->tcm_parent;
-		if (!parent) {
+		if (!parent)
 			q = dev->qdisc;
-			parent = q->handle;
-		} else {
+		else
 			q = qdisc_lookup(dev, TC_H_MAJ(tcm->tcm_parent));
-		}
 		if (!q)
 			goto out;
 		cops = q->ops->cl_ops;
@@ -2633,6 +2631,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 		block = cops->tcf_block(q, cl, NULL);
 		if (!block)
 			goto out;
+		parent = block->q->handle;
 		if (tcf_block_shared(block))
 			q = NULL;
 	}
-- 
2.26.1

Re: [Patch net] net_sched: fix tcm_parent in tc filter dump

[Cong Wang]

On Thu, Apr 30, 2020 at 6:02 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 55bd1429678f..80e93c96d2b2 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -2612,12 +2612,10 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
>                         return skb->len;
>
>                 parent = tcm->tcm_parent;
> -               if (!parent) {
> +               if (!parent)
>                         q = dev->qdisc;
> -                       parent = q->handle;
> -               } else {
> +               else
>                         q = qdisc_lookup(dev, TC_H_MAJ(tcm->tcm_parent));
> -               }
>                 if (!q)
>                         goto out;
>                 cops = q->ops->cl_ops;
> @@ -2633,6 +2631,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
>                 block = cops->tcf_block(q, cl, NULL);
>                 if (!block)
>                         goto out;
> +               parent = block->q->handle;

Hmm, block->q could be NULL, I think I should just use q->handle
here.