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 0000000..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 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)
>>>> + return 1;
>>> If you ignore status in waitpid() call, how do you know that child at
>>> MAXNEXT level failed test condition?
>> 79 /* MAX_PID_NS_LEVEL is needed for limiting size of 'struct pid' */
>> 80 #define MAX_PID_NS_LEVEL 32
>> 81
>> 82 static struct pid_namespace *create_pid_namespace(struct
>> user_namespace *user_ns,
>> 83 struct pid_namespace *parent_pid_ns)
>> 84 {
>> 85 struct pid_namespace *ns;
>> 86 unsigned int level = parent_pid_ns->level + 1;
>> 87 int i;
>> 88 int err;
>> 89
>> 90 if (level > MAX_PID_NS_LEVEL) {
>> 91 err = -EINVAL;
>> 92 goto out;
>> 93 }
>> http://lxr.free-electrons.com/source/kernel/pid_namespace.c
>>
>> I need to update the code to judge if the error is EINVAL.
>>> 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
