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

[Patch net v2] 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.

And, when tc filters are installed on a tc class, the parent
should be the classid, rather than the qdisc handle. Commit
edf6711c9840 ("net: sched: remove classid and q fields from tcf_proto")
removed the classid we save for filters, we can just restore
this classid in tcf_block.

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")
Fixes: edf6711c9840 ("net: sched: remove classid and q fields from tcf_proto")
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/sch_generic.h | 1 +
 net/sched/cls_api.c       | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 25d2ec4c8f00..8428aa614265 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -407,6 +407,7 @@ struct tcf_block {
 	struct mutex lock;
 	struct list_head chain_list;
 	u32 index; /* block index for shared blocks */
+	u32 classid; /* which class this block belongs to */
 	refcount_t refcnt;
 	struct net *net;
 	struct Qdisc *q;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 55bd1429678f..c0e5b64b3caf 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2070,6 +2070,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 		err = PTR_ERR(block);
 		goto errout;
 	}
+	block->classid = parent;
 
 	chain_index = tca[TCA_CHAIN] ? nla_get_u32(tca[TCA_CHAIN]) : 0;
 	if (chain_index > TC_ACT_EXT_VAL_MASK) {
@@ -2612,12 +2613,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 +2632,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->classid;
 		if (tcf_block_shared(block))
 			q = NULL;
 	}
-- 
2.26.1

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

[Jamal Hadi Salim]

On 2020-04-30 11:53 p.m., Cong Wang wrote:
> 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.
> 
> And, when tc filters are installed on a tc class, the parent
> should be the classid, rather than the qdisc handle. Commit
> edf6711c9840 ("net: sched: remove classid and q fields from tcf_proto")
> removed the classid we save for filters, we can just restore
> this classid in tcf_block.
> 
> 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

Note:
tc filter show dev dummy0 root
should not show that filter. OTOH,
tc filter show dev dummy0 parent ffff:
should.

root and ffff: are distinct/unique installation hooks.

cheers,
jamal

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

[Jamal Hadi Salim]

On 2020-05-02 4:48 a.m., Jamal Hadi Salim wrote:
> On 2020-04-30 11:53 p.m., Cong Wang wrote:

[..]
>> 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
> 
> Note:
> tc filter show dev dummy0 root
> should not show that filter. OTOH,
> tc filter show dev dummy0 parent ffff:
> should.
> 
> root and ffff: are distinct/unique installation hooks.
> 

Suprised no one raised this earlier - since it is so
fundamental (we should add a tdc test for it). I went back
to the oldest kernel i have from early 2018 and it was broken..

Cong, your patch is good for the case where we
want to show _all_ filters regardless of where they
were installed but only if no parent is specified. i.e if i did this:
tc filter show dev dummy0
then i am asking to see all the filters for that device.
I am actually not sure if "tc filter show dev dummy0"
ever worked that way - but it makes sense since
no dump-filtering is specified.


To illustrate, I did this:
tc filter add dev dummy0 root protocol arp prio 49151 basic action pass

And now the output looks like:
-------
#  tc filter show dev dummy0 ingressfilter protocol arp pref 49151 basic 
chain 0
filter protocol arp pref 49151 basic chain 0 handle 0x2
	action order 1: gact action pass
	 random type none pass val 0
	 index 2 ref 1 bind 1

filter protocol arp pref 49151 basic chain 0 handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 1 ref 1 bind 1

#  tc filter show dev dummy0 root
filter protocol arp pref 49151 basic chain 0
filter protocol arp pref 49151 basic chain 0 handle 0x2
	action order 1: gact action pass
	 random type none pass val 0
	 index 2 ref 1 bind 1

filter protocol arp pref 49151 basic chain 0 handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 1 ref 1 bind 1
------



If, OTOH, i specified the parent
then only that parents filters should be displayed..

cheers,
jamal

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

[Cong Wang]

On Sat, May 2, 2020 at 2:19 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2020-05-02 4:48 a.m., Jamal Hadi Salim wrote:
> > On 2020-04-30 11:53 p.m., Cong Wang wrote:
>
> [..]
> >> 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
> >
> > Note:
> > tc filter show dev dummy0 root
> > should not show that filter. OTOH,
> > tc filter show dev dummy0 parent ffff:
> > should.

Hmm, but we use TC_H_MAJ(tcm->tcm_parent) to do the
lookup, 'root' (ffff:ffff) has the same MAJ with ingress
(ffff:0000).

And qdisc_lookup() started to search for ingress since 2008,
commit 8123b421e8ed944671d7241323ed3198cccb4041.

So it is likely too late to change this behavior even if it is not
what we prefer.


> >
> > root and ffff: are distinct/unique installation hooks.
> >
>
> Suprised no one raised this earlier - since it is so
> fundamental (we should add a tdc test for it). I went back
> to the oldest kernel i have from early 2018 and it was broken..
>
> Cong, your patch is good for the case where we
> want to show _all_ filters regardless of where they
> were installed but only if no parent is specified. i.e if i did this:
> tc filter show dev dummy0
> then i am asking to see all the filters for that device.
> I am actually not sure if "tc filter show dev dummy0"
> ever worked that way - but it makes sense since
> no dump-filtering is specified.

If parent is not specified, only egress will be shown, as
we just assign q = dev->qdisc.

I agree, 'root' should mean the root qdisc on egress, matching
ingress with 'root' doesn't make much sense to me either.

But I am afraid it is too late to change ,if this behavior has been
there for 12+ years.

Thanks.

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

[Jamal Hadi Salim]

On 2020-05-02 10:28 p.m., Cong Wang wrote:
> On Sat, May 2, 2020 at 2:19 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>
>> On 2020-05-02 4:48 a.m., Jamal Hadi Salim wrote:

[..]
>>> Note:
>>> tc filter show dev dummy0 root
>>> should not show that filter. OTOH,
>>> tc filter show dev dummy0 parent ffff:
>>> should.
> 
> Hmm, but we use TC_H_MAJ(tcm->tcm_parent) to do the
> lookup, 'root' (ffff:ffff) has the same MAJ with ingress
> (ffff:0000).
>

I have some long analysis and theory below.

> And qdisc_lookup() started to search for ingress since 2008,
> commit 8123b421e8ed944671d7241323ed3198cccb4041.
> 
> So it is likely too late to change this behavior even if it is not
> what we prefer.
> 

My gut feeling is that whatever broke (likely during block addition
maybe even earlier during clsact addition) is in the code
path for adding filter. Dumping may have bugs but i would
point a finger to filter addition first.
More below.... (sorry long email).


Here's what i tried after applying your patch:

----
# $TC qd add dev $DEV ingress
# $TC qd add dev $DEV root prio
# $TC qd ls dev $DEV
qdisc noqueue 0: dev lo root refcnt 2
qdisc prio 8008: dev enp0s1 root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 
0 1 1 1 1 1 1 1 1
qdisc ingress ffff: dev enp0s1 parent ffff:fff1 ----------------
-----

egress i.e root is at 8008:
ingress is at ffff:fff1

If say:
---
# $TC filter add dev $DEV root protocol arp prio 10 basic action pass
----

i am instructing the kernel to "go and find root (which is 8008:)
and install the filter there".
IOW, I could install that filter alternatively as:
----
# $TC filter add dev $DEV parent 8008: protocol arp prio 11 basic action 
pass
---

Basically these two filters are equivalent and should end in the
same qdisc.

To test, I added those two filters (the prio is useful to visualize
in the dump).

Lets see the dump:

-------
# $TC filter show dev $DEV root
filter protocol arp pref 10 basic chain 0
filter protocol arp pref 10 basic chain 0 handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 1 ref 1 bind 1
---------

I was hoping i'd see both filters, but alas there's only
one.

Lets try a different dump explicitly specifying root qdisc id:
---------
# $TC filter show dev $DEV parent 8008:
filter protocol arp pref 11 basic chain 0
filter protocol arp pref 11 basic chain 0 handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 2 ref 1 bind 1
---------

Again i was hoping to see both filters.

This sounds like the two filters are anchored
at two different qdiscs instead of the same one
(i.e root). Hence my suspicion...

