On Sat, May 2, 2020 at 2:19 AM Jamal Hadi Salim <j...@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.
