Define Linux Network Devices

[aojea]

The proposed "netdevices" field provides a declarative way to specify which host network devices should be moved into a container's network namespace.

This approach is similar than the existing "devices" field used for block devices but uses a dictionary keyed by the interface name instead.

The proposed scheme is based on the existing representation of network device by the struct net_device
https://docs.kernel.org/networking/netdevices.html.

This proposal focuses solely on moving existing network devices into the container namespace. It does not cover the complexities of network configuration or network interface creation, emphasizing the separation of device management and network configuration.

Fixes: #1239

[aojea]

/assign @samuelkarp

[AkihiroSuda]

https://github.com/opencontainers/runtime-spec/blob/main/features.md should be updated too

[aojea]

https://github.com/opencontainers/runtime-spec/blob/main/features.md should be updated too

updated and addressed the comments

[aojea]

AI @aojea (document the cleanup and destroy of the network interfaces)

[samuelkarp]

From the in-person discussion today:

• Net device lifecycle should follow the network namespace lifecycle
• @aojea will follow up to determine whether any cleanup actions need to be taken by the OCI runtime on a container being deleted
• @kad was concerned about restarts and error handling
• Should we prohibit the new netdev addition to an existing netns? IOW only allow this for containers where a new netns is created? What about containers where the root netns is used?

[utam0k]

This spec said "MUST" but, I think it can't do it in the rootless container because the rootless container doesn't have CAP_NET_ADMIN, right?

[utam0k]

I'm not sure we should take care of the rootless container.

[samuelkarp]

Could be an error in the case of a rootless container, if the runtime is not able to satisfy the MUST condition.

[utam0k]

Could be an error in the case of a rootless container, if the runtime is not able to satisfy the MUST condition.

+1 but It'd be better to clarify it in the spec.

[aojea]

added mor explanations about runtime and network devices lifecycle and runtime checks, PTAL

[aojea]

om the in-person discussion today:

• Net device lifecycle should follow the network namespace lifecycle
• @aojea will follow up to determine whether any cleanup actions need to be taken by the OCI runtime on a container being deleted
• @kad was concerned about restarts and error handling
• Should we prohibit the new netdev addition to an existing netns? IOW only allow this for containers where a new netns is created? What about containers where the root netns is used?

Pushed a new commit addressing those comments, the changelog is

• the network namespace lifecycle will move migratebale network devices and destroy virtual devides, the runtime MAY decide to do cleanup actions
• runtime MUST check the container has enough privileges and an associated network namespace and fail if the check fail
• removed the Mask field and use the Address field with CIDR notation (IP/Prefix) to deal with IPv4 and IPv6 addresses. Only one IP is allowed to be specified on purpose to simplify the operations and reduce risks
• Add a HardwareAddress field for use cases that require to set a
  specific mac or infiniband address

[kolyshkin]

Left some more comments; to me it seems this won't be ready for 1.2.1 (or 1.3.0) if we want to cut a release soon.

[kolyshkin]

The key distinction here is between initialization and configuration. My proposal focuses on providing the bare minimum initialization required for basic network functionality within the container. This includes essential properties like the device name, MAC address, IP address, netmask, and MTU. These are the fundamental attributes that are necessary for the interface to be operational at all.

The benefits I see with this initial setup in the runtime is that it reduces the complexity of integration and avoids potential race conditions where the application might try to use the network before it's properly initialized. It also allow to reduce container size and complexity if we don't need to include scripts or logic to perform this basic network setup every time.

I like to think of this as: the runtime provides a basic "network plumbing" framework, while the application running inside the container is free to customize and configure the network according to its specific needs using tools like ip.

To me it seems that a simple StartContainer hook (which runs in a container before the container initial process is started) which calls ip (or, depending on the distro, ifconfig, etc.) with all needed parameters should be sufficient for configuring any network device. Sure, that requires a working ip (of ifconfig etc.) binary inside a container.

In case ip binary is not possible to have in container, this can be achieved via CreateRuntime hook and ip on the host.

Or, it could be done on the host by a higher-level orchestration tools just before container creation. Provided you can configure the interface while it's down, everything can be pre-configured, and the runtime tasks would be limited to:

• moving a network device into container's netns;
• optional renaming;
• bringing it up.

I'm also worried about feature creep here. The list of properties needed to be set will grow over time, needing more and more from the runtime implementation, making it even more bloated. Say, what about protocols other than IPv4 and IPv6? Various devices that need more specific properties? Static routing entries? The list can go on and on.

