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

Remove subprocess for changing netns #3291

Merged
merged 3 commits into from
Jun 5, 2018
Merged

Remove subprocess for changing netns #3291

merged 3 commits into from
Jun 5, 2018

Conversation

bboreham
Copy link
Contributor

Now that Go 1.10 has removed the issue which made us do this.

Less faffing around, more efficient.

Now that Go 1.10 has removed the issue which made us do this.
@@ -1,10 +1,8 @@
package net

This comment was marked as abuse.

@bboreham bboreham force-pushed the withnetns-inproc branch from 97729f7 to 1ee4039 Compare May 3, 2018 06:15
Copy link
Contributor

@brb brb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Left a few nit picks.

// for more details and make sure that you understand the implications before
// using the function!
func WithNetNSUnsafe(ns netns.NsHandle, work func() error) error {
func WithNetNS(ns netns.NsHandle, work func() error) error {

This comment was marked as abuse.

net/netdev.go Outdated

for _, link := range links {
if _, found := indexes[link.Attrs().Index]; found {
netdev, err := LinkToNetDev(link)

This comment was marked as abuse.

net/veth.go Outdated
for _, link := range links {
ifName := link.Attrs().Name
if strings.HasPrefix(ifName, prefix) {
ConfigureARPCache(rootPath+"/proc", ifName)

This comment was marked as abuse.

net/veth.go Outdated
// NB: This function can be used only by a process that terminates immediately
// after calling the function as it changes netns via WithNetNSLinkUnsafe.
// ConfigureARP is a helper for the Docker plugin which doesn't set the addresses itself
func ConfigureArp(prefix, rootPath string) error {

This comment was marked as abuse.

@brb brb added this to the 2.4 milestone Jun 4, 2018
Copy link
Contributor

@brb brb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. However, some tests became flaky (maybe because of faster weave?).

@brb brb merged commit 2eda624 into master Jun 5, 2018
@brb brb mentioned this pull request Oct 15, 2018
bboreham added a commit that referenced this pull request Mar 26, 2019
Use of `ethtool` was removed in #1958
`util-linux` was needed for `nsenter` which was removed in #3291
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.

2 participants