[PATCH v2 0/2] bpf-next: Introduced to support the ULP to get or set sockets

[PATCH v2 0/2] bpf-next: Introduced to support the ULP to get or set sockets

[zhangmingyi]

From: Mingyi Zhang <zhangmingyi5@huawei.com>

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
after the user-defined ko file is loaded. 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.

Change list:
- v1 -> v2:
  - modified the do_tcp_setsockopt(TCP_ULP) process by referring to
  section do_tcp_setsockopt(TCP_CONGESTION), avoid sleep
  - The selftest case is modified. An independent file is selected
  for the test to avoid affecting the original file in setget_sockopt.c
  - fixed some formatting errors, such as Signed and Subject

Revisions:
- v1
  https://lore.kernel.org/bpf/20250127090724.3168791-1-zhangmingyi5@huawei.com/

Mingyi Zhang (2):
  bpf-next: Introduced to support the ULP to get or set sockets
  bpf-next: selftest for TCP_ULP in bpf_setsockopt

 include/net/tcp.h                             |  2 +-
 net/core/filter.c                             |  1 +
 net/ipv4/tcp.c                                |  2 +-
 net/ipv4/tcp_ulp.c                            | 28 +++----
 net/mptcp/subflow.c                           |  2 +-
 .../bpf/prog_tests/setget_sockopt_tcp_ulp.c   | 78 +++++++++++++++++++
 .../bpf/progs/setget_sockopt_tcp_ulp.c        | 33 ++++++++
 7 files changed, 130 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
 create mode 100644 tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c

-- 
2.43.0

[PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

[zhangmingyi]

From: Mingyi Zhang <zhangmingyi5@huawei.com>

Note that tcp_getsockopt and tcp_setsockopt support TCP_ULP, while
bpf_getsockopt and bpf_setsockopt do not support TCP_ULP.
I think we can add the handling of this case.

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.

Signed-off-by: Mingyi Zhang <zhangmingyi5@huawei.com>
Signed-off-by: Xin Liu <liuxin350@huawei.com>
---
 include/net/tcp.h   |  2 +-
 net/core/filter.c   |  1 +
 net/ipv4/tcp.c      |  2 +-
 net/ipv4/tcp_ulp.c  | 28 +++++++++++++++-------------
 net/mptcp/subflow.c |  2 +-
 5 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index e9b37b76e894..f26e92099b86 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2582,7 +2582,7 @@ struct tcp_ulp_ops {
 };
 int tcp_register_ulp(struct tcp_ulp_ops *type);
 void tcp_unregister_ulp(struct tcp_ulp_ops *type);
-int tcp_set_ulp(struct sock *sk, const char *name);
+int tcp_set_ulp(struct sock *sk, const char *name, bool load);
 void tcp_get_available_ulp(char *buf, size_t len);
 void tcp_cleanup_ulp(struct sock *sk);
 void tcp_update_ulp(struct sock *sk, struct proto *p,
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;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 0d704bda6c41..88ccd0e211f9 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3744,7 +3744,7 @@ int do_tcp_setsockopt(struct sock *sk, int level, int optname,
 		name[val] = 0;
 
 		sockopt_lock_sock(sk);
-		err = tcp_set_ulp(sk, name);
+		err = tcp_set_ulp(sk, name, !has_current_bpf_ctx());
 		sockopt_release_sock(sk);
 		return err;
 	}
diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
index 2aa442128630..c1c39dbef417 100644
--- a/net/ipv4/tcp_ulp.c
+++ b/net/ipv4/tcp_ulp.c
@@ -33,10 +33,7 @@ static struct tcp_ulp_ops *tcp_ulp_find(const char *name)
 
 static const struct tcp_ulp_ops *__tcp_ulp_find_autoload(const char *name)
 {
-	const struct tcp_ulp_ops *ulp = NULL;
-
-	rcu_read_lock();
-	ulp = tcp_ulp_find(name);
+	const struct tcp_ulp_ops *ulp = tcp_ulp_find(name);
 
 #ifdef CONFIG_MODULES
 	if (!ulp && capable(CAP_NET_ADMIN)) {
@@ -46,10 +43,6 @@ static const struct tcp_ulp_ops *__tcp_ulp_find_autoload(const char *name)
 		ulp = tcp_ulp_find(name);
 	}
 #endif
-	if (!ulp || !try_module_get(ulp->owner))
-		ulp = NULL;
-
-	rcu_read_unlock();
 	return ulp;
 }
 
@@ -154,15 +147,24 @@ static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
 	return err;
 }
 
-int tcp_set_ulp(struct sock *sk, const char *name)
+int tcp_set_ulp(struct sock *sk, const char *name, bool load)
 {
 	const struct tcp_ulp_ops *ulp_ops;
+	int err = 0;
 
 	sock_owned_by_me(sk);
 
-	ulp_ops = __tcp_ulp_find_autoload(name);
-	if (!ulp_ops)
-		return -ENOENT;
+	rcu_read_lock();
+	if (!load)
+		ulp_ops = tcp_ulp_find(name);
+	else
+		ulp_ops = __tcp_ulp_find_autoload(name);
+
+	if (!ulp_ops || !try_module_get(ulp_ops->owner))
+		err = -ENOENT;
+	else
+		err = __tcp_set_ulp(sk, ulp_ops);
 
-	return __tcp_set_ulp(sk, ulp_ops);
+	rcu_read_unlock();
+	return err;
 }
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index fd021cf8286e..fb936d280b83 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1776,7 +1776,7 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
 	sf->sk->sk_net_refcnt = 1;
 	get_net_track(net, &sf->sk->ns_tracker, GFP_KERNEL);
 	sock_inuse_add(net, 1);
-	err = tcp_set_ulp(sf->sk, "mptcp");
+	err = tcp_set_ulp(sf->sk, "mptcp", true);
 	if (err)
 		goto err_free;
 
-- 
2.43.0

[PATCH v2 2/2] bpf-next: selftest for TCP_ULP in bpf_setsockopt

[zhangmingyi]

From: Mingyi Zhang <zhangmingyi5@huawei.com>

We try to use bpf_set/getsockopt to set/get TCP_ULP in sockops, and "tls"
need connect is established.To avoid impacting other test cases, I have
written a separate test case file.

Signed-off-by: Mingyi Zhang <zhangmingyi5@huawei.com>
Signed-off-by: Xin Liu <liuxin350@huawei.com>
---
 .../bpf/prog_tests/setget_sockopt_tcp_ulp.c   | 78 +++++++++++++++++++
 .../bpf/progs/setget_sockopt_tcp_ulp.c        | 33 ++++++++
 2 files changed, 111 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
 create mode 100644 tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c

diff --git a/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c b/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
new file mode 100644
index 000000000000..748da2c7d255
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) Meta Platforms, Inc. and affiliates. */
+
+#define _GNU_SOURCE
+#include <net/if.h>
+
+#include "test_progs.h"
+#include "network_helpers.h"
+
+#include "setget_sockopt_tcp_ulp.skel.h"
+
+#define CG_NAME "/setget-sockopt-tcp-ulp-test"
+
+static const char addr4_str[] = "127.0.0.1";
+static const char addr6_str[] = "::1";
+static struct setget_sockopt_tcp_ulp *skel;
+static int cg_fd;
+
+static int create_netns(void)
+{
+	if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns"))
+		return -1;
+	if (!ASSERT_OK(system("ip link set dev lo up"), "set lo up"))
+		return -1;
+	return 0;
+}
+
+static int modprobe_tls(void)
+{
+	if (!ASSERT_OK(system("modprobe tls"), "tls modprobe failed"))
+		return -1;
+	return 0;
+}
+
+static void test_tcp_ulp(int family)
+{
+	struct setget_sockopt_tcp_ulp__bss *bss = skel->bss;
+	int sfd, cfd;
+
+	memset(bss, 0, sizeof(*bss));
+	sfd = start_server(family, SOCK_STREAM,
+			   family == AF_INET6 ? addr6_str : addr4_str, 0, 0);
+	if (!ASSERT_GE(sfd, 0, "start_server"))
+		return;
+
+	cfd = connect_to_fd(sfd, 0);
+	if (!ASSERT_GE(cfd, 0, "connect_to_fd_server")) {
+		close(sfd);
+		return;
+	}
+	close(sfd);
+	close(cfd);
+
+	ASSERT_EQ(bss->nr_tcp_ulp, 3, "nr_tcp_ulp");
+}
+
+void test_setget_sockopt_tcp_ulp(void)
+{
+	cg_fd = test__join_cgroup(CG_NAME);
+	if (cg_fd < 0)
+		return;
+	if (create_netns() && modprobe_tls())
+		goto done;
+	skel = setget_sockopt_tcp_ulp__open();
+	if (!ASSERT_OK_PTR(skel, "open skel"))
+		goto done;
+	if (!ASSERT_OK(setget_sockopt_tcp_ulp__load(skel), "load skel"))
+		goto done;
+	skel->links.skops_sockopt_tcp_ulp =
+		bpf_program__attach_cgroup(skel->progs.skops_sockopt_tcp_ulp, cg_fd);
+	if (!ASSERT_OK_PTR(skel->links.skops_sockopt_tcp_ulp, "attach_cgroup"))
+		goto done;
+	test_tcp_ulp(AF_INET);
+	test_tcp_ulp(AF_INET6);
+done:
+	setget_sockopt_tcp_ulp__destroy(skel);
+	close(cg_fd);
+}
diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c b/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
new file mode 100644
index 000000000000..bd1009766463
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) Meta Platforms, Inc. and affiliates. */
+
+#include "vmlinux.h"
+#include "bpf_tracing_net.h"
+#include <bpf/bpf_helpers.h>
+
+int nr_tcp_ulp;
+
+SEC("sockops")
+int skops_sockopt_tcp_ulp(struct bpf_sock_ops *skops)
+{
+	static const char target_ulp[] = "tls";
+	char verify_ulp[sizeof(target_ulp)];
+
+	switch (skops->op) {
+	case BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB:
+		if (bpf_setsockopt(skops, IPPROTO_TCP, TCP_ULP, (void *)target_ulp,
+							sizeof(target_ulp)) != 0)
+			return 1;
+		nr_tcp_ulp++;
+		if (bpf_getsockopt(skops, IPPROTO_TCP, TCP_ULP, verify_ulp,
+							sizeof(verify_ulp)) != 0)
+			return 1;
+		nr_tcp_ulp++;
+		if (bpf_strncmp(verify_ulp, sizeof(target_ulp), "tls") != 0)
+			return 1;
+		nr_tcp_ulp++;
+	}
+	return 1;
+}
+
+char _license[] SEC("license") = "GPL";
\ No newline at end of file
-- 
2.43.0

Re: [PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

[Martin KaFai Lau]

On 2/10/25 5:45 AM, zhangmingyi wrote:
> From: Mingyi Zhang <zhangmingyi5@huawei.com>
> 
> Note that tcp_getsockopt and tcp_setsockopt support TCP_ULP, while
> bpf_getsockopt and bpf_setsockopt do not support TCP_ULP.
> I think we can add the handling of this case.

The commit message should talk about the "bool load" related changes in v2 and 
why it is needed.

The subject line is confusing. How about,
"Support TCP_ULP in bpf_get/setsockopt"

The code changes lgtm.

> 
> 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.

Re: [PATCH v2 2/2] bpf-next: selftest for TCP_ULP in bpf_setsockopt

[Martin KaFai Lau]

On 2/10/25 5:45 AM, zhangmingyi wrote:
> From: Mingyi Zhang <zhangmingyi5@huawei.com>
> 
> We try to use bpf_set/getsockopt to set/get TCP_ULP in sockops, and "tls"
> need connect is established.To avoid impacting other test cases, I have
> written a separate test case file.
> 
> Signed-off-by: Mingyi Zhang <zhangmingyi5@huawei.com>
> Signed-off-by: Xin Liu <liuxin350@huawei.com>
> ---
>   .../bpf/prog_tests/setget_sockopt_tcp_ulp.c   | 78 +++++++++++++++++++
>   .../bpf/progs/setget_sockopt_tcp_ulp.c        | 33 ++++++++
>   2 files changed, 111 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
>   create mode 100644 tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c b/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
> new file mode 100644
> index 000000000000..748da2c7d255
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) Meta Platforms, Inc. and affiliates. */

