From 7296dc171238291f4fc5336bbb2fba3d5cb70109 Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Wed, 31 Mar 2021 17:05:00 +0300 Subject: [PATCH 1/5] libcontainer/intelrdt: refactor clos path handling Simplify the code and make path a property of the container (via intelRdtManager). Signed-off-by: Markus Lehtonen --- libcontainer/container_linux.go | 7 ++--- libcontainer/intelrdt/intelrdt.go | 45 +++++++++---------------------- 2 files changed, 16 insertions(+), 36 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 19a82bbf6f6..39932c751a5 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -1902,9 +1902,10 @@ func (c *linuxContainer) currentState() (*State, error) { startTime, _ = c.initProcess.startTime() externalDescriptors = c.initProcess.externalDescriptors() } - intelRdtPath, err := intelrdt.GetIntelRdtPath(c.ID()) - if err != nil { - intelRdtPath = "" + + intelRdtPath := "" + if c.intelRdtManager != nil { + intelRdtPath = c.intelRdtManager.GetPath() } state := &State{ BaseState: BaseState{ diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 9c32aabd337..2b62af10486 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -207,7 +207,6 @@ var ( type intelRdtData struct { root string config *configs.Config - pid int } // Check if Intel RDT sub-features are enabled in featuresInit() @@ -405,18 +404,6 @@ func writeFile(dir, file, data string) error { return nil } -func getIntelRdtData(c *configs.Config, pid int) (*intelRdtData, error) { - rootPath, err := getIntelRdtRoot() - if err != nil { - return nil, err - } - return &intelRdtData{ - root: rootPath, - config: c, - pid: pid, - }, nil -} - // Get the read-only L3 cache information func getL3CacheInfo() (*L3CacheInfo, error) { l3CacheInfo := &L3CacheInfo{} @@ -532,14 +519,13 @@ func IsMBAScEnabled() bool { } // Get the 'container_id' path in Intel RDT "resource control" filesystem -func GetIntelRdtPath(id string) (string, error) { +func (m *intelRdtManager) getIntelRdtPath() (string, error) { rootPath, err := getIntelRdtRoot() if err != nil { return "", err } - path := filepath.Join(rootPath, id) - return path, nil + return filepath.Join(rootPath, m.id), nil } // Applies Intel RDT configuration to the process with the specified pid @@ -548,16 +534,21 @@ func (m *intelRdtManager) Apply(pid int) (err error) { if m.config.IntelRdt == nil { return nil } - d, err := getIntelRdtData(m.config, pid) + + path, err := m.getIntelRdtPath() if err != nil { return err } m.mu.Lock() defer m.mu.Unlock() - path, err := d.join(m.id) - if err != nil { - return err + + if err := os.MkdirAll(path, 0o755); err != nil { + return newLastCmdError(err) + } + + if err := WriteIntelRdtTasks(path, pid); err != nil { + return newLastCmdError(err) } m.path = path @@ -579,7 +570,7 @@ func (m *intelRdtManager) Destroy() error { // restore the object later func (m *intelRdtManager) GetPath() string { if m.path == "" { - m.path, _ = GetIntelRdtPath(m.id) + m.path, _ = m.getIntelRdtPath() } return m.path } @@ -747,18 +738,6 @@ func (m *intelRdtManager) Set(container *configs.Config) error { return nil } -func (raw *intelRdtData) join(id string) (string, error) { - path := filepath.Join(raw.root, id) - if err := os.MkdirAll(path, 0o755); err != nil { - return "", newLastCmdError(err) - } - - if err := WriteIntelRdtTasks(path, raw.pid); err != nil { - return "", err - } - return path, nil -} - func newLastCmdError(err error) error { status, err1 := getLastCmdStatus() if err1 == nil { From 17e3b41dd002a6c869379f5e19a76e8b868e74bf Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Wed, 31 Mar 2021 19:25:49 +0300 Subject: [PATCH 2/5] libcontainer/intelrdt: support ClosID parameter Handle ClosID parameter of IntelRdt. Makes it possible to use pre-configured classes/ClosIDs and avoid running out of available IDs which easily happens with per-container classes. Remove validator checks for empty L3CacheSchema and MemBwSchema fields in order to be able to leave them empty, and only specify ClosID for a pre-configured class. Signed-off-by: Markus Lehtonen --- libcontainer/configs/intelrdt.go | 3 +++ libcontainer/configs/validate/validator.go | 11 ++++------- libcontainer/intelrdt/intelrdt.go | 22 ++++++++++++++++------ libcontainer/specconv/spec_linux.go | 1 + 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/libcontainer/configs/intelrdt.go b/libcontainer/configs/intelrdt.go index 57e9f037d97..f8d951ab8b9 100644 --- a/libcontainer/configs/intelrdt.go +++ b/libcontainer/configs/intelrdt.go @@ -1,6 +1,9 @@ package configs type IntelRdt struct { + // The identity for RDT Class of Service + ClosID string `json:"closID,omitempty"` + // The schema for L3 cache id and capacity bitmask (CBM) // Format: "L3:=;=;..." L3CacheSchema string `json:"l3_cache_schema,omitempty"` diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go index c2500a63052..ab43a17f247 100644 --- a/libcontainer/configs/validate/validator.go +++ b/libcontainer/configs/validate/validator.go @@ -205,19 +205,16 @@ func (v *ConfigValidator) intelrdt(config *configs.Config) error { return errors.New("intelRdt is specified in config, but Intel RDT is not supported or enabled") } + if config.IntelRdt.ClosID == "." || config.IntelRdt.ClosID == ".." || strings.Contains(config.IntelRdt.ClosID, "/") { + return fmt.Errorf("invalid intelRdt.ClosID %q", config.IntelRdt.ClosID) + } + if !intelrdt.IsCATEnabled() && config.IntelRdt.L3CacheSchema != "" { return errors.New("intelRdt.l3CacheSchema is specified in config, but Intel RDT/CAT is not enabled") } if !intelrdt.IsMBAEnabled() && config.IntelRdt.MemBwSchema != "" { return errors.New("intelRdt.memBwSchema is specified in config, but Intel RDT/MBA is not enabled") } - - if intelrdt.IsCATEnabled() && config.IntelRdt.L3CacheSchema == "" { - return errors.New("Intel RDT/CAT is enabled and intelRdt is specified in config, but intelRdt.l3CacheSchema is empty") - } - if intelrdt.IsMBAEnabled() && config.IntelRdt.MemBwSchema == "" { - return errors.New("Intel RDT/MBA is enabled and intelRdt is specified in config, but intelRdt.memBwSchema is empty") - } } return nil diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 2b62af10486..18d74f2b1ad 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -525,7 +525,12 @@ func (m *intelRdtManager) getIntelRdtPath() (string, error) { return "", err } - return filepath.Join(rootPath, m.id), nil + clos := m.id + if m.config.IntelRdt != nil && m.config.IntelRdt.ClosID != "" { + clos = m.config.IntelRdt.ClosID + } + + return filepath.Join(rootPath, clos), nil } // Applies Intel RDT configuration to the process with the specified pid @@ -557,12 +562,17 @@ func (m *intelRdtManager) Apply(pid int) (err error) { // Destroys the Intel RDT 'container_id' group func (m *intelRdtManager) Destroy() error { - m.mu.Lock() - defer m.mu.Unlock() - if err := os.RemoveAll(m.GetPath()); err != nil { - return err + // Don't remove resctrl group if closid has been explicitly specified. The + // group is likely externally managed, i.e. by some other entity than us. + // There are probably other containers/tasks sharing the same group. + if m.config.IntelRdt == nil || m.config.IntelRdt.ClosID == "" { + m.mu.Lock() + defer m.mu.Unlock() + if err := os.RemoveAll(m.GetPath()); err != nil { + return err + } + m.path = "" } - m.path = "" return nil } diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 89172316358..bbecf2b33fc 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -301,6 +301,7 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) { } if spec.Linux.IntelRdt != nil { config.IntelRdt = &configs.IntelRdt{ + ClosID: spec.Linux.IntelRdt.ClosID, L3CacheSchema: spec.Linux.IntelRdt.L3CacheSchema, MemBwSchema: spec.Linux.IntelRdt.MemBwSchema, } From 79d292b9ff79f71b8f561c0c3f1fb0fca875142d Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Mon, 26 Apr 2021 13:28:21 +0300 Subject: [PATCH 3/5] libcontainer/intelrdt: verify ClosID existence Check that the ClosID directory pre-exists if no L3 or MB schema has been specified. Conform with the following line from runtime-spec (config-linux): If closID is set, and neither of l3CacheSchema and memBwSchema are set, runtime MUST check if corresponding pre-configured directory closID is present in mounted resctrl. If such pre-configured directory closID exists, runtime MUST assign container to this closID and generate an error if directory does not exist. Add a TODO note for verifying existing schemata against L3/MB parameters. Signed-off-by: Markus Lehtonen --- libcontainer/intelrdt/intelrdt.go | 15 ++++++++++++- libcontainer/intelrdt/intelrdt_test.go | 31 ++++++++++++++++++++++++++ libcontainer/intelrdt/util_test.go | 4 ++-- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 18d74f2b1ad..a12aca9b9c3 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -205,7 +205,6 @@ var ( ) type intelRdtData struct { - root string config *configs.Config } @@ -548,6 +547,14 @@ func (m *intelRdtManager) Apply(pid int) (err error) { m.mu.Lock() defer m.mu.Unlock() + if m.config.IntelRdt.ClosID != "" && m.config.IntelRdt.L3CacheSchema == "" && m.config.IntelRdt.MemBwSchema == "" { + // Check that the CLOS exists, i.e. it has been pre-configured to + // conform with the runtime spec + if _, err := os.Stat(path); err != nil { + return fmt.Errorf("clos dir not accessible (must be pre-created when l3CacheSchema and memBwSchema are empty): %w", err) + } + } + if err := os.MkdirAll(path, 0o755); err != nil { return newLastCmdError(err) } @@ -723,6 +730,12 @@ func (m *intelRdtManager) Set(container *configs.Config) error { l3CacheSchema := container.IntelRdt.L3CacheSchema memBwSchema := container.IntelRdt.MemBwSchema + // TODO: verify that l3CacheSchema and/or memBwSchema match the + // existing schemata if ClosID has been specified. This is a more + // involved than reading the file and doing plain string comparison as + // the value written in does not necessarily match what gets read out + // (leading zeros, cache id ordering etc). + // Write a single joint schema string to schemata file if l3CacheSchema != "" && memBwSchema != "" { if err := writeFile(path, "schemata", l3CacheSchema+"\n"+memBwSchema); err != nil { diff --git a/libcontainer/intelrdt/intelrdt_test.go b/libcontainer/intelrdt/intelrdt_test.go index 31df5186ee9..f1cff8f5474 100644 --- a/libcontainer/intelrdt/intelrdt_test.go +++ b/libcontainer/intelrdt/intelrdt_test.go @@ -5,6 +5,8 @@ package intelrdt import ( "errors" "io" + "os" + "path/filepath" "strings" "testing" ) @@ -111,6 +113,35 @@ func TestIntelRdtSetMemBwScSchema(t *testing.T) { } } +func TestApply(t *testing.T) { + helper := NewIntelRdtTestUtil(t) + + const closID = "test-clos" + + helper.IntelRdtData.config.IntelRdt.ClosID = closID + intelrdt := NewManager(helper.IntelRdtData.config, "", helper.IntelRdtPath) + if err := intelrdt.Apply(1234); err == nil { + t.Fatal("unexpected success when applying pid") + } + if _, err := os.Stat(filepath.Join(helper.IntelRdtPath, closID)); err == nil { + t.Fatal("closid dir should not exist") + } + + // Dir should be created if some schema has been specified + intelrdt.(*intelRdtManager).config.IntelRdt.L3CacheSchema = "L3:0=f" + if err := intelrdt.Apply(1235); err != nil { + t.Fatalf("Apply() failed: %v", err) + } + + pids, err := getIntelRdtParamString(intelrdt.GetPath(), "tasks") + if err != nil { + t.Fatalf("failed to read tasks file: %v", err) + } + if pids != "1235" { + t.Fatalf("unexpected tasks file, expected '1235', got %q", pids) + } +} + const ( mountinfoValid = `18 40 0:18 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs rw 19 40 0:3 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw diff --git a/libcontainer/intelrdt/util_test.go b/libcontainer/intelrdt/util_test.go index ced12779f12..a471c5e26f9 100644 --- a/libcontainer/intelrdt/util_test.go +++ b/libcontainer/intelrdt/util_test.go @@ -30,9 +30,9 @@ func NewIntelRdtTestUtil(t *testing.T) *intelRdtTestUtil { config: &configs.Config{ IntelRdt: &configs.IntelRdt{}, }, - root: t.TempDir(), } - testIntelRdtPath := filepath.Join(d.root, "resctrl") + intelRdtRoot = t.TempDir() + testIntelRdtPath := filepath.Join(intelRdtRoot, "resctrl") // Ensure the full mock Intel RDT "resource control" filesystem path exists if err := os.MkdirAll(testIntelRdtPath, 0o755); err != nil { From 1b4c30fd8b719834a5492a17893994a996c0a7ce Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Mon, 26 Apr 2021 14:23:47 +0300 Subject: [PATCH 4/5] libcontainer/intelrdt: always run unit tests Run unit tests irrespective of the underlying system configuration, i.e. even if RDT has not been enabled or is not supported. The tests do not depend on real kernel interfaces. Signed-off-by: Markus Lehtonen --- libcontainer/intelrdt/intelrdt_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt_test.go b/libcontainer/intelrdt/intelrdt_test.go index f1cff8f5474..2525789690f 100644 --- a/libcontainer/intelrdt/intelrdt_test.go +++ b/libcontainer/intelrdt/intelrdt_test.go @@ -12,10 +12,6 @@ import ( ) func TestIntelRdtSetL3CacheSchema(t *testing.T) { - if !IsCATEnabled() { - return - } - helper := NewIntelRdtTestUtil(t) const ( @@ -46,10 +42,6 @@ func TestIntelRdtSetL3CacheSchema(t *testing.T) { } func TestIntelRdtSetMemBwSchema(t *testing.T) { - if !IsMBAEnabled() { - return - } - helper := NewIntelRdtTestUtil(t) const ( @@ -80,10 +72,6 @@ func TestIntelRdtSetMemBwSchema(t *testing.T) { } func TestIntelRdtSetMemBwScSchema(t *testing.T) { - if !IsMBAScEnabled() { - return - } - helper := NewIntelRdtTestUtil(t) const ( From 93937000031ac20a96c5bd7c522fc67223908a6d Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Thu, 19 Aug 2021 21:33:38 +0300 Subject: [PATCH 5/5] libcontainer/intelrdt: update code comments Use the term "clos group" instead of "container_id group" as the group that a container belongs to is not necessarily tied to its container id. Signed-off-by: Markus Lehtonen --- libcontainer/intelrdt/intelrdt.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index a12aca9b9c3..0d5628b3be3 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -72,7 +72,7 @@ import ( * |-- ... * |-- schemata * |-- tasks - * |-- + * |-- * |-- ... * |-- schemata * |-- tasks @@ -155,7 +155,7 @@ type Manager interface { // Returns statistics for Intel RDT GetStats() (*Stats, error) - // Destroys the Intel RDT 'container_id' group + // Destroys the Intel RDT container-specific 'container_id' group Destroy() error // Returns Intel RDT path to save in a state file and to be able to @@ -517,7 +517,7 @@ func IsMBAScEnabled() bool { return mbaScEnabled } -// Get the 'container_id' path in Intel RDT "resource control" filesystem +// Get the path of the clos group in "resource control" filesystem that the container belongs to func (m *intelRdtManager) getIntelRdtPath() (string, error) { rootPath, err := getIntelRdtRoot() if err != nil { @@ -567,7 +567,7 @@ func (m *intelRdtManager) Apply(pid int) (err error) { return nil } -// Destroys the Intel RDT 'container_id' group +// Destroys the Intel RDT container-specific 'container_id' group func (m *intelRdtManager) Destroy() error { // Don't remove resctrl group if closid has been explicitly specified. The // group is likely externally managed, i.e. by some other entity than us. @@ -614,7 +614,7 @@ func (m *intelRdtManager) GetStats() (*Stats, error) { } schemaRootStrings := strings.Split(tmpRootStrings, "\n") - // The L3 cache and memory bandwidth schemata in 'container_id' group + // The L3 cache and memory bandwidth schemata in container's clos group containerPath := m.GetPath() tmpStrings, err := getIntelRdtParamString(containerPath, "schemata") if err != nil { @@ -637,7 +637,7 @@ func (m *intelRdtManager) GetStats() (*Stats, error) { } } - // The L3 cache schema in 'container_id' group + // The L3 cache schema in container's clos group for _, schema := range schemaStrings { if strings.Contains(schema, "L3") { stats.L3CacheSchema = strings.TrimSpace(schema) @@ -660,7 +660,7 @@ func (m *intelRdtManager) GetStats() (*Stats, error) { } } - // The memory bandwidth schema in 'container_id' group + // The memory bandwidth schema in container's clos group for _, schema := range schemaStrings { if strings.Contains(schema, "MB") { stats.MemBwSchema = strings.TrimSpace(schema)