ethernet: T3163: only change ring-buffer settings if required

Only update the RX/TX ring-buffer settings if they are different from the ones
currently programmed to the hardware. There is no need to write the same value
to the hardware again - this could cause traffic disruption on some NICs.

3 files changed, 34 insertions, 14 deletions

diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py
index e803e28a1..f5796358d 100644
--- a/python/vyos/ethtool.py
+++ b/python/vyos/ethtool.py
@@ -43,6 +43,7 @@ class Ethtool:
     #  }
     _speed_duplex = { }
     _ring_buffers = { }
+    _ring_buffers_max = { }
     _driver_name = None
     _auto_negotiation = None
 
@@ -99,7 +100,7 @@ class Ethtool:
                     'fixed' : fixed
                 }
 
-        out, err = popen(f'ethtool -g {ifname}')
+        out, err = popen(f'ethtool --show-ring {ifname}')
         # We are only interested in line 2-5 which contains the device maximum
         # ringbuffers
         for line in out.splitlines()[2:6]:
@@ -110,6 +111,16 @@ class Ethtool:
                 # output format from 0 -> n/a. As we are only interested in the
                 # tx/rx keys we do not care about RX Mini/Jumbo.
                 if value.isdigit():
+                    self._ring_buffers_max[key] = int(value)
+        # Now we wan't to get the current RX/TX ringbuffer values - used for
+        for line in out.splitlines()[7:11]:
+            if ':' in line:
+                key, value = [s.strip() for s in line.strip().split(":", 1)]
+                key = key.lower().replace(' ', '_')
+                # T3645: ethtool version used on Debian Bullseye changed the
+                # output format from 0 -> n/a. As we are only interested in the
+                # tx/rx keys we do not care about RX Mini/Jumbo.
+                if value.isdigit():
                     self._ring_buffers[key] = int(value)
 
     def _get_generic(self, feature):
@@ -143,15 +154,19 @@ class Ethtool:
     def get_tcp_segmentation_offload(self):
         return self._get_generic('tcp-segmentation-offload')
 
-    def get_rx_buffer(self):
-        # Configuration of RX ring-buffers is not supported on every device,
+    def get_ring_buffer_max(self, rx_tx):
+        # Configuration of RX/TX ring-buffers is not supported on every device,
         # thus when it's impossible return None
-        return self._ring_buffers.get('rx', None)
+        if rx_tx not in ['rx', 'tx']:
+            ValueError('Ring-buffer type must be either "rx" or "tx"')
+        return self._ring_buffers_max.get(rx_tx, None)
 
-    def get_tx_buffer(self):
-        # Configuration of TX ring-buffers is not supported on every device,
+    def get_ring_buffer(self, rx_tx):
+        # Configuration of RX/TX ring-buffers is not supported on every device,
         # thus when it's impossible return None
-        return self._ring_buffers.get('tx', None)
+        if rx_tx not in ['rx', 'tx']:
+            ValueError('Ring-buffer type must be either "rx" or "tx"')
+        return self._ring_buffers.get(rx_tx, None)
 
     def check_speed_duplex(self, speed, duplex):
         """ Check if the passed speed and duplex combination is supported by
diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py
index d4fa3f655..e5da3ac25 100644
--- a/python/vyos/ifconfig/ethernet.py
+++ b/python/vyos/ifconfig/ethernet.py
@@ -316,21 +316,26 @@ class EthernetIf(Interface):
                 print('Adapter does not support changing tcp-segmentation-offload settings!')
         return False
 
-    def set_ring_buffer(self, b_type, b_size):
+    def set_ring_buffer(self, rx_tx, size):
         """
         Example:
         >>> from vyos.ifconfig import EthernetIf
         >>> i = EthernetIf('eth0')
         >>> i.set_ring_buffer('rx', '4096')
         """
+        current_size = self.ethtool.get_ring_buffer(rx_tx)
+        if current_size == size:
+            # bail out early if nothing is about to change
+            return None
+
         ifname = self.config['ifname']
-        cmd = f'ethtool -G {ifname} {b_type} {b_size}'
+        cmd = f'ethtool --set-ring {ifname} {rx_tx} {size}'
         output, code = self._popen(cmd)
         # ethtool error codes:
         #  80 - value already setted
         #  81 - does not possible to set value
         if code and code != 80:
-            print(f'could not set "{b_type}" ring-buffer for {ifname}')
+            print(f'could not set "{rx_tx}" ring-buffer for {ifname}')
         return output
 
     def update(self, config):
@@ -369,8 +374,8 @@ class EthernetIf(Interface):
 
         # Set interface ring buffer
         if 'ring_buffer' in config:
-            for b_type in config['ring_buffer']:
-                self.set_ring_buffer(b_type, config['ring_buffer'][b_type])
+            for rx_tx, size in config['ring_buffer'].items():
+                self.set_ring_buffer(rx_tx, size)
 
         # call base class first
         super().update(config)
diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py
index 889f4856f..f604f787c 100755
--- a/src/conf_mode/interfaces-ethernet.py
+++ b/src/conf_mode/interfaces-ethernet.py
@@ -89,11 +89,11 @@ def verify(ethernet):
                               f'settings to: {speed}/{duplex}!')
 
     if 'ring_buffer' in ethernet:
-        max_rx = ethtool.get_rx_buffer()
+        max_rx = ethtool.get_ring_buffer_max('rx')
         if not max_rx:
             raise ConfigError('Driver does not support RX ring-buffer configuration!')
 
-        max_tx = ethtool.get_tx_buffer()
+        max_tx = ethtool.get_ring_buffer_max('tx')
         if not max_tx:
             raise ConfigError('Driver does not support TX ring-buffer configuration!')