netdev - Re: [PATCH net-next 3/6] selftests: tc-testing: use netns delete from pyroute2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231120173515.GD245676@kernel.org>
Date: Mon, 20 Nov 2023 17:35:15 +0000
From: Simon Horman <horms@...nel.org>
To: Pedro Tammela <pctammela@...atatu.com>
Cc: netdev@...r.kernel.org, jhs@...atatu.com, xiyou.wangcong@...il.com,
	jiri@...nulli.us, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, shuah@...nel.org,
	victor@...atatu.com
Subject: Re: [PATCH net-next 3/6] selftests: tc-testing: use netns delete
 from pyroute2

On Fri, Nov 17, 2023 at 02:12:05PM -0300, Pedro Tammela wrote:
> When pyroute2 is available, use the native netns delete routine instead
> of calling iproute2 to do it. As forks are expensive with some kernel
> configs, minimize its usage to avoid kselftests timeouts.
> 
> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>

I have a suggestion for a follow up below, but this change looks good to me.

Reviewed-by: Simon Horman <horms@...nel.org>

> ---
>  .../testing/selftests/tc-testing/plugin-lib/nsPlugin.py  | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
> index 2b8cbfdf1083..920dcbedc395 100644
> --- a/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
> +++ b/tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
> @@ -64,7 +64,10 @@ class SubPlugin(TdcPlugin):
>          if self.args.verbose:
>              print('{}.post_case'.format(self.sub_class))
>  
> -        self._ns_destroy()
> +        if netlink == True:
> +            self._nl_ns_destroy()
> +        else:
> +            self._ns_destroy()

As an aside, I think it would to rename _ns_* to
_iproute2_ns_* or similar, to make the distinction with _nl_ns_* clearer.

>  
>      def post_suite(self, index):
>          if self.args.verbose:
> @@ -174,6 +177,10 @@ class SubPlugin(TdcPlugin):
>          '''
>          self._exec_cmd_batched('pre', self._ns_create_cmds())
>  
> +    def _nl_ns_destroy(self):
> +        ns = self.args.NAMES['NS']
> +        netns.remove(ns)
> +
>      def _ns_destroy_cmd(self):
>          return self._replace_keywords('netns delete {}'.format(self.args.NAMES['NS']))
>  
> -- 
> 2.40.1
>