netdev - Re: [PATCH net] sched: consistently handle layer3 header accesses in the presence of VLANs

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4297936b4cc7d6cdcb51ccc10331467f39978795.camel@redhat.com>
Date:   Fri, 03 Jul 2020 14:53:50 +0200
From:   Davide Caratti <dcaratti@...hat.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        davem@...emloft.net
Cc:     netdev@...r.kernel.org, cake@...ts.bufferbloat.net,
        Jiri Pirko <jiri@...nulli.us>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Ilya Ponetayev <i.ponetaev@...systems.com>,
        Cong Wang <xiyou.wangcong@...il.com>
Subject: Re: [PATCH net] sched: consistently handle layer3 header accesses
 in the presence of VLANs

hello Toke,

thanks for answering!

On Fri, 2020-07-03 at 14:05 +0200, Toke Høiland-Jørgensen wrote:
>   while (proto == htons(ETH_P_8021Q) || proto == htons(ETH_P_8021AD)) {

maybe this line be shortened, since if_vlan.h has [1]:

while (eth_type_vlan(proto)) {
 	...
}

If I read well, the biggest change from functional point of view is that
now qdiscs can set the ECN bit also on non-accelerated VLAN packets and
QinQ-tagged packets, if the IP header is the outer-most header after VLAN;
and the same applies to almost all net/sched former users of skb->protocol 
or tc_skb_protocol().

Question (sorry in advance because it might be a dumb one :) ):

do you know why cls_flower, act_ct, act_mpls and act_connmark keep reading
skb->protocol? is that intentional?

(for act_mpls that doesn't look intentional, and probably the result is
that the BOS bit is not set correctly if someone tries to push/pop a label
for a non-accelerated or QinQ packet. But I didn't try it experimentally
:) )

[1] https://elixir.bootlin.com/linux/latest/source/include/linux/if_vlan.h#L300

-- 
davide