lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 3 May 2020 08:48:18 -0400 From: Jamal Hadi Salim <jhs@...atatu.com> To: Cong Wang <xiyou.wangcong@...il.com> Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>, Jiri Pirko <jiri@...nulli.us> Subject: Re: [Patch net v2] net_sched: fix tcm_parent in tc filter dump 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@...atatu.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
Powered by blists - more mailing lists