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 <j...@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
