Add support for (Linux) net device injection. #269

klihub · 2025-04-09T13:29:17Z

Note: This PR is stacked on #294.

This PR adds support for injecting Linux NetDevices to an OCI Spec. In particular, the PR

adds NetDevices to ContainerEdits in the CDI Spec
implements network device injection to the OCI Spec
bumps the CDI Spec version to 1.1.0
adds CDI Spec version validation to the cmd/cdi test/cli binary

specs-go/version.go

specs-go/config.go

pkg/cdi/container-edits.go

github-actions · 2025-07-10T04:32:13Z

This PR is stale because it has been open 90 days with no activity. This PR will be closed in 30 days unless new comments are made or the stale label is removed. To skip these checks, apply the "lifecycle/frozen" label.

elezar · 2025-08-07T13:57:24Z

@klihub is there a version of the OCI runtime spec that includes these fields now? Could we update this PR so that we can look at pushing out a v1.1.0 of the spec together with the changes from #278 and #279?

klihub · 2025-08-07T14:06:58Z

@klihub is there a version of the OCI runtime spec that includes these fields now?

@elezar Not with a tagged version yet, unfortunately.

Could we update this PR so that we can look at pushing out a v1.1.0 of the spec together with the changes from #278 and #279?

I think we don't need to wait for this to get merged to tag a new version. We can tag a release without this right away, if everything else has been already merged. But we can't merge #279 yet, for the same reason. intelRdt.enableMonitoring is not available yet in a tagged version of the OCI Spec.

elezar · 2025-11-05T10:58:34Z

It seems as if the v1.3.0 spec has been tagged.

klihub · 2025-11-07T10:13:11Z

It seems as if the v1.3.0 spec has been tagged.

Yes, but I think we'll need for opencontainers/runtime-tools#795 (and opencontainers/runtime-tools#797) to get merged.

bart0sh · 2025-11-17T08:27:23Z

@klihub lgtm. Do we need to update/add any tests for the new fields?

.github/workflows/sanity.yml

klihub · 2025-11-17T09:19:23Z

@klihub lgtm. Do we need to update/add any tests for the new fields?

@bart0sh Yes, I think we'd need those and they are missing now. I'll add them.

klihub · 2025-11-17T09:53:30Z

@klihub lgtm. Do we need to update/add any tests for the new fields?

@bartosh Added test for injecting and validating net devices. Updated the schema to mark both host and container interface names as required for LinuxNetDevice. Updated ContainerEdit validation to require non-empty host and container net device names.

bart0sh

/lgtm

klihub · 2025-11-25T08:20:19Z

Ping @elezar, PTAL.

pkg/cdi/container-edits.go

elezar · 2025-11-25T09:13:10Z

specs-go/config.go

+
+// LinuxNetDevice represents an OCI LinuxNetDevice to be added to the OCI Spec.
+type LinuxNetDevice struct {
+	HostIf string `json:"hostIf" yaml:"hostIf"`


Question: Since HostIf is the field that we define, is it descriptive enough? In the OCI runtime spec we have:

// NetDevices are key-value pairs, keyed by network device name on the host, moved to the container's network namespace. NetDevices map[string]LinuxNetDevice `json:"netDevices,omitempty"`

Yeah, I don't know what would be descriptive enough. I deliberately chose a struct with two fields instead of a map, in the hope of better descriptiveness, because now I have a name for both parameters compared to a string-keyed map.

So maybe a HostInterface and ContainerInterface would be better (would not like to call them devices, because of all the unices it is exactly linux which does not model network devices as unix devices) ? @elezar @bart0sh WDYT ?

Or HostInterface and Name. I only ended up with Name to have it be called the same as the corresponding OCI Spec data. Although now that you spelled this out, I'm not sure if it helps or hurts more...

I was also considering HostInterface/ContainerName and HostName/ContainerName, but I disliked both HostName and ContainerName as they can sound a misleading, at least when out of context.

@bart0sh @elezar Any thoughts ? Would HostInterface / ContainerInterface be descriptive enough ?

I think Name is fine since this is the corresponding OCI Spec field name. (we do the same for DeviceNode.Path)

What about HostInterfaceName and Name? (I would suggest HostName, but that could also be more confusing).

As a follow-up question: Do we want to allow HostIf == "" and use the value for Name in this case, or are these expected to always be different?

I think Name is fine since this is the corresponding OCI Spec field name. (we do the same for DeviceNode.Path)

What about HostInterfaceName and Name? (I would suggest HostName, but that could also be more confusing).

Okay, I'll update it accordingly.

As a follow-up question: Do we want to allow HostIf == "" and use the value for Name in this case, or are these expected to always be different?

I don't think there is any expectation regarding those except that they are not empty. I'd probably would not allow HostInterfaceName to be empty. Allowing the other way around (the container interace) Name to be empty and set to the same as HostInterface) would sound intuitively more natural to me, but I would not allow that either.

On inferring information. Let's make them both required in the spec. This makes reasoning about the implementation quite a bit simpler.

pkg/cdi/container-edits.go

Signed-off-by: Krisztian Litkey <[email protected]>

Implement OCI Spec (Linux) NetDevice injection. Signed-off-by: Krisztian Litkey <[email protected]>

Bump current version to 1.1.0, include net devices in minimum required version check. Signed-off-by: Krisztian Litkey <[email protected]>

Signed-off-by: Krisztian Litkey <[email protected]>

elezar · 2025-11-26T15:30:58Z

specs-go/version.go

 const (
 	// CurrentVersion is the current version of the Spec.
-	CurrentVersion = "1.0.0"
+	CurrentVersion = "1.1.0"


I think we may have missed this for the new / updated RDT fields too, but we should probably have a SPEC.md change as well.

cc @marquiz

elezar · 2025-11-26T15:32:08Z

specs-go/config.go

 	EnableMonitoring bool     `json:"enableMonitoring,omitempty" yaml:"enableMonitoring,omitempty"`
 }
+
+// LinuxNetDevice represents an OCI LinuxNetDevice to be added to the OCI Spec.


Do we need to add a docstring that describes what the two fields are? (I'm ok to not do so if we feel these are self-explanatory).

elezar · 2025-11-26T15:32:54Z

specs-go/config.go

 	ClosID           string   `json:"closID,omitempty"           yaml:"closID,omitempty"`
 	L3CacheSchema    string   `json:"l3CacheSchema,omitempty"    yaml:"l3CacheSchema,omitempty"`
 	MemBwSchema      string   `json:"memBwSchema,omitempty"      yaml:"memBwSchema,omitempty"`
 	Schemata         []string `json:"schemata,omitempty"         yaml:"schemata,omitempty"`


Missed the //added in v1.1.0 suffis to the new fields here: cc @marquiz

elezar

@klihub this looks good now.

Some minor questions regarding whether this should include edits to SPEC.md, but I am approving this provisionally.

klihub force-pushed the devel/linux-net-device branch 3 times, most recently from 658b35b to 9900940 Compare April 9, 2025 13:37