This is not right.

> +
> +#define _GNU_SOURCE
> +#include <net/if.h>
> +
> +#include "test_progs.h"
> +#include "network_helpers.h"
> +
> +#include "setget_sockopt_tcp_ulp.skel.h"
> +
> +#define CG_NAME "/setget-sockopt-tcp-ulp-test"
> +
> +static const char addr4_str[] = "127.0.0.1";
> +static const char addr6_str[] = "::1";
> +static struct setget_sockopt_tcp_ulp *skel;
> +static int cg_fd;
> +
> +static int create_netns(void)
> +{
> +	if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns"))
> +		return -1;
> +	if (!ASSERT_OK(system("ip link set dev lo up"), "set lo up"))
> +		return -1;
> +	return 0;
> +}
> +
> +static int modprobe_tls(void)
> +{
> +	if (!ASSERT_OK(system("modprobe tls"), "tls modprobe failed"))
> +		return -1;
> +	return 0;
> +}
> +
> +static void test_tcp_ulp(int family)

First, the bpf CI still fails to compile for the same reason as v1. You should 
have received an email from bpf CI bot. Please ensure it is addressed first 
before reposting. This repeated bpf CI error is an automatic nack.

pw-bot: cr

The subject tagging should be "[PATCH v2 bpf-next ...] selftests/bpf: ... ". 
There are many examples to follow in the mailing list if it is not clear.

Regarding the v1 comment: "...separate it out into its own BPF program..."

The comment was made at the bpf prog file, "progs/setget_sockopt.c". I meant 
only create a separate bpf program. The user space part can stay in 
prog_tests/setget_sockopt.c.

Move this function, test_tcp_ulp, to prog_tests/setget_sockopt.c. Then all the 
above config preparation codes and the test_setget_sockopt_tcp_ulp() can be 
saved. modprobe_tls() is not needed also. Do it after the test_ktls().
Take a look at test_nonstandard_opt() in prog_tests/setget_sockopt.c.
It is testing another bpf prog in the same prog_tests/setget_sockopt.c.

Also, for the bpf prog, do you really need to test at 
BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB? If not, just testing at 
SEC("lsm_cgroup/socket_post_create") will be easier. You can detach the 
previously attached skel->links.socket_post_create by using bpf_link__destroy() 
first and then attach yours bpf prog to do the test.

> +{
> +	struct setget_sockopt_tcp_ulp__bss *bss = skel->bss;
> +	int sfd, cfd;
> +
> +	memset(bss, 0, sizeof(*bss));
> +	sfd = start_server(family, SOCK_STREAM,
> +			   family == AF_INET6 ? addr6_str : addr4_str, 0, 0);
> +	if (!ASSERT_GE(sfd, 0, "start_server"))
> +		return;
> +
> +	cfd = connect_to_fd(sfd, 0);
> +	if (!ASSERT_GE(cfd, 0, "connect_to_fd_server")) {
> +		close(sfd);
> +		return;
> +	}
> +	close(sfd);
> +	close(cfd);
> +
> +	ASSERT_EQ(bss->nr_tcp_ulp, 3, "nr_tcp_ulp");
> +}
> +
> +void test_setget_sockopt_tcp_ulp(void)
> +{
> +	cg_fd = test__join_cgroup(CG_NAME);
> +	if (cg_fd < 0)
> +		return;
> +	if (create_netns() && modprobe_tls())
> +		goto done;
> +	skel = setget_sockopt_tcp_ulp__open();
> +	if (!ASSERT_OK_PTR(skel, "open skel"))
> +		goto done;
> +	if (!ASSERT_OK(setget_sockopt_tcp_ulp__load(skel), "load skel"))
> +		goto done;
> +	skel->links.skops_sockopt_tcp_ulp =
> +		bpf_program__attach_cgroup(skel->progs.skops_sockopt_tcp_ulp, cg_fd);
> +	if (!ASSERT_OK_PTR(skel->links.skops_sockopt_tcp_ulp, "attach_cgroup"))
> +		goto done;
> +	test_tcp_ulp(AF_INET);
> +	test_tcp_ulp(AF_INET6);
> +done:
> +	setget_sockopt_tcp_ulp__destroy(skel);
> +	close(cg_fd);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c b/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
> new file mode 100644
> index 000000000000..bd1009766463
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) Meta Platforms, Inc. and affiliates. */

Same here.

