After changing namespace, other goroutines may switch to that namespace

[bboreham]

In this repo you have NetNS.Do, but it is only called when you need a "specific namespace". Turns out you also need to call it when you need the root namespace.

See discussion at vishvananda/netns#17, details at moby/libnetwork#1113 or weaveworks/weave#2388 (comment)

The problem is that the Go runtime can create a new OS thread while you have changed namespace, and that new thread inherits that changed namespace, so now any operation elsewhere in the program can suddenly switch to the other namespace as Go schedules goroutines onto the new thread.

Given the nature of CNI, that each process tends to exit quickly after doing its work, it's perhaps hard to hit the problem. But still possible.

[rosenhouse]

Yes, thanks for bringing this up. We've struggled with this issue for a while now, e.g. #183 (comment)

A reexec pattern (like docker) can work, though its not exactly elegant. Other process-level granularity would be good. I know on some other projects that are otherwise Go-based, they still use a small C process to do the namespace switching.

Some static analysis tools might be useful here.

For now, the only suggestion I can make is that CNI plugins should obey the following 3 rules:

1. be short-lived (as you said)
2. be single-threaded, single-goroutine
3. never re-enter NetNS.Do()

Note that these rules apply to an entire process -- its impossible to enforce them via library code alone.

If anyone sees violations of any of these rules in CNI plugins in this repo, please bring them to the attention of myself or other maintainers.

[bboreham]

My apologies: I thought I had read through #183 and found it was a different issue, hence I filed a new one. But I see that your comment does make the exact point.

I have come up with roughly the same rules, but they are probably unachievable in any nontrivial program, considering they must apply to all libraries called. Nonetheless, short of a fix in the Go runtime, loudly announcing them may be the best we can do.

[bboreham]

This came up again today; I'm wondering if https://github.com/containernetworking/cni/tree/master/pkg/ns/README.md should be updated to explain that "All code dependent on a particular network namespace" really means "All code that uses networking or may call into a library that uses networking".

[rosenhouse]

Sounds good to me. PR?

[bboreham]

Issue now open for this specific point in Go golang/go#20676

[bboreham]

This is fixed in Go 1.10