Re: [LTP] [PATCH V2] container: new testcase pidns32
Hi! Hi, I'm still not entirely convinced about this test. So am I. What about checking that we can do at least MAXNEST levels? -- Cyril Hrubis chru...@suse.cz -- ___ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list
Re: [LTP] [PATCH V2] container: new testcase pidns32
Hi Jan,
See in-line comments.
On 2015/8/11 16:05, Jan Stancek wrote:
- Original Message -
From: Yuan Sun sunyu...@huawei.com
To: jstan...@redhat.com
Cc: ltp-list@lists.sourceforge.net
Sent: Friday, 7 August, 2015 6:55:08 AM
Subject: [PATCH V2] container: new testcase pidns32
Kernel imposes a limit of 32 nested levels of pid namespaces.
Signed-off-by: Yuan Sun sunyu...@huawei.com
---
runtest/containers | 1 +
testcases/kernel/containers/pidns/.gitignore | 1 +
testcases/kernel/containers/pidns/pidns32.c | 96
3 files changed, 98 insertions(+)
create mode 100644 testcases/kernel/containers/pidns/pidns32.c
diff --git a/runtest/containers b/runtest/containers
index 9dc44bc..ac37ab2 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -13,6 +13,7 @@ pidns17 pidns17
pidns20 pidns20
pidns30 pidns30
pidns31 pidns31
+pidns32 pidns32
mqns_01 mqns_01
mqns_01_clone mqns_01 -clone
diff --git a/testcases/kernel/containers/pidns/.gitignore
b/testcases/kernel/containers/pidns/.gitignore
index e56c1f9..488b045 100644
--- a/testcases/kernel/containers/pidns/.gitignore
+++ b/testcases/kernel/containers/pidns/.gitignore
@@ -12,3 +12,4 @@
/pidns20
/pidns30
/pidns31
+/pidns32
diff --git a/testcases/kernel/containers/pidns/pidns32.c
b/testcases/kernel/containers/pidns/pidns32.c
new file mode 100644
index 000..d0d3e1e
--- /dev/null
+++ b/testcases/kernel/containers/pidns/pidns32.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) Huawei Technologies Co., Ltd., 2015
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ */
+
+/*
Hi,
I'm still not entirely convinced about this test.
+ * Verify that:
+ * The kernel imposes a limit of 32 nested levels of pid namespaces.
If it doesn't, is that a bad thing? Are there some adverse effects, when
such limitation is lacking?
It is a kernel feature. The following information is from
http://www.serverphorums.com/read.php?12,580615
'struct pid' is a variable sized struct - a header with an array
of upids at the end.
A size of the array depends on a level (depth) of pid namespaces. Now
a level of pidns is not limited, so 'struct pid' can be more than one
page.
Looks reasonable, that it should be less than a page. MAX_PIS_NS_LEVEL
is not calculated from PAGE_SIZE, because in this case it depends on
architectures, config options and it will be reduced, if someone adds a
new fields in struct pid or struct upid.
I suggest to set MAX_PID_NS_LEVEL = 32, because it saves ability to
expand struct pid and it's more than enough for all known for me
use-cases. When someone finds a reasonable use case, we can add a
config option or a sysctl parameter.
In addition it will reduce effect of another problem, when we have many
nested namespaces and the oldest one starts dying. zap_pid_ns_processe
will be called for each namespace and find_vpid will be called for each
process in a namespace. find_vpid will be called minimum max_level^2 / 2
times. The reason of that is that when we found a bit in pidmap, we
can't determine this pidns is top for this process or it isn't.
vpid is a heavy operation, so a fork bomb, which create many nested
namespace, can do a system inaccessible for a long time.
+ */
+
+#define _GNU_SOURCE
+#include sys/wait.h
+#include assert.h
+#include stdio.h
+#include stdlib.h
+#include unistd.h
+#include string.h
+#include errno.h
+#include test.h
+#include pidns_helper.h
+
+#define MAXNEST 32
+
+char *TCID = pidns32;
+int TST_TOTAL = 1;
+
+static void setup(void)
+{
+if (tst_kvercmp(3, 7, 0) 0)
+tst_brkm(TCONF, NULL, nest depth limitation not supported);
+tst_require_root();
+check_newpid();
+tst_tmpdir();
+}
+
+static void cleanup(void)
+{
+tst_rmdir();
+}
+
+static int child_fn1(void *arg)
+{
+pid_t cpid1;
+long level = (long)arg;
+
+cpid1 = ltp_clone_quick(CLONE_NEWPID | SIGCHLD,
+(void *)child_fn1, (void *)(level + 1));
You are relying on fact that clone will fail at MAXNEST levels.
If it doesn't then this keeps making child processes presumably
until you hit OOM.
First action of new child should be to check current level. If it's
over, test can end there.
Yes, you are right. I will update the code.
+if (cpid1 0) {
+if (level == MAXNEST)
+return 0;
+return 1;
+}
+if (waitpid(cpid1, NULL, 0) == -1)
+
Re: [LTP] [PATCH V2] container: new testcase pidns32
- Original Message -
From: Yuan Sun sunyu...@huawei.com
To: jstan...@redhat.com
Cc: ltp-list@lists.sourceforge.net
Sent: Friday, 7 August, 2015 6:55:08 AM
Subject: [PATCH V2] container: new testcase pidns32
Kernel imposes a limit of 32 nested levels of pid namespaces.
Signed-off-by: Yuan Sun sunyu...@huawei.com
---
runtest/containers | 1 +
testcases/kernel/containers/pidns/.gitignore | 1 +
testcases/kernel/containers/pidns/pidns32.c | 96
3 files changed, 98 insertions(+)
create mode 100644 testcases/kernel/containers/pidns/pidns32.c
diff --git a/runtest/containers b/runtest/containers
index 9dc44bc..ac37ab2 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -13,6 +13,7 @@ pidns17 pidns17
pidns20 pidns20
pidns30 pidns30
pidns31 pidns31
+pidns32 pidns32
mqns_01 mqns_01
mqns_01_clone mqns_01 -clone
diff --git a/testcases/kernel/containers/pidns/.gitignore
b/testcases/kernel/containers/pidns/.gitignore
index e56c1f9..488b045 100644
--- a/testcases/kernel/containers/pidns/.gitignore
+++ b/testcases/kernel/containers/pidns/.gitignore
@@ -12,3 +12,4 @@
/pidns20
/pidns30
/pidns31
+/pidns32
diff --git a/testcases/kernel/containers/pidns/pidns32.c
b/testcases/kernel/containers/pidns/pidns32.c
new file mode 100644
index 000..d0d3e1e
--- /dev/null
+++ b/testcases/kernel/containers/pidns/pidns32.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) Huawei Technologies Co., Ltd., 2015
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ */
+
+/*
Hi,
I'm still not entirely convinced about this test.
+ * Verify that:
+ * The kernel imposes a limit of 32 nested levels of pid namespaces.
If it doesn't, is that a bad thing? Are there some adverse effects, when
such limitation is lacking?
+ */
+
+#define _GNU_SOURCE
+#include sys/wait.h
+#include assert.h
+#include stdio.h
+#include stdlib.h
+#include unistd.h
+#include string.h
+#include errno.h
+#include test.h
+#include pidns_helper.h
+
+#define MAXNEST 32
+
+char *TCID = pidns32;
+int TST_TOTAL = 1;
+
+static void setup(void)
+{
+ if (tst_kvercmp(3, 7, 0) 0)
+ tst_brkm(TCONF, NULL, nest depth limitation not supported);
+ tst_require_root();
+ check_newpid();
+ tst_tmpdir();
+}
+
+static void cleanup(void)
+{
+ tst_rmdir();
+}
+
+static int child_fn1(void *arg)
+{
+ pid_t cpid1;
+ long level = (long)arg;
+
+ cpid1 = ltp_clone_quick(CLONE_NEWPID | SIGCHLD,
+ (void *)child_fn1, (void *)(level + 1));
You are relying on fact that clone will fail at MAXNEST levels.
If it doesn't then this keeps making child processes presumably
until you hit OOM.
First action of new child should be to check current level. If it's
over, test can end there.
+ if (cpid1 0) {
+ if (level == MAXNEST)
+ return 0;
+ return 1;
+ }
+ if (waitpid(cpid1, NULL, 0) == -1)
+ return 1;
If you ignore status in waitpid() call, how do you know that child at
MAXNEXT level failed test condition?
Regards,
Jan
+ if (level MAXNEST) {
+ printf(MAX_PIS_NS_LEVEL doestn't take effect\n);
+ return 1;
+ }
+ return 0;
+}
+
+static void test_max_nest(void)
+{
+ pid_t cpid1;
+
+ cpid1 = ltp_clone_quick(CLONE_NEWPID | SIGCHLD,
+ (void *)child_fn1, (void *)1);
+ if (cpid1 0)
+ tst_brkm(TBROK | TERRNO, cleanup, clone failed);
+
+ tst_record_childstatus(cleanup, cpid1);
+}
+
+int main(int argc, char *argv[])
+{
+ int lc;
+
+ setup();
+ tst_parse_opts(argc, argv, NULL, NULL);
+
+ for (lc = 0; TEST_LOOPING(lc); lc++) {
+ tst_count = 0;
+ test_max_nest();
+ }
+
+ cleanup();
+ tst_exit();
+}
--
1.9.1
--
___
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
Re: [LTP] [PATCH V2] container: new testcase pidns32
Hi Jan and Cyril, I have send a message in reply. What is your opinion? If you convinced about this test, I will update the code. Or it is unnecessary to update the code. Thanks. Yuan On 2015/8/11 16:33, Cyril Hrubis wrote: Hi! Hi, I'm still not entirely convinced about this test. So am I. What about checking that we can do at least MAXNEST levels? -- ___ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list
Re: [LTP] [PATCH V2] container: new testcase pidns32
- Original Message -
From: Yuan Sun sunyu...@huawei.com
To: Jan Stancek jstan...@redhat.com
Cc: ltp-list@lists.sourceforge.net
Sent: Tuesday, 11 August, 2015 10:43:02 AM
Subject: Re: [PATCH V2] container: new testcase pidns32
Hi Jan,
See in-line comments.
On 2015/8/11 16:05, Jan Stancek wrote:
- Original Message -
From: Yuan Sun sunyu...@huawei.com
To: jstan...@redhat.com
Cc: ltp-list@lists.sourceforge.net
Sent: Friday, 7 August, 2015 6:55:08 AM
Subject: [PATCH V2] container: new testcase pidns32
Kernel imposes a limit of 32 nested levels of pid namespaces.
Signed-off-by: Yuan Sun sunyu...@huawei.com
---
runtest/containers | 1 +
testcases/kernel/containers/pidns/.gitignore | 1 +
testcases/kernel/containers/pidns/pidns32.c | 96
3 files changed, 98 insertions(+)
create mode 100644 testcases/kernel/containers/pidns/pidns32.c
diff --git a/runtest/containers b/runtest/containers
index 9dc44bc..ac37ab2 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -13,6 +13,7 @@ pidns17 pidns17
pidns20 pidns20
pidns30 pidns30
pidns31 pidns31
+pidns32 pidns32
mqns_01 mqns_01
mqns_01_clone mqns_01 -clone
diff --git a/testcases/kernel/containers/pidns/.gitignore
b/testcases/kernel/containers/pidns/.gitignore
index e56c1f9..488b045 100644
--- a/testcases/kernel/containers/pidns/.gitignore
+++ b/testcases/kernel/containers/pidns/.gitignore
@@ -12,3 +12,4 @@
/pidns20
/pidns30
/pidns31
+/pidns32
diff --git a/testcases/kernel/containers/pidns/pidns32.c
b/testcases/kernel/containers/pidns/pidns32.c
new file mode 100644
index 000..d0d3e1e
--- /dev/null
+++ b/testcases/kernel/containers/pidns/pidns32.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) Huawei Technologies Co., Ltd., 2015
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ */
+
+/*
Hi,
I'm still not entirely convinced about this test.
+ * Verify that:
+ * The kernel imposes a limit of 32 nested levels of pid namespaces.
If it doesn't, is that a bad thing? Are there some adverse effects, when
such limitation is lacking?
It is a kernel feature.
My concern is that it's not documented, and can be changed at any time.
There's no way to learn what that value is, unless you look at source code.
As suggested by Cyril, test could verify that we can create at least MAXNEST-1
pid
namespaces and stop there. What do you think?
Regards,
Jan
The following information is from
http://www.serverphorums.com/read.php?12,580615
'struct pid' is a variable sized struct - a header with an array
of upids at the end.
A size of the array depends on a level (depth) of pid namespaces. Now
a level of pidns is not limited, so 'struct pid' can be more than one
page.
Looks reasonable, that it should be less than a page. MAX_PIS_NS_LEVEL
is not calculated from PAGE_SIZE, because in this case it depends on
architectures, config options and it will be reduced, if someone adds a
new fields in struct pid or struct upid.
I suggest to set MAX_PID_NS_LEVEL = 32, because it saves ability to
expand struct pid and it's more than enough for all known for me
use-cases. When someone finds a reasonable use case, we can add a
config option or a sysctl parameter.
In addition it will reduce effect of another problem, when we have many
nested namespaces and the oldest one starts dying. zap_pid_ns_processe
will be called for each namespace and find_vpid will be called for each
process in a namespace. find_vpid will be called minimum max_level^2 / 2
times. The reason of that is that when we found a bit in pidmap, we
can't determine this pidns is top for this process or it isn't.
vpid is a heavy operation, so a fork bomb, which create many nested
namespace, can do a system inaccessible for a long time.
+ */
+
+#define _GNU_SOURCE
+#include sys/wait.h
+#include assert.h
+#include stdio.h
+#include stdlib.h
+#include unistd.h
+#include string.h
+#include errno.h
+#include test.h
+#include pidns_helper.h
+
+#define MAXNEST 32
+
+char *TCID = pidns32;
+int TST_TOTAL = 1;
+
+static void setup(void)
+{
+ if (tst_kvercmp(3, 7, 0) 0)
+ tst_brkm(TCONF, NULL, nest depth limitation not supported);
+ tst_require_root();
+ check_newpid();
+ tst_tmpdir();
+}
+
+static void cleanup(void)
+{
+ tst_rmdir();
+}
Re: [LTP] [PATCH V2] container: new testcase pidns32
On 2015/8/11 21:58, Jan Stancek wrote:
- Original Message -
From: Yuan Sun sunyu...@huawei.com
To: Jan Stancek jstan...@redhat.com
Cc: ltp-list@lists.sourceforge.net
Sent: Tuesday, 11 August, 2015 10:43:02 AM
Subject: Re: [PATCH V2] container: new testcase pidns32
Hi Jan,
See in-line comments.
On 2015/8/11 16:05, Jan Stancek wrote:
- Original Message -
From: Yuan Sun sunyu...@huawei.com
To: jstan...@redhat.com
Cc: ltp-list@lists.sourceforge.net
Sent: Friday, 7 August, 2015 6:55:08 AM
Subject: [PATCH V2] container: new testcase pidns32
Kernel imposes a limit of 32 nested levels of pid namespaces.
Signed-off-by: Yuan Sun sunyu...@huawei.com
---
runtest/containers | 1 +
testcases/kernel/containers/pidns/.gitignore | 1 +
testcases/kernel/containers/pidns/pidns32.c | 96
3 files changed, 98 insertions(+)
create mode 100644 testcases/kernel/containers/pidns/pidns32.c
diff --git a/runtest/containers b/runtest/containers
index 9dc44bc..ac37ab2 100644
--- a/runtest/containers
+++ b/runtest/containers
@@ -13,6 +13,7 @@ pidns17 pidns17
pidns20 pidns20
pidns30 pidns30
pidns31 pidns31
+pidns32 pidns32
mqns_01 mqns_01
mqns_01_clone mqns_01 -clone
diff --git a/testcases/kernel/containers/pidns/.gitignore
b/testcases/kernel/containers/pidns/.gitignore
index e56c1f9..488b045 100644
--- a/testcases/kernel/containers/pidns/.gitignore
+++ b/testcases/kernel/containers/pidns/.gitignore
@@ -12,3 +12,4 @@
/pidns20
/pidns30
/pidns31
+/pidns32
diff --git a/testcases/kernel/containers/pidns/pidns32.c
b/testcases/kernel/containers/pidns/pidns32.c
new file mode 100644
index 000..d0d3e1e
--- /dev/null
+++ b/testcases/kernel/containers/pidns/pidns32.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) Huawei Technologies Co., Ltd., 2015
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+ * the GNU General Public License for more details.
+ */
+
+/*
Hi,
I'm still not entirely convinced about this test.
+ * Verify that:
+ * The kernel imposes a limit of 32 nested levels of pid namespaces.
If it doesn't, is that a bad thing? Are there some adverse effects, when
such limitation is lacking?
It is a kernel feature.
My concern is that it's not documented, and can be changed at any time.
There's no way to learn what that value is, unless you look at source code.
As suggested by Cyril, test could verify that we can create at least
MAXNEST-1 pid
namespaces and stop there. What do you think?
Regards,
Jan
I understand your concern. Ok. I agree with you.
I will update the code.
Thank you.
Yuan
The following information is from
http://www.serverphorums.com/read.php?12,580615
'struct pid' is a variable sized struct - a header with an array
of upids at the end.
A size of the array depends on a level (depth) of pid namespaces. Now
a level of pidns is not limited, so 'struct pid' can be more than one
page.
Looks reasonable, that it should be less than a page. MAX_PIS_NS_LEVEL
is not calculated from PAGE_SIZE, because in this case it depends on
architectures, config options and it will be reduced, if someone adds a
new fields in struct pid or struct upid.
I suggest to set MAX_PID_NS_LEVEL = 32, because it saves ability to
expand struct pid and it's more than enough for all known for me
use-cases. When someone finds a reasonable use case, we can add a
config option or a sysctl parameter.
In addition it will reduce effect of another problem, when we have many
nested namespaces and the oldest one starts dying. zap_pid_ns_processe
will be called for each namespace and find_vpid will be called for each
process in a namespace. find_vpid will be called minimum max_level^2 / 2
times. The reason of that is that when we found a bit in pidmap, we
can't determine this pidns is top for this process or it isn't.
vpid is a heavy operation, so a fork bomb, which create many nested
namespace, can do a system inaccessible for a long time.
+ */
+
+#define _GNU_SOURCE
+#include sys/wait.h
+#include assert.h
+#include stdio.h
+#include stdlib.h
+#include unistd.h
+#include string.h
+#include errno.h
+#include test.h
+#include pidns_helper.h
+
+#define MAXNEST 32
+
+char *TCID = pidns32;
+int TST_TOTAL = 1;
+
+static void setup(void)
+{
+ if (tst_kvercmp(3, 7, 0) 0)
+ tst_brkm(TCONF, NULL, nest depth limitation not supported);
+ tst_require_root();
+ check_newpid();
+ tst_tmpdir();
+}
+
+static