> +
> +#include "vmlinux.h"
> +#include "bpf_tracing_net.h"
> +#include <bpf/bpf_helpers.h>
> +
> +int nr_tcp_ulp;
> +
> +SEC("sockops")
> +int skops_sockopt_tcp_ulp(struct bpf_sock_ops *skops)
> +{
> +	static const char target_ulp[] = "tls";
> +	char verify_ulp[sizeof(target_ulp)];
> +
> +	switch (skops->op) {
> +	case BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB:
> +		if (bpf_setsockopt(skops, IPPROTO_TCP, TCP_ULP, (void *)target_ulp,
> +							sizeof(target_ulp)) != 0)
> +			return 1;
> +		nr_tcp_ulp++;
> +		if (bpf_getsockopt(skops, IPPROTO_TCP, TCP_ULP, verify_ulp,
> +							sizeof(verify_ulp)) != 0)
> +			return 1;
> +		nr_tcp_ulp++;
> +		if (bpf_strncmp(verify_ulp, sizeof(target_ulp), "tls") != 0)
> +			return 1;
> +		nr_tcp_ulp++;
> +	}
> +	return 1;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> \ No newline at end of file

Re: [PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

[kernel test robot]

Hello,

kernel test robot noticed "BUG:sleeping_function_called_from_invalid_context_at_kernel/locking/mutex.c" on:

commit: 8f510de3f26b2fabaf47eacd59053469e9c32754 ("[PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets")
url: https://github.com/intel-lab-lkp/linux/commits/zhangmingyi/bpf-next-Introduced-to-support-the-ULP-to-get-or-set-sockets/20250210-215203
base: https://git.kernel.org/cgit/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/all/20250210134550.3189616-2-zhangmingyi5@huawei.com/
patch subject: [PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

in testcase: trinity
version: trinity-i386-abe9de86-1_20230429
with following parameters:

	runtime: 300s
	group: group-03
	nr_groups: 5



config: i386-randconfig-054-20250212
compiler: gcc-12
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+-----------------------------------------------------------------------------+------------+------------+
|                                                                             | 9b6cdaf2ac | 8f510de3f2 |
+-----------------------------------------------------------------------------+------------+------------+
| BUG:sleeping_function_called_from_invalid_context_at_kernel/locking/mutex.c | 0          | 6          |
+-----------------------------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202502140959.f66e2ba6-lkp@intel.com


[   71.099773][ T3759] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:562
[   71.101798][ T3759] in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 3759, name: trinity-c4
[   71.103659][ T3759] preempt_count: 0, expected: 0
[   71.104658][ T3759] RCU nest depth: 1, expected: 0
[   71.105669][ T3759] 2 locks held by trinity-c4/3759:
[ 71.106777][ T3759] #0: ecffcd80 (sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock (include/net/sock.h:1625) 
[ 71.108460][ T3759] #1: c3500498 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire (include/linux/rcupdate.h:336) 
[   71.110397][ T3759] CPU: 1 UID: 65534 PID: 3759 Comm: trinity-c4 Tainted: G                T  6.14.0-rc1-00030-g8f510de3f26b #1 8ad64aae41fa4cb8babad52c8f50e0a7d5e34569
[   71.110406][ T3759] Tainted: [T]=RANDSTRUCT
[   71.110407][ T3759] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   71.110410][ T3759] Call Trace:
[ 71.110416][ T3759] dump_stack_lvl (lib/dump_stack.c:123) 
[ 71.110423][ T3759] dump_stack (lib/dump_stack.c:130) 
[ 71.110428][ T3759] __might_resched (kernel/sched/core.c:8767) 
[ 71.110440][ T3759] __might_sleep (kernel/sched/core.c:8696 (discriminator 17)) 
[ 71.110446][ T3759] __mutex_lock (include/linux/kernel.h:73 kernel/locking/mutex.c:562 kernel/locking/mutex.c:730) 
[ 71.110452][ T3759] ? rcu_read_unlock (include/linux/rcupdate.h:335) 
[ 71.110462][ T3759] ? mark_held_locks (kernel/locking/lockdep.c:4323) 
[ 71.110470][ T3759] ? lock_sock_nested (net/core/sock.c:3653) 
[ 71.110481][ T3759] mutex_lock_nested (kernel/locking/mutex.c:783) 
[ 71.110486][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993) 
[ 71.110494][ T3759] tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993) 
[ 71.110505][ T3759] tcp_set_ulp (net/ipv4/tcp_ulp.c:140 net/ipv4/tcp_ulp.c:166) 
[ 71.110513][ T3759] do_tcp_setsockopt (net/ipv4/tcp.c:3747) 
[ 71.110534][ T3759] tcp_setsockopt (net/ipv4/tcp.c:4032) 
[ 71.110542][ T3759] ? sock_common_recvmsg (net/core/sock.c:3833) 
[ 71.110548][ T3759] sock_common_setsockopt (net/core/sock.c:3838) 
[ 71.110561][ T3759] do_sock_setsockopt (net/socket.c:2298) 
[ 71.110577][ T3759] __sys_setsockopt (net/socket.c:2323) 
[ 71.110592][ T3759] __ia32_sys_setsockopt (net/socket.c:2326) 
[ 71.110599][ T3759] ia32_sys_call (kbuild/obj/consumer/i386-randconfig-054-20250212/./arch/x86/include/generated/asm/syscalls_32.h:367) 
[ 71.110607][ T3759] do_int80_syscall_32 (arch/x86/entry/common.c:165 arch/x86/entry/common.c:339) 
[ 71.110616][ T3759] entry_INT80_32 (arch/x86/entry/entry_32.S:942) 
[   71.110621][ T3759] EIP: 0xb4014092
[ 71.110626][ T3759] Code: 00 00 00 e9 90 ff ff ff ff a3 24 00 00 00 68 30 00 00 00 e9 80 ff ff ff ff a3 f8 ff ff ff 66 90 00 00 00 00 00 00 00 00 cd 80 <c3> 8d b4 26 00 00 00 00 8d b6 00 00 00 00 8b 1c 24 c3 8d b4 26 00
All code
========
   0:	00 00                	add    %al,(%rax)
   2:	00 e9                	add    %ch,%cl
   4:	90                   	nop
   5:	ff                   	(bad)
   6:	ff                   	(bad)
   7:	ff                   	(bad)
   8:	ff a3 24 00 00 00    	jmp    *0x24(%rbx)
   e:	68 30 00 00 00       	push   $0x30
  13:	e9 80 ff ff ff       	jmp    0xffffffffffffff98
  18:	ff a3 f8 ff ff ff    	jmp    *-0x8(%rbx)
  1e:	66 90                	xchg   %ax,%ax
	...
  28:	cd 80                	int    $0x80
  2a:*	c3                   	ret		<-- trapping instruction
  2b:	8d b4 26 00 00 00 00 	lea    0x0(%rsi,%riz,1),%esi
  32:	8d b6 00 00 00 00    	lea    0x0(%rsi),%esi
  38:	8b 1c 24             	mov    (%rsp),%ebx
  3b:	c3                   	ret
  3c:	8d                   	.byte 0x8d
  3d:	b4 26                	mov    $0x26,%ah
	...

Code starting with the faulting instruction
===========================================
   0:	c3                   	ret
   1:	8d b4 26 00 00 00 00 	lea    0x0(%rsi,%riz,1),%esi
   8:	8d b6 00 00 00 00    	lea    0x0(%rsi),%esi
   e:	8b 1c 24             	mov    (%rsp),%ebx
  11:	c3                   	ret
  12:	8d                   	.byte 0x8d
  13:	b4 26                	mov    $0x26,%ah
	...
[   71.110630][ T3759] EAX: ffffffda EBX: 00000134 ECX: 00000006 EDX: 0000001f
[   71.110634][ T3759] ESI: 08fee650 EDI: 00000004 EBP: 000012cf ESP: bfc1c538
[   71.110638][ T3759] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000296
[   71.182507][ T3759]
[   71.182999][ T3759] =============================
[   71.183907][ T3759] [ BUG: Invalid wait context ]
[   71.184819][ T3759] 6.14.0-rc1-00030-g8f510de3f26b #1 Tainted: G        W       T
[   71.186327][ T3759] -----------------------------
[   71.187265][ T3759] trinity-c4/3759 is trying to lock:
[ 71.188287][ T3759] c37b35e0 (tcpv4_prot_mutex){....}-{4:4}, at: tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993) 
[   71.189847][ T3759] other info that might help us debug this:
[   71.191018][ T3759] context-{5:5}
[   71.191678][ T3759] 2 locks held by trinity-c4/3759:
[ 71.192635][ T3759] #0: ecffcd80 (sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock (include/net/sock.h:1625) 
[ 71.194220][ T3759] #1: c3500498 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire (include/linux/rcupdate.h:336) 
[   71.196078][ T3759] stack backtrace:
[   71.196797][ T3759] CPU: 0 UID: 65534 PID: 3759 Comm: trinity-c4 Tainted: G        W       T  6.14.0-rc1-00030-g8f510de3f26b #1 8ad64aae41fa4cb8babad52c8f50e0a7d5e34569
[   71.196807][ T3759] Tainted: [W]=WARN, [T]=RANDSTRUCT
[   71.196809][ T3759] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   71.196812][ T3759] Call Trace:
[ 71.196818][ T3759] dump_stack_lvl (lib/dump_stack.c:123) 
[ 71.196825][ T3759] dump_stack (lib/dump_stack.c:130) 
[ 71.196830][ T3759] __lock_acquire (kernel/locking/lockdep.c:4830 kernel/locking/lockdep.c:4900 kernel/locking/lockdep.c:5178) 
[ 71.196840][ T3759] lock_acquire (kernel/locking/lockdep.c:469 kernel/locking/lockdep.c:5853) 
[ 71.196846][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993) 
[ 71.196856][ T3759] ? __schedule (kernel/sched/core.c:5380) 
[ 71.196866][ T3759] __mutex_lock (kernel/locking/mutex.c:587 kernel/locking/mutex.c:730) 
[ 71.196872][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993) 
[ 71.196878][ T3759] ? rcu_read_unlock (include/linux/rcupdate.h:335) 
[ 71.196885][ T3759] ? mark_held_locks (kernel/locking/lockdep.c:4323) 
[ 71.196889][ T3759] ? lock_sock_nested (net/core/sock.c:3653) 
[ 71.196898][ T3759] mutex_lock_nested (kernel/locking/mutex.c:783) 
[ 71.196904][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993) 
[ 71.196909][ T3759] tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993) 
[ 71.196916][ T3759] tcp_set_ulp (net/ipv4/tcp_ulp.c:140 net/ipv4/tcp_ulp.c:166) 
[ 71.196923][ T3759] do_tcp_setsockopt (net/ipv4/tcp.c:3747) 
[ 71.196934][ T3759] tcp_setsockopt (net/ipv4/tcp.c:4032) 
[ 71.196939][ T3759] ? sock_common_recvmsg (net/core/sock.c:3833) 
[ 71.196946][ T3759] sock_common_setsockopt (net/core/sock.c:3838) 
[ 71.196952][ T3759] do_sock_setsockopt (net/socket.c:2298) 
[ 71.196961][ T3759] __sys_setsockopt (net/socket.c:2323) 
[ 71.196967][ T3759] __ia32_sys_setsockopt (net/socket.c:2326) 
[ 71.196972][ T3759] ia32_sys_call (kbuild/obj/consumer/i386-randconfig-054-20250212/./arch/x86/include/generated/asm/syscalls_32.h:367) 
[ 71.196979][ T3759] do_int80_syscall_32 (arch/x86/entry/common.c:165 arch/x86/entry/common.c:339) 
[ 71.196985][ T3759] entry_INT80_32 (arch/x86/entry/entry_32.S:942) 
[   71.196990][ T3759] EIP: 0xb4014092
[ 71.196995][ T3759] Code: 00 00 00 e9 90 ff ff ff ff a3 24 00 00 00 68 30 00 00 00 e9 80 ff ff ff ff a3 f8 ff ff ff 66 90 00 00 00 00 00 00 00 00 cd 80 <c3> 8d b4 26 00 00 00 00 8d b6 00 00 00 00 8b 1c 24 c3 8d b4 26 00
All code
========
   0:	00 00                	add    %al,(%rax)
   2:	00 e9                	add    %ch,%cl
   4:	90                   	nop
   5:	ff                   	(bad)
   6:	ff                   	(bad)
   7:	ff                   	(bad)
   8:	ff a3 24 00 00 00    	jmp    *0x24(%rbx)
   e:	68 30 00 00 00       	push   $0x30
  13:	e9 80 ff ff ff       	jmp    0xffffffffffffff98
  18:	ff a3 f8 ff ff ff    	jmp    *-0x8(%rbx)
  1e:	66 90                	xchg   %ax,%ax
	...
  28:	cd 80                	int    $0x80
  2a:*	c3                   	ret		<-- trapping instruction
  2b:	8d b4 26 00 00 00 00 	lea    0x0(%rsi,%riz,1),%esi
  32:	8d b6 00 00 00 00    	lea    0x0(%rsi),%esi
  38:	8b 1c 24             	mov    (%rsp),%ebx
  3b:	c3                   	ret
  3c:	8d                   	.byte 0x8d
  3d:	b4 26                	mov    $0x26,%ah
	...

