ifconfig: T2653: implement update() in derived classes for admin up/down

Every derived class must implement update() to set the interfaces admin
up/down state. This is required to prevend extensive link flaps when e.g.
reconfiguring bond interfaces.

7 files changed, 90 insertions, 6 deletions

diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py
index af950b35d..da4e1a289 100644
--- a/python/vyos/ifconfig/bridge.py
+++ b/python/vyos/ifconfig/bridge.py
@@ -195,7 +195,7 @@ class BridgeIf(Interface):
         interface setup code and provide a single point of entry when workin
         on any interface. """
 
-        # now call the regular function from within our base class
+        # call base class first
         super().update(config)
 
         # Set ageing time
@@ -251,3 +251,13 @@ class BridgeIf(Interface):
                 # set bridge port path priority
                 value = interface_config.get('priority')
                 tmp.set_path_priority(value)
+
+        # Enable/Disable of an interface must always be done at the end of the
+        # derived class to make use of the ref-counting set_admin_state()
+        # function. We will only enable the interface if 'up' was called as
+        # often as 'down'. This is required by some interface implementations
+        # as certain parameters can only be changed when the interface is
+        # in admin-down state. This ensures the link does not flap during
+        # reconfiguration.
+        state = 'down' if 'disable' in config else 'up'
+        self.set_admin_state(state)
diff --git a/python/vyos/ifconfig/dummy.py b/python/vyos/ifconfig/dummy.py
index 404c490c7..43614cd1c 100644
--- a/python/vyos/ifconfig/dummy.py
+++ b/python/vyos/ifconfig/dummy.py
@@ -35,3 +35,22 @@ class DummyIf(Interface):
             'prefixes': ['dum', ],
         },
     }
+
+    def update(self, config):
+        """ General helper function which works on a dictionary retrived by
+        get_config_dict(). It's main intention is to consolidate the scattered
+        interface setup code and provide a single point of entry when workin
+        on any interface. """
+
+        # call base class first
+        super().update(config)
+
+        # Enable/Disable of an interface must always be done at the end of the
+        # derived class to make use of the ref-counting set_admin_state()
+        # function. We will only enable the interface if 'up' was called as
+        # often as 'down'. This is required by some interface implementations
+        # as certain parameters can only be changed when the interface is
+        # in admin-down state. This ensures the link does not flap during
+        # reconfiguration.
+        state = 'down' if 'disable' in config else 'up'
+        self.set_admin_state(state)
diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py
index 7641e82fd..77128633d 100644
--- a/python/vyos/ifconfig/ethernet.py
+++ b/python/vyos/ifconfig/ethernet.py
@@ -261,7 +261,7 @@ class EthernetIf(Interface):
         interface setup code and provide a single point of entry when workin
         on any interface. """
 
-        # now call the regular function from within our base class
+        # call base class first
         super().update(config)
 
         # disable ethernet flow control (pause frames)
@@ -326,3 +326,13 @@ class EthernetIf(Interface):
         for vif_id, vif in config.get('vif', {}).items():
             vlan = self.add_vlan(vif_id)
             vlan.update(vif)
+
+        # Enable/Disable of an interface must always be done at the end of the
+        # derived class to make use of the ref-counting set_admin_state()
+        # function. We will only enable the interface if 'up' was called as
+        # often as 'down'. This is required by some interface implementations
+        # as certain parameters can only be changed when the interface is
+        # in admin-down state. This ensures the link does not flap during
+        # reconfiguration.
+        state = 'down' if 'disable' in config else 'up'
+        self.set_admin_state(state)
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 2d2017b7a..5942904b5 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -951,6 +951,3 @@ class Interface(Control):
             bridge = config.get('is_bridge_member')
             self.add_to_bridge(bridge)
 
-        # Interface administrative state
-        state = 'down' if 'disable' in config else 'up'
-        self.set_admin_state(state)
diff --git a/python/vyos/ifconfig/loopback.py b/python/vyos/ifconfig/loopback.py
index 7ebd13b54..2b4ebfdcc 100644
--- a/python/vyos/ifconfig/loopback.py
+++ b/python/vyos/ifconfig/loopback.py
@@ -75,5 +75,15 @@ class LoopbackIf(Interface):
         # Update IP address entry in our dictionary
         config.update({'address' : addr})
 
-        # now call the regular function from within our base class
+        # call base class
         super().update(config)
+
+        # Enable/Disable of an interface must always be done at the end of the
+        # derived class to make use of the ref-counting set_admin_state()
+        # function. We will only enable the interface if 'up' was called as
+        # often as 'down'. This is required by some interface implementations
+        # as certain parameters can only be changed when the interface is
+        # in admin-down state. This ensures the link does not flap during
+        # reconfiguration.
+        state = 'down' if 'disable' in config else 'up'
+        self.set_admin_state(state)
diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index ea8c9807e..6f570d162 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -71,3 +71,22 @@ class MACsecIf(Interface):
             'source_interface': '',
         }
         return config
+
+    def update(self, config):
+        """ General helper function which works on a dictionary retrived by
+        get_config_dict(). It's main intention is to consolidate the scattered
+        interface setup code and provide a single point of entry when workin
+        on any interface. """
+
+        # call base class first
+        super().update(config)
+
+        # Enable/Disable of an interface must always be done at the end of the
+        # derived class to make use of the ref-counting set_admin_state()
+        # function. We will only enable the interface if 'up' was called as
+        # often as 'down'. This is required by some interface implementations
+        # as certain parameters can only be changed when the interface is
+        # in admin-down state. This ensures the link does not flap during
+        # reconfiguration.
+        state = 'down' if 'disable' in config else 'up'
+        self.set_admin_state(state)
diff --git a/python/vyos/ifconfig/macvlan.py b/python/vyos/ifconfig/macvlan.py
index b5481f4a7..b068ce873 100644
--- a/python/vyos/ifconfig/macvlan.py
+++ b/python/vyos/ifconfig/macvlan.py
@@ -68,3 +68,22 @@ class MACVLANIf(Interface):
         >> dict = MACVLANIf().get_config()
         """
         return deepcopy(cls.default)
+
+    def update(self, config):
+        """ General helper function which works on a dictionary retrived by
+        get_config_dict(). It's main intention is to consolidate the scattered
+        interface setup code and provide a single point of entry when workin
+        on any interface. """
+
+        # call base class first
+        super().update(config)
+
+        # Enable/Disable of an interface must always be done at the end of the
+        # derived class to make use of the ref-counting set_admin_state()
+        # function. We will only enable the interface if 'up' was called as
+        # often as 'down'. This is required by some interface implementations
+        # as certain parameters can only be changed when the interface is
+        # in admin-down state. This ensures the link does not flap during
+        # reconfiguration.
+        state = 'down' if 'disable' in config else 'up'
+        self.set_admin_state(state)