[PATCH mptcp-next 1/3] selftests/bpf: Handle SIGINT when creating netns

Geliang Tang posted 3 patches 5 months ago
There is a newer version of this series
[PATCH mptcp-next 1/3] selftests/bpf: Handle SIGINT when creating netns
Posted by Geliang Tang 5 months ago
From: Geliang Tang <tanggeliang@kylinos.cn>

It's necessary to delete netns during BPF selftests interrupt, otherwise
the next tests run will fail due to unable to create netns.

This patch adds a new SIGINT handle netns_sig_handler() in create_netns(),
and deletes NS_TEST in this handler.

For passing argument to signal handler, a trick mentioned in [1] is
used.

[1]
https://stackoverflow.com/questions/6970224/providing-passing-argument-to-signal-handler

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

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 0b25b008f4f6..7e233b8c75d9 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -519,6 +519,16 @@ void cleanup_netns(struct nstoken *token)
 	close_netns(token);
 }
 
+static int netns_sig_handler(const int sig, void *ptr)
+{
+	struct nstoken *token = (struct nstoken *)ptr;
+
+	signal(sig, SIG_IGN);
+	if (sig == SIGINT)
+		cleanup_netns(token);
+	return 0;
+}
+
 struct nstoken *create_netns(const char *name)
 {
 	struct nstoken *token = NULL;
@@ -539,6 +549,8 @@ struct nstoken *create_netns(const char *name)
 		goto fail;
 	}
 
+	signal(SIGINT, (__sighandler_t)netns_sig_handler);
+	netns_sig_handler(SIGUSR1, (void *)token);
 	return token;
 
 fail:
-- 
2.43.0
Re: [PATCH mptcp-next 1/3] selftests/bpf: Handle SIGINT when creating netns
Posted by Matthieu Baerts 5 months ago
Hi Geliang,

On 15/05/2024 08:44, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> It's necessary to delete netns during BPF selftests interrupt, otherwise
> the next tests run will fail due to unable to create netns.

I think this patch can be part of the series you sent to BPF
maintainers, because that's not specific to MPTCP, no?
(If yes, don't forget to wait 24h between posting).

Or do you send it here to have a pre-review? If yes, please see below:

> This patch adds a new SIGINT handle netns_sig_handler() in create_netns(),
> and deletes NS_TEST in this handler.
> 
> For passing argument to signal handler, a trick mentioned in [1] is
> used.
> 
> [1]
> https://stackoverflow.com/questions/6970224/providing-passing-argument-to-signal-handler

Interesting!

I guess you are referring to this answer, right?

  Link: https://stackoverflow.com/a/43400143 [1]

(instead of sharing the link to the question with multiple answers)

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/bpf/network_helpers.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index 0b25b008f4f6..7e233b8c75d9 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -519,6 +519,16 @@ void cleanup_netns(struct nstoken *token)
>  	close_netns(token);
>  }
>  
> +static int netns_sig_handler(const int sig, void *ptr)
> +{
> +	struct nstoken *token = (struct nstoken *)ptr;
> +
> +	signal(sig, SIG_IGN);
> +	if (sig == SIGINT)
> +		cleanup_netns(token);
> +	return 0;
> +}
> +
>  struct nstoken *create_netns(const char *name)
>  {
>  	struct nstoken *token = NULL;
> @@ -539,6 +549,8 @@ struct nstoken *create_netns(const char *name)
>  		goto fail;
>  	}
>  
> +	signal(SIGINT, (__sighandler_t)netns_sig_handler);
> +	netns_sig_handler(SIGUSR1, (void *)token);

It doesn't look like it will work with multiple netns per test, right?
It sounds a bit restricting to limit to only one netns. Or the helper
name should be clearer about that: create_netns_single()? (not even sure
people will understand this helper can only be used if there is only one
netns to create...)

Can you not use an array (xfrm_info.c seems to be the one creating the
maximum: 3) or a list in a global variable instead?

>  	return token;
>  
>  fail:

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.