Having said that, I can agree that a way to move a network device to a container would be nice to have. But I strongly believe that configuring its properties is a bit out of scope for container runtime.

[aojea]

Having said that, I can agree that a way to move a network device to a container would be nice to have. But I strongly believe that configuring its properties is a bit out of scope for container runtime.

I'm completely fine with that and that is what I want and need for Kubernetes, the bootstrapping configuration options are very nice to have things but not essential, as you mentioned the scope was creeping during the review, so my question is if all of you we'll be fine just removing that part and focusing only on the "moving interfaces part", @kad you were very vocal about IP configuration, is this acceptable?

[aojea]

Or, it could be done on the host by a higher-level orchestration tools just before container creation. Provided you can configure the interface while it's down, everything can be pre-configured, and the runtime tasks would be limited to:

• moving a network device into container's netns;
• optional renaming;
• bringing it up.

I'm also worried about feature creep here. The list of properties needed to be set will grow over time, needing more and more from the runtime implementation, making it even more bloated. Say, what about protocols other than IPv4 and IPv6? Various devices that need more specific properties? Static routing entries? The list can go on and on

@kolyshkin see last commit removing all the network configuration options and leaving the proposal to only the network interface plumbing.

I think that this is a good next step for everyone involved in the review since it allows to unblock a lot of scenarios that today we need to workaround with out of band mechanisms and a lot of redtape ... and this is also not blocking a future expansion in case someone wants to pursue the network initialization side , but for Kubernetes this new feature in current proposal is really beneficial so I appreciate if we can still consider for next release

[kad]

I'm completely fine with that and that is what I want and need for Kubernetes, the bootstrapping configuration options are very nice to have things but not essential, as you mentioned the scope was creeping during the review, so my question is if all of you we'll be fine just removing that part and focusing only on the "moving interfaces part", @kad you were very vocal about IP configuration, is this acceptable?

we have case where simple assign list of addresses is going to be helpful. Removing dependency to external hook would be very nice to have.

Speaking of hooks, I need to remind that hooks usually leads to contamination of host OS (in addition to increasing complexity of deployment such hooks that sole purpose would be to call ip addr add in particular namespace) or container image. I understand concern about "feature creep", but for things that are easily available via few netlink api calls, I don't see any big reasons why it be too complex to implement in runtime. Especially if we start thinking outside of runc/crun implementations, some other runtimes might implement "namespaces" differently (vm, usermodelinux, etc), so "moving interfaces" might have some specifics. I think original proposal about having minimal list of properties to configure was good enough middle ground.

[kolyshkin]

@aojea can you squash the commits? Otherwise LGTM, and this is definitely something we can build upon.

[aojea]

@aojea can you squash the commits? Otherwise LGTM, and this is definitely something we can build upon.

@kolyshkin squashed, thanks

[kolyshkin]

LGTM except for one nit.

[tianon]

for things that are easily available via few netlink api calls, I don't see any big reasons why it be too complex to implement in runtime

The problem in my mind is where do you draw the line? At what point does "a few netlink api calls" become "a few too many" and now it's no longer acceptable?

I agree with Kir that we should keep the scope really limited here so we don't accidentally stray too far into network management in the low level runtime, which is something we've traditionally left as a higher level concern for a lot of good reasons.

[kolyshkin]

One more comment -- why is the data structure a map? A map makes sense when we want to quickly find an element based on a key. Is it needed here?

Perhaps a simple array is easier.

[aojea]

One more comment -- why is the data structure a map? A map makes sense when we want to quickly find an element based on a key. Is it needed here?

interfaces names are unique per namespace, a map guarantees this avoiding user fat finger problems or misconfiguration, otherwise we need to validate that the names in the array are unique

[kolyshkin]

interfaces names are unique per namespace, a map guarantees this avoiding user fat finger problems or misconfiguration, otherwise we need to validate that the names in the array are unique

I don't think the uniqueness requirement justifies using map, and I don't think we should be concerned much about uniqueness. In case a network device will be listed twice, the second entry will fail with "no such device" because the first one will already be moved.

Also, we have other similar fields (Devices, Namespaces, Mounts etc.) as arrays. I see that time namespace offsets is implemented via maps (and I think it was a mistake, for similar reasons -- an array would work just fine).

Having said that, this is not a deal breaker. Also, I'm sure other maintainers may have different opinions. WDYT @opencontainers/runtime-spec-maintainers ?

[pfl]

