From 18fc512fc4d50c2463fa4de8845d33c04a0ed529 Mon Sep 17 00:00:00 2001 From: Sachi King Date: Sun, 10 Feb 2019 21:00:04 +1100 Subject: [PATCH] Bond: Monitor bond mii_status not link operstate (#1124) With a bond interface the state of the slave interface from the bond's point of view is reflected in `mii_status` and is independent of the link's `operstate`. When a bond is monitored with `miimon`, `mii_status` will reflect the state of the physical link as configured via the operator. When a bond is monitored via `arp_interval` the `mii_status` will reflect the results of the bond ARP checking. This means the link can be down from the bond's point of view, but up from a physical connection point of view. If a bond is not monitored via miimon or arp, the `mii_status` should likely be always `up`, however I have observed a case where this is not true and the `operstate` is `up` while `mii_status` is `down`. Kernel bond documentation stresses that a bond should not be configured without one of `mii_mon` or `arp_interval` configured however. This change results in the metric 'node_bonding_active' matching the up/down state of the bond's point of view rather than operstate. Signed-off-by: Sachi King --- CHANGELOG.md | 2 ++ collector/bonding_linux.go | 4 ++-- collector/fixtures/sys.ttar | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77f84cbc..6d6abb2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ other network metrics #1224 * The cpufreq metrics now separate the `cpufreq` and `scaling` data based on what the driver provides. #1248 * The labels for the network_up metric have changed, see issue #1236 +* Bonding collector now uses `mii_status` instead of `operstatus` #1124 ### Changes @@ -21,6 +22,7 @@ * [FEATURE] Add kstat-based Solaris metrics for boottime, cpu and zfs collectors #1197 * [FEATURE] Add uname collector for FreeBSD #1239 * [FEATURE] Add diskstats collector for OpenBSD #1250 +* [CHANGE] Bonding state uses mii_status #1124 ## 0.17.0 / 2018-11-30 diff --git a/collector/bonding_linux.go b/collector/bonding_linux.go index 829ab6dd..f67e2a36 100644 --- a/collector/bonding_linux.go +++ b/collector/bonding_linux.go @@ -82,10 +82,10 @@ func readBondingStats(root string) (status map[string][2]int, err error) { } sstat := [2]int{0, 0} for _, slave := range strings.Fields(string(slaves)) { - state, err := ioutil.ReadFile(filepath.Join(root, master, fmt.Sprintf("lower_%s", slave), "operstate")) + state, err := ioutil.ReadFile(filepath.Join(root, master, fmt.Sprintf("lower_%s", slave), "bonding_slave", "mii_status")) if os.IsNotExist(err) { // some older? kernels use slave_ prefix - state, err = ioutil.ReadFile(filepath.Join(root, master, fmt.Sprintf("slave_%s", slave), "operstate")) + state, err = ioutil.ReadFile(filepath.Join(root, master, fmt.Sprintf("slave_%s", slave), "bonding_slave", "mii_status")) } if err != nil { return nil, err diff --git a/collector/fixtures/sys.ttar b/collector/fixtures/sys.ttar index 27b0cb40..ee30c814 100644 --- a/collector/fixtures/sys.ttar +++ b/collector/fixtures/sys.ttar @@ -704,6 +704,14 @@ Mode: 644 Directory: sys/class/net/dmz/slave_eth0 Mode: 755 # ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Directory: sys/class/net/dmz/slave_eth0/bonding_slave +Mode: 755 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Path: sys/class/net/dmz/slave_eth0/bonding_slave/mii_status +Lines: 1 +up +Mode: 644 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Path: sys/class/net/dmz/slave_eth0/operstate Lines: 1 up @@ -712,6 +720,14 @@ Mode: 644 Directory: sys/class/net/dmz/slave_eth4 Mode: 755 # ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Directory: sys/class/net/dmz/slave_eth4/bonding_slave +Mode: 755 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Path: sys/class/net/dmz/slave_eth4/bonding_slave/mii_status +Lines: 1 +up +Mode: 644 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Path: sys/class/net/dmz/slave_eth4/operstate Lines: 1 up @@ -860,6 +876,14 @@ Mode: 644 Directory: sys/class/net/int/slave_eth1 Mode: 755 # ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Directory: sys/class/net/int/slave_eth1/bonding_slave +Mode: 755 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Path: sys/class/net/int/slave_eth1/bonding_slave/mii_status +Lines: 1 +down +Mode: 644 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Path: sys/class/net/int/slave_eth1/operstate Lines: 1 down @@ -868,6 +892,14 @@ Mode: 644 Directory: sys/class/net/int/slave_eth5 Mode: 755 # ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Directory: sys/class/net/int/slave_eth5/bonding_slave +Mode: 755 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +Path: sys/class/net/int/slave_eth5/bonding_slave/mii_status +Lines: 1 +up +Mode: 644 +# ttar - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Path: sys/class/net/int/slave_eth5/operstate Lines: 1 up