Merge pull request #299 from thomas-mangin/T2205

ifconfig: T2205: silence ethtool harmless failures

3 files changed, 67 insertions, 28 deletions

diff --git a/python/vyos/ifconfig/control.py b/python/vyos/ifconfig/control.py
index c1a073aef..c7a2fa2d6 100644
--- a/python/vyos/ifconfig/control.py
+++ b/python/vyos/ifconfig/control.py
@@ -15,8 +15,9 @@
 
 
 import os
-from subprocess import Popen, PIPE, STDOUT
 
+from vyos.util import debug, debug_msg
+from vyos.util import popen, cmd
 from vyos.ifconfig.register import Register
 
 
@@ -32,27 +33,18 @@ class Control(Register):
         # to prevent this, debugging can be explicitely disabled
 
         # if debug is not explicitely disabled the the config, enable it
-        self.debug = kargs.get('debug', True)
+        self.debug = ''
+        if kargs.get('debug', True):
+            self.debug = debug('ifconfig')
 
-    def _debug_msg(self, msg):
-        if os.path.isfile('/tmp/vyos.ifconfig.debug') and self.debug:
-            print('DEBUG/{:<6} {}'.format(self.config['ifname'], msg))
+    def _debug_msg (self, message):
+        return debug_msg(message, self.debug)
 
     def _popen(self, command):
-        p = Popen(command, stdout=PIPE, stderr=STDOUT, shell=True)
-        tmp = p.communicate()[0].strip()
-        self._debug_msg(f"cmd '{command}'")
-        decoded = tmp.decode()
-        if decoded:
-            self._debug_msg(f"returned:\n{decoded}")
-        return decoded, p.returncode
+        return popen(command, self.debug)
 
     def _cmd(self, command):
-        decoded, code = self._popen(command)
-        if code != 0:
-            # error code can be recovered with .errno
-            raise OSError(code, f'{command}\nreturned: {decoded}')
-        return decoded
+        return cmd(command, self.debug)
 
     def _get_command(self, config, name):
         """
@@ -79,6 +71,10 @@ class Control(Register):
         if convert:
             value = convert(value)
 
+        possible = self._command_set[name].get('possible', None)
+        if possible and not possible(config['ifname'], value):
+            return False
+
         config = {**config, **{'value': value}}
 
         cmd = self._command_set[name]['shellcmd'].format(**config)
diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py
index 50552dc71..c18d2e72f 100644
--- a/python/vyos/ifconfig/ethernet.py
+++ b/python/vyos/ifconfig/ethernet.py
@@ -18,7 +18,7 @@ import re
 
 from vyos.ifconfig.interface import Interface
 from vyos.ifconfig.vlan import VLAN
-
+from vyos.util import popen
 from vyos.validate import *
 
 
@@ -43,27 +43,36 @@ class EthernetIf(Interface):
         }
     }
 
+    @staticmethod
+    def feature(ifname, option, value):
+        out, code = popen(f'/sbin/ethtool -K {ifname} {option} {value}','ifconfig')
+        return False
 
     _command_set = {**Interface._command_set, **{
         'gro': {
             'validate': lambda v: assert_list(v, ['on', 'off']),
-            'shellcmd': '/sbin/ethtool -K {ifname} gro {value}',
+            'possible': lambda i, v: EthernetIf.feature(i, 'gro', v),
+            # 'shellcmd': '/sbin/ethtool -K {ifname} gro {value}',
         },
         'gso': {
             'validate': lambda v: assert_list(v, ['on', 'off']),
-            'shellcmd': '/sbin/ethtool -K {ifname} gso {value}',
+            'possible': lambda i, v: EthernetIf.feature(i, 'gso', v),
+            # 'shellcmd': '/sbin/ethtool -K {ifname} gso {value}',
         },
         'sg': {
             'validate': lambda v: assert_list(v, ['on', 'off']),
-            'shellcmd': '/sbin/ethtool -K {ifname} sg {value}',
+            'possible': lambda i, v: EthernetIf.feature(i, 'sg', v),
+            # 'shellcmd': '/sbin/ethtool -K {ifname} sg {value}',
         },
         'tso': {
             'validate': lambda v: assert_list(v, ['on', 'off']),
-            'shellcmd': '/sbin/ethtool -K {ifname} tso {value}',
+            'possible': lambda i, v: EthernetIf.feature(i, 'tso', v),
+            # 'shellcmd': '/sbin/ethtool -K {ifname} tso {value}',
         },
         'ufo': {
             'validate': lambda v: assert_list(v, ['on', 'off']),
-            'shellcmd': '/sbin/ethtool -K {ifname} ufo {value}',
+            'possible': lambda i, v: EthernetIf.feature(i, 'ufo', v),
+            # 'shellcmd': '/sbin/ethtool -K {ifname} ufo {value}',
         },
     }}
 
@@ -245,8 +254,4 @@ class EthernetIf(Interface):
         >>> i = EthernetIf('eth0')
         >>> i.set_udp_offload('on')
         """
-        if state not in ['on', 'off']:
-            raise ValueError('state must be "on" or "off"')
-
-        cmd = '/sbin/ethtool -K {} ufo {}'.format(self.config['ifname'], state)
-        return self._cmd(cmd)
+        return self.set_interface('ufo', state)
diff --git a/python/vyos/util.py b/python/vyos/util.py
index 5807c837d..781fd9a2c 100644
--- a/python/vyos/util.py
+++ b/python/vyos/util.py
@@ -16,6 +16,44 @@
 import os
 import re
 import sys
+from subprocess import Popen, PIPE, STDOUT
+
+
+# debugging
+
+
+def debug(flag):
+    return flag if os.path.isfile(f'/tmp/vyos.{flag}.debug') else ''
+
+
+def debug_msg(message, section=''):
+    if section:
+        print(f'DEBUG/{section:<6} {message}')
+
+
+#  commands
+
+
+def popen(command, section=''):
+    p = Popen(command, stdout=PIPE, stderr=STDOUT, shell=True)
+    tmp = p.communicate()[0].strip()
+    debug_msg(f"cmd '{command}'", section)
+    decoded = tmp.decode()
+    if decoded:
+        debug_msg(f"returned:\n{decoded}", section)
+    return decoded, p.returncode
+
+
+def cmd(command, section=''):
+    decoded, code = popen(command, section)
+    if code != 0:
+        # error code can be recovered with .errno
+        raise OSError(code, f'{command}\nreturned: {decoded}')
+    return decoded
+
+
+# file manipulation
+
 
 
 def subprocess_cmd(command):