I agree with Kir that we should keep the scope really limited here so we don't accidentally stray too far into network management in the low level runtime, which is something we've traditionally left as a higher level concern for a lot of good reasons.

With the IP configuration part now removed, am I correct that the "easy" solution configuring IP networking for additional interfaces is still via a custom runtime hook? In our case the containers are static and cannot be amended with IP configuration tools or the necessary networking capabilities, and I doubt we are able to inject another container into the pod without causing a stir. IP (and yes, a route configuration) would have been a good and standard solution for simple secondary network needs.

[kad]

for things that are easily available via few netlink api calls, I don't see any big reasons why it be too complex to implement in runtime

The problem in my mind is where do you draw the line? At what point does "a few netlink api calls" become "a few too many" and now it's no longer acceptable?

My "border line" is about features and fields provided by kernel netlink api. If something is done with it, I don't see big difference between call "move interface xyz to namespace abc" vs. "set property foo to value bar on interface xyz in namespace abc". Or to what it worth, "put value 123 into cgroup foobar".
Things like DHCP that require more than plain kernel api call are clearly outside of the boundaries that can be implemented in runtime.

[kolyshkin]

what is 23 here?

[aojea]

an integer to cause a bad config since it is expecting a string

[kolyshkin]

for things that are easily available via few netlink api calls, I don't see any big reasons why it be too complex to implement in runtime

The problem in my mind is where do you draw the line? At what point does "a few netlink api calls" become "a few too many" and now it's no longer acceptable?

My "border line" is about features and fields provided by kernel netlink api. If something is done with it, I don't see big difference between call "move interface xyz to namespace abc" vs. "set property foo to value bar on interface xyz in namespace abc". Or to what it worth, "put value 123 into cgroup foobar". Things like DHCP that require more than plain kernel api call are clearly outside of the boundaries that can be implemented in runtime.

To me, this sounds like a runtime should implement most of what ip(8) is capable of. Which is a lot.

If that's the case, here's a crazy idea.

Considering that ip(8) is a de facto standard on Linux, perhaps such configuration can be achieved by supplying a sequence of ip command arguments in runtime-spec. A runtime when can use ip binary on the host to get to the needed configuration.

Something like

"linux": {
        "netDevices": [
            "name": "enp34s0u2u1u2",
            "ct_name": "eth0",
            "config": [
                "ip addr add 10.2.3.4/25 dev $IF",
                "ip link set $IF mtu 1350",
                "ip route add 10.10.0.0/16 via 10.2.3.99 dev $IF proto static metric 100"
                "ip route add default dev $IF"
            ]
        ]
    }

Which will result in runtime running the host's ip binary in the container's network namespace (after moving the device into container's namespace and giving it a new name). It can even be a single ip invocation if -b (batch mode) is supported.

The upsides are:

• this uses a well known, de facto standard "domain language" for network-related configuration;
• this allows for a quite complex configuration to be performed (limited to whatever ip understands);
• this seems trivial to implement for any runtime;
• this doesn't impose any specific container image requirements (no need for any extra binaries etc.);
• this should be secure (as ip is running inside a netns);
• some advanced runtimes can implement a subset of this internally (i.e. without the call to ip);

The biggest downside, I guess, is the scope is not well defined, as ip is evolving.

[aojea]

I don't like the idea of execing commands, specially in golang implementations that are problematic with goroutines and namespaces
I think that once we have just the network interfaces on the runtimes we'll get much better feedback and signal from the implementations to evaluate the next steps and to know if something else is needed and what exactly

I still think that preventing to detect failures at runtime is desirable, you can argue you should not add duplicates, but bugs exist and if somebody accidentally add a duplicate interface name it will cause a problem in production we could have avoided just not allowing to define it

[kad]

Considering that ip(8) is a de facto standard on Linux, perhaps such configuration can be achieved by supplying a sequence of ip command arguments in runtime-spec. A runtime when can use ip binary on the host to get to the needed configuration.

I believe that this is really bad idea, compared to have limited set of config options implemented. exec() is always slower and less reliable than calling native kernel APIs. It opens doors to all various injection vulnerabilities and other potential misuses and scope creeps than strict API subset usage. As well as Antonio mentioned proper error handling.

To provide more insights on our use case, here is snip of the code that we want to get rid of, currently implemented as OCI runtime hook (btw, including already unreliable call to ip in it): https://github.com/HabanaAI/habana-container-runtime/blob/main/cmd/habana-container-cli/netdevices.go#L131-L182