From bc9b28132ca7270f226968d0c1207adefe768049 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 24 Sep 2019 19:24:45 +0200
Subject: Python/ifconfig: T1557: refactor BondIf 'arp_interval' property to
 set_arp_interval()

---
 python/vyos/ifconfig.py            | 81 ++++++++++++++------------------------
 src/conf_mode/interface-bonding.py |  7 +---
 2 files changed, 32 insertions(+), 56 deletions(-)

diff --git a/python/vyos/ifconfig.py b/python/vyos/ifconfig.py
index 324f341da..8b333b81d 100644
--- a/python/vyos/ifconfig.py
+++ b/python/vyos/ifconfig.py
@@ -1115,23 +1115,41 @@ class BondIf(VLANIf):
         return self._write_sysfs('/sys/class/net/{}/bonding/xmit_hash_policy'
                                  .format(self._ifname), mode)
 
-    def set_arp_interval(self, time):
+    def set_arp_interval(self, interval):
         """
-        Specifies the IP addresses to use as ARP monitoring peers when
-        arp_interval is > 0. These are the targets of the ARP request sent to
-        determine the health of the link to the targets. Specify these values
-        in ddd.ddd.ddd.ddd format. Multiple IP addresses must be separated by
-        a comma. At least one IP address must be given for ARP monitoring to
-        function. The maximum number of targets that can be specified is 16.
+        Specifies the ARP link monitoring frequency in milliseconds.
 
-        The default value is no IP addresses.
+        The ARP monitor works by periodically checking the slave devices
+        to determine whether they have sent or received traffic recently
+        (the precise criteria depends upon the bonding mode, and the
+        state of the slave). Regular traffic is generated via ARP probes
+        issued for the addresses specified by the arp_ip_target option.
+
+        If ARP monitoring is used in an etherchannel compatible mode
+        (modes 0 and 2), the switch should be configured in a mode that
+        evenly distributes packets across all links. If the switch is
+        configured to distribute the packets in an XOR fashion, all
+        replies from the ARP targets will be received on the same link
+        which could cause the other team members to fail.
+
+        value of 0 disables ARP monitoring. The default value is 0.
 
         Example:
         >>> from vyos.ifconfig import BondIf
-        >>> BondIf('bond0').set_arp_interval = '100'
-        """
-        return self._write_sysfs('/sys/class/net/{}/bonding/arp_interval'
-                                 .format(self._ifname), time)
+        >>> BondIf('bond0').set_arp_interval('100')
+        """
+        if int(interval) == 0:
+            """
+            Specifies the MII link monitoring frequency in milliseconds.
+            This determines how often the link state of each slave is
+            inspected for link failures. A value of zero disables MII
+            link monitoring. A value of 100 is a good starting point.
+            """
+            return self._write_sysfs('/sys/class/net/{}/bonding/miimon'
+                                     .format(self._ifname), interval)
+        else:
+            return self._write_sysfs('/sys/class/net/{}/bonding/arp_interval'
+                                     .format(self._ifname), interval)
 
     def get_arp_ip_target(self):
         """
@@ -1172,45 +1190,6 @@ class BondIf(VLANIf):
         return self._write_sysfs('/sys/class/net/{}/bonding/arp_ip_target'
                                  .format(self._ifname), target)
 
-    @property
-    def miimon(self):
-        """
-        Specifies the MII link monitoring frequency in milliseconds.
-        This determines how often the link state of each slave is
-        inspected for link failures. A value of zero disables MII
-        link monitoring. A value of 100 is a good starting point.
-
-        The default value is 0.
-
-        Example:
-        >>> from vyos.ifconfig import BondIf
-        >>> BondIf('bond0').miimon
-        '250'
-        """
-        return self._read_sysfs('/sys/class/net/{}/bonding/miimon'
-                                .format(self._ifname))
-
-
-    @miimon.setter
-    def miimon(self, time):
-        """
-        Specifies the MII link monitoring frequency in milliseconds.
-        This determines how often the link state of each slave is
-        inspected for link failures. A value of zero disables MII
-        link monitoring. A value of 100 is a good starting point.
-
-        The default value is 0.
-
-        Example:
-        >>> from vyos.ifconfig import BondIf
-        >>> BondIf('bond0').miimon = 250
-        >>> BondIf('bond0').miimon
-        '250'
-        """
-        return self._write_sysfs('/sys/class/net/{}/bonding/miimon'
-                                 .format(self._ifname), time)
-
-
     def add_port(self, interface):
         """
         Enslave physical interface to bond.
diff --git a/src/conf_mode/interface-bonding.py b/src/conf_mode/interface-bonding.py
index 01df0e3d2..6092d0732 100755
--- a/src/conf_mode/interface-bonding.py
+++ b/src/conf_mode/interface-bonding.py
@@ -345,11 +345,8 @@ def apply(bond):
             b.del_port(intf)
 
         # ARP link monitoring frequency, reset miimon when arp-montior is inactive
-        if bond['arp_mon_intvl'] == 0:
-            # reset miimon to default
-            b.miimon = 250
-        else:
-            b.set_arp_interval(bond['arp_mon_intvl'])
+        # this is done inside BondIf automatically
+        b.set_arp_interval(bond['arp_mon_intvl'])
 
         # ARP monitor targets need to be synchronized between sysfs and CLI.
         # Unfortunately an address can't be send twice to sysfs as this will
-- 
cgit v1.2.3