lkml.org 
[lkml]   [2022]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH bpf v2] bpf: Update bpf_{g,s}etsockopt() documentation
From
On 2022/11/27 11:27, Ji Rongfeng wrote:
> 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@outlook.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

new features

> 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

if there're

> customary
> rules outside bpf_devel_QA. Thanks!

I just found that SO_BINDTODEVICE is bpf_setsockopt() only. I checked
sock_getbindtodevice() and there's nothing special comparing with
sock_setbindtodevice(), except "down_read(&devnet_rename_sem);" and
"up_read(&devnet_rename_sem);". Martin once wrote:

> The only exception is SO_BINDTODEVICE because it needs to acquire a
> blocking lock. Thus, SO_BINDTODEVICE is not supported.

Were you referring to "down_read(&devnet_rename_sem);"? Seems it's not
acquiring a blocking lock. Maybe "devnet_rename_sem" has been locked for
writing somewhere in bpf before? Please let me know. Thanks!

\
 
 \ /
  Last update: 2022-11-29 15:17    [W:0.074 / U:1.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site