From 1537b1dbd72ca3c971cdc498f558f986e0b9ffa1 Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Tue, 19 Sep 2023 10:05:04 +0300 Subject: [PATCH] Add IntelRdt to CDI spec Add support for specifying the OCI Linux.IntelRdt configuration that is the control point for cache and memory bandwidth allocation technologies sucn as Intel RDT (Resource Director Technology). There can only be one IntelRdt configuration per container so in case multiple specs happened to specify it the last one applied prevails. Also, the IntelRdt configuration is always applied as a whole - editing specific sub-fields is not supported. Signed-off-by: Markus Lehtonen --- .codespellrc | 3 +- SPEC.md | 22 ++++++- pkg/cdi/container-edits.go | 26 +++++++- pkg/cdi/container-edits_test.go | 106 ++++++++++++++++++++++++++++++++ pkg/cdi/spec_test.go | 27 ++++++++ pkg/cdi/version.go | 17 +++++ schema/defs.json | 23 +++++++ specs-go/config.go | 10 +++ specs-go/oci.go | 11 ++++ 9 files changed, 239 insertions(+), 6 deletions(-) diff --git a/.codespellrc b/.codespellrc index 05a029b0..c109564a 100644 --- a/.codespellrc +++ b/.codespellrc @@ -1,4 +1,3 @@ [codespell] skip = .git,*.pdf,*.svg,*.sum,*.mod -# -# ignore-words-list = +ignore-words-list = clos diff --git a/SPEC.md b/SPEC.md index e3c05d5b..324aa6d3 100644 --- a/SPEC.md +++ b/SPEC.md @@ -8,7 +8,7 @@ ## Version -This is CDI **spec** version **0.6.0**. +This is CDI **spec** version **0.7.0**. ### Update policy @@ -27,7 +27,8 @@ Released versions of the spec are available as Git tags. | v0.4.0 | | Added `type` field to Mount specification | | v0.5.0 | | Add `HostPath` to `DeviceNodes` | | v0.6.0 | | Add `Annotations` field to `Spec` and `Device` specifications | -| | | Allow dots (`.`) in name segment of `Kind` field | +| | | Allow dots (`.`) in name segment of `Kind` field. | +| v0.7.0 | | Add `IntelRdt`field. | *Note*: The initial release of a **spec** with version `v0.x.0` will be tagged as `v0.x.0` with subsequent changes to the API applicable to this version tagged as `v0.x.y`. @@ -82,7 +83,7 @@ The keywords "must", "must not", "required", "shall", "shall not", "should", "sh ``` { - "cdiVersion": "0.6.0", + "cdiVersion": "0.7.0", "kind": "", // This field contains a set of key-value pairs that may be used to provide @@ -150,6 +151,13 @@ The keywords "must", "must not", "required", "shall", "shall not", "should", "sh "timeout": (optional) } ] + "intelRdt": { (optional) + "closID": "", (optional) + "l3CacheSchema": "string" (optional) + "memBwSchema": "string" (optional) + "enableCMT": "" (optional) + "enableMBM": "" (optional) + } } ] } @@ -220,6 +228,12 @@ The `containerEdits` field has the following definition: * `args` (array of strings, OPTIONAL) with the same semantics as IEEE Std 1003.1-2008 execv's argv. * `env` (array of strings, OPTIONAL) with the same semantics as IEEE Std 1003.1-2008's environ. * `timeout` (int, OPTIONAL) is the number of seconds before aborting the hook. If set, timeout MUST be greater than zero. If not set container runtime will wait for the hook to return. + * `intelRdt` (object, OPTIONAL) describes the Linux [resctrl][resctrl] settings for the container (object, OPTIONAL) + * `closID` (string, OPTIONAL) name of the `CLOS` (Class of Service). + * `l3CacheSchema` (string, OPTIONAL) L3 cache allocation schema for the `CLOS`. + * `memBwSchema` (string, OPTIONAL) memory bandwidth allocation schema for the `CLOS`. + * `enableCMT` (boolean, OPTIONAL) whether to enable cache monitoring + * `enableMBM` (boolean, OPTIONAL) whether to enable memory bandwidth monitoring ## Error Handling * Kind requested is not present in any CDI file. @@ -231,3 +245,5 @@ The `containerEdits` field has the following definition: This is because a resource does not need to exist when the spec is written, but it needs to exist when the container is created. * Hook fails to execute. Container runtimes should surface an error when hooks fails to execute. + +[resctrl]: https://docs.kernel.org/arch/x86/resctrl.html diff --git a/pkg/cdi/container-edits.go b/pkg/cdi/container-edits.go index 688ddf78..7f92cbbc 100644 --- a/pkg/cdi/container-edits.go +++ b/pkg/cdi/container-edits.go @@ -144,6 +144,13 @@ func (e *ContainerEdits) Apply(spec *oci.Spec) error { } } + if e.IntelRdt != nil { + // The specgen is missing functionality to set all parameters so we + // just piggy-back on it to initialize all structs and the copy over. + specgen.SetLinuxIntelRdtClosID(e.IntelRdt.ClosID) + spec.Linux.IntelRdt = e.IntelRdt.ToOCI() + } + return nil } @@ -171,6 +178,11 @@ func (e *ContainerEdits) Validate() error { return err } } + if e.IntelRdt != nil { + if err := ValidateIntelRdt(e.IntelRdt); err != nil { + return err + } + } return nil } @@ -192,6 +204,9 @@ func (e *ContainerEdits) Append(o *ContainerEdits) *ContainerEdits { e.DeviceNodes = append(e.DeviceNodes, o.DeviceNodes...) e.Hooks = append(e.Hooks, o.Hooks...) e.Mounts = append(e.Mounts, o.Mounts...) + if o.IntelRdt != nil { + e.IntelRdt = o.IntelRdt + } return e } @@ -202,7 +217,7 @@ func (e *ContainerEdits) isEmpty() bool { if e == nil { return false } - return len(e.Env)+len(e.DeviceNodes)+len(e.Hooks)+len(e.Mounts) == 0 + return len(e.Env)+len(e.DeviceNodes)+len(e.Hooks)+len(e.Mounts) == 0 && e.IntelRdt == nil } // ValidateEnv validates the given environment variables. @@ -280,6 +295,15 @@ func (m *Mount) Validate() error { return nil } +// ValidateIntelRdt validates the IntelRdt configuration. +func ValidateIntelRdt(i *specs.IntelRdt) error { + // ClosID must be a valid Linux filename + if len(i.ClosID) >= 4096 || i.ClosID == "." || i.ClosID == ".." || strings.ContainsAny(i.ClosID, "/\n") { + return errors.New("invalid ClosID") + } + return nil +} + // Ensure OCI Spec hooks are not nil so we can add hooks. func ensureOCIHooks(spec *oci.Spec) { if spec.Hooks == nil { diff --git a/pkg/cdi/container-edits_test.go b/pkg/cdi/container-edits_test.go index 3bfc1dff..b9b53bd3 100644 --- a/pkg/cdi/container-edits_test.go +++ b/pkg/cdi/container-edits_test.go @@ -245,6 +245,50 @@ func TestValidateContainerEdits(t *testing.T) { }, invalid: true, }, + { + name: "valid rdt config", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo.bar", + }, + }, + }, + { + name: "invalid rdt config, invalid closID (slash)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo/bar", + }, + }, + invalid: true, + }, + { + name: "invalid rdt config, invalid closID (dot)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: ".", + }, + }, + invalid: true, + }, + { + name: "invalid rdt config, invalid closID (double dot)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "..", + }, + }, + invalid: true, + }, + { + name: "invalid rdt config, invalid closID (newline)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo\nbar", + }, + }, + invalid: true, + }, } { t.Run(tc.name, func(t *testing.T) { edits := ContainerEdits{tc.edits} @@ -467,6 +511,58 @@ func TestApplyContainerEdits(t *testing.T) { }, }, }, + { + name: "empty spec, rdt", + spec: &oci.Spec{}, + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "clos-1", + L3CacheSchema: "L3:0=ff;1=ff", + MemBwSchema: "MB:0=50;1=50", + EnableCMT: true, + EnableMBM: true, + }, + }, + result: &oci.Spec{ + Linux: &oci.Linux{ + IntelRdt: &oci.LinuxIntelRdt{ + ClosID: "clos-1", + L3CacheSchema: "L3:0=ff;1=ff", + MemBwSchema: "MB:0=50;1=50", + EnableCMT: true, + EnableMBM: true, + }, + }, + }, + }, + { + name: "non-empty spec, overriding rdt", + spec: &oci.Spec{ + Linux: &oci.Linux{ + IntelRdt: &oci.LinuxIntelRdt{ + ClosID: "clos-1", + L3CacheSchema: "L3:0=ff", + MemBwSchema: "MB:0=100", + EnableCMT: true, + EnableMBM: true, + }, + }, + }, + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "clos-2", + L3CacheSchema: "L3:0=f", + }, + }, + result: &oci.Spec{ + Linux: &oci.Linux{ + IntelRdt: &oci.LinuxIntelRdt{ + ClosID: "clos-2", + L3CacheSchema: "L3:0=f", + }, + }, + }, + }, } { t.Run(tc.name, func(t *testing.T) { edits := ContainerEdits{tc.edits} @@ -555,6 +651,10 @@ func TestAppend(t *testing.T) { Path: "/dev/dev1", }, }, + IntelRdt: &cdi.IntelRdt{ + ClosID: "clos-1", + L3CacheSchema: "L3:0=ff", + }, }, }, { @@ -583,6 +683,9 @@ func TestAppend(t *testing.T) { Path: "/dev/dev4", }, }, + IntelRdt: &cdi.IntelRdt{ + ClosID: "clos-2", + }, }, }, }, @@ -609,6 +712,9 @@ func TestAppend(t *testing.T) { Path: "/dev/dev4", }, }, + IntelRdt: &cdi.IntelRdt{ + ClosID: "clos-2", + }, }, }, }, diff --git a/pkg/cdi/spec_test.go b/pkg/cdi/spec_test.go index 7bdd0631..2c9cdc6f 100644 --- a/pkg/cdi/spec_test.go +++ b/pkg/cdi/spec_test.go @@ -669,6 +669,33 @@ func TestRequiredVersion(t *testing.T) { }, expectedVersion: "0.6.0", }, + { + description: "IntelRdt requires v0.7.0", + spec: &cdi.Spec{ + ContainerEdits: cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo", + }, + }, + }, + expectedVersion: "0.7.0", + }, + { + description: "IntelRdt (on devices) requires v0.7.0", + spec: &cdi.Spec{ + Devices: []cdi.Device{ + { + Name: "device0", + ContainerEdits: cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo", + }, + }, + }, + }, + }, + expectedVersion: "0.7.0", + }, } for _, tc := range testCases { diff --git a/pkg/cdi/version.go b/pkg/cdi/version.go index a6178127..c7750608 100644 --- a/pkg/cdi/version.go +++ b/pkg/cdi/version.go @@ -39,6 +39,7 @@ const ( v040 version = "v0.4.0" v050 version = "v0.5.0" v060 version = "v0.6.0" + v070 version = "v0.7.0" // vEarliest is the earliest supported version of the CDI specification vEarliest version = v030 @@ -54,6 +55,7 @@ var validSpecVersions = requiredVersionMap{ v040: requiresV040, v050: requiresV050, v060: requiresV060, + v070: requiresV070, } // MinimumRequiredVersion determines the minimum spec version for the input spec. @@ -118,6 +120,21 @@ func (r requiredVersionMap) requiredVersion(spec *cdi.Spec) version { return minVersion } +// requiresV070 returns true if the spec uses v0.7.0 features +func requiresV070(spec *cdi.Spec) bool { + if spec.ContainerEdits.IntelRdt != nil { + return true + } + + for _, d := range spec.Devices { + if d.ContainerEdits.IntelRdt != nil { + return true + } + } + + return false +} + // requiresV060 returns true if the spec uses v0.6.0 features func requiresV060(spec *cdi.Spec) bool { // The v0.6.0 spec allows annotations to be specified at a spec level diff --git a/schema/defs.json b/schema/defs.json index 012de1a4..e23aca51 100644 --- a/schema/defs.json +++ b/schema/defs.json @@ -17,6 +17,9 @@ "type": "string" } }, + "FileName": { + "type": "string" + }, "FilePath": { "type": "string" }, @@ -134,6 +137,26 @@ "items": { "$ref": "#/definitions/Hook" } + }, + "intelRdt": { + "type": "object", + "properties": { + "closID": { + "$ref": "#/definitions/FileName" + }, + "l3CacheSchema": { + "type": "string" + }, + "memBwSchema": { + "type": "string" + }, + "enableCMT": { + "type": "boolean" + }, + "enableMBM": { + "type": "boolean" + } + } } } }, diff --git a/specs-go/config.go b/specs-go/config.go index 4043b858..69c39689 100644 --- a/specs-go/config.go +++ b/specs-go/config.go @@ -29,6 +29,7 @@ type ContainerEdits struct { DeviceNodes []*DeviceNode `json:"deviceNodes,omitempty"` Hooks []*Hook `json:"hooks,omitempty"` Mounts []*Mount `json:"mounts,omitempty"` + IntelRdt *IntelRdt `json:"intelRdt,omitempty"` } // DeviceNode represents a device node that needs to be added to the OCI spec. @@ -60,3 +61,12 @@ type Hook struct { Env []string `json:"env,omitempty"` Timeout *int `json:"timeout,omitempty"` } + +// IntelRdt describes the Linux IntelRdt parameters to set in the OCI spec. +type IntelRdt struct { + ClosID string `json:"closID,omitempty"` + L3CacheSchema string `json:"l3CacheSchema,omitempty"` + MemBwSchema string `json:"memBwSchema,omitempty"` + EnableCMT bool `json:"enableCMT,omitempty"` + EnableMBM bool `json:"enableMBM,omitempty"` +} diff --git a/specs-go/oci.go b/specs-go/oci.go index 229ad52e..1b4d576b 100644 --- a/specs-go/oci.go +++ b/specs-go/oci.go @@ -36,3 +36,14 @@ func (d *DeviceNode) ToOCI() spec.LinuxDevice { GID: d.GID, } } + +// ToOCI returns the opencontainers runtime Spec LinuxIntelRdt for this IntelRdt config. +func (i *IntelRdt) ToOCI() *spec.LinuxIntelRdt { + return &spec.LinuxIntelRdt{ + ClosID: i.ClosID, + L3CacheSchema: i.L3CacheSchema, + MemBwSchema: i.MemBwSchema, + EnableCMT: i.EnableCMT, + EnableMBM: i.EnableMBM, + } +}