Code starting with the faulting instruction
===========================================
   0:	c3                   	ret
   1:	8d b4 26 00 00 00 00 	lea    0x0(%rsi,%riz,1),%esi
   8:	8d b6 00 00 00 00    	lea    0x0(%rsi),%esi
   e:	8b 1c 24             	mov    (%rsp),%ebx
  11:	c3                   	ret
  12:	8d                   	.byte 0x8d
  13:	b4 26                	mov    $0x26,%ah
	...
[   71.196999][ T3759] EAX: ffffffda EBX: 00000134 ECX: 00000006 EDX: 0000001f
[   71.197004][ T3759] ESI: 08fee650 EDI: 00000004 EBP: 000012cf ESP: bfc1c538
[   71.197008][ T3759] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000296



The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250214/202502140959.f66e2ba6-lkp@intel.com


-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

[Martin KaFai Lau]

On 2/13/25 6:13 PM, kernel test robot wrote:
> [   71.182999][ T3759] =============================
> [   71.183907][ T3759] [ BUG: Invalid wait context ]
> [   71.184819][ T3759] 6.14.0-rc1-00030-g8f510de3f26b #1 Tainted: G        W       T
> [   71.186327][ T3759] -----------------------------
> [   71.187265][ T3759] trinity-c4/3759 is trying to lock:
> [ 71.188287][ T3759] c37b35e0 (tcpv4_prot_mutex){....}-{4:4}, at: tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
> [   71.189847][ T3759] other info that might help us debug this:
> [   71.191018][ T3759] context-{5:5}
> [   71.191678][ T3759] 2 locks held by trinity-c4/3759:
> [ 71.192635][ T3759] #0: ecffcd80 (sk_lock-AF_INET){+.+.}-{0:0}, at: lock_sock (include/net/sock.h:1625)
> [ 71.194220][ T3759] #1: c3500498 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire (include/linux/rcupdate.h:336)
> [   71.196078][ T3759] stack backtrace:
> [   71.196797][ T3759] CPU: 0 UID: 65534 PID: 3759 Comm: trinity-c4 Tainted: G        W       T  6.14.0-rc1-00030-g8f510de3f26b #1 8ad64aae41fa4cb8babad52c8f50e0a7d5e34569
> [   71.196807][ T3759] Tainted: [W]=WARN, [T]=RANDSTRUCT
> [   71.196809][ T3759] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> [   71.196812][ T3759] Call Trace:
> [ 71.196818][ T3759] dump_stack_lvl (lib/dump_stack.c:123)
> [ 71.196825][ T3759] dump_stack (lib/dump_stack.c:130)
> [ 71.196830][ T3759] __lock_acquire (kernel/locking/lockdep.c:4830 kernel/locking/lockdep.c:4900 kernel/locking/lockdep.c:5178)
> [ 71.196840][ T3759] lock_acquire (kernel/locking/lockdep.c:469 kernel/locking/lockdep.c:5853)
> [ 71.196846][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
> [ 71.196856][ T3759] ? __schedule (kernel/sched/core.c:5380)
> [ 71.196866][ T3759] __mutex_lock (kernel/locking/mutex.c:587 kernel/locking/mutex.c:730)
> [ 71.196872][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
> [ 71.196878][ T3759] ? rcu_read_unlock (include/linux/rcupdate.h:335)
> [ 71.196885][ T3759] ? mark_held_locks (kernel/locking/lockdep.c:4323)
> [ 71.196889][ T3759] ? lock_sock_nested (net/core/sock.c:3653)
> [ 71.196898][ T3759] mutex_lock_nested (kernel/locking/mutex.c:783)

This is probably because __tcp_set_ulp is now under the rcu_read_lock() in patch 1.

Even fixing patch 1 will not be enough. The bpf cgrp prog (e.g. sockops) cannot 
sleep now, so it still cannot call bpf_setsockopt(TCP_ULP, "tls") which will 
take a mutex. This is a blocker :(

> [ 71.196904][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
> [ 71.196909][ T3759] tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
> [ 71.196916][ T3759] tcp_set_ulp (net/ipv4/tcp_ulp.c:140 net/ipv4/tcp_ulp.c:166)
> [ 71.196923][ T3759] do_tcp_setsockopt (net/ipv4/tcp.c:3747)
> [ 71.196934][ T3759] tcp_setsockopt (net/ipv4/tcp.c:4032)
> [ 71.196939][ T3759] ? sock_common_recvmsg (net/core/sock.c:3833)
> [ 71.196946][ T3759] sock_common_setsockopt (net/core/sock.c:3838)
> [ 71.196952][ T3759] do_sock_setsockopt (net/socket.c:2298)
> [ 71.196961][ T3759] __sys_setsockopt (net/socket.c:2323)
> [ 71.196967][ T3759] __ia32_sys_setsockopt (net/socket.c:2326)
> [ 71.196972][ T3759] ia32_sys_call (kbuild/obj/consumer/i386-randconfig-054-20250212/./arch/x86/include/generated/asm/syscalls_32.h:367)
> [ 71.196979][ T3759] do_int80_syscall_32 (arch/x86/entry/common.c:165 arch/x86/entry/common.c:339)
> [ 71.196985][ T3759] entry_INT80_32 (arch/x86/entry/entry_32.S:942)

Re: [PATCH v2 2/2] bpf-next: selftest for TCP_ULP in bpf_setsockopt

[kernel test robot]

Hi zhangmingyi,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]
[also build test ERROR on bpf/master mptcp/export mptcp/export-net linus/master v6.14-rc2 next-20250214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/zhangmingyi/bpf-next-Introduced-to-support-the-ULP-to-get-or-set-sockets/20250210-215203
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20250210134550.3189616-3-zhangmingyi5%40huawei.com
patch subject: [PATCH v2 2/2] bpf-next: selftest for TCP_ULP in bpf_setsockopt
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250215/202502150104.1LKDPqiq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502150104.1LKDPqiq-lkp@intel.com/

All errors (new ones prefixed by >>):

>> progs/setget_sockopt_tcp_ulp.c:18:42: error: use of undeclared identifier 'TCP_ULP'; did you mean 'MCP_UC'?
      18 |                 if (bpf_setsockopt(skops, IPPROTO_TCP, TCP_ULP, (void *)target_ulp,
         |                                                        ^~~~~~~
         |                                                        MCP_UC
   tools/testing/selftests/bpf/tools/include/vmlinux.h:18434:2: note: 'MCP_UC' declared here
    18434 |         MCP_UC = 2,
          |         ^
   progs/setget_sockopt_tcp_ulp.c:22:42: error: use of undeclared identifier 'TCP_ULP'; did you mean 'MCP_UC'?
      22 |                 if (bpf_getsockopt(skops, IPPROTO_TCP, TCP_ULP, verify_ulp,
         |                                                        ^~~~~~~
         |                                                        MCP_UC
   tools/testing/selftests/bpf/tools/include/vmlinux.h:18434:2: note: 'MCP_UC' declared here
    18434 |         MCP_UC = 2,
          |         ^
   2 errors generated.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

[Jakub Kicinski]

On Thu, 13 Feb 2025 22:23:39 -0800 Martin KaFai Lau wrote:
> On 2/13/25 6:13 PM, kernel test robot wrote:
> > [ 71.196846][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
> > [ 71.196856][ T3759] ? __schedule (kernel/sched/core.c:5380)
> > [ 71.196866][ T3759] __mutex_lock (kernel/locking/mutex.c:587 kernel/locking/mutex.c:730)
> > [ 71.196872][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
> > [ 71.196878][ T3759] ? rcu_read_unlock (include/linux/rcupdate.h:335)
> > [ 71.196885][ T3759] ? mark_held_locks (kernel/locking/lockdep.c:4323)
> > [ 71.196889][ T3759] ? lock_sock_nested (net/core/sock.c:3653)
> > [ 71.196898][ T3759] mutex_lock_nested (kernel/locking/mutex.c:783)  
> 
> This is probably because __tcp_set_ulp is now under the rcu_read_lock() in patch 1.
> 
> Even fixing patch 1 will not be enough. The bpf cgrp prog (e.g. sockops) cannot 
> sleep now, so it still cannot call bpf_setsockopt(TCP_ULP, "tls") which will 
> take a mutex. This is a blocker :(

Oh, kbuild bot was nice enough to CC netdev, it wasn't CCed on 
the submission.

I'd really rather we didn't allow setting ULP from BPF unless there 
is a strong and clear use case. The ULP configuration and stacking
is a source of many bugs. And the use case here AFAIU is to allow
attaching some ULP from an OOT module to a socket, which I think
won't make core BPF folks happy either, right?

Re: [PATCH v2 1/2] bpf-next: Introduced to support the ULP to get or set sockets

[Martin KaFai Lau]

On 2/14/25 1:20 PM, Jakub Kicinski wrote:
> On Thu, 13 Feb 2025 22:23:39 -0800 Martin KaFai Lau wrote:
>> On 2/13/25 6:13 PM, kernel test robot wrote:
>>> [ 71.196846][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
>>> [ 71.196856][ T3759] ? __schedule (kernel/sched/core.c:5380)
>>> [ 71.196866][ T3759] __mutex_lock (kernel/locking/mutex.c:587 kernel/locking/mutex.c:730)
>>> [ 71.196872][ T3759] ? tls_init (net/tls/tls_main.c:934 net/tls/tls_main.c:993)
>>> [ 71.196878][ T3759] ? rcu_read_unlock (include/linux/rcupdate.h:335)
>>> [ 71.196885][ T3759] ? mark_held_locks (kernel/locking/lockdep.c:4323)
>>> [ 71.196889][ T3759] ? lock_sock_nested (net/core/sock.c:3653)
>>> [ 71.196898][ T3759] mutex_lock_nested (kernel/locking/mutex.c:783)
>>
>> This is probably because __tcp_set_ulp is now under the rcu_read_lock() in patch 1.
>>
>> Even fixing patch 1 will not be enough. The bpf cgrp prog (e.g. sockops) cannot
>> sleep now, so it still cannot call bpf_setsockopt(TCP_ULP, "tls") which will
>> take a mutex. This is a blocker :(
> 
> Oh, kbuild bot was nice enough to CC netdev, it wasn't CCed on
> the submission.

Ah. I also didn't notice netdev was not cc-ed. will pay attention in the future.

> 
> I'd really rather we didn't allow setting ULP from BPF unless there
> is a strong and clear use case. The ULP configuration and stacking
> is a source of many bugs. And the use case here AFAIU is to allow
> attaching some ULP from an OOT module to a socket, which I think
> won't make core BPF folks happy either, right?

If the in-tree ulp does not work, there is little reason to do it for the 
out-of-tree module only.

My question on the ulp use case went to silence in v1, so we can assume it is 
out-of-tree ulp only. I also asked to replace the "smc" ulp testing with a more 
real "tls" ulp testing to see how it goes first. It does not work as the bot 
reported it.

Re: [PATCH v2 2/2] bpf-next: selftest for TCP_ULP in bpf_setsockopt

[zhangmingyi]

On 2/13/25 3:19 PM, Martin KaFai Lau wrote:
> On 2/10/25 5:45 AM, zhangmingyi wrote:
> > From: Mingyi Zhang <zhangmingyi5@huawei.com>
> > 
> > We try to use bpf_set/getsockopt to set/get TCP_ULP in sockops, and "tls"
> > need connect is established.To avoid impacting other test cases, I 
> > have written a separate test case file.
> > 
> > Signed-off-by: Mingyi Zhang <zhangmingyi5@huawei.com>
> > Signed-off-by: Xin Liu <liuxin350@huawei.com>
> > ---
> >   .../bpf/prog_tests/setget_sockopt_tcp_ulp.c   | 78 +++++++++++++++++++
> >   .../bpf/progs/setget_sockopt_tcp_ulp.c        | 33 ++++++++
> >   2 files changed, 111 insertions(+)
> >   create mode 100644 tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
> >   create mode 100644 
> > tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
> > 
> > diff --git 
> > a/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c 
> > b/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
> > new file mode 100644
> > index 000000000000..748da2c7d255
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/setget_sockopt_tcp_ulp.c
> > @@ -0,0 +1,78 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) Meta Platforms, Inc. and affiliates. */
> 
> This is not right.
> 
> > +
> > +#define _GNU_SOURCE
> > +#include <net/if.h>
> > +
> > +#include "test_progs.h"
> > +#include "network_helpers.h"
> > +
> > +#include "setget_sockopt_tcp_ulp.skel.h"
> > +
> > +#define CG_NAME "/setget-sockopt-tcp-ulp-test"
> > +
> > +static const char addr4_str[] = "127.0.0.1"; static const char 
> > +addr6_str[] = "::1"; static struct setget_sockopt_tcp_ulp *skel; 
> > +static int cg_fd;
> > +
> > +static int create_netns(void)
> > +{
> > +	if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns"))
> > +		return -1;
> > +	if (!ASSERT_OK(system("ip link set dev lo up"), "set lo up"))
> > +		return -1;
> > +	return 0;
> > +}
> > +
> > +static int modprobe_tls(void)
> > +{
> > +	if (!ASSERT_OK(system("modprobe tls"), "tls modprobe failed"))
> > +		return -1;
> > +	return 0;
> > +}
> > +
> > +static void test_tcp_ulp(int family)
> 
> First, the bpf CI still fails to compile for the same reason as v1. You should 
> have received an email from bpf CI bot. Please ensure it is addressed first 
> before reposting. This repeated bpf CI error is an automatic nack.
> 

I'm sorry I didn't notice this and I'll fix this in the next patch

> pw-bot: cr
> 
> The subject tagging should be "[PATCH v2 bpf-next ...] selftests/bpf: ... ". 
> There are many examples to follow in the mailing list if it is not clear.
> 
> Regarding the v1 comment: "...separate it out into its own BPF program..."
> 
> The comment was made at the bpf prog file, "progs/setget_sockopt.c". I meant 
> only create a separate bpf program. The user space part can stay in 
> prog_tests/setget_sockopt.c.
> 
> Move this function, test_tcp_ulp, to prog_tests/setget_sockopt.c. Then all the 
> above config preparation codes and the test_setget_sockopt_tcp_ulp() can be 
> saved. modprobe_tls() is not needed also. Do it after the test_ktls().
> Take a look at test_nonstandard_opt() in prog_tests/setget_sockopt.c.
> It is testing another bpf prog in the same prog_tests/setget_sockopt.c.
>

Well, it seems that I misunderstood you, and I'll change it according to
your intentions.
 
> Also, for the bpf prog, do you really need to test at 
> BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB? If not, just testing at 
> SEC("lsm_cgroup/socket_post_create") will be easier. You can detach the 
> previously attached skel->links.socket_post_create by using bpf_link__destroy() 
> first and then attach yours bpf prog to do the test.

My idea is that since tls needs to be setockopt after the link is established,
it would be easier for me to test BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB in
sockops ebpf program.

> > +{
> > +	struct setget_sockopt_tcp_ulp__bss *bss = skel->bss;
> > +	int sfd, cfd;
> > +
> > +	memset(bss, 0, sizeof(*bss));
> > +	sfd = start_server(family, SOCK_STREAM,
> > +			   family == AF_INET6 ? addr6_str : addr4_str, 0, 0);
> > +	if (!ASSERT_GE(sfd, 0, "start_server"))
> > +		return;
> > +
> > +	cfd = connect_to_fd(sfd, 0);
> > +	if (!ASSERT_GE(cfd, 0, "connect_to_fd_server")) {
> > +		close(sfd);
> > +		return;
> > +	}
> > +	close(sfd);
> > +	close(cfd);
> > +
> > +	ASSERT_EQ(bss->nr_tcp_ulp, 3, "nr_tcp_ulp"); }
> > +
> > +void test_setget_sockopt_tcp_ulp(void) {
> > +	cg_fd = test__join_cgroup(CG_NAME);
> > +	if (cg_fd < 0)
> > +		return;
> > +	if (create_netns() && modprobe_tls())
> > +		goto done;
> > +	skel = setget_sockopt_tcp_ulp__open();
> > +	if (!ASSERT_OK_PTR(skel, "open skel"))
> > +		goto done;
> > +	if (!ASSERT_OK(setget_sockopt_tcp_ulp__load(skel), "load skel"))
> > +		goto done;
> > +	skel->links.skops_sockopt_tcp_ulp =
> > +		bpf_program__attach_cgroup(skel->progs.skops_sockopt_tcp_ulp, cg_fd);
> > +	if (!ASSERT_OK_PTR(skel->links.skops_sockopt_tcp_ulp, "attach_cgroup"))
> > +		goto done;
> > +	test_tcp_ulp(AF_INET);
> > +	test_tcp_ulp(AF_INET6);
> > +done:
> > +	setget_sockopt_tcp_ulp__destroy(skel);
> > +	close(cg_fd);
> > +}
> > diff --git 
> > a/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c 
> > b/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
> > new file mode 100644
> > index 000000000000..bd1009766463
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/setget_sockopt_tcp_ulp.c
> > @@ -0,0 +1,33 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) Meta Platforms, Inc. and affiliates. */
> 
> Same here.
> 
> > +
> > +#include "vmlinux.h"
> > +#include "bpf_tracing_net.h"
> > +#include <bpf/bpf_helpers.h>
> > +
> > +int nr_tcp_ulp;
> > +
> > +SEC("sockops")
> > +int skops_sockopt_tcp_ulp(struct bpf_sock_ops *skops) {
> > +	static const char target_ulp[] = "tls";
> > +	char verify_ulp[sizeof(target_ulp)];
> > +
> > +	switch (skops->op) {
> > +	case BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB:
> > +		if (bpf_setsockopt(skops, IPPROTO_TCP, TCP_ULP, (void *)target_ulp,
> > +							sizeof(target_ulp)) != 0)
> > +			return 1;
> > +		nr_tcp_ulp++;
> > +		if (bpf_getsockopt(skops, IPPROTO_TCP, TCP_ULP, verify_ulp,
> > +							sizeof(verify_ulp)) != 0)
> > +			return 1;
> > +		nr_tcp_ulp++;
> > +		if (bpf_strncmp(verify_ulp, sizeof(target_ulp), "tls") != 0)
> > +			return 1;
> > +		nr_tcp_ulp++;
> > +	}
> > +	return 1;
> > +}
> > +
> > +char _license[] SEC("license") = "GPL";
> > \ No newline at end of file
>