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 <nakato@nakato.io>

3 files changed, 36 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 77f84cb..6d6abb2 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 829ab6d..f67e2a3 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 27b0cb4..ee30c81 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