From 6f5fb5c503b5df96d0686002355da3633b1fc597 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 31 Aug 2021 21:28:08 +0200
Subject: vyos.ethtool: T3163: purify code to read current speed and duplex
 settings

It makes no sense to have a parser for the ethtool value sin ethtool.py
and ethernet.py - one instance ios more then enough!
---
 python/vyos/ethtool.py           | 13 ++++++++++++-
 python/vyos/ifconfig/ethernet.py | 22 +++++-----------------
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py
index 968e2e2a3..e803e28a1 100644
--- a/python/vyos/ethtool.py
+++ b/python/vyos/ethtool.py
@@ -44,6 +44,7 @@ class Ethtool:
     _speed_duplex = { }
     _ring_buffers = { }
     _driver_name = None
+    _auto_negotiation = None
 
     def __init__(self, ifname):
         # Get driver used for interface
@@ -65,7 +66,6 @@ class Ethtool:
                 reading = True
             if 'Supported pause frame use:' in line:
                 reading = False
-                break
             if reading:
                 for block in line.split():
                     if pattern.search(block):
@@ -75,6 +75,15 @@ class Ethtool:
                             self._speed_duplex.update({ speed : {}})
                         if duplex not in self._speed_duplex[speed]:
                             self._speed_duplex[speed].update({ duplex : ''})
+            if 'Auto-negotiation:' in line:
+                # Split the following string: Auto-negotiation: off
+                # we are only interested in off or on
+                tmp = line.split()[-1]
+                self._auto_negotiation = bool(tmp == 'on')
+
+        if self._auto_negotiation == None:
+            raise ValueError(f'Could not determine auto-negotiation settings '\
+                             f'for interface {ifname}!')
 
         # Now populate features dictionaty
         out, err = popen(f'ethtool --show-features {ifname}')
@@ -162,3 +171,5 @@ class Ethtool:
                 return True
         return False
 
+    def get_auto_negotiation(self):
+        return self._auto_negotiation
diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py
index 76ed3fd92..d4fa3f655 100644
--- a/python/vyos/ifconfig/ethernet.py
+++ b/python/vyos/ifconfig/ethernet.py
@@ -20,6 +20,7 @@ from vyos.ethtool import Ethtool
 from vyos.ifconfig.interface import Interface
 from vyos.util import run
 from vyos.util import dict_search
+from vyos.util import read_file
 from vyos.validate import assert_list
 
 @Interface.register
@@ -181,32 +182,19 @@ class EthernetIf(Interface):
 
         # Get current speed and duplex settings:
         ifname = self.config['ifname']
-        cmd = f'ethtool {ifname}'
-        tmp = self._cmd(cmd)
-
-        if re.search("\tAuto-negotiation: on", tmp):
+        if self.ethtool.get_auto_negotiation():
             if speed == 'auto' and duplex == 'auto':
                 # bail out early as nothing is to change
                 return
         else:
             # read in current speed and duplex settings
-            cur_speed = 0
-            cur_duplex = ''
-            for line in tmp.splitlines():
-                if line.lstrip().startswith("Speed:"):
-                    non_decimal = re.compile(r'[^\d.]+')
-                    cur_speed = non_decimal.sub('', line)
-                    continue
-
-                if line.lstrip().startswith("Duplex:"):
-                    cur_duplex = line.split()[-1].lower()
-                    break
-
+            cur_speed = read_file(f'/sys/class/net/{ifname}/speed')
+            cur_duplex = read_file(f'/sys/class/net/{ifname}/duplex')
             if (cur_speed == speed) and (cur_duplex == duplex):
                 # bail out early as nothing is to change
                 return
 
-        cmd = f'ethtool -s {ifname}'
+        cmd = f'ethtool --change {ifname}'
         if speed == 'auto' or duplex == 'auto':
             cmd += ' autoneg on'
         else:
-- 
cgit v1.2.3