[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6bb42e9b-83ff-497d-8052-27ac609a2af7@linux.dev>
Date: Tue, 14 Jan 2025 15:20:40 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, dsahern@...nel.org,
willemdebruijn.kernel@...il.com, willemb@...gle.com, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, eddyz87@...il.com, song@...nel.org,
yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org,
sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, horms@...nel.org
Subject: Re: [PATCH net-next v5 01/15] net-timestamp: add support for
bpf_setsockopt()
On 1/12/25 3:37 AM, Jason Xing wrote:
> Users can write the following code to enable the bpf extension:
> bpf_setsockopt(skops, SOL_SOCKET, SK_BPF_CB_FLAGS, &flags, sizeof(flags));
>
> Signed-off-by: Jason Xing <kerneljasonxing@...il.com>
> ---
> include/net/sock.h | 7 +++++++
> include/uapi/linux/bpf.h | 8 ++++++++
> net/core/filter.c | 25 +++++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 1 +
> 4 files changed, 41 insertions(+)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index ccf86c8a7a8a..f5447b4b78fd 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -303,6 +303,7 @@ struct sk_filter;
> * @sk_stamp: time stamp of last packet received
> * @sk_stamp_seq: lock for accessing sk_stamp on 32 bit architectures only
> * @sk_tsflags: SO_TIMESTAMPING flags
> + * @sk_bpf_cb_flags: used for bpf_setsockopt
> * @sk_use_task_frag: allow sk_page_frag() to use current->task_frag.
> * Sockets that can be used under memory reclaim should
> * set this to false.
> @@ -445,6 +446,12 @@ struct sock {
> u32 sk_reserved_mem;
> int sk_forward_alloc;
> u32 sk_tsflags;
> +#ifdef CONFIG_BPF_SYSCALL
The CONFIG_BPF is used instead in the existing "u8 bpf_sock_ops_cb_flags;" in
tcp_sock. afaik, CONFIG_BPF is selected by CONFIG_NET. It is why the test bot
fails when CONFIG_BPF_SYSCALL is used here but not with the existing
bpf_sock_ops_cb_flags. Considering CONFIG_BPF is also mostly useless here
because of CONFIG_NET, I would remove this ifdef usage altogether. If there is
really a need to distinguish CONFIG_BPF_SYSCALL is enabled or not, this can be
improved together with the existing bpf_sock_ops_cb_flags.
> +#define SK_BPF_CB_FLAG_TEST(SK, FLAG) ((SK)->sk_bpf_cb_flags & (FLAG))
> + u32 sk_bpf_cb_flags;
> +#else
> +#define SK_BPF_CB_FLAG_TEST(SK, FLAG) 0
> +#endif
> __cacheline_group_end(sock_write_rxtx);
>
> __cacheline_group_begin(sock_write_tx);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4162afc6b5d0..e629e09b0b31 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -6903,6 +6903,13 @@ enum {
> BPF_SOCK_OPS_ALL_CB_FLAGS = 0x7F,
> };
>
> +/* Definitions for bpf_sk_cb_flags */
> +enum {
> + SK_BPF_CB_TX_TIMESTAMPING = 1<<0,
> + SK_BPF_CB_MASK = (SK_BPF_CB_TX_TIMESTAMPING - 1) |
> + SK_BPF_CB_TX_TIMESTAMPING
> +};
> +
> /* List of known BPF sock_ops operators.
> * New entries can only be added at the end
> */
> @@ -7081,6 +7088,7 @@ enum {
> TCP_BPF_SYN_IP = 1006, /* Copy the IP[46] and TCP header */
> TCP_BPF_SYN_MAC = 1007, /* Copy the MAC, IP[46], and TCP header */
> TCP_BPF_SOCK_OPS_CB_FLAGS = 1008, /* Get or Set TCP sock ops flags */
> + SK_BPF_CB_FLAGS = 1009, /* Used to set socket bpf flags */
> };
>
> enum {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b957cf57299e..c6dd2d2e44c8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5222,6 +5222,23 @@ static const struct bpf_func_proto bpf_get_socket_uid_proto = {
> .arg1_type = ARG_PTR_TO_CTX,
> };
>
> +static int sk_bpf_set_cb_flags(struct sock *sk, char *optval, bool getopt)
> +{
> + u32 sk_bpf_cb_flags;
> +
> + if (getopt)
I may have this in my earlier sample code? This is probably because of my
laziness for a quick example. getopt should also be supported, similar to the
existing TCP_BPF_SOCK_OPS_CB_FLAGS.
> + return -EINVAL;
> +
> + sk_bpf_cb_flags = *(u32 *)optval;
> +
> + if (sk_bpf_cb_flags & ~SK_BPF_CB_MASK)
> + return -EINVAL;
> +
> + sk->sk_bpf_cb_flags = sk_bpf_cb_flags;
> +
> + return 0;
> +}
> +
> static int sol_socket_sockopt(struct sock *sk, int optname,
> char *optval, int *optlen,
> bool getopt)
> @@ -5238,6 +5255,7 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> case SO_MAX_PACING_RATE:
> case SO_BINDTOIFINDEX:
> case SO_TXREHASH:
> + case SK_BPF_CB_FLAGS:
> if (*optlen != sizeof(int))
> return -EINVAL;
> break;
> @@ -5247,6 +5265,13 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
> return -EINVAL;
> }
>
> + if (optname == SK_BPF_CB_FLAGS)
> +#ifdef CONFIG_BPF_SYSCALL
> + return sk_bpf_set_cb_flags(sk, optval, getopt);
> +#else
> + return -EINVAL;
> +#endif
> +
> if (getopt) {
> if (optname == SO_BINDTODEVICE)
> return -EINVAL;
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 4162afc6b5d0..6b0a5b787b12 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -7081,6 +7081,7 @@ enum {
> TCP_BPF_SYN_IP = 1006, /* Copy the IP[46] and TCP header */
> TCP_BPF_SYN_MAC = 1007, /* Copy the MAC, IP[46], and TCP header */
> TCP_BPF_SOCK_OPS_CB_FLAGS = 1008, /* Get or Set TCP sock ops flags */
> + SK_BPF_CB_FLAGS = 1009, /* Used to set socket bpf flags */
> };
>
> enum {
Powered by blists - more mailing lists