Lets add a filter at ingress:
-----
$TC filter add dev $DEV parent ffff:fff1 protocol arp basic action pass
-------

Ok lets dump this from ingress:

-----
# $TC filter show dev $DEV parent ffff:fff1
filter protocol arp pref 10 basic chain 0
filter protocol arp pref 10 basic chain 0 handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 1 ref 1 bind 1

filter protocol arp pref 49152 basic chain 0
filter protocol arp pref 49152 basic chain 0 handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 3 ref 1 bind 1
-------------

Same result if i said "root".
I was only expecting to see the one with pref 49152 in
the above output.

It _feels_ like those two filters(ingress and egress) are
installed in the same struct.

Ok, last dump without specifying a parent, which should
pick root qdisc per code:
------
# $TC filter show dev $DEV
filter parent 8008: protocol arp pref 11 basic chain 0
filter parent 8008: protocol arp pref 11 basic chain 0 handle 0x1
	action order 1: gact action pass
	 random type none pass val 0
	 index 2 ref 1 bind 1
----

Semantically this should have dumped all 3 filters and
not just pick root. But that issue has been there
for a decade like you said. So it is reasonable to specify
parent ffff:fff1 for dumping just ingress side.
Also reasonable not to see ingress when dumping root.

> If parent is not specified, only egress will be shown, as
> we just assign q = dev->qdisc.
>

Which is the root egress qdisc.
That is the "$TC filter show dev $DEV" scenario.
See my comment above.

> I agree, 'root' should mean the root qdisc on egress, matching
> ingress with 'root' doesn't make much sense to me either.
> 
> But I am afraid it is too late to change ,if this behavior has been
> there for 12+ years.
> 

Although i cant pinpoint exactly when - this used to work (I dont think
its 12+ years but I could be wrong). These semantics are really broken.

Do you have time to look at the theory that things break at install?
If you dont have time i could try to debug it by Tuesday.

cheers,
jamal

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

[Jamal Hadi Salim]

On 2020-05-03 8:02 a.m., Jamal Hadi Salim wrote:
> On 2020-05-02 10:28 p.m., Cong Wang wrote:
>> On Sat, May 2, 2020 at 2:19 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>>
>>> On 2020-05-02 4:48 a.m., Jamal Hadi Salim wrote:
> 
> [..]
>>>> Note:
>>>> tc filter show dev dummy0 root
>>>> should not show that filter. OTOH,
>>>> tc filter show dev dummy0 parent ffff:
>>>> should.
>>
>> Hmm, but we use TC_H_MAJ(tcm->tcm_parent) to do the
>> lookup, 'root' (ffff:ffff) has the same MAJ with ingress
>> (ffff:0000).
>>
> 
> I have some long analysis and theory below.
> 
>> And qdisc_lookup() started to search for ingress since 2008,
>> commit 8123b421e8ed944671d7241323ed3198cccb4041.
>>
>> So it is likely too late to change this behavior even if it is not
>> what we prefer.
>>
> 
> My gut feeling is that whatever broke (likely during block addition
> maybe even earlier during clsact addition) is in the code
> path for adding filter. Dumping may have bugs but i would
> point a finger to filter addition first.
> More below.... (sorry long email).
> 
> 
> Here's what i tried after applying your patch:
> 
> ----
> # $TC qd add dev $DEV ingress
> # $TC qd add dev $DEV root prio
> # $TC qd ls dev $DEV
> qdisc noqueue 0: dev lo root refcnt 2
> qdisc prio 8008: dev enp0s1 root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 
> 0 1 1 1 1 1 1 1 1
> qdisc ingress ffff: dev enp0s1 parent ffff:fff1 ----------------
> -----
> 
> egress i.e root is at 8008:
> ingress is at ffff:fff1
> 
> If say:
> ---
> # $TC filter add dev $DEV root protocol arp prio 10 basic action pass
> ----
> 
> i am instructing the kernel to "go and find root (which is 8008:)
> and install the filter there".

Ok, I went backwards and looked at many kernel sources.
It is true we install the filters in two different locations
i.e just specifying TC_H_ROOT does not equate to picking
the egress qdisc with that flag.
And has been broken for way too long - so we have to live
with it.
I wish we had more tdc tests and earlier.

