On Mon, Jun 29, 2020 at 11:13:07AM -0700, Andrii Nakryiko wrote:

On Mon, Jun 29, 2020 at 11:00 AM Martin KaFai Lau [off-list ref] wrote:

quoted

On Sat, Jun 27, 2020 at 01:31:42PM -0700, Andrii Nakryiko wrote:

quoted

On Fri, Jun 26, 2020 at 5:23 PM Martin KaFai Lau [off-list ref] wrote:

quoted

On Fri, Jun 26, 2020 at 03:45:04PM -0700, Andrii Nakryiko wrote:

quoted

On Fri, Jun 26, 2020 at 10:56 AM Martin KaFai Lau [off-list ref] wrote:

quoted

It is common for networking tests creating its netns and making its own
setting under this new netns (e.g. changing tcp sysctl).  If the test
forgot to restore to the original netns, it would affect the
result of other tests.

This patch saves the original netns at the beginning and then restores it
after every test.  Since the restore "setns()" is not expensive, it does it
on all tests without tracking if a test has created a new netns or not.

Signed-off-by: Martin KaFai Lau <redacted>
---
 tools/testing/selftests/bpf/test_progs.c | 21 +++++++++++++++++++++
 tools/testing/selftests/bpf/test_progs.h |  2 ++
 2 files changed, 23 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 54fa5fa688ce..b521ce366381 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c

@@ -121,6 +121,24 @@ static void reset_affinity() {
        }
 }

+static void save_netns(void)
+{
+       env.saved_netns_fd = open("/proc/self/ns/net", O_RDONLY);
+       if (env.saved_netns_fd == -1) {
+               perror("open(/proc/self/ns/net)");
+               exit(-1);
+       }
+}
+
+static void restore_netns(void)
+{
+       if (setns(env.saved_netns_fd, CLONE_NEWNET) == -1) {
+               stdio_restore();
+               perror("setns(CLONE_NEWNS)");
+               exit(-1);
+       }
+}
+
 void test__end_subtest()
 {
        struct prog_test_def *test = env.test;

@@ -643,6 +661,7 @@ int main(int argc, char **argv)
                return -1;
        }

+       save_netns();

you should probably do this also after each sub-test in test__end_subtest()?

You mean restore_netns()?

oops, yeah :)

quoted

It is a tough call.
Some tests may only want to create a netns at the beginning for all the subtests
to use (e.g. sk_assign.c).  restore_netns() after each subtest may catch
tester in surprise that the netns is not in its full control while its
own test is running.

Wouldn't it be better to update such self-tests to setns on each
sub-test properly? It should be a simple code re-use exercise, unless
I'm missing some other implications of having to do it before each
sub-test?

It should be simple, I think.  Haven't looked into details of each test.
However, I won't count re-running the same piece of code in a for-loop
as a re-use exercise ;)

In my vm, a quick try in forcing sk_assign.c to reconfigure netns in each
subtest in the for loop increased the runtime from 1s to 8s.
I guess it is not a big deal for test_progs.

Oh, no, thank you very much, no one needs extra 7 seconds of
test_progs run. Can you please remove reset_affinity() from sub-test
clean up then, and consistently do clean ups only between tests?

Sure.

reset_affinity() has already been called after each test, so should be
fine as is.