Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add neighbor state #3191

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add neighbor state #3191

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b1004d8
Add neighbor state
eugercek Nov 23, 2024
2246604
Move to count of states
eugercek Dec 6, 2024
d679000
Merge branch 'master' of github.com:prometheus/node_exporter into add…
eugercek Jan 13, 2025
6b0be65
Fix linter
eugercek Feb 2, 2025
ed49ea9
Fix `NUD_NOARP` skip
eugercek Feb 2, 2025
81e1321
Implement reviews
eugercek Feb 2, 2025
3ca9f7d
review: Optimize `getArpEntriesRTNL` via deferring string lookup
eugercek Feb 2, 2025
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
59 changes: 51 additions & 8 deletions collector/arp_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,25 @@
fs procfs.FS
deviceFilter deviceFilter
entries *prometheus.Desc
states *prometheus.Desc
logger *slog.Logger
}

var neighborStatesMap = map[uint16]string{
unix.NUD_INCOMPLETE: "incomplete",
unix.NUD_REACHABLE: "reachable",
unix.NUD_STALE: "stale",
unix.NUD_DELAY: "delay",
unix.NUD_PROBE: "probe",
unix.NUD_FAILED: "failed",
unix.NUD_PERMANENT: "permanent",
}

type neighborState struct {

Check failure on line 54 in collector/arp_linux.go

View workflow job for this annotation

GitHub Actions / lint 

type `neighborState` is unused (unused)
ip string
state string
}

func init() {
registerCollector("arp", defaultEnabled, NewARPCollector)
}
Expand All @@ -59,6 +75,11 @@
"ARP entries by device",
[]string{"device"}, nil,
),
states: prometheus.NewDesc(
prometheus.BuildFQName(namespace, "arp", "states"),
"ARP states by device",
[]string{"device", "state"}, nil,
),
logger: logger,
}, nil
}
Expand All @@ -73,40 +94,54 @@
return entries
}

func getTotalArpEntriesRTNL() (map[string]uint32, error) {
func getArpEntriesRTNL() (map[string]uint32, map[string]map[string]int, error) {
conn, err := rtnl.Dial(nil)
if err != nil {
return nil, err
return nil, nil, err
}
defer conn.Close()

// Neighbors will also contain IPv6 neighbors, but since this is purely an ARP collector,
// restrict to AF_INET.
neighbors, err := conn.Neighbours(nil, unix.AF_INET)
if err != nil {
return nil, err
return nil, nil, err
}

// Map of interface name to ARP neighbor count.
entries := make(map[string]uint32)
// Map of map[InterfaceName]map[StateName]int
states := make(map[string]map[string]int)

for _, n := range neighbors {
// Skip entries which have state NUD_NOARP to conform to output of /proc/net/arp.
if n.State&unix.NUD_NOARP == 0 {
entries[n.Interface.Name]++
if n.State&unix.NUD_NOARP != unix.NUD_NOARP {
Copy link
Author

Choose a reason for hiding this comment

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

Maybe I can say?

Suggested change
if n.State&unix.NUD_NOARP != unix.NUD_NOARP {
if n.State != unix.NUD_NOARP {

Copy link
Contributor

@dswarbrick dswarbrick Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No, n.State is a bit mask. The bitwise AND is necessary to isolate the NUD_NOARP flag that we wish to test for. Other bits may be set in n.State, so you cannot just do a simple comparison as you suggest.
cf. man 7 rtnetlink:

              ndm_state is a bit mask of the following states:
              NUD_INCOMPLETE   a currently resolving cache entry
              NUD_REACHABLE    a confirmed working cache entry
              NUD_STALE        an expired cache entry
              NUD_DELAY        an entry waiting for a timer
              NUD_PROBE        a cache entry that is currently reprobed
              NUD_FAILED       an invalid cache entry
              NUD_NOARP        a device with no destination cache
              NUD_PERMANENT    a static entry

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for explanation, makes sense. Then current implementation looks ok. I changed n.State&unix.NUD_NOARP == 0 to have early return, can revert that if you wish.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed the bug after i changed here, btw.

continue
}

entries[n.Interface.Name]++

_, ok := states[n.Interface.Name]
if !ok {
eugercek marked this conversation as resolved.
Show resolved Hide resolved
states[n.Interface.Name] = make(map[string]int)
}

states[n.Interface.Name][neighborStatesMap[n.State]]++
}

return entries, nil
return entries, states, nil
}

func (c *arpCollector) Update(ch chan<- prometheus.Metric) error {
var enumeratedEntry map[string]uint32
var (
enumeratedEntry map[string]uint32
enumStates map[string]map[string]int
eugercek marked this conversation as resolved.
Show resolved Hide resolved
)

if *arpNetlink {
var err error

enumeratedEntry, err = getTotalArpEntriesRTNL()
enumeratedEntry, enumStates, err = getArpEntriesRTNL()
if err != nil {
return fmt.Errorf("could not get ARP entries: %w", err)
}
Expand All @@ -125,6 +160,14 @@
}
ch <- prometheus.MustNewConstMetric(
c.entries, prometheus.GaugeValue, float64(entryCount), device)

if *arpNetlink {
states := enumStates[device]
for state, count := range states {
eugercek marked this conversation as resolved.
Show resolved Hide resolved
ch <- prometheus.MustNewConstMetric(
c.states, prometheus.GaugeValue, float64(count), device, state)
}
}
}

return nil
Expand Down
Loading