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] [day] [month] [year] [list]
Message-ID: <20250228064442.3218835-1-zhangmingyi5@huawei.com>
Date: Fri, 28 Feb 2025 14:44:42 +0800
From: zhangmingyi <zhangmingyi5@...wei.com>
To: <martin.lau@...ux.dev>
CC: <ast@...nel.org>, <daniel@...earbox.net>, <andrii@...nel.org>,
	<song@...nel.org>, <yhs@...com>, <john.fastabend@...il.com>,
	<kpsingh@...nel.org>, <sdf@...gle.com>, <haoluo@...gle.com>,
	<jolsa@...nel.org>, <bpf@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<yanan@...wei.com>, <wuchangye@...wei.com>, <xiesongyang@...wei.com>,
	<liuxin350@...wei.com>, <liwei883@...wei.com>, <tianmuyang@...wei.com>,
	<zhangmingyi5@...wei.com>
Subject: Re: [PATCH v2 2/2] bpf-next: selftest for TCP_ULP in bpf_setsockopt

On 2/13/25 3:19 PM, Martin KaFai Lau wrote:
> On 2/10/25 5:45 AM, zhangmingyi wrote:
> > From: Mingyi Zhang <zhangmingyi5@...wei.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@...wei.com>
> > Signed-off-by: Xin Liu <liuxin350@...wei.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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