From 91b36f1c0341814d92fd3b1f1732d5f2aae1f122 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Sat, 19 Sep 2026 12:25:02 -0700 Subject: [PATCH 1/3] fix(infiniband): skip sysfs reads for excluded devices Motivation: --collector.infiniband.device-exclude (and device-include) correctly filtered excluded devices out of emitted metrics, but the collector's Update() called sysfs.FS.InfiniBandClass(), which eagerly parses every device under /sys/class/infiniband -- including reading each port's counter files -- before the device filter is ever applied. On hosts with firmware-managed restricted ports, reading those counter files can trigger a firmware command that fails and floods the kernel log, even though the offending device was explicitly excluded. Approach: Enumerate device names first via sysfs.FS.InfiniBandClassDevices(), which only lists directory entries and performs no attribute or counter reads. Apply the existing device filter against the name, and only call sysfs.FS.InfiniBandDevice(name) -- which does the actual sysfs reads -- for devices that pass the filter. This does not change emitted metrics for included devices; it only avoids touching excluded devices' sysfs files at all. Validation: Added TestInfiniBandCollectorSkipsSysfsReadsForExcludedDevices, which builds a synthetic sysfs tree with a healthy device and a device missing its required fw_ver file (simulating a device whose attributes/counters cannot be read), then excludes the broken device via device-exclude and asserts Update() succeeds and only the included device's metrics are emitted. Report: https://github.com/prometheus/node_exporter/issues/3823 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code) --- collector/infiniband_linux.go | 15 +++- collector/infiniband_linux_test.go | 120 +++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 collector/infiniband_linux_test.go diff --git a/collector/infiniband_linux.go b/collector/infiniband_linux.go index fb83c152a6..04f0be0b73 100644 --- a/collector/infiniband_linux.go +++ b/collector/infiniband_linux.go @@ -158,7 +158,7 @@ func (c *infinibandCollector) pushCounter(ch chan<- prometheus.Metric, name stri } func (c *infinibandCollector) Update(ch chan<- prometheus.Metric) error { - devices, err := c.fs.InfiniBandClass() + deviceNames, err := c.fs.InfiniBandClassDevices() if err != nil { if errors.Is(err, os.ErrNotExist) { c.logger.Debug("infiniband statistics not found, skipping") @@ -167,11 +167,20 @@ func (c *infinibandCollector) Update(ch chan<- prometheus.Metric) error { return fmt.Errorf("error obtaining InfiniBand class info: %w", err) } - for _, device := range devices { - if c.deviceFilter.ignored(device.Name) { + for _, name := range deviceNames { + if c.deviceFilter.ignored(name) { continue } + // Only read the device's attributes and port counters from sysfs once we + // know it is not excluded: on some hardware (e.g. firmware-managed + // restricted ports), reading these counters triggers a firmware command + // that fails and floods the kernel log. + device, err := c.fs.InfiniBandDevice(name) + if err != nil { + return fmt.Errorf("error obtaining InfiniBand device info: %w", err) + } + infoDesc := prometheus.NewDesc( prometheus.BuildFQName(namespace, c.subsystem, "info"), "Non-numeric data from /sys/class/infiniband/, value is always 1.", diff --git a/collector/infiniband_linux_test.go b/collector/infiniband_linux_test.go new file mode 100644 index 0000000000..12cb055ee4 --- /dev/null +++ b/collector/infiniband_linux_test.go @@ -0,0 +1,120 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !noinfiniband + +package collector + +import ( + "io" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" +) + +// writeInfiniBandTestDevice creates the minimal set of sysfs files under +// sysRoot/class/infiniband/ needed for sysfs.FS to parse a device. When +// withFwVer is false, the required fw_ver file is omitted so that parsing the +// device fails, simulating a restricted/firmware-managed port whose counters +// cannot be read (see https://github.com/prometheus/node_exporter/issues/3823). +func writeInfiniBandTestDevice(t *testing.T, sysRoot, name string, withFwVer bool) { + t.Helper() + + devPath := filepath.Join(sysRoot, "class", "infiniband", name) + portPath := filepath.Join(devPath, "ports", "1") + if err := os.MkdirAll(portPath, 0o755); err != nil { + t.Fatal(err) + } + + if withFwVer { + if err := os.WriteFile(filepath.Join(devPath, "fw_ver"), []byte("1.2.3\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + portFiles := map[string]string{ + "link_layer": "InfiniBand\n", + "state": "4: ACTIVE\n", + "phys_state": "5: LinkUp\n", + "rate": "100 Gb/sec (4X EDR)\n", + } + for f, content := range portFiles { + if err := os.WriteFile(filepath.Join(portPath, f), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } +} + +// TestInfiniBandCollectorSkipsSysfsReadsForExcludedDevices verifies that a +// device matched by --collector.infiniband.device-exclude is never parsed +// from sysfs. Before the fix, the collector unconditionally parsed every +// device (via sysfs.FS.InfiniBandClass) before applying the device filter, so +// a single unreadable/excluded device would fail the whole scrape. +func TestInfiniBandCollectorSkipsSysfsReadsForExcludedDevices(t *testing.T) { + sysRoot := t.TempDir() + // mlx5_0 is a healthy device that should still be scraped normally. + writeInfiniBandTestDevice(t, sysRoot, "mlx5_0", true) + // mlx5_12 simulates a restricted port: its fw_ver file cannot be read, so + // parsing it errors out. It must never be parsed once excluded. + writeInfiniBandTestDevice(t, sysRoot, "mlx5_12", false) + + oldSysPath, oldExclude := *sysPath, *infinibandDeviceExclude + *sysPath = sysRoot + *infinibandDeviceExclude = "^mlx5_12$" + defer func() { + *sysPath = oldSysPath + *infinibandDeviceExclude = oldExclude + }() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + collector, err := NewInfiniBandCollector(logger) + if err != nil { + t.Fatal(err) + } + + ch := make(chan prometheus.Metric, 100) + if err := collector.Update(ch); err != nil { + t.Fatalf("Update() failed, meaning the excluded device's sysfs files were still read: %v", err) + } + close(ch) + + sawExcluded, sawIncluded := false, false + for m := range ch { + var out dto.Metric + if err := m.Write(&out); err != nil { + t.Fatal(err) + } + for _, l := range out.GetLabel() { + if l.GetName() != "device" { + continue + } + switch l.GetValue() { + case "mlx5_12": + sawExcluded = true + case "mlx5_0": + sawIncluded = true + } + } + } + + if sawExcluded { + t.Error("metrics for excluded device mlx5_12 were emitted") + } + if !sawIncluded { + t.Error("expected metrics for included device mlx5_0, got none") + } +} From 83e207772520e0603d84d2d113c220c9b3cd7984 Mon Sep 17 00:00:00 2001 From: Ben Kochie Date: Wed, 28 Oct 2026 16:37:54 +0100 Subject: [PATCH 2/3] Update collector/infiniband_linux.go Signed-off-by: Ben Kochie --- collector/infiniband_linux.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/collector/infiniband_linux.go b/collector/infiniband_linux.go index fa073fb5fe..6a869ef188 100644 --- a/collector/infiniband_linux.go +++ b/collector/infiniband_linux.go @@ -172,10 +172,6 @@ func (c *infinibandCollector) Update(ch chan<- prometheus.Metric) error { continue } - // Only read the device's attributes and port counters from sysfs once we - // know it is not excluded: on some hardware (e.g. firmware-managed - // restricted ports), reading these counters triggers a firmware command - // that fails and floods the kernel log. device, err := c.fs.InfiniBandDevice(name) if err != nil { return fmt.Errorf("error obtaining InfiniBand device info: %w", err) From 5df1f21af971cd6db7b36cb9bd33f99748d9b4db Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:00:56 -0700 Subject: [PATCH 3/3] infiniband: drop ad-hoc sysfs-writing test Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- collector/infiniband_linux_test.go | 120 ----------------------------- 1 file changed, 120 deletions(-) delete mode 100644 collector/infiniband_linux_test.go diff --git a/collector/infiniband_linux_test.go b/collector/infiniband_linux_test.go deleted file mode 100644 index 12cb055ee4..0000000000 --- a/collector/infiniband_linux_test.go +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//go:build !noinfiniband - -package collector - -import ( - "io" - "log/slog" - "os" - "path/filepath" - "testing" - - "github.com/prometheus/client_golang/prometheus" - dto "github.com/prometheus/client_model/go" -) - -// writeInfiniBandTestDevice creates the minimal set of sysfs files under -// sysRoot/class/infiniband/ needed for sysfs.FS to parse a device. When -// withFwVer is false, the required fw_ver file is omitted so that parsing the -// device fails, simulating a restricted/firmware-managed port whose counters -// cannot be read (see https://github.com/prometheus/node_exporter/issues/3823). -func writeInfiniBandTestDevice(t *testing.T, sysRoot, name string, withFwVer bool) { - t.Helper() - - devPath := filepath.Join(sysRoot, "class", "infiniband", name) - portPath := filepath.Join(devPath, "ports", "1") - if err := os.MkdirAll(portPath, 0o755); err != nil { - t.Fatal(err) - } - - if withFwVer { - if err := os.WriteFile(filepath.Join(devPath, "fw_ver"), []byte("1.2.3\n"), 0o644); err != nil { - t.Fatal(err) - } - } - - portFiles := map[string]string{ - "link_layer": "InfiniBand\n", - "state": "4: ACTIVE\n", - "phys_state": "5: LinkUp\n", - "rate": "100 Gb/sec (4X EDR)\n", - } - for f, content := range portFiles { - if err := os.WriteFile(filepath.Join(portPath, f), []byte(content), 0o644); err != nil { - t.Fatal(err) - } - } -} - -// TestInfiniBandCollectorSkipsSysfsReadsForExcludedDevices verifies that a -// device matched by --collector.infiniband.device-exclude is never parsed -// from sysfs. Before the fix, the collector unconditionally parsed every -// device (via sysfs.FS.InfiniBandClass) before applying the device filter, so -// a single unreadable/excluded device would fail the whole scrape. -func TestInfiniBandCollectorSkipsSysfsReadsForExcludedDevices(t *testing.T) { - sysRoot := t.TempDir() - // mlx5_0 is a healthy device that should still be scraped normally. - writeInfiniBandTestDevice(t, sysRoot, "mlx5_0", true) - // mlx5_12 simulates a restricted port: its fw_ver file cannot be read, so - // parsing it errors out. It must never be parsed once excluded. - writeInfiniBandTestDevice(t, sysRoot, "mlx5_12", false) - - oldSysPath, oldExclude := *sysPath, *infinibandDeviceExclude - *sysPath = sysRoot - *infinibandDeviceExclude = "^mlx5_12$" - defer func() { - *sysPath = oldSysPath - *infinibandDeviceExclude = oldExclude - }() - - logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - collector, err := NewInfiniBandCollector(logger) - if err != nil { - t.Fatal(err) - } - - ch := make(chan prometheus.Metric, 100) - if err := collector.Update(ch); err != nil { - t.Fatalf("Update() failed, meaning the excluded device's sysfs files were still read: %v", err) - } - close(ch) - - sawExcluded, sawIncluded := false, false - for m := range ch { - var out dto.Metric - if err := m.Write(&out); err != nil { - t.Fatal(err) - } - for _, l := range out.GetLabel() { - if l.GetName() != "device" { - continue - } - switch l.GetValue() { - case "mlx5_12": - sawExcluded = true - case "mlx5_0": - sawIncluded = true - } - } - } - - if sawExcluded { - t.Error("metrics for excluded device mlx5_12 were emitted") - } - if !sawIncluded { - t.Error("expected metrics for included device mlx5_0, got none") - } -}