Proposal of fix for kernel segfault

[helloIAmPau]

Hi all,
I propose a solution for #79 by avoiding the creation of the same interface on the same container.

The script, moreover, shows all the bridged interfaces used inside the container.

I hope this will help.

Bye.

[jpetazzo]

Hi!

First of all, I'm sorry for the delay!

The corresponding kernel bug has been fixed. If you think that this fix is still necessary, I'll be happy to apply, but can you please change it to try to fit on 80 cols?

(Just cosmetic change, that's all...)

Thank you for the contribution in any case!

[helloIAmPau]

Hi @jpetazzo,
about recently unsupported linux distributions (like Ubuntu 12.04), are they still affected to this bug? If so, I think this fix should be merged.

If you agree with me, I make the cosmetic changes :)

[jpetazzo]

Agreed. If we can avoid kernel panics on older setups, it's cool :-)

[helloIAmPau]

Hi @jpetazzo, I've updated my code, I hope it's ok.
Bye

[jpetazzo]

Cool! I'm trying to understand the usage of IFS here; could you explain?

Thank you once again; this will be useful!

[helloIAmPau]

IFS is the list of the interfaces created inside the container.
Can be useful if you want to add a new interface and you need to know the names which are available.

[jpetazzo]

Oh, so it has no relation whatsoever with the built-in shell IFS variable, right?

[helloIAmPau]

No, IFS is just an unlucky name :)

If you want, I can change it in something else

[jpetazzo]

OK, so I think we shouldn't use IFS, since it's a built-in variable :-)

I'm not sure I understand the goal of that line... Can you explain what it does? It should list the content of a directory, but I don't understand why, sorry!

[helloIAmPau]

I'll try with an example:

$ docker run -d -t ubuntu  /bin/bash
7e2d4c0a4ec411f9849eda0185da4aada1c1f7b820cdbca534852d75e01aeaf6
$ sudo ./pipework/pipework eth0 -i eth1 compassionate_torvalds 10.11.12.13/[email protected]
$ sudo ./pipework/pipework eth0 -i eth999 compassionate_torvalds 192.168.100.200/[email protected]
# So far all goes fine; now if a try to call pipework using one of the names above
sudo ./pipework/pipework eth0 -i eth999 compassionate_torvalds 192.168.100.200/[email protected]
A bridge on eth999 already exists for compassionate_torvalds
eth1
eth999
are already taken

I used the IFS variable to obtain the list of the interfaces created inside the container.

(I know, my english is so bad. :) Thank you for your patience)

[jpetazzo]

Hi!

Sorry again that it took forever for me to reply.

I think I found a slightly nicer way to find if the interface name was taken:

ip netns exec $NSPID [ -d /sys/class/net/$CONTAINER_IFNAME ] && echo "Name taken"

What do you think?

[dubglan]

There's one issue with this change. There's a bug in macvlan - I'm not sure whether it's the same bug @jpetazzo is taking about. When container is removed, orphaned upper interface remains on the system.

Now imagine that PIDs wrap around and you are creating container with PID that matches PID of the another container created and removed earlier. And let's assume I'm provisioning all containers with the same set of macvlan interfaces (eth1 linked to host eth1). In this case second container wiring will fail (now without kernel panic). But it'll still fail and the only workaround would be to recreate the container or reboot the host. As I mentioned in #79, we had to use mktemp to generate unique interface names instead of basing them on container PID+interface name.

@jpetazzo, could you please provide some reference to the kernel issue.

[jpetazzo]

Unfortunately, I don't have a reference to the kernel issue, sorry!

[helloIAmPau]

Wow @jpetazzo what a nice trick :)... I update the code immediately