Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Add a safer version of WithNetNS* #2443

Closed
wants to merge 8 commits into from
Closed

Add a safer version of WithNetNS* #2443

wants to merge 8 commits into from

Conversation

brb
Copy link
Contributor

@brb brb commented Jul 14, 2016

This PR introduces a safer version of WithNetNS*. The new function executes work() by shelling out weaveutil which before executing it enters a given network namespace (passed via --netns-fd).

EDIT Unfortunately, the function isn't totally safe - any go-routine spawned bywork() might be run in a wrong netns. To fix that we either need to create a small in non-Go shim or to compile weaveutil with a custom constructor function which gets called before the runtime initialization (EDITED: I'll file an issue for it once we have discussed in this PR). nsenter is used when calling weaveutil.

Also, the PR replaces the usage of WithNetNS*Unsafe with the newly introduced function where needed.

Fixes #2419

@rade rade added this to the 1.6.1 milestone Jul 20, 2016
@brb brb assigned awh Jul 26, 2016
@brb brb force-pushed the issues/2419-withnetns branch from f4a1570 to c2c5bdd Compare August 3, 2016 16:31
brb added 6 commits August 3, 2016 17:39
The flag is used to denote that weaveutil has to enter the given
netns (identified by fd) before executing a cmd.

Keep in mind, that this is still not the safest way to run some
Go code in custom netns, because some runtime sched threads
might reside in the root netns.
The introduced functions are executing a work function via
creating a process which executes "weaveutil --netns-fd".
The module introduces the Dev type (copy of common/utils.go:NetDev)
which is used to represent network dev info (iface name, IPs, MAC).
The module is used to implement cmds which have to be run
inside a custom netns (passed via --netns-fd) and it is
intended to replace the usage of WithNetNS*Unsafe.
The relevant functions have been moved to net/netdev.go
@brb brb force-pushed the issues/2419-withnetns branch from c2c5bdd to 8c37ad5 Compare August 3, 2016 16:41
nsenter ensures that the entire Go runtime runs in the same
network namespace.
@brb brb force-pushed the issues/2419-withnetns branch from 8c37ad5 to 7463776 Compare August 3, 2016 17:21
@brb
Copy link
Contributor Author

brb commented Aug 3, 2016

@awh PTAL. I have replaced weaveutil --netns-fd=... <cmd> with nsenter <..> weaveutil <cmd>.

@awh
Copy link
Contributor

awh commented Aug 10, 2016

@brb as discussed too invasive for a bugfix release - please reopen against master. Thanks!

@awh awh assigned brb and unassigned awh Aug 10, 2016
@awh awh closed this Aug 10, 2016
@awh awh modified the milestones: n/a, 1.6.1 Aug 10, 2016
@brb
Copy link
Contributor Author

brb commented Aug 10, 2016

Done: #2475

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants