linux-kernel - Re: [PATCH bpf v2] bpf: Update bpf

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] [day] [month] [year] [list]
Message-ID: <DU0P192MB15474DA29DF17452DC3E95A9D6109@DU0P192MB1547.EURP192.PROD.OUTLOOK.COM>
Date:   Sun, 27 Nov 2022 11:27:12 +0800
From:   Ji Rongfeng <SikoJobs@...look.com>
To:     Martin KaFai Lau <martin.lau@...ux.dev>,
        Daniel Borkmann <daniel@...earbox.net>
Cc:     ast@...nel.org, andrii@...nel.org, song@...nel.org, yhs@...com,
        john.fastabend@...il.com, kpsingh@...nel.org, sdf@...gle.com,
        haoluo@...gle.com, jolsa@...nel.org, joannelkoong@...il.com,
        kuifeng@...com, lorenzo@...nel.org, maximmi@...dia.com,
        quentin@...valent.com, bpf@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf v2] bpf: Update bpf_{g,s}etsockopt() documentation

On 2022/11/24 8:40, Martin KaFai Lau wrote:
> On 11/23/22 4:18 PM, Daniel Borkmann wrote:
>> On 11/18/22 9:18 AM, Ji Rongfeng wrote:
>>> * append missing optnames to the end
>>> * simplify bpf_getsockopt()'s doc
>>>
>>> Signed-off-by: Ji Rongfeng <SikoJobs@...look.com>
>>> ---
>>>   include/uapi/linux/bpf.h       | 20 ++++++++++++--------
>>>   tools/include/uapi/linux/bpf.h | 20 ++++++++++++--------
>>>   2 files changed, 24 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>>> index 51b9aa640ad2..14f29d95ea71 100644
>>> --- a/include/uapi/linux/bpf.h
>>> +++ b/include/uapi/linux/bpf.h
>>> @@ -2576,14 +2576,19 @@ union bpf_attr {
>>>    *         * **SOL_SOCKET**, which supports the following 
>>> *optname*\ s:
>>>    *           **SO_RCVBUF**, **SO_SNDBUF**, **SO_MAX_PACING_RATE**,
>>>    *           **SO_PRIORITY**, **SO_RCVLOWAT**, **SO_MARK**,
>>> - *           **SO_BINDTODEVICE**, **SO_KEEPALIVE**.
>>> + *           **SO_BINDTODEVICE**, **SO_KEEPALIVE**, **SO_REUSEADDR**,
>>> + *           **SO_REUSEPORT**, **SO_BINDTOIFINDEX**, **SO_TXREHASH**.
>>>    *         * **IPPROTO_TCP**, which supports the following 
>>> *optname*\ s:
>>>    *           **TCP_CONGESTION**, **TCP_BPF_IW**,
>>>    *           **TCP_BPF_SNDCWND_CLAMP**, **TCP_SAVE_SYN**,
>>>    *           **TCP_KEEPIDLE**, **TCP_KEEPINTVL**, **TCP_KEEPCNT**,
>>> - *          **TCP_SYNCNT**, **TCP_USER_TIMEOUT**, 
>>> **TCP_NOTSENT_LOWAT**.
>>> + *           **TCP_SYNCNT**, **TCP_USER_TIMEOUT**, 
>>> **TCP_NOTSENT_LOWAT**,
>>> + *           **TCP_NODELAY**, **TCP_MAXSEG**, **TCP_WINDOW_CLAMP**,
>>> + *           **TCP_THIN_LINEAR_TIMEOUTS**, **TCP_BPF_DELACK_MAX**,
>>> + *           **TCP_BPF_RTO_MIN**.
>>>    *         * **IPPROTO_IP**, which supports *optname* **IP_TOS**.
>>> - *         * **IPPROTO_IPV6**, which supports *optname* 
>>> **IPV6_TCLASS**.
>>> + *         * **IPPROTO_IPV6**, which supports the following 
>>> *optname*\ s:
>>> + *           **IPV6_TCLASS**, **IPV6_AUTOFLOWLABEL**.
>>>    *     Return
>>>    *         0 on success, or a negative error in case of failure.
>>>    *
>>> @@ -2800,12 +2805,11 @@ union bpf_attr {
>>>    *           and **BPF_CGROUP_INET6_CONNECT**.
>>>    *
>>>    *         This helper actually implements a subset of 
>>> **getsockopt()**.
>>> - *         It supports the following *level*\ s:
>>> + *         It supports the same set of *optname*\ s that supported by
>>
>> nit: that is supported by
>>
>>> + *         **bpf_setsockopt**\ () helper with a few exceptions:
>>>    *
>>> - *         * **IPPROTO_TCP**, which supports *optname*
>>> - *           **TCP_CONGESTION**.
>>> - *         * **IPPROTO_IP**, which supports *optname* **IP_TOS**.
>>> - *         * **IPPROTO_IPV6**, which supports *optname* 
>>> **IPV6_TCLASS**.
>>> + *         * **bpf_setsockopt**\ () helper only: **TCP_BPF_***.
>>> + *         * **bpf_getsockopt**\ () helper only: **TCP_SAVED_SYNC**.
>>
>> I think from a user PoV the above is a bit hard to follow, maybe take 
>> Martin's
>> earlier feedback into account and add a proper sentence; it will be 
>> much easier
>> to understand.
>
> +1  Made the change and also fixed TCP_SAVED_SYNC with s/SYNC/SYN/ 
> while applying.  Thanks!

Thanks for the helpful reviews. I chose the form of lists was because we 
could
append more optnames easily in the future. But I believe it's not late 
to apply
that form when we really need it : )

In my opinion, this patch doesn't contain any new feature, but just a 
kind of fix
to the documentation, according to the corresponding code in the bpf tree,
which hasn't been modified yet in the bpf-next tree. So I targeted the 
former,
as this patch could be useful there. Please let me know if there's any 
customary
rules outside bpf_devel_QA. Thanks!