diff --git a/cmd/plugins/topology-aware/policy/resources.go b/cmd/plugins/topology-aware/policy/resources.go index 9cb6bae8d..6307d743e 100644 --- a/cmd/plugins/topology-aware/policy/resources.go +++ b/cmd/plugins/topology-aware/policy/resources.go @@ -1136,7 +1136,7 @@ func (cs *supply) GetScore(req Request) Score { score.hints = make(map[string]float64, len(hints)) for provider, hint := range cr.container.GetTopologyHints() { - log.Debugf(" - evaluating topology hint %s", hint) + log.Debugf(" - evaluating topology hint %s", hint.String()) score.hints[provider] = cs.node.HintScore(hint) } diff --git a/pkg/topology/go.mod b/pkg/topology/go.mod index 4e70f71ea..17778b3de 100644 --- a/pkg/topology/go.mod +++ b/pkg/topology/go.mod @@ -1,5 +1,5 @@ module github.com/containers/nri-plugins/pkg/topology -go 1.20 +go 1.23 require golang.org/x/sys v0.18.0 diff --git a/pkg/topology/test-data.tar.gz b/pkg/topology/test-data.tar.gz index 515f69df0..9cd6c129b 100644 Binary files a/pkg/topology/test-data.tar.gz and b/pkg/topology/test-data.tar.gz differ diff --git a/pkg/topology/topology.go b/pkg/topology/topology.go index d3957a1c8..4b51f9aec 100644 --- a/pkg/topology/topology.go +++ b/pkg/topology/topology.go @@ -16,8 +16,12 @@ package topology import ( "fmt" + "maps" "os" "path/filepath" + "regexp" + "slices" + "strconv" "strings" ) @@ -33,12 +37,50 @@ const ( ProviderKubelet = "kubelet" ) +// PCIeHopType tells what kind of node a PCIeHop entry refers to. +type PCIeHopType string + +const ( + // PCIeHopBridge marks a PCI-to-PCI bridge (class 0604): a root + // port, a switch upstream or downstream port, or a plain bridge. + // Sysfs has no separate class for switches, so a switch just + // shows up as consecutive bridge hops in the chain. + PCIeHopBridge PCIeHopType = "bridge" + // PCIeHopRoot marks the host bridge / root complex at the top of + // a device's PCI hierarchy. + PCIeHopRoot PCIeHopType = "root" +) + +// PCIeHop is one ancestor bridge or the root complex above a device +// in its PCIe hierarchy. +type PCIeHop struct { + // Address is the PCI address (e.g. "0000:00:1c.0"), or the sysfs + // root bridge directory name (e.g. "pci0000:00") when Type is + // PCIeHopRoot. + Address string + Type PCIeHopType +} + // Hint represents various hints that can be detected from sysfs for the device. type Hint struct { + // Provider is the sysfs path this hint was collected from. Provider string - CPUs string - NUMAs string - Sockets string + // CPUs is the CPU list the device is affine to, in Linux list + // format (e.g. "0-3,7"). + CPUs string + // NUMAs is the list of NUMA nodes the device is attached to. + NUMAs string + // Sockets is the list of physical sockets the device is on. Set + // only as a fallback, when a kernel/BIOS quirk leaves NUMAs set + // but no usable CPU list to derive it from. + Sockets string + // PCIeChain lists the device's ancestor PCI bridges and its root + // complex, ordered from the nearest bridge to the root. Empty for + // devices with no PCI ancestry. + PCIeChain []PCIeHop + // IRQs lists the interrupt numbers tied to the device, read from + // its own sysfs "irq" file and "msi_irqs" entries. + IRQs []int } // Hints represents set of hints collected from multiple providers. @@ -173,6 +215,9 @@ func getTopologyHint(sysFSPath string) (*Hint, error) { } } + hint.PCIeChain = pcieChain(sysFSPath) + hint.IRQs = deviceIRQs(sysFSPath) + if hint.CPUs != "" || hint.NUMAs != "" || hint.Sockets != "" { log.Debugf(" => %s", hint.String()) } @@ -180,6 +225,85 @@ func getTopologyHint(sysFSPath string) (*Hint, error) { return &hint, nil } +var ( + // pciAddressRe matches a PCI BDF address directory name, e.g. + // "0000:00:1c.0". + pciAddressRe = regexp.MustCompile(`^[0-9a-f]{4}:[0-9a-f]{2}:[0-9a-f]{2}\.[0-9a-f]$`) + // pciRootNameRe matches the sysfs root bridge directory name, + // e.g. "pci0000:00". + pciRootNameRe = regexp.MustCompile(`^pci[0-9a-f]{4}:[0-9a-f]{2}$`) +) + +// isPCIBridgeClass reports whether a sysfs "class" file's content +// (e.g. "0x060400") denotes a PCI-to-PCI bridge (base class 06, +// sub-class 04). +func isPCIBridgeClass(class string) bool { + class = strings.TrimPrefix(strings.TrimSpace(class), "0x") + return len(class) >= 4 && class[:4] == "0604" +} + +// pcieChain walks up from devPath's parent directory and returns the +// device's ancestor PCI bridges and root complex, nearest first. It +// stops at the first non-PCI ancestor, so a device with no PCI +// ancestry gets an empty chain. Every step is best effort: a read +// error or an unexpected path shape just ends the walk with whatever +// was found so far, rather than failing the whole hint. +func pcieChain(devPath string) []PCIeHop { + chain := []PCIeHop{} + dir := filepath.Dir(devPath) + for { + name := filepath.Base(dir) + switch { + case pciRootNameRe.MatchString(name): + return append(chain, PCIeHop{Address: name, Type: PCIeHopRoot}) + case pciAddressRe.MatchString(name): + class, err := os.ReadFile(filepath.Join(dir, "class")) + if err != nil { + log.Debugf("pcie chain for %s: stopping at %s, no class file: %v", devPath, dir, err) + return chain + } + if !isPCIBridgeClass(string(class)) { + return chain + } + chain = append(chain, PCIeHop{Address: name, Type: PCIeHopBridge}) + default: + return chain + } + parent := filepath.Dir(dir) + if parent == dir { + return chain + } + dir = parent + } +} + +// deviceIRQs collects the interrupt numbers tied to the device at +// devPath: its legacy "irq" file, if set to a positive number, plus +// every entry under its "msi_irqs" directory. Missing files or +// directories are not an error, they just mean nothing was found +// from that source. When MSI/MSI-X is enabled, the legacy "irq" file +// can report a vector that also shows up under "msi_irqs", so values +// are collected into a set and returned sorted, each appearing once. +func deviceIRQs(devPath string) []int { + irqSet := map[int]struct{}{} + + if b, err := os.ReadFile(filepath.Join(devPath, "irq")); err == nil { + if n, err := strconv.Atoi(strings.TrimSpace(string(b))); err == nil && n > 0 { + irqSet[n] = struct{}{} + } + } + + if entries, err := os.ReadDir(filepath.Join(devPath, "msi_irqs")); err == nil { + for _, e := range entries { + if n, err := strconv.Atoi(e.Name()); err == nil { + irqSet[n] = struct{}{} + } + } + } + + return slices.Sorted(maps.Keys(irqSet)) +} + // NewTopologyHints return array of hints for the main device and its // depended devices (e.g. RAID). func NewTopologyHints(devPath string) (hints Hints, err error) { @@ -240,7 +364,7 @@ func (hints Hints) ResolvePartialHints(resolve func(NUMAs string) string) { // String returns the hints as a string. func (h *Hint) String() string { - cpus, nodes, sockets, sep := "", "", "", "" + cpus, nodes, sockets, irqs, sep := "", "", "", "", "" if h.CPUs != "" { cpus = "CPUs:" + h.CPUs @@ -252,9 +376,17 @@ func (h *Hint) String() string { } if h.Sockets != "" { sockets = sep + "sockets:" + h.Sockets + sep = ", " + } + if len(h.IRQs) > 0 { + list := make([]string, len(h.IRQs)) + for i, irq := range h.IRQs { + list[i] = strconv.Itoa(irq) + } + irqs = sep + "IRQs:" + strings.Join(list, ",") } - return "" + return "" } // FindGivenSysFsDevice returns the physical device with the given device type, diff --git a/pkg/topology/topology_test.go b/pkg/topology/topology_test.go index 9455caace..6776166c9 100644 --- a/pkg/topology/topology_test.go +++ b/pkg/topology/topology_test.go @@ -367,10 +367,33 @@ func TestNewTopologyHints(t *testing.T) { input: "/sys/devices/pci0000:00/0000:00:02.0/drm/card1", output: Hints{ "/sys/devices/pci0000:00/0000:00:02.0": Hint{ - Provider: "/sys/devices/pci0000:00/0000:00:02.0", - CPUs: "0-7", - NUMAs: "", - Sockets: ""}, + Provider: "/sys/devices/pci0000:00/0000:00:02.0", + CPUs: "0-7", + NUMAs: "", + Sockets: "", + PCIeChain: []PCIeHop{{Address: "pci0000:00", Type: PCIeHopRoot}}, + IRQs: []int{16, 24, 25}, + }, + }, + expectedErr: false, + }, + { + name: "pci endpoint behind a bridge", + input: "/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0", + output: Hints{ + "/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0": Hint{ + Provider: "/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0", + CPUs: "8-15", + NUMAs: "1", + Sockets: "", + PCIeChain: []PCIeHop{ + {Address: "0000:00:1c.0", Type: PCIeHopBridge}, + {Address: "pci0000:00", Type: PCIeHopRoot}, + }, + // irq is 0 (no legacy interrupt), only the + // MSI vector should show up here. + IRQs: []int{33}, + }, }, expectedErr: false, }, @@ -389,3 +412,89 @@ func TestNewTopologyHints(t *testing.T) { }) } } + +func TestPcieChain(t *testing.T) { + teardown := setupTestEnv(t) + defer teardown() + pwd, err := os.Getwd() + if err != nil { + t.Fatal("unable to get current directory") + } + root := pwd + "/testdata" + + cases := []struct { + name string + devDir string + want []PCIeHop + }{ + { + name: "endpoint directly on the root complex", + devDir: root + "/sys/devices/pci0000:00/0000:00:02.0", + want: []PCIeHop{{Address: "pci0000:00", Type: PCIeHopRoot}}, + }, + { + name: "endpoint behind one bridge", + devDir: root + "/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0", + want: []PCIeHop{ + {Address: "0000:00:1c.0", Type: PCIeHopBridge}, + {Address: "pci0000:00", Type: PCIeHopRoot}, + }, + }, + { + name: "non-PCI device has no chain", + devDir: root + "/sys/devices/virtual/mem/null", + want: []PCIeHop{}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := pcieChain(tc.devDir); !reflect.DeepEqual(got, tc.want) { + t.Errorf("pcieChain(%s) = %+v, want %+v", tc.devDir, got, tc.want) + } + }) + } +} + +func TestDeviceIRQs(t *testing.T) { + teardown := setupTestEnv(t) + defer teardown() + pwd, err := os.Getwd() + if err != nil { + t.Fatal("unable to get current directory") + } + root := pwd + "/testdata" + + cases := []struct { + name string + devDir string + want []int + }{ + { + name: "legacy irq plus non-overlapping MSI vectors", + devDir: root + "/sys/devices/pci0000:00/0000:00:02.0", + want: []int{16, 24, 25}, + }, + { + name: "legacy irq overlapping an MSI vector is deduplicated", + devDir: root + "/sys/devices/pci0000:00/0000:00:03.0", + want: []int{24, 26}, + }, + { + name: "irq==0 means MSI-only", + devDir: root + "/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0", + want: []int{33}, + }, + { + name: "no irq file, no msi_irqs dir", + devDir: root + "/sys/devices/virtual/mem/null", + want: nil, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := deviceIRQs(tc.devDir); !reflect.DeepEqual(got, tc.want) { + t.Errorf("deviceIRQs(%s) = %v, want %v", tc.devDir, got, tc.want) + } + }) + } +}