diff mbox series

[mptcp-next,v8,4/9] Squash to "selftests/bpf: Add bpf scheduler test" 3 MPTCP_SCHED_TEST

Message ID ad3d2ce4863dc542d754471d8e566c084a0bc52c.1712714407.git.tanggeliang@kylinos.cn (mailing list archive)
State Superseded, archived
Delegated to: Mat Martineau
Headers show
Series refactor mptcp bpf tests | expand

Checks

Context Check Description
matttbe/build success Build and static analysis OK
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 36 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf__only_bpftest_all_ success Success! ✅

Commit Message

Geliang Tang April 10, 2024, 2:05 a.m. UTC 
From: Geliang Tang <tanggeliang@kylinos.cn>

Please append this into commit log:

'''
This patch defines MPTCP_SCHED_TEST macro, a template for all scheduler
tests. Every scheduler is identified by argument name, and use sysctl
to set net.mptcp.scheduler as "bpf_name" to use this sched. Add two
veth net devices to simulate the multiple addresses case. Use 'ip mptcp
endpoint' command to add the new endpoint ADDR2 to PM netlink. Arguments
addr1/add2 means whether the data has been sent on the first/second subflow
or not. Send data and check bytes_sent of 'ss' output after it using
send_data_and_verify().
'''

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/bpf/prog_tests/mptcp.c  | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

Mat Martineau April 12, 2024, 11:30 p.m. UTC | #1 
On Wed, 10 Apr 2024, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Please append this into commit log:
>
> '''
> This patch defines MPTCP_SCHED_TEST macro, a template for all scheduler
> tests. Every scheduler is identified by argument name, and use sysctl
> to set net.mptcp.scheduler as "bpf_name" to use this sched. Add two
> veth net devices to simulate the multiple addresses case. Use 'ip mptcp
> endpoint' command to add the new endpoint ADDR2 to PM netlink. Arguments
> addr1/add2 means whether the data has been sent on the first/second subflow
> or not. Send data and check bytes_sent of 'ss' output after it using
> send_data_and_verify().
> '''
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> .../testing/selftests/bpf/prog_tests/mptcp.c  | 30 +++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index e8df18c28961..0144db1425ed 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -500,6 +500,36 @@ static void test_default(void)
> 	cleanup_netns(nstoken);
> }
>
> +#define MPTCP_SCHED_TEST(sched, addr1, addr2)			\
Hi Geliang -

I saw Matthieu's question on v6 about the use of this large macro. 
(https://lore.kernel.org/mptcp/f44d5960-8aa3-4914-bae3-4f0c072e07b4@kernel.org/)

I agree with him that some common code could be factored out without 
needing the big macro.

> +static void test_##sched(void)					\
> +{								\
> +	struct mptcp_bpf_##sched *skel;				\

This struct is specific to each scheduler's skeleton header

> +	struct nstoken *nstoken;				\
> +	struct bpf_link *link;					\
> +	struct bpf_map *map;					\
> +								\
> +	skel = mptcp_bpf_##sched##__open_and_load();		\
> +	if (!ASSERT_OK_PTR(skel, "open_and_load:" #sched))	\
> +		return;						\

and so is this __open_and_load() function call. However, only skel->obj is 
used after this point (except for the final line) and it is generic. So 
the code below here can be refactored as regular code to reduce the 
boilerplate.

> +								\
> +	map = bpf_object__find_map_by_name(skel->obj, #sched);	\
> +	link = bpf_map__attach_struct_ops(map);			\
> +	if (!ASSERT_OK_PTR(link, "attach_struct_ops:" #sched))	\
> +		goto skel_destroy;				\
> +								\
> +	nstoken = sched_init("subflow", "bpf_" #sched);		\
> +	if (!ASSERT_OK_PTR(nstoken, "sched_init:" #sched))	\
> +		goto link_destroy;				\
> +								\
> +	send_data_and_verify(#sched, addr1, addr2);		\
> +								\
> +	cleanup_netns(nstoken);					\
> +link_destroy:							\
> +	bpf_link__destroy(link);				\

Boilerplate ends here.

> +skel_destroy:							\
> +	mptcp_bpf_##sched##__destroy(skel);			\
> +}
> +

I think non-macro refactoring would also help in creating future 
scheduler-specific tests.


- Mat
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index e8df18c28961..0144db1425ed 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -500,6 +500,36 @@  static void test_default(void)
 	cleanup_netns(nstoken);
 }
 
+#define MPTCP_SCHED_TEST(sched, addr1, addr2)			\
+static void test_##sched(void)					\
+{								\
+	struct mptcp_bpf_##sched *skel;				\
+	struct nstoken *nstoken;				\
+	struct bpf_link *link;					\
+	struct bpf_map *map;					\
+								\
+	skel = mptcp_bpf_##sched##__open_and_load();		\
+	if (!ASSERT_OK_PTR(skel, "open_and_load:" #sched))	\
+		return;						\
+								\
+	map = bpf_object__find_map_by_name(skel->obj, #sched);	\
+	link = bpf_map__attach_struct_ops(map);			\
+	if (!ASSERT_OK_PTR(link, "attach_struct_ops:" #sched))	\
+		goto skel_destroy;				\
+								\
+	nstoken = sched_init("subflow", "bpf_" #sched);		\
+	if (!ASSERT_OK_PTR(nstoken, "sched_init:" #sched))	\
+		goto link_destroy;				\
+								\
+	send_data_and_verify(#sched, addr1, addr2);		\
+								\
+	cleanup_netns(nstoken);					\
+link_destroy:							\
+	bpf_link__destroy(link);				\
+skel_destroy:							\
+	mptcp_bpf_##sched##__destroy(skel);			\
+}
+
 static void test_first(void)
 {
 	struct mptcp_bpf_first *first_skel;