[PATCH] IPC initialize shmmax and shmall from the current value not the default

[PATCH] IPC initialize shmmax and shmall from the current value not the default

[Marian Marinov]

When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the 
values of the current shmmax and shmall to the new namespace.

Copying the values of the init_ipc_ns would allow us to create new ipc namespaces with different values without granting 
them privileges to change those values.

Here is the proposed patch:

---
  ipc/shm.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 7a51443..b7a4728 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -74,8 +74,13 @@ static int sysvipc_shm_proc_show(struct seq_file *s, void *it);

  void shm_init_ns(struct ipc_namespace *ns)
  {
-       ns->shm_ctlmax = SHMMAX;
-       ns->shm_ctlall = SHMALL;
+       if (ns == &init_ipc_ns) {
+               ns->shm_ctlmax = SHMMAX;
+               ns->shm_ctlall = SHMALL;
+       } else {
+               ns->shm_ctlmax = init_ipc_ns.shm_ctlmax;
+               ns->shm_ctlall = init_ipc_ns.shm_ctlall;
+       }
         ns->shm_ctlmni = SHMMNI;
         ns->shm_rmid_forced = 0;
         ns->shm_tot = 0;
-- 
1.8.4

-- 
Marian Marinov
Founder & CEO of 1H Ltd.
Jabber/GTalk: hackman@jabber.org
ICQ: 7556201
Mobile: +359 886 660 270

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Davidlohr Bueso]

On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the 
> values of the current shmmax and shmall to the new namespace.

Why is this a good idea?

This would break userspace that relies on the current behavior.
Furthermore we've recently changed the default value of both these
limits to be as large as you can get, thus deprecating them. I don't
like the idea of this being replaced by namespaces.

Thanks,
Davidlohr

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Marian Marinov]

On 05/04/2014 02:53 AM, Davidlohr Bueso wrote:
> On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
>> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the
>> values of the current shmmax and shmall to the new namespace.
>
> Why is this a good idea?
>
> This would break userspace that relies on the current behavior.
> Furthermore we've recently changed the default value of both these
> limits to be as large as you can get, thus deprecating them. I don't
> like the idea of this being replaced by namespaces.
>
> Thanks,
> Davidlohr
>

The current behavior is create_ipc_ns()->shm_init_ns()

void shm_init_ns(struct ipc_namespace *ns)
{
     ns->shm_ctlmax = SHMMAX;
     ns->shm_ctlall = SHMALL;
     ns->shm_ctlmni = SHMMNI;
     ns->shm_rmid_forced = 0;
     ns->shm_tot = 0;
     ipc_init_ids(&shm_ids(ns));
}

This means that whenever you are creating an IPC namespace it gets its SHMMAX and SHMALL values from the defaults for 
the kernel.
If for some reason you want to have smaller(or bigger, for older kernels) limit. This means changing the values in 
/proc/sys/kernel/shmmax and /proc/sys/kernel/shmall. However the program that is started with the new IPC namespace may 
lack privileges to write to these files and so it can not modify them.

What I'm proposing is simply to copy the current values of the host machine, as set by a privileged process before the 
namespace creation.

Maybe a better approach would be to allow the changes to be done by processes having CAP_SYS_RESOURCE inside the new 
namespace?

Marian

-- 
Marian Marinov
Founder & CEO of 1H Ltd.
Jabber/GTalk: hackman@jabber.org
ICQ: 7556201
Mobile: +359 886 660 270

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Davidlohr Bueso]

On Sun, 2014-05-04 at 03:28 +0300, Marian Marinov wrote:
> On 05/04/2014 02:53 AM, Davidlohr Bueso wrote:
> > On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
> >> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the
> >> values of the current shmmax and shmall to the new namespace.
> >
> > Why is this a good idea?
> >
> > This would break userspace that relies on the current behavior.
> > Furthermore we've recently changed the default value of both these
> > limits to be as large as you can get, thus deprecating them. I don't
> > like the idea of this being replaced by namespaces.
> >
> > Thanks,
> > Davidlohr
> >
> 
> The current behavior is create_ipc_ns()->shm_init_ns()
> 
> void shm_init_ns(struct ipc_namespace *ns)
> {
>      ns->shm_ctlmax = SHMMAX;
>      ns->shm_ctlall = SHMALL;
>      ns->shm_ctlmni = SHMMNI;
>      ns->shm_rmid_forced = 0;
>      ns->shm_tot = 0;
>      ipc_init_ids(&shm_ids(ns));
> }
> 
> This means that whenever you are creating an IPC namespace it gets its SHMMAX and SHMALL values from the defaults for 
> the kernel.

