[PATCH 0/2] ipv4, bpf: Introduced to support the ULP to get or set sockets

[PATCH 0/2] ipv4, bpf: Introduced to support the ULP to get or set sockets

[zhangmingyi]

We want call bpf_setsockopt to replace the kernel module in the TCP_ULP
case. The purpose is to customize the behavior in connect and sendmsg.
We have an open source community project kmesh (kmesh.net). Based on
this, we refer to some processes of tcp fastopen to implement delayed
connet and perform HTTP DNAT when sendmsg.In this case, we need to parse
HTTP packets in the bpf program and set TCP_ULP for the specified socket.

Note that tcp_getsockopt and tcp_setsockopt support TCP_ULP, while
bpf_getsockopt and bpf_setsockopt do not support TCP_ULP.
I'm not sure why there is such a difference, but I noticed that
tcp_setsockopt is called in bpf_setsockopt.I think we can add the
handling of this case.

zhangmingyi (2):
  ipv4, bpf: Introduced to support the ULP to get or set sockets
  add selftest for TCP_ULP in bpf_setsockopt

 net/core/filter.c                             |  1 +
 .../selftests/bpf/progs/setget_sockopt.c      | 21 ++++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/2] ipv4, bpf: Introduced to support the ULP to get or set sockets

[zhangmingyi]

Note that tcp_getsockopt and tcp_setsockopt support TCP_ULP, while
bpf_getsockopt and bpf_setsockopt do not support TCP_ULP.
I'm not sure why there is such a difference, but I noticed that
tcp_setsockopt is called in bpf_setsockopt.I think we can add the
handling of this case.

Signed-off-by: zhangmingyi <zhangmingyi5@huawei.com>
---
 net/core/filter.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 713d6f454df3..bdb5c43d6fb0 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5380,6 +5380,7 @@ static int sol_tcp_sockopt(struct sock *sk, int optname,
 	case TCP_CONGESTION:
 		return sol_tcp_sockopt_congestion(sk, optval, optlen, getopt);
 	case TCP_SAVED_SYN:
+	case TCP_ULP:
 		if (*optlen < 1)
 			return -EINVAL;
 		break;
-- 
2.43.0

Re: [PATCH 1/2] ipv4, bpf: Introduced to support the ULP to get or set sockets

[Martin KaFai Lau]

On 1/27/25 1:07 AM, zhangmingyi wrote:
> Note that tcp_getsockopt and tcp_setsockopt support TCP_ULP, while
> bpf_getsockopt and bpf_setsockopt do not support TCP_ULP.
> I'm not sure why there is such a difference, but I noticed that
> tcp_setsockopt is called in bpf_setsockopt.I think we can add the
> handling of this case.
> 
> Signed-off-by: zhangmingyi <zhangmingyi5@huawei.com>

A nit that I found it useful to recognize different authors of the mailing list. 
It would be easier to have a formatted name. It seems "Mingyi Zhang" was used 
earlier, as seen by searching the git log history.

[PATCH 2/2] add selftest for TCP_ULP in bpf_setsockopt

[zhangmingyi]

This case invokes bpf_setsockopt and bpf_getsockopt to set ulp. 
The existing smc_ulp_ops of the kernel is used as a test case to test
whether the setting and get operations can be performed normally.

Signed-off-by: zhangmingyi <zhangmingyi5@huawei.com>
---
 .../selftests/bpf/progs/setget_sockopt.c      | 21 ++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c
index 6dd4318debbf..dcdf26ef41c4 100644
--- a/tools/testing/selftests/bpf/progs/setget_sockopt.c
+++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c
@@ -327,6 +327,18 @@ static int test_tcp_maxseg(void *ctx, struct sock *sk)
 	return 0;
 }
 
+static int test_tcp_ulp(void *ctx, struct sock *sk)
+{
+	__u8 saved_syn[20];
+
+	if (sk->sk_state == TCP_SYN_SENT)
+		return bpf_setsockopt(ctx, IPPROTO_TCP, TCP_ULP,
+						"smc", sizeof("smc"));
+
+	return bpf_getsockopt(ctx, IPPROTO_TCP, TCP_ULP,
+			    saved_syn, sizeof(saved_syn));
+}
+
 static int test_tcp_saved_syn(void *ctx, struct sock *sk)
 {
 	__u8 saved_syn[20];
@@ -395,16 +407,19 @@ int skops_sockopt(struct bpf_sock_ops *skops)
 		break;
 	case BPF_SOCK_OPS_TCP_CONNECT_CB:
 		nr_connect += !(bpf_test_sockopt(skops, sk) ||
-				test_tcp_maxseg(skops, sk));
+				test_tcp_maxseg(skops, sk) ||
+				test_tcp_ulp(skops, sk));
 		break;
 	case BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB:
 		nr_active += !(bpf_test_sockopt(skops, sk) ||
-			       test_tcp_maxseg(skops, sk));
+			       test_tcp_maxseg(skops, sk) ||
+				   test_tcp_ulp(skops, sk));
 		break;
 	case BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB:
 		nr_passive += !(bpf_test_sockopt(skops, sk) ||
 				test_tcp_maxseg(skops, sk) ||
-				test_tcp_saved_syn(skops, sk));
+				test_tcp_saved_syn(skops, sk) ||
+				test_tcp_ulp(skops, sk));
 		flags = skops->bpf_sock_ops_cb_flags | BPF_SOCK_OPS_STATE_CB_FLAG;
 		bpf_setsockopt(skops, SOL_TCP, TCP_BPF_SOCK_OPS_CB_FLAGS, &flags, sizeof(flags));
 		break;
-- 
2.43.0

Re: [PATCH 2/2] add selftest for TCP_ULP in bpf_setsockopt

[Martin KaFai Lau]

