diff mbox series

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

Message ID 20190626190343.22031-2-aring@mojatatu.com (mailing list archive)
State New, archived
Headers show
Series iproute2 netns mount race issue and solution? | expand

Commit Message

Alexander Aring June 26, 2019, 7:03 p.m. UTC
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(+)

Comments

Nicolas Dichtel June 27, 2019, 12:08 p.m. UTC | #1
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
David Howells June 28, 2019, 4:26 p.m. UTC | #2
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
Matteo Croce June 28, 2019, 5:06 p.m. UTC | #3
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 June 29, 2019, 9:45 p.m. UTC | #4
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,
Nicolas Dichtel July 1, 2019, 12:34 p.m. UTC | #5
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
diff mbox series

Patch

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) {