Advise to users is not to use semantics like "root" or ingress
but rather explicitly specify the parent.

So ignore what i said above. I will ACK your patch.

cheers,
jamal

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

[Jamal Hadi Salim]

On 2020-04-30 11:53 p.m., Cong Wang wrote:
> 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.
> 
> And, when tc filters are installed on a tc class, the parent
> should be the classid, rather than the qdisc handle. Commit
> edf6711c9840 ("net: sched: remove classid and q fields from tcf_proto")
> removed the classid we save for filters, we can just restore
> this classid in tcf_block.
> 
> 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")
> Fixes: edf6711c9840 ("net: sched: remove classid and q fields from tcf_proto")
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>


cheers,
jamal

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

[Cong Wang]

On Sun, May 3, 2020 at 5:48 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2020-05-03 8:02 a.m., Jamal Hadi Salim wrote:
> > On 2020-05-02 10:28 p.m., Cong Wang wrote:
> >> On Sat, May 2, 2020 at 2:19 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >>>
> >>> On 2020-05-02 4:48 a.m., Jamal Hadi Salim wrote:
> >
> > [..]
> >>>> Note:
> >>>> tc filter show dev dummy0 root
> >>>> should not show that filter. OTOH,
> >>>> tc filter show dev dummy0 parent ffff:
> >>>> should.
> >>
> >> Hmm, but we use TC_H_MAJ(tcm->tcm_parent) to do the
> >> lookup, 'root' (ffff:ffff) has the same MAJ with ingress
> >> (ffff:0000).
> >>
> >
> > I have some long analysis and theory below.
> >
> >> And qdisc_lookup() started to search for ingress since 2008,
> >> commit 8123b421e8ed944671d7241323ed3198cccb4041.
> >>
> >> So it is likely too late to change this behavior even if it is not
> >> what we prefer.
> >>
> >
> > My gut feeling is that whatever broke (likely during block addition
> > maybe even earlier during clsact addition) is in the code
> > path for adding filter. Dumping may have bugs but i would
> > point a finger to filter addition first.
> > More below.... (sorry long email).
> >
> >
> > Here's what i tried after applying your patch:
> >
> > ----
> > # $TC qd add dev $DEV ingress
> > # $TC qd add dev $DEV root prio
> > # $TC qd ls dev $DEV
> > qdisc noqueue 0: dev lo root refcnt 2
> > qdisc prio 8008: dev enp0s1 root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0
> > 0 1 1 1 1 1 1 1 1
> > qdisc ingress ffff: dev enp0s1 parent ffff:fff1 ----------------
> > -----
> >
> > egress i.e root is at 8008:
> > ingress is at ffff:fff1
> >
> > If say:
> > ---
> > # $TC filter add dev $DEV root protocol arp prio 10 basic action pass
> > ----
> >
> > i am instructing the kernel to "go and find root (which is 8008:)
> > and install the filter there".
>
> Ok, I went backwards and looked at many kernel sources.
> It is true we install the filters in two different locations
> i.e just specifying TC_H_ROOT does not equate to picking
> the egress qdisc with that flag.
> And has been broken for way too long - so we have to live
> with it.
> I wish we had more tdc tests and earlier.
>
> Advise to users is not to use semantics like "root" or ingress
> but rather explicitly specify the parent.

Yeah, I was confused too when my colleagues reported this
to me.

>
> So ignore what i said above. I will ACK your patch.

Thanks for review!

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

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu, 30 Apr 2020 20:53:49 -0700

> 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.
> 
> And, when tc filters are installed on a tc class, the parent
> should be the classid, rather than the qdisc handle. Commit
> edf6711c9840 ("net: sched: remove classid and q fields from tcf_proto")
> removed the classid we save for filters, we can just restore
> this classid in tcf_block.
> 
> Steps to reproduce this:
 ...
> Fixes: a10fa20101ae ("net: sched: propagate q and parent from caller down to tcf_fill_node")
> Fixes: edf6711c9840 ("net: sched: remove classid and q fields from tcf_proto")
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied, thanks.