Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions features.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ var featuresCommand = cli.Command{
Enabled: &t,
},
IntelRdt: &features.IntelRdt{
Enabled: &t,
Schemata: &t,
Enabled: &t,
Schemata: &t,
Monitoring: &t,
},
MountExtensions: &features.MountExtensions{
IDMap: &features.IDMap{
Expand Down
3 changes: 3 additions & 0 deletions libcontainer/configs/intelrdt.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@ type IntelRdt struct {
// The unit of memory bandwidth is specified in "percentages" by
// default, and in "MBps" if MBA Software Controller is enabled.
MemBwSchema string `json:"memBwSchema,omitempty"`

// Create a monitoring group for the container.
EnableMonitoring bool `json:"enableMonitoring,omitempty"`
}
17 changes: 4 additions & 13 deletions libcontainer/configs/validate/intelrdt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,24 @@ func TestValidateIntelRdt(t *testing.T) {
isErr bool
}{
{
name: "rdt not supported, no config",
rdtEnabled: false,
config: nil,
isErr: false,
name: "rdt not supported, no config",
},
{
name: "rdt not supported, with config",
rdtEnabled: false,
config: &configs.IntelRdt{},
isErr: true,
name: "rdt not supported, with config",
config: &configs.IntelRdt{},
isErr: true,
},
{
name: "empty config",
rdtEnabled: true,
config: &configs.IntelRdt{},
isErr: false,
},
{
name: "root clos",
rdtEnabled: true,
config: &configs.IntelRdt{
ClosID: "/",
},
isErr: false,
},
{
name: "invalid ClosID (.)",
Expand Down Expand Up @@ -71,7 +65,6 @@ func TestValidateIntelRdt(t *testing.T) {
{
name: "cat not supported",
rdtEnabled: true,
catEnabled: false,
config: &configs.IntelRdt{
L3CacheSchema: "0=ff",
},
Expand All @@ -80,7 +73,6 @@ func TestValidateIntelRdt(t *testing.T) {
{
name: "mba not supported",
rdtEnabled: true,
mbaEnabled: false,
config: &configs.IntelRdt{
MemBwSchema: "0=100",
},
Expand All @@ -96,7 +88,6 @@ func TestValidateIntelRdt(t *testing.T) {
L3CacheSchema: "0=ff",
MemBwSchema: "0=100",
},
isErr: false,
},
}
for _, tc := range testCases {
Expand Down
8 changes: 8 additions & 0 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ type State struct {

// Intel RDT "resource control" filesystem path.
IntelRdtPath string `json:"intel_rdt_path,omitempty"`

// Path of the container specific monitoring group in resctrl filesystem.
// Empty if the container does not have aindividual dedicated monitoring
// group.
IntelRdtMonPath string `json:"intel_rdt_mon_path,omitempty"`
}

// ID returns the container's unique ID
Expand Down Expand Up @@ -942,8 +947,10 @@ func (c *Container) currentState() *State {
}

intelRdtPath := ""
intelRdtMonPath := ""
if c.intelRdtManager != nil {
intelRdtPath = c.intelRdtManager.GetPath()
intelRdtMonPath = c.intelRdtManager.GetMonPath()
}
state := &State{
BaseState: BaseState{
Expand All @@ -956,6 +963,7 @@ func (c *Container) currentState() *State {
Rootless: c.config.RootlessEUID && c.config.RootlessCgroups,
CgroupPaths: c.cgroupManager.GetPaths(),
IntelRdtPath: intelRdtPath,
IntelRdtMonPath: intelRdtMonPath,
NamespacePaths: make(map[configs.NamespaceType]string),
ExternalDescriptors: externalDescriptors,
}
Expand Down
47 changes: 45 additions & 2 deletions libcontainer/intelrdt/intelrdt.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,22 +468,41 @@ func (m *Manager) Apply(pid int) (err error) {
return newLastCmdError(err)
}

// Create MON group
if monPath := m.GetMonPath(); monPath != "" {
if err := os.Mkdir(monPath, 0o755); err != nil && !os.IsExist(err) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who has control of this path? In runc this is trusted, okay, but is it exposed in k8s or containerd or some other to the user?

Not sure with the https://github.com/intel/k8s-rdt-controller what is exposed to an end user

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's runc, it's in this very same file (and patch):

func (m *Manager) GetMonPath() string {
	if closPath := m.GetPath(); closPath != "" && m.config.IntelRdt.EnableMonitoring {
		path, err := securejoin.SecureJoin(filepath.Join(closPath, "mon_groups"), m.id)

So it's basically <clos-dir>/mon_groups/<container-id>

Copy link
Member

@rata rata Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and that comes from intelRdtRoot. But where does that come from? Is there any way an unprivileged user (or just anyone that is not the sysadmin or so) can control any path component (or the intelRDT root)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That, in turn, comes from parsing the output of statfs syscall (unix.Statfs()). Note that in the case of runc update the closPath is taken from the config.json of the container.

In any case I cannot see any way that an unprivileged user can control the path

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So statfs will have some string value and we will mkdir it on the host, outside of the container rootfs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conversation was broken out of this thread, it continued in the main PR discussion. The final answer I found for this is posted here: #4832 (comment)

Basically, the fs will be mounted somewhere on the host by the admin and we might create/delete some dirs inside that. None of that is accessible to the container process.

@marquiz correct me if I'm getting something wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kolyshkin PTAL, I'd like someone else to look at this part too, just in case I'm missing something :)

return newLastCmdError(err)
}
if err := WriteIntelRdtTasks(monPath, pid); err != nil {
return newLastCmdError(err)
}
}

m.path = path
return nil
}

// Destroy destroys the Intel RDT container-specific container_id group.
func (m *Manager) Destroy() error {
if m.config.IntelRdt == nil {
return nil
}
// 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 == "" {
if m.config.IntelRdt.ClosID == "" {
m.mu.Lock()
defer m.mu.Unlock()
if err := os.Remove(m.GetPath()); err != nil && !os.IsNotExist(err) {
return err
}
m.path = ""
} else if monPath := m.GetMonPath(); monPath != "" {
// If ClosID is not specified the possible monintoring group was
// removed with the CLOS above.
if err := os.Remove(monPath); err != nil && !os.IsNotExist(err) {
return err
}
}
return nil
}
Expand All @@ -494,6 +513,21 @@ func (m *Manager) GetPath() string {
return m.path
}

// GetMonPath returns path of the monitoring group of the container. Returns an
// empty string if the container does not have a individual dedicated
// monitoring group.
func (m *Manager) GetMonPath() string {
if !m.config.IntelRdt.EnableMonitoring {
return ""
}
closPath := m.GetPath()
if closPath == "" {
return ""
}

return filepath.Join(closPath, "mon_groups", m.id)
}

// GetStats returns statistics for Intel RDT.
func (m *Manager) GetStats() (*Stats, error) {
// If intelRdt is not specified in config
Expand Down Expand Up @@ -573,7 +607,16 @@ func (m *Manager) GetStats() (*Stats, error) {
}

if IsMBMEnabled() || IsCMTEnabled() {
err = getMonitoringStats(containerPath, stats)
monPath := m.GetMonPath()
if monPath == "" {
// NOTE: If per-container monitoring is not enabled, the monitoring
// data we get here might have little to do with this container as
// there might be anything from this single container to the half
// of the system assigned in the group. Should consider not
// exposing stats in this case(?)
monPath = containerPath
}
err = getMonitoringStats(monPath, stats)
if err != nil {
return nil, err
}
Expand Down
186 changes: 166 additions & 20 deletions libcontainer/intelrdt/intelrdt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"os"
"path/filepath"
"slices"
"strconv"
"strings"
"testing"

Expand Down Expand Up @@ -126,31 +127,176 @@ func TestIntelRdtSet(t *testing.T) {
}

func TestApply(t *testing.T) {
helper := NewIntelRdtTestUtil(t)
const pid = 1234
tests := []struct {
name string
config configs.IntelRdt
precreateClos bool
isError bool
postApplyAssert func(*Manager)
}{
{
name: "failure because non-pre-existing CLOS",
config: configs.IntelRdt{
ClosID: "non-existing-clos",
},
isError: true,
postApplyAssert: func(m *Manager) {
if _, err := os.Stat(m.path); err == nil {
t.Fatal("closid dir should not exist")
}
},
},
{
name: "CLOS dir should be created if some schema has been specified",
config: configs.IntelRdt{
ClosID: "clos-to-be-created",
L3CacheSchema: "L3:0=f",
},
postApplyAssert: func(m *Manager) {
pids, err := getIntelRdtParamString(m.path, "tasks")
if err != nil {
t.Fatalf("failed to read tasks file: %v", err)
}
if pids != strconv.Itoa(pid) {
t.Fatalf("unexpected tasks file, expected '%d', got %q", pid, pids)
}
},
},
{
name: "clos and monitoring group should be created if EnableMonitoring is true",
config: configs.IntelRdt{
EnableMonitoring: true,
},
precreateClos: true,
postApplyAssert: func(m *Manager) {
pids, err := getIntelRdtParamString(m.path, "tasks")
if err != nil {
t.Fatalf("failed to read tasks file: %v", err)
}
if pids != strconv.Itoa(pid) {
t.Fatalf("unexpected tasks file, expected '%d', got %q", pid, pids)
}
},
},
}

const closID = "test-clos"
closPath := filepath.Join(helper.IntelRdtPath, closID)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
NewIntelRdtTestUtil(t)
id := "abcd-1234"
closPath := filepath.Join(intelRdtRoot, id)
if tt.config.ClosID != "" {
closPath = filepath.Join(intelRdtRoot, tt.config.ClosID)
}

helper.config.IntelRdt.ClosID = closID
intelrdt := newManager(helper.config, "container-1", closPath)
if err := intelrdt.Apply(1234); err == nil {
t.Fatal("unexpected success when applying pid")
}
if _, err := os.Stat(closPath); err == nil {
t.Fatal("closid dir should not exist")
if tt.precreateClos {
if err := os.MkdirAll(filepath.Join(closPath, "mon_groups"), 0o755); err != nil {
t.Fatal(err)
}
}
m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath)
err := m.Apply(pid)
if tt.isError && err == nil {
t.Fatal("expected error, got nil")
} else if !tt.isError && err != nil {
t.Fatalf("unexpected error: %v", err)
}
tt.postApplyAssert(m)
})
}
}

// Dir should be created if some schema has been specified
intelrdt.config.IntelRdt.L3CacheSchema = "L3:0=f"
if err := intelrdt.Apply(1235); err != nil {
t.Fatalf("Apply() failed: %v", err)
}
func TestDestroy(t *testing.T) {
tests := []struct {
name string
config configs.IntelRdt
testFunc func(*Manager)
}{
{
name: "per-container CLOS dir should be removed",
testFunc: func(m *Manager) {
closPath := m.path
if _, err := os.Stat(closPath); err != nil {
t.Fatal("CLOS dir should exist")
}
// Need to delete the tasks file so that the dir is empty
if err := os.Remove(filepath.Join(closPath, "tasks")); err != nil {
t.Fatalf("failed to remove tasks file: %v", err)
}
if err := m.Destroy(); err != nil {
t.Fatalf("Destroy() failed: %v", err)
}
if _, err := os.Stat(closPath); err == nil {
t.Fatal("CLOS dir should not exist")
}
},
},
{
name: "pre-existing CLOS should not be removed",
config: configs.IntelRdt{
ClosID: "pre-existing-clos",
},
testFunc: func(m *Manager) {
closPath := m.path

pids, err := getIntelRdtParamString(intelrdt.GetPath(), "tasks")
if err != nil {
t.Fatalf("failed to read tasks file: %v", err)
if _, err := os.Stat(closPath); err != nil {
t.Fatal("CLOS dir should exist")
}
if err := m.Destroy(); err != nil {
t.Fatalf("Destroy() failed: %v", err)
}
if _, err := os.Stat(closPath); err != nil {
t.Fatal("CLOS dir should exist")
}
},
},
{
name: "per-container MON dir in pre-existing CLOS should be removed",
config: configs.IntelRdt{
ClosID: "pre-existing-clos",
EnableMonitoring: true,
},
testFunc: func(m *Manager) {
closPath := m.path

monPath := filepath.Join(closPath, "mon_groups", m.id)
if _, err := os.Stat(monPath); err != nil {
t.Fatal("MON dir should exist")
}
// Need to delete the tasks file so that the dir is empty
os.Remove(filepath.Join(monPath, "tasks"))
if err := m.Destroy(); err != nil {
t.Fatalf("Destroy() failed: %v", err)
}
if _, err := os.Stat(closPath); err != nil {
t.Fatalf("CLOS dir should exist: %f", err)
}
if _, err := os.Stat(monPath); err == nil {
t.Fatal("MON dir should not exist")
}
},
},
}
if pids != "1235" {
t.Fatalf("unexpected tasks file, expected '1235', got %q", pids)

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
NewIntelRdtTestUtil(t)

id := "abcd-1234"
closPath := filepath.Join(intelRdtRoot, id)
if tt.config.ClosID != "" {
closPath = filepath.Join(intelRdtRoot, tt.config.ClosID)
// Pre-create the CLOS directory
if err := os.MkdirAll(filepath.Join(closPath, "mon_groups"), 0o755); err != nil {
t.Fatal(err)
}
}
m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath)
if err := m.Apply(1234); err != nil {
t.Fatalf("Apply() failed: %v", err)
}
tt.testFunc(m)
})
}
}
Loading
Loading