This is exactly what I meant by 'current behavior'.

> If for some reason you want to have smaller(or bigger, for older kernels) limit. This means changing the values in 
> /proc/sys/kernel/shmmax and /proc/sys/kernel/shmall. However the program that is started with the new IPC namespace may 
> lack privileges to write to these files and so it can not modify them.

I see no reason why namespaces should behave any different than the rest
of the system, wrt this. And this changes how *and* when these limits
are set, which impacts at a userspace level with no justification.

> What I'm proposing is simply to copy the current values of the host machine, as set by a privileged process before the 
> namespace creation.
> 
> Maybe a better approach would be to allow the changes to be done by processes having CAP_SYS_RESOURCE inside the new 
> namespace?

Why do you need this? Is there any real impact/issue you're seeing?

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Marian Marinov]

On 05/04/2014 04:20 AM, Davidlohr Bueso wrote:
> On Sun, 2014-05-04 at 03:28 +0300, Marian Marinov wrote:
>> On 05/04/2014 02:53 AM, Davidlohr Bueso wrote:
>>> On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
>>>> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the
>>>> values of the current shmmax and shmall to the new namespace.
>>>
>>> Why is this a good idea?
>>>
>>> This would break userspace that relies on the current behavior.
>>> Furthermore we've recently changed the default value of both these
>>> limits to be as large as you can get, thus deprecating them. I don't
>>> like the idea of this being replaced by namespaces.
>>>
>>> Thanks,
>>> Davidlohr
>>>
>>
>> The current behavior is create_ipc_ns()->shm_init_ns()
>>
>> void shm_init_ns(struct ipc_namespace *ns)
>> {
>>       ns->shm_ctlmax = SHMMAX;
>>       ns->shm_ctlall = SHMALL;
>>       ns->shm_ctlmni = SHMMNI;
>>       ns->shm_rmid_forced = 0;
>>       ns->shm_tot = 0;
>>       ipc_init_ids(&shm_ids(ns));
>> }
>>
>> This means that whenever you are creating an IPC namespace it gets its SHMMAX and SHMALL values from the defaults for
>> the kernel.
>
> This is exactly what I meant by 'current behavior'.
>
>> If for some reason you want to have smaller(or bigger, for older kernels) limit. This means changing the values in
>> /proc/sys/kernel/shmmax and /proc/sys/kernel/shmall. However the program that is started with the new IPC namespace may
>> lack privileges to write to these files and so it can not modify them.
>
> I see no reason why namespaces should behave any different than the rest
> of the system, wrt this. And this changes how *and* when these limits
> are set, which impacts at a userspace level with no justification.
>
>> What I'm proposing is simply to copy the current values of the host machine, as set by a privileged process before the
>> namespace creation.
>>
>> Maybe a better approach would be to allow the changes to be done by processes having CAP_SYS_RESOURCE inside the new
>> namespace?
>
> Why do you need this? Is there any real impact/issue you're seeing?
>
I'm using Linux Containers and I need to be able to either start containers with different SHMMAX or set different 
SHMMAX to already running containers without giving them full root access.

-Marian

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Manfred Spraul]

Hi Marian,

Note: The limits will soon be increased to (nearly) ULONG_MAX.
I.e.: If you propose the patch because you are running into issues with 
a too small SEMMAX after an unshare(CLONE_NEWIPC), then this will be 
fixed soon.


On 05/04/2014 01:53 AM, Davidlohr Bueso wrote:
> On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
>> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the
>> values of the current shmmax and shmall to the new namespace.
The idea sounds reasonable:
If an admin has reduced the limits, then the reduction should also apply 
after a unshare(CLONE_NEWIPC).

But:
Your patch doesn't use the current shmmax, it uses the shmmax from 
init_ipc_ns.
Would it be possible to use the current values?

> Why is this a good idea?
>
> This would break userspace that relies on the current behavior.
> Furthermore we've recently changed the default value of both these
> limits to be as large as you can get, thus deprecating them. I don't
> like the idea of this being replaced by namespaces.
Davidlohr: We are not deprecating them, we make the default huge.
The limits should stay as usable as they were.

