1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v7 0/9] refactor mptcp bpf tests
  4. View patch

[PATCH mptcp-next v7 2/9] Squash to "selftests/bpf: Add bpf scheduler test" 1 verify

Geliang Tang posted 9 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH mptcp-next v7 2/9] Squash to "selftests/bpf: Add bpf scheduler test" 1 verify
Posted by Geliang Tang 1 year, 10 months ago 
From: Geliang Tang <tanggeliang@kylinos.cn>

Add send_data_and_verify helper to avoid duplicated code.

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

diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index c29c81239603..5080e680fbe0 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -457,23 +457,44 @@ static int has_bytes_sent(char *addr)
 	return system(cmd);
 }
 
-static void test_default(void)
+static void send_data_and_verify(char *sched, char *addr1, char *addr2)
 {
 	int server_fd, client_fd;
-	struct nstoken *nstoken;
 
-	nstoken = sched_init("subflow", "default");
-	if (!ASSERT_OK_PTR(nstoken, "sched_init:default"))
-		goto fail;
 	server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
+	if (CHECK(server_fd < 0, sched, "start_mptcp_server: %d\n", errno))
+		return;
+
 	client_fd = connect_to_fd(server_fd, 0);
+	if (CHECK(client_fd < 0, sched, "connect_to_fd: %d\n", errno))
+		goto close_server;
 
-	send_data(server_fd, client_fd, "default");
-	ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr_1");
-	ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent addr_2");
+	send_data(server_fd, client_fd, sched);
+
+	if (!strcmp(addr1, "WITH_DATA"))
+		CHECK(has_bytes_sent(ADDR_1), sched, "should have bytes_sent on addr1\n");
+	else if (!strcmp(addr1, "WITHOUT_DATA"))
+		CHECK(!has_bytes_sent(ADDR_1), sched, "shouldn't have bytes_sent on addr1\n");
+	if (!strcmp(addr2, "WITH_DATA"))
+		CHECK(has_bytes_sent(ADDR_2), sched, "should have bytes_sent on addr2\n");
+	else if (!strcmp(addr2, "WITHOUT_DATA"))
+		CHECK(!has_bytes_sent(ADDR_2), sched, "shouldn't have bytes_sent on addr2\n");
 
 	close(client_fd);
+close_server:
 	close(server_fd);
+}
+
+static void test_default(void)
+{
+	struct nstoken *nstoken;
+
+	nstoken = sched_init("subflow", "default");
+	if (!ASSERT_OK_PTR(nstoken, "sched_init:default"))
+		goto fail;
+
+	send_data_and_verify("default", "WITH_DATA", "WITH_DATA");
+
 fail:
 	cleanup_netns(nstoken);
 }
