[RFC iproute2 0/1] iproute2 netns mount race issue and solution?

[RFC iproute2 0/1] iproute2 netns mount race issue and solution?

[Alexander Aring]

Hi,

We found an issue how we can react on namespaces created by iproute2.
As state of the current Linux kernel there exists no way to get events
on new mounts. Polling is not an option because you can miss mounts.

It's an RFC to see that might people seeing the same issue here and
would like to talk about possible solutions how to deal with that.

I cc linux-fs here that they might can tell me a solution which maybe
already exists if not this solution should be backwards compatible.

I know this solution only works for iproute2 but isn't iproute2 not the
standard defintion how /var/run/netns works?

- Alex

Alexander Aring (1):
  ip: netns: add mounted state file for each netns

 ip/ipnetns.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

-- 
2.11.0

[RFC iproute2 1/1] ip: netns: add mounted state file for each netns

[Alexander Aring]

This patch adds a state file for each generated namespace to ensure the
namespace is mounted. There exists no way to tell another programm that
the namespace is mounted when iproute is creating one. An example
application would be an inotify watcher to use the generated namespace
when it's discovers one. In this case we cannot use the generated
namespace file in /var/run/netns in the time when it's not mounted yet.
A primitiv approach is to generate another file after the mount
systemcall was done. In my case inotify waits until the mount statefile
is generated to be sure that iproute2 did a mount bind.

Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 ip/ipnetns.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index a883f210..339a9ffc 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -606,6 +606,13 @@ static int on_netns_del(char *nsname, void *arg)
 			netns_path, strerror(errno));
 		return -1;
 	}
+	snprintf(netns_path, sizeof(netns_path), "%s/%s.mounted",
+		 NETNS_RUN_DIR, nsname);
+	if (unlink(netns_path) < 0) {
+		fprintf(stderr, "Cannot remove namespace file \"%s\": %s\n",
+			netns_path, strerror(errno));
+		return -1;
+	}
 	return 0;
 }
 
@@ -758,6 +765,15 @@ static int netns_add(int argc, char **argv, bool create)
 	}
 	netns_restore();
 
+	snprintf(netns_path, sizeof(netns_path), "%s/%s.mounted", NETNS_RUN_DIR, name);
+	fd = open(netns_path, O_RDONLY|O_CREAT|O_EXCL, 0);
+	if (fd < 0) {
+		fprintf(stderr, "Cannot create namespace file \"%s\": %s\n",
+			netns_path, strerror(errno));
+		goto out_delete;
+	}
+	close(fd);
+
 	return 0;
 out_delete:
 	if (create) {
-- 
2.11.0

Re: [RFC iproute2 1/1] ip: netns: add mounted state file for each netns

[Nicolas Dichtel]

Le 26/06/2019 à 21:03, Alexander Aring a écrit :
> This patch adds a state file for each generated namespace to ensure the
> namespace is mounted. There exists no way to tell another programm that
> the namespace is mounted when iproute is creating one. An example
> application would be an inotify watcher to use the generated namespace
> when it's discovers one. In this case we cannot use the generated
> namespace file in /var/run/netns in the time when it's not mounted yet.
> A primitiv approach is to generate another file after the mount
> systemcall was done. In my case inotify waits until the mount statefile
> is generated to be sure that iproute2 did a mount bind.
We (at 6WIND) already hit this problem. The solution was: if setns() fails, wait
a bit and retry the setns() and continue this loop with a predefined timeout.
netns may be created by other app than iproute2, it would be nice to find a
generic solution.

David Howells was working on a mount notification mechanism:
https://lwn.net/Articles/760714/
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications

I don't know what is the status of this series.


Regards,
Nicolas

Re: [RFC iproute2 1/1] ip: netns: add mounted state file for each netns

[David Howells]

Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:

> David Howells was working on a mount notification mechanism:
> https://lwn.net/Articles/760714/
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
> 
> I don't know what is the status of this series.

It's still alive.  I just posted a new version on it.  I'm hoping, possibly
futiley, to get it in in this merge window.

David

Re: [RFC iproute2 1/1] ip: netns: add mounted state file for each netns

[Matteo Croce]

On Fri, Jun 28, 2019 at 6:27 PM David Howells <dhowells@redhat.com> wrote:
>
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
> > David Howells was working on a mount notification mechanism:
> > https://lwn.net/Articles/760714/
> > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
> >
> > I don't know what is the status of this series.
>
> It's still alive.  I just posted a new version on it.  I'm hoping, possibly
> futiley, to get it in in this merge window.
>
> David

Hi all,

this could cause a clash if I create a netns with name ending with .mounted.

$ sudo ip/ip netns add ns1.mounted
$ sudo ip/ip netns add ns1
Cannot create namespace file "/var/run/netns/ns1.mounted": File exists
Cannot remove namespace file "/var/run/netns/ns1.mounted": Device or
resource busy

If you want to go along this road, please either:
- disallow netns creation with names ending with .mounted
- or properly document it in the manpage

Regards,
-- 
Matteo Croce
per aspera ad upstream

Re: [RFC iproute2 1/1] ip: netns: add mounted state file for each netns

[Matteo Croce]

On Fri, Jun 28, 2019 at 7:06 PM Matteo Croce <mcroce@redhat.com> wrote:
>
> On Fri, Jun 28, 2019 at 6:27 PM David Howells <dhowells@redhat.com> wrote:
> >
> > Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> >
> > > David Howells was working on a mount notification mechanism:
> > > https://lwn.net/Articles/760714/
> > > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
> > >
> > > I don't know what is the status of this series.
> >
> > It's still alive.  I just posted a new version on it.  I'm hoping, possibly
> > futiley, to get it in in this merge window.
> >
> > David
>
> Hi all,
>
> this could cause a clash if I create a netns with name ending with .mounted.
>
> $ sudo ip/ip netns add ns1.mounted
> $ sudo ip/ip netns add ns1
> Cannot create namespace file "/var/run/netns/ns1.mounted": File exists
> Cannot remove namespace file "/var/run/netns/ns1.mounted": Device or
> resource busy
>
> If you want to go along this road, please either:
> - disallow netns creation with names ending with .mounted
> - or properly document it in the manpage
>
> Regards,
> --
> Matteo Croce
> per aspera ad upstream

BTW, this breaks the namespace listing:

# ip netns add test
# ip netns list
Error: Peer netns reference is invalid.
Error: Peer netns reference is invalid.
test.mounted
test

A better choice IMHO could be to create a temporary file before the
placeholder, and delete it after the bind mount, so an inotify watcher
can listen for the delete event.
For example, when creating the namespace "foo":

- create /var/run/netns/.foo.mounting
- create /var/run/netns/foo
- bind mount from /proc/.. to /var/run/netns/foo
- remove /var/run/netns/.foo.mounting

and exclude .*.mounting from the netns listing

Or, announce netns creation/deletion in some other way (dbus?).

Regards,
-- 
Matteo Croce
per aspera ad upstream

Re: [RFC iproute2 1/1] ip: netns: add mounted state file for each netns

[Nicolas Dichtel]

Le 28/06/2019 à 18:26, David Howells a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> 
>> David Howells was working on a mount notification mechanism:
>> https://lwn.net/Articles/760714/
>> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=notifications
>>
>> I don't know what is the status of this series.
> 
> It's still alive.  I just posted a new version on it.  I'm hoping, possibly
> futiley, to get it in in this merge window.
Nice to hear. It will help to properly solve this issue.


Thank you,
Nicolas