netdev - Re: [PATCH bpf-next] bpf: add tcp_notsent

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 8 Oct 2020 23:27:09 -0700
From:   Yonghong Song <yhs@...com>
To:     "Nikita V. Shirokov" <tehnerd@...nerd.com>, <bpf@...r.kernel.org>
CC:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next] bpf: add tcp_notsent_lowat bpf setsockopt



On 10/8/20 10:08 PM, Nikita V. Shirokov wrote:
> Adding support for TCP_NOTSENT_LOWAT sockoption
> (https://lwn.net/Articles/560082/ ) in tcpbpf
> 
> Signed-off-by: Nikita V. Shirokov <tehnerd@...nerd.com>
> ---
>   include/uapi/linux/bpf.h                          |  2 +-
>   net/core/filter.c                                 |  4 ++++
>   tools/testing/selftests/bpf/progs/connect4_prog.c | 15 +++++++++++++++
>   3 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d83561e8cd2c..42d2df799397 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1698,7 +1698,7 @@ union bpf_attr {
>    * 		  **TCP_CONGESTION**, **TCP_BPF_IW**,
>    * 		  **TCP_BPF_SNDCWND_CLAMP**, **TCP_SAVE_SYN**,
>    * 		  **TCP_KEEPIDLE**, **TCP_KEEPINTVL**, **TCP_KEEPCNT**,
> - * 		  **TCP_SYNCNT**, **TCP_USER_TIMEOUT**.
> + *		  **TCP_SYNCNT**, **TCP_USER_TIMEOUT**, **TCP_NOTSENT_LOWAT**.
>    * 		* **IPPROTO_IP**, which supports *optname* **IP_TOS**.
>    * 		* **IPPROTO_IPV6**, which supports *optname* **IPV6_TCLASS**.
>    * 	Return
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 05df73780dd3..5da44b11e1ec 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4827,6 +4827,10 @@ static int _bpf_setsockopt(struct sock *sk, int level, int optname,
>   				else
>   					icsk->icsk_user_timeout = val;
>   				break;
> +			case TCP_NOTSENT_LOWAT:
> +				tp->notsent_lowat = val;
> +				sk->sk_write_space(sk);
> +				break;

This looks good to me. It is the same as in do_tcp_setsockopt().

>   			default:
>   				ret = -EINVAL;
>   			}
> diff --git a/tools/testing/selftests/bpf/progs/connect4_prog.c b/tools/testing/selftests/bpf/progs/connect4_prog.c
> index b1b2773c0b9d..b10e7fbace7b 100644
> --- a/tools/testing/selftests/bpf/progs/connect4_prog.c
> +++ b/tools/testing/selftests/bpf/progs/connect4_prog.c
> @@ -128,6 +128,18 @@ static __inline int set_keepalive(struct bpf_sock_addr *ctx)
>   	return 0;
>   }
>   
> +static __inline int set_notsent_lowat(struct bpf_sock_addr *ctx)
> +{
> +	int lowat = 65535;
> +
> +	if (ctx->type == SOCK_STREAM) {
> +		if (bpf_setsockopt(ctx, SOL_TCP, TCP_NOTSENT_LOWAT, &lowat, sizeof(lowat)))

In my build system, I hit a compilation error.

progs/connect4_prog.c:137:36: error: use of undeclared identifier 
'TCP_NOTSENT_LOWAT'
                 if (bpf_setsockopt(ctx, SOL_TCP, TCP_NOTSENT_LOWAT, 
&lowat, sizeof(lowat)))

TCP_NOTSENT_LOWAT is included in /usr/include/linux/tcp.h. But this file
includes netinet/tcp.h and it contains some same symbol definitions as
linux/tcp.h so I can include both.

Adding the following can fix the issue

#ifndef TCP_NOTSENT_LOWAT
#define TCP_NOTSENT_LOWAT       25
#endif

Not sure where TCP_NOTSENT_LOWAT is defined in your system.

> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
>   SEC("cgroup/connect4")
>   int connect_v4_prog(struct bpf_sock_addr *ctx)
>   {
> @@ -148,6 +160,9 @@ int connect_v4_prog(struct bpf_sock_addr *ctx)
>   	if (set_keepalive(ctx))
>   		return 0;
>   
> +	if (set_notsent_lowat(ctx))
> +		return 0;
> +
>   	if (ctx->type != SOCK_STREAM && ctx->type != SOCK_DGRAM)
>   		return 0;
>   	else if (ctx->type == SOCK_STREAM)
>