@@ -663,8 +684,7 @@ void test_mptcp(void)
 {
 	RUN_MPTCP_TEST(base);
 	RUN_MPTCP_TEST(mptcpify);
-	if (test__start_subtest("default"))
-		test_default();
+	RUN_MPTCP_TEST(default);
 	if (test__start_subtest("first"))
 		test_first();
 	if (test__start_subtest("bkup"))
-- 
2.40.1
Re: [PATCH mptcp-next v7 2/9] Squash to "selftests/bpf: Add bpf scheduler test" 1 verify
Posted by Matthieu Baerts 1 year, 10 months ago 
Hi Geliang,

On 08/04/2024 05:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add send_data_and_verify helper to avoid duplicated code.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 40 ++++++++++++++-----
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index c29c81239603..5080e680fbe0 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -457,23 +457,44 @@ static int has_bytes_sent(char *addr)
>  	return system(cmd);
>  }
>  
> -static void test_default(void)
> +static void send_data_and_verify(char *sched, char *addr1, char *addr2)
>  {
>  	int server_fd, client_fd;
> -	struct nstoken *nstoken;
>  
> -	nstoken = sched_init("subflow", "default");
> -	if (!ASSERT_OK_PTR(nstoken, "sched_init:default"))
> -		goto fail;
>  	server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
> +	if (CHECK(server_fd < 0, sched, "start_mptcp_server: %d\n", errno))
> +		return;
> +
>  	client_fd = connect_to_fd(server_fd, 0);
> +	if (CHECK(client_fd < 0, sched, "connect_to_fd: %d\n", errno))
> +		goto close_server;
>  
> -	send_data(server_fd, client_fd, "default");
> -	ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr_1");
> -	ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent addr_2");
> +	send_data(server_fd, client_fd, sched);
> +
> +	if (!strcmp(addr1, "WITH_DATA"))

Out of curiosity, why did you use strings? Why not using booleans? (or a
bitmask)

  #define WITH_DATA true
  static void send_data_and_verify(char *sched, bool addr1_with_data,
                                   bool addr1_with_data)

  send_data_and_verify("...", WITH_DATA, !WITH_DATA);


I guess that's fine for the tests, just strange in C :)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v7 2/9] Squash to "selftests/bpf: Add bpf scheduler test" 1 verify
Posted by Geliang Tang 1 year, 10 months ago 
Hi Matt,

On Mon, 2024-04-08 at 21:58 +0200, Matthieu Baerts wrote:
> > Hi Geliang,
> > 
> > On 08/04/2024 05:01, Geliang Tang wrote:
> > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > 
> > > > Add send_data_and_verify helper to avoid duplicated code.
> > > > 
> > > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > > > ---
> > > >  .../testing/selftests/bpf/prog_tests/mptcp.c  | 40
> > > > ++++++++++++++-----
> > > >  1 file changed, 30 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > index c29c81239603..5080e680fbe0 100644
> > > > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > @@ -457,23 +457,44 @@ static int has_bytes_sent(char *addr)
> > > >   return system(cmd);
> > > >  }
> > > >  
> > > > -static void test_default(void)
> > > > +static void send_data_and_verify(char *sched, char *addr1,
> > > > char
> > > > *addr2)
> > > >  {
> > > >   int server_fd, client_fd;
> > > > - struct nstoken *nstoken;
> > > >  
> > > > - nstoken = sched_init("subflow", "default");
> > > > - if (!ASSERT_OK_PTR(nstoken, "sched_init:default"))
> > > > - goto fail;
> > > >   server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
> > > > + if (CHECK(server_fd < 0, sched, "start_mptcp_server: %d\n",
> > > > errno))
> > > > + return;
> > > > +
> > > >   client_fd = connect_to_fd(server_fd, 0);
> > > > + if (CHECK(client_fd < 0, sched, "connect_to_fd: %d\n",
> > > > errno))
> > > > + goto close_server;
> > > >  
> > > > - send_data(server_fd, client_fd, "default");
> > > > - ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr_1");
> > > > - ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent addr_2");
> > > > + send_data(server_fd, client_fd, sched);
> > > > +
> > > > + if (!strcmp(addr1, "WITH_DATA"))
> > 
> > Out of curiosity, why did you use strings? Why not using booleans?
> > (or a
> > bitmask)
> > 

Since arguments addr1 and addr2 of macro MPTCP_SCHED_TEST are strings:

#define MPTCP_SCHED_TEST(sched, addr1, addr2)                   \
static void test_##sched(void)                                  \
{                                                               \
...
        send_data_and_verify(#sched, #addr1, #addr2);           \
...

It's sample to use string arguments for send_data_and_verify too.

Thanks,
-Geliang

> >   #define WITH_DATA true
> >   static void send_data_and_verify(char *sched, bool
> > addr1_with_data,
> >                                    bool addr1_with_data)
> > 
> >   send_data_and_verify("...", WITH_DATA, !WITH_DATA);
> > 
> > 
> > I guess that's fine for the tests, just strange in C :)
> > 
> > Cheers,
> > Matt
Re: [PATCH mptcp-next v7 2/9] Squash to "selftests/bpf: Add bpf scheduler test" 1 verify
Posted by Matthieu Baerts 1 year, 10 months ago 
Hi Geliang,

On 09/04/2024 14:42, Geliang Tang wrote:
> Hi Matt,
> 
> On Mon, 2024-04-08 at 21:58 +0200, Matthieu Baerts wrote:
>>> Hi Geliang,
>>>
>>> On 08/04/2024 05:01, Geliang Tang wrote:
>>>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>>>
>>>>> Add send_data_and_verify helper to avoid duplicated code.
>>>>>
>>>>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>>>>> ---
>>>>>  .../testing/selftests/bpf/prog_tests/mptcp.c  | 40
>>>>> ++++++++++++++-----
>>>>>  1 file changed, 30 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>>>> b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>>>> index c29c81239603..5080e680fbe0 100644
>>>>> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>>>> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
>>>>> @@ -457,23 +457,44 @@ static int has_bytes_sent(char *addr)
>>>>>   return system(cmd);
>>>>>  }
>>>>>  
>>>>> -static void test_default(void)
>>>>> +static void send_data_and_verify(char *sched, char *addr1,
>>>>> char
>>>>> *addr2)
>>>>>  {
>>>>>   int server_fd, client_fd;
>>>>> - struct nstoken *nstoken;
>>>>>  
>>>>> - nstoken = sched_init("subflow", "default");
>>>>> - if (!ASSERT_OK_PTR(nstoken, "sched_init:default"))
>>>>> - goto fail;
>>>>>   server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1, 0);
>>>>> + if (CHECK(server_fd < 0, sched, "start_mptcp_server: %d\n",
>>>>> errno))
>>>>> + return;
>>>>> +
>>>>>   client_fd = connect_to_fd(server_fd, 0);
>>>>> + if (CHECK(client_fd < 0, sched, "connect_to_fd: %d\n",
>>>>> errno))
>>>>> + goto close_server;
>>>>>  
>>>>> - send_data(server_fd, client_fd, "default");
>>>>> - ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent addr_1");
>>>>> - ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent addr_2");
>>>>> + send_data(server_fd, client_fd, sched);
>>>>> +
>>>>> + if (!strcmp(addr1, "WITH_DATA"))
>>>
>>> Out of curiosity, why did you use strings? Why not using booleans?
>>> (or a
>>> bitmask)
>>>
> 
> Since arguments addr1 and addr2 of macro MPTCP_SCHED_TEST are strings:

They are strings because you asked the preprocessor to convert them as
string ...

> #define MPTCP_SCHED_TEST(sched, addr1, addr2)                   \
> static void test_##sched(void)                                  \
> {                                                               \
> ...
>         send_data_and_verify(#sched, #addr1, #addr2);           \

... here, by using "#addr1" and "#addr2".

Why not simply removing the "#" not to do any conversion?

e.g.

================================= 8< =================================
#include <stdio.h>
#include <stdbool.h>

static void send_data_and_verify(char *sched, bool addr1, bool addr2)
{
	printf("%s: %d %d\n", sched, addr1, addr2);
}

#define MPTCP_SCHED_TEST(sched, addr1, addr2)		\
static void test_##sched(void)				\
{							\
	send_data_and_verify(#sched, addr1, addr2);	\
}

#define WITH_DATA true
MPTCP_SCHED_TEST(first, WITH_DATA, !WITH_DATA);

int main()
{
	test_first();
}
================================= 8< =================================

This prints 'first: 1 0', what you want, no?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v7 2/9] Squash to "selftests/bpf: Add bpf scheduler test" 1 verify
Posted by Geliang Tang 1 year, 10 months ago 
Hi Matt,

On Tue, 2024-04-09 at 17:20 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 09/04/2024 14:42, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Mon, 2024-04-08 at 21:58 +0200, Matthieu Baerts wrote:
> > > > Hi Geliang,
> > > > 
> > > > On 08/04/2024 05:01, Geliang Tang wrote:
> > > > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > > > 
> > > > > > Add send_data_and_verify helper to avoid duplicated code.
> > > > > > 
> > > > > > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > > > > > ---
> > > > > >  .../testing/selftests/bpf/prog_tests/mptcp.c  | 40
> > > > > > ++++++++++++++-----
> > > > > >  1 file changed, 30 insertions(+), 10 deletions(-)
> > > > > > 
> > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > > > b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > > > index c29c81239603..5080e680fbe0 100644
> > > > > > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > > > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> > > > > > @@ -457,23 +457,44 @@ static int has_bytes_sent(char *addr)
> > > > > >   return system(cmd);
> > > > > >  }
> > > > > >  
> > > > > > -static void test_default(void)
> > > > > > +static void send_data_and_verify(char *sched, char *addr1,
> > > > > > char
> > > > > > *addr2)
> > > > > >  {
> > > > > >   int server_fd, client_fd;
> > > > > > - struct nstoken *nstoken;
> > > > > >  
> > > > > > - nstoken = sched_init("subflow", "default");
> > > > > > - if (!ASSERT_OK_PTR(nstoken, "sched_init:default"))
> > > > > > - goto fail;
> > > > > >   server_fd = start_mptcp_server(AF_INET, ADDR_1, PORT_1,
> > > > > > 0);
> > > > > > + if (CHECK(server_fd < 0, sched, "start_mptcp_server:
> > > > > > %d\n",
> > > > > > errno))
> > > > > > + return;
> > > > > > +
> > > > > >   client_fd = connect_to_fd(server_fd, 0);
> > > > > > + if (CHECK(client_fd < 0, sched, "connect_to_fd: %d\n",
> > > > > > errno))
> > > > > > + goto close_server;
> > > > > >  
> > > > > > - send_data(server_fd, client_fd, "default");
> > > > > > - ASSERT_OK(has_bytes_sent(ADDR_1), "has_bytes_sent
> > > > > > addr_1");
> > > > > > - ASSERT_OK(has_bytes_sent(ADDR_2), "has_bytes_sent
> > > > > > addr_2");
> > > > > > + send_data(server_fd, client_fd, sched);
> > > > > > +
> > > > > > + if (!strcmp(addr1, "WITH_DATA"))
> > > > 
> > > > Out of curiosity, why did you use strings? Why not using
> > > > booleans?
> > > > (or a
> > > > bitmask)
> > > > 
> > 
> > Since arguments addr1 and addr2 of macro MPTCP_SCHED_TEST are
> > strings:
> 
> They are strings because you asked the preprocessor to convert them
> as
> string ...
> 
> > #define MPTCP_SCHED_TEST(sched, addr1, addr2)                   \
> > static void test_##sched(void)                                  \
> > {                                                               \
> > ...
> >         send_data_and_verify(#sched, #addr1, #addr2);           \
> 
> ... here, by using "#addr1" and "#addr2".
> 
> Why not simply removing the "#" not to do any conversion?
> 
> e.g.
> 
> ================================= 8<
> =================================
> #include <stdio.h>
> #include <stdbool.h>
> 
> static void send_data_and_verify(char *sched, bool addr1, bool addr2)
> {
> 	printf("%s: %d %d\n", sched, addr1, addr2);
> }
> 
> #define MPTCP_SCHED_TEST(sched, addr1, addr2)		\
> static void test_##sched(void)				\
> {							\
> 	send_data_and_verify(#sched, addr1, addr2);	\
> }
> 
> #define WITH_DATA true
> MPTCP_SCHED_TEST(first, WITH_DATA, !WITH_DATA);
> 
> int main()
> {
> 	test_first();
> }
> ================================= 8<
> =================================
> 
> This prints 'first: 1 0', what you want, no?

Thank you for providing such a detailed example. I have updated it to
v8.

-Geliang

> 
> Cheers,
> Matt

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.