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]
Date:   Mon, 24 Jan 2022 15:46:33 +0000
From:   Pavel Begunkov <>
To:     Stanislav Fomichev <>,
        Martin KaFai Lau <>
        Alexei Starovoitov <>,
        Daniel Borkmann <>,
        Andrii Nakryiko <>,
        Song Liu <>,
Subject: Re: [PATCH v3] cgroup/bpf: fast path skb BPF filtering

On 12/16/21 18:24, Stanislav Fomichev wrote:
> On Thu, Dec 16, 2021 at 10:14 AM Martin KaFai Lau <> wrote:
>> On Thu, Dec 16, 2021 at 01:21:26PM +0000, Pavel Begunkov wrote:
>>> On 12/15/21 22:07, Stanislav Fomichev wrote:
>>>>> I'm skeptical I'll be able to measure inlining one function,
>>>>> variability between boots/runs is usually greater and would hide it.
>>>> Right, that's why I suggested to mirror what we do in set/getsockopt
>>>> instead of the new extra CGROUP_BPF_TYPE_ENABLED. But I'll leave it up
>>>> to you, Martin and the rest.
>> I also suggested to try to stay with one way for fullsock context in v2
>> but it is for code readability reason.
>> How about calling CGROUP_BPF_TYPE_ENABLED() just next to cgroup_bpf_enabled()
>> in BPF_CGROUP_RUN_PROG_*SOCKOPT_*() instead ?
> SG!
>> It is because both cgroup_bpf_enabled() and CGROUP_BPF_TYPE_ENABLED()
>> want to check if there is bpf to run before proceeding everything else
>> and then I don't need to jump to the non-inline function itself to see
>> if there is other prog array empty check.
>> Stan, do you have concern on an extra inlined sock_cgroup_ptr()
>> when there is bpf prog to run for set/getsockopt()?  I think
>> it should be mostly noise from looking at
>> __cgroup_bpf_run_filter_*sockopt()?
> Yeah, my concern is also mostly about readability/consistency. Either
> __cgroup_bpf_prog_array_is_empty everywhere or this new
> CGROUP_BPF_TYPE_ENABLED everywhere. I'm slightly leaning towards
> __cgroup_bpf_prog_array_is_empty because I don't believe direct
> function calls add any visible overhead and macros are ugly :-) But
> either way is fine as long as it looks consistent.

Martin, Stanislav, do you think it's good to go? Any other concerns?
It feels it might end with bikeshedding and would be great to finally
get it done, especially since I find the issue to be pretty simple.

Pavel Begunkov

Powered by blists - more mailing lists