Re: [PATCH bpf-next 1/3] bpf/bpf_get,set_sockopt: add option to set TCP-BPF sock ops flags

[Martin KaFai Lau <martin.lau@xxxxxxxxx>]

On 8/2/24 8:29 AM, Alan Maguire wrote:
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 78a6f746ea0b..570ca3f12175 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5278,6 +5278,11 @@ static int bpf_sol_tcp_setsockopt(struct sock *sk, int optname,
>   			return -EINVAL;
>   		inet_csk(sk)->icsk_rto_min = timeout;
>   		break;
> +	case TCP_BPF_SOCK_OPS_CB_FLAGS:
> +		if (val & ~(BPF_SOCK_OPS_ALL_CB_FLAGS))
> +			return -EINVAL;
> +		tp->bpf_sock_ops_cb_flags = val;
> +		break;
>   	default:
>   		return -EINVAL;
>   	}
> @@ -5366,6 +5371,17 @@ static int sol_tcp_sockopt(struct sock *sk, int optname,
>   		if (*optlen < 1)
>   			return -EINVAL;
>   		break;
> +	case TCP_BPF_SOCK_OPS_CB_FLAGS:
> +		if (*optlen != sizeof(int))
> +			return -EINVAL;
> +		if (getopt) {
> +			struct tcp_sock *tp = tcp_sk(sk);
> +			int val = READ_ONCE(tp->bpf_sock_ops_cb_flags);

READ_ONCE() here looks suspicious. There is no existing WRITE_ONCE.

Is it needed? The read here should have already passed the sock_owned_by_me 
test. The existing write side should also have the sock_owned_by_me.