Merge pull request #275 from thomas-mangin/T2057-oper-admin

ifconfig: T2057: explicity name state functions

6 files changed, 43 insertions, 28 deletions

diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py
index 3c26b9b95..e2ff71490 100644
--- a/python/vyos/ifconfig/bond.py
+++ b/python/vyos/ifconfig/bond.py
@@ -101,7 +101,7 @@ class BondIf(Interface):
         for s in self.get_slaves():
             slave = {
                 'ifname': s,
-                'state': Interface(s).get_state()
+                'state': Interface(s).get_admin_state()
             }
             slave_list.append(slave)
 
@@ -112,7 +112,7 @@ class BondIf(Interface):
         # physical interface
         for slave in slave_list:
              i = Interface(slave['ifname'])
-             i.set_state(slave['state'])
+             i.set_admin_state(slave['state'])
 
     def set_hash_policy(self, mode):
         """
@@ -211,7 +211,7 @@ class BondIf(Interface):
         # An interface can only be added to a bond if it is in 'down' state. If
         # interface is in 'up' state, the following Kernel error will  be thrown:
         # bond0: eth1 is up - this may be due to an out of date ifenslave.
-        Interface(interface).set_state('down')
+        Interface(interface).set_admin_state('down')
         return self.set_interface('bond_add_port', f'+{interface}')
 
     def del_port(self, interface):
diff --git a/python/vyos/ifconfig/control.py b/python/vyos/ifconfig/control.py
index 39b6945c8..1c9f7e284 100644
--- a/python/vyos/ifconfig/control.py
+++ b/python/vyos/ifconfig/control.py
@@ -49,7 +49,7 @@ class Control(Register):
         Using the defined names, set data write to sysfs.
         """
         cmd = self._command_get[name]['shellcmd'].format(**config)
-        return self._cmd(cmd)
+        return self._command_get[name].get('format', lambda _: _)(self._cmd(cmd))
 
     def _set_command(self, config, name, value):
         """
@@ -72,7 +72,7 @@ class Control(Register):
         config = {**config, **{'value': value}}
 
         cmd = self._command_set[name]['shellcmd'].format(**config)
-        return self._cmd(cmd)
+        return self._command_set[name].get('format', lambda _: _)(self._cmd(cmd))
 
     _sysfs_get = {}
     _sysfs_set = {}
diff --git a/python/vyos/ifconfig/geneve.py b/python/vyos/ifconfig/geneve.py
index f27786417..0c1cdade9 100644
--- a/python/vyos/ifconfig/geneve.py
+++ b/python/vyos/ifconfig/geneve.py
@@ -49,7 +49,7 @@ class GeneveIf(Interface):
         self._cmd(cmd)
 
         # interface is always A/D down. It needs to be enabled explicitly
-        self.set_state('down')
+        self.set_admin_state('down')
 
     @classmethod
     def get_config(cls):
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 2db5aa76f..0fddc67f3 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -49,8 +49,15 @@ class Interface(DHCP):
         'bridgeable':  False,
     }
 
+    _command_get = {
+        'admin_state': {
+            'shellcmd': 'ip -json link show dev {ifname}',
+            'format': lambda j: 'up' if 'UP' in json.loads(j)[0]['flags'] else 'down',
+        }
+    }
+
     _command_set = {
-        'state': {
+        'admin_state': {
             'validate': lambda v: assert_list(v, ['up', 'down']),
             'shellcmd': 'ip link set dev {ifname} {value}',
         },
@@ -74,6 +81,9 @@ class Interface(DHCP):
         'mtu': {
             'location': '/sys/class/net/{ifname}/mtu',
         },
+        'oper_state':{
+            'location': '/sys/class/net/{ifname}/operstate',
+        },
     }
 
     _sysfs_set = {
@@ -267,9 +277,9 @@ class Interface(DHCP):
             return None
 
         # MAC address can only be changed if interface is in 'down' state
-        prev_state = self.get_state()
+        prev_state = self.get_admin_state()
         if prev_state == 'up':
-            self.set_state('down')
+            self.set_admin_state('down')
 
         self.set_interface('mac', mac)
 
@@ -410,36 +420,41 @@ class Interface(DHCP):
         """
         self.set_interface('alias', ifalias)
 
-    def get_state(self):
+    def get_admin_state(self):
         """
         Get interface administrative state. Function will return 'up' or 'down'
 
         Example:
         >>> from vyos.ifconfig import Interface
-        >>> Interface('eth0').get_state()
+        >>> Interface('eth0').get_admin_state()
         'up'
         """
-        cmd = 'ip -json link show dev {}'.format(self.config['ifname'])
-        tmp = self._cmd(cmd)
-        out = json.loads(tmp)
+        return self.get_interface('admin_state')
 
-        state = 'down'
-        if 'UP' in out[0]['flags']:
-            state = 'up'
-
-        return state
-
-    def set_state(self, state):
+    def set_admin_state(self, state):
         """
         Set interface administrative state to be 'up' or 'down'
 
         Example:
         >>> from vyos.ifconfig import Interface
-        >>> Interface('eth0').set_state('down')
-        >>> Interface('eth0').get_state()
+        >>> Interface('eth0').set_admin_state('down')
+        >>> Interface('eth0').get_admin_state()
         'down'
         """
-        return self.set_interface('state', state)
+        return self.set_interface('admin_state', state)
+
+    def get_oper_state(self):
+        """
+        Get interface operational state
+
+        Example:
+        >>> from vyos.ifconfig import Interface
+        >>> Interface('eth0').get_oper_sate()
+        'up'
+        """
+        # https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-net
+        # "unknown", "notpresent", "down", "lowerlayerdown", "testing", "dormant", "up"
+        return self.get_interface('oper_state')
 
     def set_proxy_arp(self, enable):
         """
diff --git a/python/vyos/ifconfig/l2tpv3.py b/python/vyos/ifconfig/l2tpv3.py
index fbfab4c6e..07f1cf8a3 100644
--- a/python/vyos/ifconfig/l2tpv3.py
+++ b/python/vyos/ifconfig/l2tpv3.py
@@ -62,7 +62,7 @@ class L2TPv3If(Interface):
         self._cmd(cmd)
 
         # interface is always A/D down. It needs to be enabled explicitly
-        self.set_state('down')
+        self.set_admin_state('down')
 
     def remove(self):
         """
@@ -76,7 +76,7 @@ class L2TPv3If(Interface):
 
         if os.path.exists('/sys/class/net/{}'.format(self.config['ifname'])):
             # interface is always A/D down. It needs to be enabled explicitly
-            self.set_state('down')
+            self.set_admin_state('down')
 
             if self._config['tunnel_id'] and self._config['session_id']:
                 cmd = 'ip l2tp del session tunnel_id {} '.format(
diff --git a/python/vyos/ifconfig/tunnel.py b/python/vyos/ifconfig/tunnel.py
index a49bdd51c..1bbb9eb6a 100644
--- a/python/vyos/ifconfig/tunnel.py
+++ b/python/vyos/ifconfig/tunnel.py
@@ -98,10 +98,10 @@ class _Tunnel(Interface):
         options = " ".join(["{} {}".format(k, self.config[k])
                             for k in self.options if k in self.config and self.config[k]])
         self._cmd('{} {}'.format(self.create.format(**self.config), options))
-        self.set_interface('state', 'down')
+        self.set_admin_state('down')
 
     def _delete(self):
-        self.set_interface('state', 'down')
+        self.set_admin_state('down')
         cmd = self.delete.format(**self.config)
         return self._cmd(cmd)