From 0d666d9aa45bc76322480baf30f863d714f1b0fa Mon Sep 17 00:00:00 2001 From: Thomas Mangin Date: Thu, 26 Mar 2020 13:04:49 +0000 Subject: ifconfig: T2158: never fail when setting flowcontrol the result of the commands used to setup the interface is now checked. flowcontrol can not always be set on all interfaces, and when/if it fails, it prevents the interace to come up. This is problematic as it may prevent the router to come up. Therefore flowcontrol must be allowed to fail gracefully/silently. --- python/vyos/ifconfig/control.py | 11 ++++++++--- python/vyos/ifconfig/ethernet.py | 29 ++++++++++++++++++----------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/python/vyos/ifconfig/control.py b/python/vyos/ifconfig/control.py index 28adc80d1..f7b032478 100644 --- a/python/vyos/ifconfig/control.py +++ b/python/vyos/ifconfig/control.py @@ -28,15 +28,20 @@ class Control(Register): if os.path.isfile('/tmp/vyos.ifconfig.debug'): print('DEBUG/{:<6} {}'.format(self.config['ifname'], msg)) - def _cmd(self, command): + 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}") - if p.returncode != 0: - raise RuntimeError(f'{command}\nreturned: {decoded}') + return decoded, p.returncode + + 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 def _get_command(self, config, name): diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index b3e652409..50552dc71 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -101,6 +101,8 @@ class EthernetIf(Interface): >>> i = EthernetIf('eth0') >>> i.set_flow_control(True) """ + ifname = self.config['ifname'] + if enable not in ['on', 'off']: raise ValueError("Value out of range") @@ -110,8 +112,15 @@ class EthernetIf(Interface): return # Get current flow control settings: - cmd = '/sbin/ethtool --show-pause {0}'.format(self.config['ifname']) - tmp = self._cmd(cmd) + cmd = f'/sbin/ethtool --show-pause {ifname}' + output, code = self._popen(cmd) + if code == 76: + # the interface does not support it + return '' + if code: + # never fail here as it prevent vyos to boot + print(f'unexpected return code {code} from {cmd}') + return '' # The above command returns - with tabs: # @@ -119,23 +128,21 @@ class EthernetIf(Interface): # Autonegotiate: on # RX: off # TX: off - if re.search("Autonegotiate:\ton", tmp): + if re.search("Autonegotiate:\ton", output): if enable == "on": # flowcontrol is already enabled - no need to re-enable it again # this will prevent the interface from flapping as applying the # flow-control settings will take the interface down and bring # it back up every time. - return + return '' # Assemble command executed on system. Unfortunately there is no way # to change this setting via sysfs - cmd = '/sbin/ethtool --pause {0} autoneg {1} tx {1} rx {1}'.format( - self.config['ifname'], enable) - try: - # An exception will be thrown if the settings are not changed - return self._cmd(cmd) - except RuntimeError: - pass + cmd = f'/sbin/ethtool --pause {ifname} autoneg {enable} tx {enable} rx {enable}' + output, code = self._popen(cmd) + if code: + print(f'could not set flowcontrol for {ifname}') + return output def set_speed_duplex(self, speed, duplex): """ -- cgit v1.2.3