netdev - RE: [net v5 2/3] net: sched: add check tc_skip

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: <61b383c6373ca_1f50e20816@john.notmuch>
Date:   Fri, 10 Dec 2021 08:43:50 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     xiangxia.m.yue@...il.com, netdev@...r.kernel.org
Cc:     Tonghao Zhang <xiangxia.m.yue@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Antoine Tenart <atenart@...nel.org>,
        Alexander Lobakin <alexandr.lobakin@...el.com>,
        Wei Wang <weiwan@...gle.com>, Arnd Bergmann <arnd@...db.de>
Subject: RE: [net v5 2/3] net: sched: add check tc_skip_classify in sch egress

xiangxia.m.yue@ wrote:
> From: Tonghao Zhang <xiangxia.m.yue@...il.com>
> 
> Try to resolve the issues as below:
> * We look up and then check tc_skip_classify flag in net
>   sched layer, even though skb don't want to be classified.
>   That case may consume a lot of cpu cycles. This patch
>   is useful when there are a lot of filters with different
>   prio. There is ~5 prio in in production, ~1% improvement.
> 
>   Rules as below:
>   $ for id in $(seq 1 5); do
>   $       tc filter add ... egress prio $id ... action mirred egress redirect dev ifb0
>   $ done
> 
> * bpf_redirect may be invoked in egress path. If we don't
>   check the flags and then return immediately, the packets
>   will loopback.

This would be the naive case right? Meaning the BPF program is
doing a redirect without any logic or is buggy?

Can you map out how this happens for me, I'm not fully sure I
understand the exact concern. Is it possible for BPF programs
that used to see packets no longer see the packet as expected?

Is this the path you are talking about?

 rx ethx  ->
   execute BPF program on ethx with bpf_redirect(ifb0) ->
     __skb_dequeue @ifb tc_skip_classify = 1 ->
       dev_queue_xmit() -> 
          sch_handle_egress() ->
            execute BPF program again

I can't see why you want to skip that second tc BPF program,
or for that matter any tc filter there. In general how do you
know that is the correct/expected behavior? Before the above
change it would have been called, what if its doing useful
work.

Also its not clear how your ifb setup is built or used. That
might help understand your use case. I would just remove the
IFB altogether and the above discussion is mute.

Thanks,
John