On 1/27/25 1:07 AM, zhangmingyi wrote:
> This case invokes bpf_setsockopt and bpf_getsockopt to set ulp.
> The existing smc_ulp_ops of the kernel is used as a test case to test
> whether the setting and get operations can be performed normally.
> 
> Signed-off-by: zhangmingyi <zhangmingyi5@huawei.com>
> ---
>   .../selftests/bpf/progs/setget_sockopt.c      | 21 ++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt.c b/tools/testing/selftests/bpf/progs/setget_sockopt.c
> index 6dd4318debbf..dcdf26ef41c4 100644
> --- a/tools/testing/selftests/bpf/progs/setget_sockopt.c
> +++ b/tools/testing/selftests/bpf/progs/setget_sockopt.c
> @@ -327,6 +327,18 @@ static int test_tcp_maxseg(void *ctx, struct sock *sk)
>   	return 0;
>   }
>   
> +static int test_tcp_ulp(void *ctx, struct sock *sk)
> +{
> +	__u8 saved_syn[20];
> +
> +	if (sk->sk_state == TCP_SYN_SENT)
> +		return bpf_setsockopt(ctx, IPPROTO_TCP, TCP_ULP,
> +						"smc", sizeof("smc"));

The test_progs/setget_sockopt.c is using "tls" in a setsockopt(TCP_ULP) call. I 
would rather not to introduce another ulp in this selftest. Let stay with "tls".

btw, the indentation is off...

> +
> +	return bpf_getsockopt(ctx, IPPROTO_TCP, TCP_ULP,
> +			    saved_syn, sizeof(saved_syn));

same here on indentation.

Also, the getsockopt test should ensure it gets the same ulp name back (i.e. 
"tls"). Take a look at bpf_strncmp.

> +}
> +
>   static int test_tcp_saved_syn(void *ctx, struct sock *sk)
>   {
>   	__u8 saved_syn[20];
> @@ -395,16 +407,19 @@ int skops_sockopt(struct bpf_sock_ops *skops)
>   		break;
>   	case BPF_SOCK_OPS_TCP_CONNECT_CB:
>   		nr_connect += !(bpf_test_sockopt(skops, sk) ||
> -				test_tcp_maxseg(skops, sk));
> +				test_tcp_maxseg(skops, sk) ||
> +				test_tcp_ulp(skops, sk));

For other optnames, it makes sense to reuse the existing "skops_sockopt" BPF 
program. For ulp, it could change the sendmsg, recvmsg, and a few other 
behaviors. I would prefer to separate it out into its own BPF program to avoid 
future surprises on the existing tests in prog_tests/setget_sockopt.c. Keep the 
new BPF program simple, e.g. implement a new BPF program for 
"lsm_cgroup/socket_post_create" and only check for bpf_set/getsockopt(TCP_ULP).

Please tag the set for bpf-next. The "ipv4" in the patch 1's subject is not 
accurate also. afaik, ulp is not specific to ipv4.

Also, the bpf CI complains that the test cannot compile.

pw-bot: cr

>   		break;
>   	case BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB:
>   		nr_active += !(bpf_test_sockopt(skops, sk) ||
> -			       test_tcp_maxseg(skops, sk));
> +			       test_tcp_maxseg(skops, sk) ||
> +				   test_tcp_ulp(skops, sk));
>   		break;
>   	case BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB:
>   		nr_passive += !(bpf_test_sockopt(skops, sk) ||
>   				test_tcp_maxseg(skops, sk) ||
> -				test_tcp_saved_syn(skops, sk));
> +				test_tcp_saved_syn(skops, sk) ||
> +				test_tcp_ulp(skops, sk));
>   		flags = skops->bpf_sock_ops_cb_flags | BPF_SOCK_OPS_STATE_CB_FLAG;
>   		bpf_setsockopt(skops, SOL_TCP, TCP_BPF_SOCK_OPS_CB_FLAGS, &flags, sizeof(flags));
>   		break;

Re: [PATCH 0/2] ipv4, bpf: Introduced to support the ULP to get or set sockets

[Martin KaFai Lau]

On 1/27/25 1:07 AM, zhangmingyi wrote:
> We want call bpf_setsockopt to replace the kernel module in the TCP_ULP
> case. The purpose is to customize the behavior in connect and sendmsg.
> We have an open source community project kmesh (kmesh.net). Based on
> this, we refer to some processes of tcp fastopen to implement delayed
> connet and perform HTTP DNAT when sendmsg.In this case, we need to parse
> HTTP packets in the bpf program and set TCP_ULP for the specified socket.

The ulp could be a kernel module. Which ulp is needed in your use case?

> Note that tcp_getsockopt and tcp_setsockopt support TCP_ULP, while
> bpf_getsockopt and bpf_setsockopt do not support TCP_ULP.
> I'm not sure why there is such a difference, but I noticed that

You are right that bpf_get/setsockopt should be able to support most of the 
TCP_* optname.

After looking at tcp_set_ulp, I believe TCP_ULP is one of the few exceptions. I 
didn't drill down further and I stopped at __tcp_ulp_find_autoload which I 
believe it might_sleep. The BPF programs that support bpf_setsockopt cannot 
sleep. Take a look at how do_tcp_setsockopt(TCP_CONGESTION) is done.

pw-bot: cr

> tcp_setsockopt is called in bpf_setsockopt.I think we can add the
> handling of this case.
> 
> zhangmingyi (2):
>    ipv4, bpf: Introduced to support the ULP to get or set sockets
>    add selftest for TCP_ULP in bpf_setsockopt
> 
>   net/core/filter.c                             |  1 +
>   .../selftests/bpf/progs/setget_sockopt.c      | 21 ++++++++++++++++---
>   2 files changed, 19 insertions(+), 3 deletions(-)
>