With regards to breaking user space, I must think about it a bit more.
Right now, each new namespace starts with SEMMAX=32MB, i.e. an often 
unusable default.

--
     Manfred

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Davidlohr Bueso]

On Sun, 2014-05-04 at 13:17 +0200, Manfred Spraul wrote:
> Hi Marian,
> 
> Note: The limits will soon be increased to (nearly) ULONG_MAX.
> I.e.: If you propose the patch because you are running into issues with 
> a too small SEMMAX after an unshare(CLONE_NEWIPC), then this will be 
> fixed soon.
> 
> 
> On 05/04/2014 01:53 AM, Davidlohr Bueso wrote:
> > On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
> >> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the
> >> values of the current shmmax and shmall to the new namespace.
> The idea sounds reasonable:
> If an admin has reduced the limits, then the reduction should also apply 
> after a unshare(CLONE_NEWIPC).
> 
> But:
> Your patch doesn't use the current shmmax, it uses the shmmax from 
> init_ipc_ns.
> Would it be possible to use the current values?
> 
> > Why is this a good idea?
> >
> > This would break userspace that relies on the current behavior.
> > Furthermore we've recently changed the default value of both these
> > limits to be as large as you can get, thus deprecating them. I don't
> > like the idea of this being replaced by namespaces.
> Davidlohr: We are not deprecating them, we make the default huge.
> The limits should stay as usable as they were.

Deprecating them in the sense that hopefully users will not set them
anymore. I'm not saying lets add the word "deprecated" it in the
manpage...

> 
> With regards to breaking user space, I must think about it a bit more.
> Right now, each new namespace starts with SEMMAX=32MB, i.e. an often 
> unusable default.
> 
> --
>      Manfred

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Marian Marinov]

On 05/04/2014 02:17 PM, Manfred Spraul wrote:
> Hi Marian,
>
> Note: The limits will soon be increased to (nearly) ULONG_MAX.
> I.e.: If you propose the patch because you are running into issues with a too small SEMMAX after an
> unshare(CLONE_NEWIPC), then this will be fixed soon.
>
>
> On 05/04/2014 01:53 AM, Davidlohr Bueso wrote:
>> On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
>>> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to copy the
>>> values of the current shmmax and shmall to the new namespace.
> The idea sounds reasonable:
> If an admin has reduced the limits, then the reduction should also apply after a unshare(CLONE_NEWIPC).
>
> But:
> Your patch doesn't use the current shmmax, it uses the shmmax from init_ipc_ns.
> Would it be possible to use the current values?

In my tests it worked exactly as expected.
Here is an example:

[root@sp2 ~]# sysctl -a|grep shmmax
kernel.shmmax = 68719476736
[root@sp2 ~]# lxc-attach -n cent_plain
[root@localhost ~]# sysctl -a|grep shmmax
kernel.shmmax = 68719476736
[root@localhost ~]# halt
[root@sp2 ~]# sysctl -a|grep shmmax
kernel.shmmax = 68719476736
[root@sp2 ~]# sysctl kernel.shmmax=34359738368
kernel.shmmax = 34359738368
[root@sp2 ~]# lxc-start -n cent_plain -d
[root@sp2 ~]# lxc-attach -n cent_plain
[root@localhost ~]# sysctl -a|grep shmmax
kernel.shmmax = 34359738368
[root@localhost ~]#

So it seams to work as expected :)

It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns.
So when we are creating a new ipc_ns its ok to copy the values from init_ipc_ns.

-Marian

>
>> Why is this a good idea?
>>
>> This would break userspace that relies on the current behavior.
>> Furthermore we've recently changed the default value of both these
>> limits to be as large as you can get, thus deprecating them. I don't
>> like the idea of this being replaced by namespaces.
> Davidlohr: We are not deprecating them, we make the default huge.
> The limits should stay as usable as they were.
>
> With regards to breaking user space, I must think about it a bit more.
> Right now, each new namespace starts with SEMMAX=32MB, i.e. an often unusable default.
>
> --
>      Manfred
>


