Skip to content

Commit

Permalink
Merge pull request #164 from marquiz/devel/intel-rdt
Browse files Browse the repository at this point in the history
Add IntelRdt to CDI spec
  • Loading branch information
elezar authored Jan 10, 2024
2 parents 7b12d28 + 1537b1d commit 57c92f8
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 6 deletions.
3 changes: 1 addition & 2 deletions .codespellrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
[codespell]
skip = .git,*.pdf,*.svg,*.sum,*.mod
#
# ignore-words-list =
ignore-words-list = clos
22 changes: 19 additions & 3 deletions SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

## Version

This is CDI **spec** version **0.6.0**.
This is CDI **spec** version **0.7.0**.

### Update policy

Expand All @@ -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`.
Expand Down Expand Up @@ -82,7 +83,7 @@ The keywords "must", "must not", "required", "shall", "shall not", "should", "sh

```
{
"cdiVersion": "0.6.0",
"cdiVersion": "0.7.0",
"kind": "<name>",
// This field contains a set of key-value pairs that may be used to provide
Expand Down Expand Up @@ -150,6 +151,13 @@ The keywords "must", "must not", "required", "shall", "shall not", "should", "sh
"timeout": <int> (optional)
}
]
"intelRdt": { (optional)
"closID": "<name>", (optional)
"l3CacheSchema": "string" (optional)
"memBwSchema": "string" (optional)
"enableCMT": "<boolean>" (optional)
"enableMBM": "<boolean>" (optional)
}
}
]
}
Expand Down Expand Up @@ -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.
Expand All @@ -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
26 changes: 25 additions & 1 deletion pkg/cdi/container-edits.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
106 changes: 106 additions & 0 deletions pkg/cdi/container-edits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -555,6 +651,10 @@ func TestAppend(t *testing.T) {
Path: "/dev/dev1",
},
},
IntelRdt: &cdi.IntelRdt{
ClosID: "clos-1",
L3CacheSchema: "L3:0=ff",
},
},
},
{
Expand Down Expand Up @@ -583,6 +683,9 @@ func TestAppend(t *testing.T) {
Path: "/dev/dev4",
},
},
IntelRdt: &cdi.IntelRdt{
ClosID: "clos-2",
},
},
},
},
Expand All @@ -609,6 +712,9 @@ func TestAppend(t *testing.T) {
Path: "/dev/dev4",
},
},
IntelRdt: &cdi.IntelRdt{
ClosID: "clos-2",
},
},
},
},
Expand Down
27 changes: 27 additions & 0 deletions pkg/cdi/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
17 changes: 17 additions & 0 deletions pkg/cdi/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions schema/defs.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
"type": "string"
}
},
"FileName": {
"type": "string"
},
"FilePath": {
"type": "string"
},
Expand Down Expand Up @@ -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"
}
}
}
}
},
Expand Down
10 changes: 10 additions & 0 deletions specs-go/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"`
}
Loading

0 comments on commit 57c92f8

Please sign in to comment.