Specify network namespace for firecracker process #120

aaithal · 2019-03-04T19:34:09Z

Issue #, if available: N/A

Description of changes:

specify network namespace for firecracker process
start firecracker VM in non-default netns, when specified

Testing done: Modified the taskworkflow example to accept -netns argument. Tested end-to-end workflow using the same.

$ sudo ./taskworkflow  -ip 10.0.5.33  -host_device tap301 -mac aa:bb:cc:dd:ee:ff -netns vlan-301
2019/03/04 19:32:10.943694 Creating containerd client
2019/03/04 19:32:10.944011 Created containerd client
2019/03/04 19:32:20.570249 Successfully pulled docker.io/library/nginx:latest image
2019/03/04 19:32:22.827486 Successfully created task: demo for the container
2019/03/04 19:32:22.827502 Completed waiting for the container task
2019/03/04 19:32:22.864207 Successfully started the container task
2019/03/04 19:32:25.864347 Executing http GET on 10.0.5.33
2019/03/04 19:32:25.865881 Response from [10.0.5.33]:
[<!DOCTYPE html>
<html>
<head>
<title>Welcome to nginx!</title>
<style>
    body {
        width: 35em;
        margin: 0 auto;
        font-family: Tahoma, Verdana, Arial, sans-serif;
    }
</style>
</head>
<body>
<h1>Welcome to nginx!</h1>
<p>If you see this page, the nginx web server is successfully installed and
working. Further configuration is required.</p>

<p>For online documentation and support please refer to
<a href="http://nginx.org/">nginx.org</a>.<br/>
Commercial support is available at
<a href="http://nginx.com/">nginx.com</a>.</p>

<p><em>Thank you for using nginx.</em></p>
</body>
</html>
]
10.0.3.109 - - [04/Mar/2019:19:32:25 +0000] "GET / HTTP/1.1" 200 612 "-" "Go-http-client/1.1" "-"

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Modified the firecracker vm config protobuf message to add the FirecrackerNetworkNamespace field, which lets client specify the network namespace that the firecracker process should be associated with. Signed-off-by: Anirudh Aithal <[email protected]>

sipsma · 2019-03-04T19:52:03Z

runtime/service.go

+	}
+	// It's unsafe to switch network namespaces without locking down the OS
+	// thread. Lock it for the start VM operation.
+	pkgruntime.LockOSThread()


nit: it looks like the netNS.Do implementation already takes care of locking the goroutine making the call to its os thread, so we shouldn't need to do the lock/unlock ourselves: https://github.com/containernetworking/plugins/blob/master/pkg/ns/ns_linux.go#L199

nice catch! i didn't realize it was already doing that! i'll get rid of it.

sipsma · 2019-03-04T20:07:57Z

runtime/service.go