-- 
Marian Marinov
Founder & CEO of 1H Ltd.
Jabber/GTalk: hackman@jabber.org
ICQ: 7556201
Mobile: +359 886 660 270

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Marian Marinov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/05/2014 10:59 PM, Marian Marinov wrote:
> On 05/04/2014 02:17 PM, Manfred Spraul wrote:
>> Hi Marian,
>> 
>> Note: The limits will soon be increased to (nearly) ULONG_MAX. I.e.: If you propose the patch because you are
>> running into issues with a too small SEMMAX after an unshare(CLONE_NEWIPC), then this will be fixed soon.
>> 
>> 
>> On 05/04/2014 01:53 AM, Davidlohr Bueso wrote:
>>> On Sun, 2014-05-04 at 01:48 +0300, Marian Marinov wrote:
>>>> When we are creating new IPC namespace that should be cloned from the current namespace it is a good idea to
>>>> copy the values of the current shmmax and shmall to the new namespace.
>> The idea sounds reasonable: If an admin has reduced the limits, then the reduction should also apply after a
>> unshare(CLONE_NEWIPC).
>> 
>> But: Your patch doesn't use the current shmmax, it uses the shmmax from init_ipc_ns. Would it be possible to use
>> the current values?
> 
> In my tests it worked exactly as expected. Here is an example:
> 
> [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain 
> [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl
> -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax =
> 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]#
> sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#
> 
> So it seams to work as expected :)
> 
> It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are
> creating a new ipc_ns its ok to copy the values from init_ipc_ns.
> 
> -Marian
> 

Ping?

So will there be any more comments on that?

Marian

>> 
>>> Why is this a good idea?
>>> 
>>> This would break userspace that relies on the current behavior. Furthermore we've recently changed the default
>>> value of both these limits to be as large as you can get, thus deprecating them. I don't like the idea of this
>>> being replaced by namespaces.
>> Davidlohr: We are not deprecating them, we make the default huge. The limits should stay as usable as they were.
>> 
>> With regards to breaking user space, I must think about it a bit more. Right now, each new namespace starts with
>> SEMMAX=32MB, i.e. an often unusable default.
>> 
>> -- Manfred
>> 
> 
> 


- -- 
Marian Marinov
Founder & CEO of 1H Ltd.
Jabber/GTalk: hackman@jabber.org
ICQ: 7556201
Mobile: +359 886 660 270
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iEYEARECAAYFAlN99SAACgkQ4mt9JeIbjJQHrQCfdexU5xdW4A/pO66SvbcYQVqF
uREAoJ1e6hytp6435YUrpKjEG2qVulI1
=QqGi
-----END PGP SIGNATURE-----

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Manfred Spraul]

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

Hi Marian,

On 05/22/2014 03:01 PM, Marian Marinov wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 05/05/2014 10:59 PM, Marian Marinov wrote:
>>
>> In my tests it worked exactly as expected. Here is an example:
>>
>> [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain
>> [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl
>> -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax =
>> 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]#
>> sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#
>>
>> So it seams to work as expected :)
>>
>> It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are
>> creating a new ipc_ns its ok to copy the values from init_ipc_ns.
>>
>> -Marian
>>
> Ping?
>
> So will there be any more comments on that?
>
>
Attached is an untested idea:
- each new namespace copies from it's parent, i.e. nested namespaces 
should work.
- msg, sem and shm updated

--
     Manfred

[-- Attachment #2: 0001-ipc-namespace-copy-settings-from-parent-namespace.patch --]
[-- Type: text/x-patch, Size: 5643 bytes --]

>From ed73ce838fc3f55e34041591a72b3135ccaa460b Mon Sep 17 00:00:00 2001
From: Manfred Spraul <manfred@colorfullife.com>
Date: Sun, 25 May 2014 21:04:42 +0200
Subject: [PATCH] ipc namespace: copy settings from parent namespace

Right now, each IPC namespace starts with the kernel boot standard
settings.
This patch changes that:
Now each new namespace starts with the settings from the parent
namespace.

The patch updates msg, sem and shm.

Marian: Would that patch help you?

It's just a proposal - not yet tested.

Open issues:
- auto_msgmni is not yet taken from parent.

--
	Manfred
---
 ipc/msg.c       | 13 +++++++++----
 ipc/namespace.c |  6 +++---
 ipc/sem.c       | 19 +++++++++++++------
 ipc/shm.c       | 19 +++++++++++++------
 ipc/util.h      | 12 ++++++------
 5 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index 6498531..6c72d43 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -107,10 +107,15 @@ void recompute_msgmni(struct ipc_namespace *ns)
 	ns->msg_ctlmni = allowed;
 }
 
