From 0229645c8248decb5664056df8aa5cd5dff41802 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 31 Aug 2021 23:03:01 +0200
Subject: vyos.ethtool: T3163: purify code to read and change flow-control
 settings

It makes no sense to have a parser for the ethtool values in ethtool.py
and ethernet.py - one instance ios more then enough!
---
 python/vyos/ethtool.py           | 23 ++++++++++++++++++++++
 python/vyos/ifconfig/ethernet.py | 42 ++++++++++------------------------------
 2 files changed, 33 insertions(+), 32 deletions(-)

(limited to 'python')

diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py
index f5796358d..87b9d7dd0 100644
--- a/python/vyos/ethtool.py
+++ b/python/vyos/ethtool.py
@@ -46,6 +46,8 @@ class Ethtool:
     _ring_buffers_max = { }
     _driver_name = None
     _auto_negotiation = None
+    _flow_control = None
+    _flow_control_enabled = None
 
     def __init__(self, ifname):
         # Get driver used for interface
@@ -123,6 +125,15 @@ class Ethtool:
                 if value.isdigit():
                     self._ring_buffers[key] = int(value)
 
+        # Get current flow control settings, but this is not supported by
+        # all NICs (e.g. vmxnet3 does not support is)
+        out, err = popen(f'ethtool --show-pause {ifname}')
+        if len(out.splitlines()) > 1:
+            self._flow_control = True
+            # read current flow control setting, this returns:
+            # ['Autonegotiate:', 'on']
+            self._flow_control_enabled = out.splitlines()[1].split()[-1]
+
     def _get_generic(self, feature):
         """
         Generic method to read self._features and return a tuple for feature
@@ -186,5 +197,17 @@ class Ethtool:
                 return True
         return False
 
+    def check_flow_control(self):
+        """ Check if the NIC supports flow-control """
+        if self._driver_name in ['vmxnet3', 'virtio_net', 'xen_netfront']:
+            return False
+        return self._flow_control
+
+    def get_flow_control(self):
+        if self._flow_control_enabled == None:
+            raise ValueError('Interface does not support changing '\
+                             'flow-control settings!')
+        return self._flow_control_enabled
+
     def get_auto_negotiation(self):
         return self._auto_negotiation
diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py
index e5da3ac25..7bd269491 100644
--- a/python/vyos/ifconfig/ethernet.py
+++ b/python/vyos/ifconfig/ethernet.py
@@ -121,38 +121,16 @@ class EthernetIf(Interface):
                             'flow control settings!')
             return
 
-        # Get current flow control settings:
-        cmd = f'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:
-        #
-        # Pause parameters for eth0:
-        # Autonegotiate:  on
-        # RX:             off
-        # TX:             off
-        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 ''
-
-        # Assemble command executed on system. Unfortunately there is no way
-        # to change this setting via sysfs
-        cmd = f'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
+        current = self.ethtool.get_flow_control()
+        if current != enable:
+            # Assemble command executed on system. Unfortunately there is no way
+            # to change this setting via sysfs
+            cmd = f'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
+        return None
 
     def set_speed_duplex(self, speed, duplex):
         """
-- 
cgit v1.2.3