Re: [PATCH net-next v2 02/12] net-timestamp: open gate for bpf_setsockopt

[Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx>]

Jason Xing wrote:
> From: Jason Xing <kernelxing@xxxxxxxxxxx>
> 
> For now, we support bpf_setsockopt only TX timestamps flags. Users
> can use something like this in bpf program to turn on the feature:
> 
> flags = SOF_TIMESTAMPING_TX_SCHED;
> bpf_setsockopt(skops, SOL_SOCKET, SO_TIMESTAMPING, &flags, sizeof(flags));
> 
> Later, I will support each Tx flags one by one based on this.
> 
> Signed-off-by: Jason Xing <kernelxing@xxxxxxxxxxx>
> ---
>  include/net/sock.h |  2 ++
>  net/core/filter.c  | 27 +++++++++++++++++++++++++++
>  net/core/sock.c    | 35 ++++++++++++++++++++++++-----------
>  3 files changed, 53 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 8cf278c957b3..66ecd78f1dfe 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2890,6 +2890,8 @@ void sock_def_readable(struct sock *sk);
>  
>  int sock_bindtoindex(struct sock *sk, int ifindex, bool lock_sk);
>  void sock_set_timestamp(struct sock *sk, int optname, bool valbool);
> +int sock_get_timestamping(struct so_timestamping *timestamping,
> +			  sockptr_t optval, unsigned int optlen);
>  int sock_set_timestamping(struct sock *sk, int optname,
>  			  struct so_timestamping timestamping);
>  
> diff --git a/net/core/filter.c b/net/core/filter.c
> index bd0d08bf76bb..996426095bd9 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5204,10 +5204,30 @@ static const struct bpf_func_proto bpf_get_socket_uid_proto = {
>  	.arg1_type      = ARG_PTR_TO_CTX,
>  };
>  
> +static int bpf_sock_set_timestamping(struct sock *sk,
> +				     struct so_timestamping *timestamping)
> +{
> +	u32 flags = timestamping->flags;
> +
> +	if (flags & ~SOF_TIMESTAMPING_MASK)
> +		return -EINVAL;
> +
> +	if (!(flags & (SOF_TIMESTAMPING_TX_SCHED | SOF_TIMESTAMPING_TX_SOFTWARE |
> +	      SOF_TIMESTAMPING_TX_ACK)))
> +		return -EINVAL;
> +
> +	WRITE_ONCE(sk->sk_tsflags[BPFPROG_TS_REQUESTOR], flags);
> +
> +	return 0;
> +}
> +
>  static int sol_socket_sockopt(struct sock *sk, int optname,
>  			      char *optval, int *optlen,
>  			      bool getopt)
>  {
> +	struct so_timestamping ts;
> +	int ret = 0;
> +
>  	switch (optname) {
>  	case SO_REUSEADDR:
>  	case SO_SNDBUF:
> @@ -5225,6 +5245,13 @@ static int sol_socket_sockopt(struct sock *sk, int optname,
>  		break;
>  	case SO_BINDTODEVICE:
>  		break;
> +	case SO_TIMESTAMPING_NEW:
> +	case SO_TIMESTAMPING_OLD:
> +		ret = sock_get_timestamping(&ts, KERNEL_SOCKPTR(optval),
> +					    *optlen);
> +		if (!ret)
> +			ret = bpf_sock_set_timestamping(sk, &ts);
> +		return ret;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 52c8c5a5ba27..a6e0d51a5f72 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -894,6 +894,27 @@ static int sock_timestamping_bind_phc(struct sock *sk, int phc_index)
>  	return 0;
>  }
>  
> +int sock_get_timestamping(struct so_timestamping *timestamping,
> +			  sockptr_t optval, unsigned int optlen)
> +{
> +	int val;
> +
> +	if (copy_from_sockptr(&val, optval, sizeof(val)))
> +		return -EFAULT;

Ideally don't read this again.

If you do, then move it in the else clause.

> +
> +	if (optlen == sizeof(*timestamping)) {
> +		if (copy_from_sockptr(timestamping, optval,
> +				      sizeof(*timestamping))) {
> +			return -EFAULT;
> +		}
> +	} else {
> +		memset(timestamping, 0, sizeof(*timestamping));
> +		timestamping->flags = val;
> +	}
> +
> +	return 0;
> +}
> +
>  int sock_set_timestamping(struct sock *sk, int optname,
>  			  struct so_timestamping timestamping)
>  {
> @@ -1402,17 +1423,9 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
>  
>  	case SO_TIMESTAMPING_NEW:
>  	case SO_TIMESTAMPING_OLD:
> -		if (optlen == sizeof(timestamping)) {
> -			if (copy_from_sockptr(&timestamping, optval,
> -					      sizeof(timestamping))) {
> -				ret = -EFAULT;
> -				break;
> -			}
> -		} else {
> -			memset(&timestamping, 0, sizeof(timestamping));
> -			timestamping.flags = val;
> -		}
> -		ret = sock_set_timestamping(sk, optname, timestamping);
> +		ret = sock_get_timestamping(&timestamping, optval, optlen);
> +		if (!ret)
> +			ret = sock_set_timestamping(sk, optname, timestamping);
>  		break;
>  
>  	case SO_RCVLOWAT:
> -- 
> 2.37.3
>