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