MPTCP Archive mirror
 help / color / mirror / Atom feed
From: Matthieu Baerts <matttbe@kernel.org>
To: Geliang Tang <geliang@kernel.org>, mptcp@lists.linux.dev
Cc: Geliang Tang <tanggeliang@kylinos.cn>
Subject: Re: [PATCH mptcp-next 1/3] selftests/bpf: Handle SIGINT when creating netns
Date: Wed, 15 May 2024 09:32:48 +0200	[thread overview]
Message-ID: <774d7113-3517-45b9-b0fa-606a92914b70@kernel.org> (raw)
In-Reply-To: <8c4198e7855045559ab746be7d90f72fd550a857.1715755275.git.tanggeliang@kylinos.cn>

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.


  reply	other threads:[~2024-05-15  7:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-15  6:44 [PATCH mptcp-next 0/3] fixes for create_netns Geliang Tang
2024-05-15  6:44 ` [PATCH mptcp-next 1/3] selftests/bpf: Handle SIGINT when creating netns Geliang Tang
2024-05-15  7:32   ` Matthieu Baerts [this message]
2024-05-15  6:44 ` [PATCH mptcp-next 2/3] Squash to "selftests/bpf: Add mptcp subflow subtest" Geliang Tang
2024-05-15  6:44 ` [PATCH mptcp-next 3/3] Squash to "selftests/bpf: Add bpf scheduler test" Geliang Tang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=774d7113-3517-45b9-b0fa-606a92914b70@kernel.org \
    --to=matttbe@kernel.org \
    --cc=geliang@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=tanggeliang@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).