@@ -52,6 +54,7 @@ import (
 const (
 	defaultVsockPort     = 10789
 	supportedMountFSType = "ext4"
+	namedNetNSPath       = "/var/run/netns/"


Just curious, what's the reason we are requiring the ns come from under /var/run/netns? As far as I knew you can bind a net NS fd anywhere you want on a filesystem, is this just a standard location?

Named network namespaces are typically created under /var/run/netns. See ip-netns(8) man page for details. I have not seen too many (any tbh) implementations that use an alternative path. You're right that the netns fd can certainly be bound anywhere on the filesystem. However, because the assumption is the use of named network namespaces, this path was chosen. An alternative could be letting clients specify FirecrackerNetworkNamespacePath instead of FirecrackerNetworkNamespace. But, it might be a bit much to specify was my take on it.

Thanks, that makes sense. The other middle-ground option I can think of is check if the user specified an absolute or relative path; if they specified an absolute path use that unchanged, if it was relative append it to the end of /var/run/netns/.

If you agree that makes sense it's probably worth updating, but I don't consider it a blocker given it's just to support an obscure use-case.

Since the most widely used library I'm aware of (for dealing with network namespaces in golang) makes this same assumption: https://github.com/vishvananda/netns/blob/13995c7128ccc8e51e9a6bd2b551020a27180abd/netns_linux.go#L81, I'd prefer to keep it the way it is. Let me know if you feel strongly about it and we can make that change.

I see, no that works for me for now, easy to update in the future if ever needed.

If fully specified, we should use the concrete path rather than a relative path. This is useful when the ns is not bind-mounted anywhere but is instead one of the ns files from /proc, like /proc/<pid>/ns/net.

If fully specified, we should use the concrete path rather than a relative path. This is useful when the ns is not bind-mounted anywhere but is instead one of the ns files from /proc, like /proc/<pid>/ns/net.

Agree. I'll make this change since two of you have asked for it. But, I still don't think that clients would want to do that. It's much easier to manage/deal with named network namespaces and that's what will drive most of the usage of this field imo.

If the client specifies a network namespace name in the firecracker VM config, use that to start the firecracker process. Signed-off-by: Anirudh Aithal <[email protected]>

The '-netns' flag lets callers specify a network namespace for firecracker VM process. Signed-off-by: Anirudh Aithal <[email protected]>

samuelkarp

I think the right way to do this is with the jailer rather than making the runtime responsible for this.

samuelkarp · 2019-03-04T22:41:25Z

examples/taskworkflow.go

 )

 func main() {
 	var ip = flag.String("ip", "", "ip address assigned to the container. Example: -ip 172.16.0.1")
 	var gateway = flag.String("gw", "", "gateway ip address. Example: -gw 172.16.0.1")
 	var netMask = flag.String("mask", "", "subnet gatway mask. Example: -mask 255.255.255.0")
+	var netNS = flag.String("netns", "", "firecracker VM network namespace. Example: -netNS testing")


Looks like the example should read "-netns testing".

samuelkarp · 2019-03-04T22:50:22Z

examples/taskworkflow.go

 )

 func main() {
 	var ip = flag.String("ip", "", "ip address assigned to the container. Example: -ip 172.16.0.1")
 	var gateway = flag.String("gw", "", "gateway ip address. Example: -gw 172.16.0.1")
 	var netMask = flag.String("mask", "", "subnet gatway mask. Example: -mask 255.255.255.0")
+	var netNS = flag.String("netns", "", "firecracker VM network namespace. Example: -netNS testing")
+	var hostDevName = flag.String("host_device", defaultHostDevName,


nit: I'd prefer to use - inside argument options instead of _, so this would be host-device.

samuelkarp · 2019-03-04T22:50:34Z

examples/taskworkflow.go

+		"the host device name for the network interface, required when specifying 'netns'. Example: -host_device tap0")
+	var macAddress = flag.String("mac", defaultMacAddress,
+		"the mac address for the network interface, required when specifying 'netns'. Example: -mac AA:FC:00:00:00:01")
+	var kernelNetworkArgs = flag.Bool("kernel_nw_args", false,


Same as above.

+1 for hyphens rather than underscores.

I also would prefer something other than "nw" for network. Maybe kernel-ip-arg or kernel-net-arg?

samuelkarp · 2019-03-04T22:51:20Z

examples/taskworkflow.go

+	var macAddress = flag.String("mac", defaultMacAddress,
+		"the mac address for the network interface, required when specifying 'netns'. Example: -mac AA:FC:00:00:00:01")
+	var kernelNetworkArgs = flag.Bool("kernel_nw_args", false,
+		"specifies if network params for the VMs should be included with kernel args. Example: -kernel_nw_args true")


Is this always true when netns is specified? Does it default to true when netns is specified? What about when ip is specified?

Is this always true when netns is specified? Does it default to true when netns is specified? What about when ip is specified?

It's true only when someone sets it. Depending on how your VM is configured you or may not need it. My rootfs has a dhcp client. Hence, I don't need to set this at all. The taskworkflow.md file demonstrates use-cases with and without it. Please let me know if it's still unclear and I'll try to document more.

samuelkarp · 2019-03-04T22:54:50Z

runtime/service.go

@@ -52,6 +54,7 @@ import (
 const (
 	defaultVsockPort     = 10789
 	supportedMountFSType = "ext4"
+	namedNetNSPath       = "/var/run/netns/"


If fully specified, we should use the concrete path rather than a relative path. This is useful when the ns is not bind-mounted anywhere but is instead one of the ns files from /proc, like /proc/<pid>/ns/net.

samuelkarp · 2019-03-04T22:55:12Z

runtime/service.go

+		return s.machine.Start(vmmCtx)
+	}
+	// Get the network namespace handle.
+	netNS, err := ns.GetNS(namedNetNSPath + netNSName)


We should be able to accept a path like /proc/<pid>/ns/net here so that a bind-mount is not required.

samuelkarp · 2019-03-04T22:59:59Z

runtime/service.go

+	if err != nil {
+		return errors.Wrapf(err, "unable to find netns %s", netNSName)
+	}
+	return netNS.Do(func(_ ns.NetNS) error {


I'd prefer not to do this in this process. Instead, I'd want to use the jailer and use its --netns argument instead. I'm working on #121 which should unblock #122.

aaithal · 2019-03-04T23:10:09Z

I think the right way to do this is with the jailer rather than making the runtime responsible for this.

I agree that the right way to do this would be with jailer. This is something that unblocks folks who want to do this in the meantime.

We can easily switch the internal implementation to jailer once #122 is unblocked.

aaithal · 2019-06-05T18:31:43Z

Closing this for now. The code/architecture has diverged a bit since I wrote this.

Upgrade go-swagger

…irecracker-microvm#120) add kind to the list of dependencies in the dev's manual Signed-off-by: Shyam Jesal <[email protected]>

aaithal requested review from nmeyerhans, mxpv, xibz and samuelkarp March 4, 2019 19:34

aaithal force-pushed the netns branch from 751bfac to da83341 Compare March 4, 2019 19:38

sipsma reviewed Mar 4, 2019

View reviewed changes

aaithal added 2 commits March 4, 2019 20:17

start firecracker VM in non-default netns, when specified

21948c4

If the client specifies a network namespace name in the firecracker VM config, use that to start the firecracker process. Signed-off-by: Anirudh Aithal <[email protected]>

add netns flag to taskworkflow example.

71c9406

The '-netns' flag lets callers specify a network namespace for firecracker VM process. Signed-off-by: Anirudh Aithal <[email protected]>

aaithal force-pushed the netns branch from da83341 to 71c9406 Compare March 4, 2019 20:23

sipsma approved these changes Mar 4, 2019

View reviewed changes

samuelkarp suggested changes Mar 4, 2019

View reviewed changes

aaithal closed this Jun 5, 2019

fangn2 pushed a commit to fangn2/firecracker-containerd that referenced this pull request Mar 23, 2023

Merge pull request firecracker-microvm#120 from kzys/build-repro

ea85a54

Upgrade go-swagger

Specify network namespace for firecracker process #120

Specify network namespace for firecracker process #120

Uh oh!

Conversation

aaithal commented Mar 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaithal Mar 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelkarp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaithal commented Mar 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaithal commented Jun 5, 2019

Uh oh!

Uh oh!

aaithal commented Mar 4, 2019 •

edited

Loading

aaithal Mar 4, 2019 •

edited

Loading

aaithal commented Mar 4, 2019 •

edited

Loading