-void msg_init_ns(struct ipc_namespace *ns)
+void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
 {
-	ns->msg_ctlmax = MSGMAX;
-	ns->msg_ctlmnb = MSGMNB;
+	if (old_ns != NULL) {
+		ns->msg_ctlmax = old_ns->msg_ctlmax;
+		ns->msg_ctlmnb = old_ns->msg_ctlmnb;
+	} else {
+		ns->msg_ctlmax = MSGMAX;
+		ns->msg_ctlmnb = MSGMNB;
+	}
 
 	recompute_msgmni(ns);
 
@@ -129,7 +134,7 @@ void msg_exit_ns(struct ipc_namespace *ns)
 
 void __init msg_init(void)
 {
-	msg_init_ns(&init_ipc_ns);
+	msg_init_ns(&init_ipc_ns, NULL);
 
 	printk(KERN_INFO "msgmni has been set to %d\n",
 		init_ipc_ns.msg_ctlmni);
diff --git a/ipc/namespace.c b/ipc/namespace.c
index 59451c1..57b65fa 100644
--- a/ipc/namespace.c
+++ b/ipc/namespace.c
@@ -41,9 +41,9 @@ static struct ipc_namespace *create_ipc_ns(struct user_namespace *user_ns,
 	}
 	atomic_inc(&nr_ipc_ns);
 
-	sem_init_ns(ns);
-	msg_init_ns(ns);
-	shm_init_ns(ns);
+	sem_init_ns(ns, old_ns);
+	msg_init_ns(ns, old_ns);
+	shm_init_ns(ns, old_ns);
 
 	/*
 	 * msgmni has already been computed for the new ipc ns.
diff --git a/ipc/sem.c b/ipc/sem.c
index bee5554..0e232e6 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -170,12 +170,19 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
 #define sc_semopm	sem_ctls[2]
 #define sc_semmni	sem_ctls[3]
 
-void sem_init_ns(struct ipc_namespace *ns)
+void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
 {
-	ns->sc_semmsl = SEMMSL;
-	ns->sc_semmns = SEMMNS;
-	ns->sc_semopm = SEMOPM;
-	ns->sc_semmni = SEMMNI;
+	if (old_ns != NULL) {
+		ns->sc_semmsl = old_ns->sc_semmsl;
+		ns->sc_semmns = old_ns->sc_semmns;
+		ns->sc_semopm = old_ns->sc_semopm;
+		ns->sc_semmni = old_ns->sc_semmni;
+	} else {
+		ns->sc_semmsl = SEMMSL;
+		ns->sc_semmns = SEMMNS;
+		ns->sc_semopm = SEMOPM;
+		ns->sc_semmni = SEMMNI;
+	}
 	ns->used_sems = 0;
 	ipc_init_ids(&ns->ids[IPC_SEM_IDS]);
 }
@@ -190,7 +197,7 @@ void sem_exit_ns(struct ipc_namespace *ns)
 
 void __init sem_init(void)
 {
-	sem_init_ns(&init_ipc_ns);
+	sem_init_ns(&init_ipc_ns, NULL);
 	ipc_init_proc_interface("sysvipc/sem",
 				"       key      semid perms      nsems   uid   gid  cuid  cgid      otime      ctime\n",
 				IPC_SEM_IDS, sysvipc_sem_proc_show);
diff --git a/ipc/shm.c b/ipc/shm.c
index 7645961..5c54b25 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -72,12 +72,19 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp);
 static int sysvipc_shm_proc_show(struct seq_file *s, void *it);
 #endif
 
-void shm_init_ns(struct ipc_namespace *ns)
+void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
 {
-	ns->shm_ctlmax = SHMMAX;
-	ns->shm_ctlall = SHMALL;
-	ns->shm_ctlmni = SHMMNI;
-	ns->shm_rmid_forced = 0;
+	if (old_ns != NULL) {
+		ns->shm_ctlmax = old_ns->shm_ctlmax;
+		ns->shm_ctlall = old_ns->shm_ctlall;
+		ns->shm_ctlmni = old_ns->shm_ctlmni;
+		ns->shm_rmid_forced = old_ns->shm_rmid_forced;
+	} else {
+		ns->shm_ctlmax = SHMMAX;
+		ns->shm_ctlall = SHMALL;
+		ns->shm_ctlmni = SHMMNI;
+		ns->shm_rmid_forced = 0;
+	}
 	ns->shm_tot = 0;
 	ipc_init_ids(&shm_ids(ns));
 }
@@ -110,7 +117,7 @@ void shm_exit_ns(struct ipc_namespace *ns)
 
 static int __init ipc_ns_init(void)
 {
-	shm_init_ns(&init_ipc_ns);
+	shm_init_ns(&init_ipc_ns, NULL);
 	return 0;
 }
 
diff --git a/ipc/util.h b/ipc/util.h
index 9c47d6f..bb62b44 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -30,17 +30,17 @@ static inline void mq_put_mnt(struct ipc_namespace *ns) { }
 #endif
 
 #ifdef CONFIG_SYSVIPC
-void sem_init_ns(struct ipc_namespace *ns);
-void msg_init_ns(struct ipc_namespace *ns);
-void shm_init_ns(struct ipc_namespace *ns);
+void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
+void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
+void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
 
 void sem_exit_ns(struct ipc_namespace *ns);
 void msg_exit_ns(struct ipc_namespace *ns);
 void shm_exit_ns(struct ipc_namespace *ns);
 #else
-static inline void sem_init_ns(struct ipc_namespace *ns) { }
-static inline void msg_init_ns(struct ipc_namespace *ns) { }
-static inline void shm_init_ns(struct ipc_namespace *ns) { }
+static inline void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
+static inline void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
+static inline void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
 
 static inline void sem_exit_ns(struct ipc_namespace *ns) { }
 static inline void msg_exit_ns(struct ipc_namespace *ns) { }
-- 
1.9.0

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Marian Marinov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Manfred,

On 05/25/2014 11:01 PM, Manfred Spraul wrote:
> Hi Marian,
> 
> On 05/22/2014 03:01 PM, Marian Marinov wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> On 05/05/2014 10:59 PM, Marian Marinov wrote:
>>> 
>>> In my tests it worked exactly as expected. Here is an example:
>>> 
>>> [root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain 
>>> [root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]#
>>> sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax
>>> = 34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost
>>> ~]# sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#
>>> 
>>> So it seams to work as expected :)
>>> 
>>> It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are 
>>> creating a new ipc_ns its ok to copy the values from init_ipc_ns.
>>> 
>>> -Marian
>>> 
>> Ping?
>> 
>> So will there be any more comments on that?
>> 
>> 

It seams logical. I like your approach even better.
I'll test it tomorrow to confirm if all of my tests are successful.

Marian

> Attached is an untested idea: - each new namespace copies from it's parent, i.e. nested namespaces should work. -
> msg, sem and shm updated
> 
> -- Manfred


- -- 
Marian Marinov
Founder & CEO of 1H Ltd.
Jabber/GTalk: hackman@jabber.org
ICQ: 7556201
Mobile: +359 886 660 270
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iEYEARECAAYFAlOCha4ACgkQ4mt9JeIbjJTMvwCdGHV2R7Y1H7x14n53Vyihg5AB
2k4Anjp6inQsGjvof2MAx9Sh2/1K3L25
=QTO9
-----END PGP SIGNATURE-----

Re: [PATCH] IPC initialize shmmax and shmall from the current value not the default

[Serge Hallyn]

Quoting Manfred Spraul (manfred@colorfullife.com):
> Hi Marian,
> 
> On 05/22/2014 03:01 PM, Marian Marinov wrote:
> >-----BEGIN PGP SIGNED MESSAGE-----
> >Hash: SHA1
> >
> >On 05/05/2014 10:59 PM, Marian Marinov wrote:
> >>
> >>In my tests it worked exactly as expected. Here is an example:
> >>
> >>[root@sp2 ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# lxc-attach -n cent_plain
> >>[root@localhost ~]# sysctl -a|grep shmmax kernel.shmmax = 68719476736 [root@localhost ~]# halt [root@sp2 ~]# sysctl
> >>-a|grep shmmax kernel.shmmax = 68719476736 [root@sp2 ~]# sysctl kernel.shmmax=34359738368 kernel.shmmax =
> >>34359738368 [root@sp2 ~]# lxc-start -n cent_plain -d [root@sp2 ~]# lxc-attach -n cent_plain [root@localhost ~]#
> >>sysctl -a|grep shmmax kernel.shmmax = 34359738368 [root@localhost ~]#
> >>
> >>So it seams to work as expected :)
> >>
> >>It works because wen you setup a new shmmax limit it is actually the limit in the init_ipc_ns. So when we are
> >>creating a new ipc_ns its ok to copy the values from init_ipc_ns.
> >>
> >>-Marian
> >>
> >Ping?
> >
> >So will there be any more comments on that?
> >
> >
> Attached is an untested idea:
> - each new namespace copies from it's parent, i.e. nested namespaces
> should work.
> - msg, sem and shm updated

Agreed, I prefer this.

> --
>     Manfred

> From ed73ce838fc3f55e34041591a72b3135ccaa460b Mon Sep 17 00:00:00 2001
> From: Manfred Spraul <manfred@colorfullife.com>
> Date: Sun, 25 May 2014 21:04:42 +0200
> Subject: [PATCH] ipc namespace: copy settings from parent namespace
> 
> Right now, each IPC namespace starts with the kernel boot standard
> settings.
> This patch changes that:
> Now each new namespace starts with the settings from the parent
> namespace.
> 
> The patch updates msg, sem and shm.
> 
> Marian: Would that patch help you?
> 
> It's just a proposal - not yet tested.
> 
> Open issues:
> - auto_msgmni is not yet taken from parent.
> 
> --
> 	Manfred
> ---
>  ipc/msg.c       | 13 +++++++++----
>  ipc/namespace.c |  6 +++---
>  ipc/sem.c       | 19 +++++++++++++------
>  ipc/shm.c       | 19 +++++++++++++------
>  ipc/util.h      | 12 ++++++------
>  5 files changed, 44 insertions(+), 25 deletions(-)
> 
> diff --git a/ipc/msg.c b/ipc/msg.c
> index 6498531..6c72d43 100644
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -107,10 +107,15 @@ void recompute_msgmni(struct ipc_namespace *ns)
>  	ns->msg_ctlmni = allowed;
>  }
>  
> -void msg_init_ns(struct ipc_namespace *ns)
> +void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
>  {
> -	ns->msg_ctlmax = MSGMAX;
> -	ns->msg_ctlmnb = MSGMNB;
> +	if (old_ns != NULL) {
> +		ns->msg_ctlmax = old_ns->msg_ctlmax;
> +		ns->msg_ctlmnb = old_ns->msg_ctlmnb;
> +	} else {
> +		ns->msg_ctlmax = MSGMAX;
> +		ns->msg_ctlmnb = MSGMNB;
> +	}
>  
>  	recompute_msgmni(ns);
>  
> @@ -129,7 +134,7 @@ void msg_exit_ns(struct ipc_namespace *ns)
>  
>  void __init msg_init(void)
>  {
> -	msg_init_ns(&init_ipc_ns);
> +	msg_init_ns(&init_ipc_ns, NULL);
>  
>  	printk(KERN_INFO "msgmni has been set to %d\n",
>  		init_ipc_ns.msg_ctlmni);
> diff --git a/ipc/namespace.c b/ipc/namespace.c
> index 59451c1..57b65fa 100644
> --- a/ipc/namespace.c
> +++ b/ipc/namespace.c
> @@ -41,9 +41,9 @@ static struct ipc_namespace *create_ipc_ns(struct user_namespace *user_ns,
>  	}
>  	atomic_inc(&nr_ipc_ns);
>  
> -	sem_init_ns(ns);
> -	msg_init_ns(ns);
> -	shm_init_ns(ns);
> +	sem_init_ns(ns, old_ns);
> +	msg_init_ns(ns, old_ns);
> +	shm_init_ns(ns, old_ns);
>  
>  	/*
>  	 * msgmni has already been computed for the new ipc ns.
> diff --git a/ipc/sem.c b/ipc/sem.c
> index bee5554..0e232e6 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -170,12 +170,19 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
>  #define sc_semopm	sem_ctls[2]
>  #define sc_semmni	sem_ctls[3]
>  
> -void sem_init_ns(struct ipc_namespace *ns)
> +void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
>  {
> -	ns->sc_semmsl = SEMMSL;
> -	ns->sc_semmns = SEMMNS;
> -	ns->sc_semopm = SEMOPM;
> -	ns->sc_semmni = SEMMNI;
> +	if (old_ns != NULL) {
> +		ns->sc_semmsl = old_ns->sc_semmsl;
> +		ns->sc_semmns = old_ns->sc_semmns;
> +		ns->sc_semopm = old_ns->sc_semopm;
> +		ns->sc_semmni = old_ns->sc_semmni;
> +	} else {
> +		ns->sc_semmsl = SEMMSL;
> +		ns->sc_semmns = SEMMNS;
> +		ns->sc_semopm = SEMOPM;
> +		ns->sc_semmni = SEMMNI;
> +	}
>  	ns->used_sems = 0;
>  	ipc_init_ids(&ns->ids[IPC_SEM_IDS]);
>  }
> @@ -190,7 +197,7 @@ void sem_exit_ns(struct ipc_namespace *ns)
>  
>  void __init sem_init(void)
>  {
> -	sem_init_ns(&init_ipc_ns);
> +	sem_init_ns(&init_ipc_ns, NULL);
>  	ipc_init_proc_interface("sysvipc/sem",
>  				"       key      semid perms      nsems   uid   gid  cuid  cgid      otime      ctime\n",
>  				IPC_SEM_IDS, sysvipc_sem_proc_show);
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 7645961..5c54b25 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -72,12 +72,19 @@ static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp);
>  static int sysvipc_shm_proc_show(struct seq_file *s, void *it);
>  #endif
>  
> -void shm_init_ns(struct ipc_namespace *ns)
> +void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns)
>  {
> -	ns->shm_ctlmax = SHMMAX;
> -	ns->shm_ctlall = SHMALL;
> -	ns->shm_ctlmni = SHMMNI;
> -	ns->shm_rmid_forced = 0;
> +	if (old_ns != NULL) {
> +		ns->shm_ctlmax = old_ns->shm_ctlmax;
> +		ns->shm_ctlall = old_ns->shm_ctlall;
> +		ns->shm_ctlmni = old_ns->shm_ctlmni;
> +		ns->shm_rmid_forced = old_ns->shm_rmid_forced;
> +	} else {
> +		ns->shm_ctlmax = SHMMAX;
> +		ns->shm_ctlall = SHMALL;
> +		ns->shm_ctlmni = SHMMNI;
> +		ns->shm_rmid_forced = 0;
> +	}
>  	ns->shm_tot = 0;
>  	ipc_init_ids(&shm_ids(ns));
>  }
> @@ -110,7 +117,7 @@ void shm_exit_ns(struct ipc_namespace *ns)
>  
>  static int __init ipc_ns_init(void)
>  {
> -	shm_init_ns(&init_ipc_ns);
> +	shm_init_ns(&init_ipc_ns, NULL);
>  	return 0;
>  }
>  
> diff --git a/ipc/util.h b/ipc/util.h
> index 9c47d6f..bb62b44 100644
> --- a/ipc/util.h
> +++ b/ipc/util.h
> @@ -30,17 +30,17 @@ static inline void mq_put_mnt(struct ipc_namespace *ns) { }
>  #endif
>  
>  #ifdef CONFIG_SYSVIPC
> -void sem_init_ns(struct ipc_namespace *ns);
> -void msg_init_ns(struct ipc_namespace *ns);
> -void shm_init_ns(struct ipc_namespace *ns);
> +void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
> +void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
> +void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns);
>  
>  void sem_exit_ns(struct ipc_namespace *ns);
>  void msg_exit_ns(struct ipc_namespace *ns);
>  void shm_exit_ns(struct ipc_namespace *ns);
>  #else
> -static inline void sem_init_ns(struct ipc_namespace *ns) { }
> -static inline void msg_init_ns(struct ipc_namespace *ns) { }
> -static inline void shm_init_ns(struct ipc_namespace *ns) { }
> +static inline void sem_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
> +static inline void msg_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
> +static inline void shm_init_ns(struct ipc_namespace *ns, struct ipc_namespace *old_ns) { }
>  
>  static inline void sem_exit_ns(struct ipc_namespace *ns) { }
>  static inline void msg_exit_ns(struct ipc_namespace *ns) { }
> -- 
> 1.9.0
> 
> 

> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/containers