ethernet: T3163: not all NIC drivers support ring-buffer configuration

In addition to commit cf1156a60e ("ethernet: T3163: probe driver for maximum
rx/tx ring-buffer size") this extends the logic in a way as not every driver
supports setting the buffers at all so it will properly error out.

When invoking "ethtool -g" both stdout and stderr are captured and no exception
is raised if it's an unsupported driver feature. The verify() section will
inform the user about the illegal operation.

5 files changed, 49 insertions, 15 deletions

diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py
index cef7d476f..68c8a6656 100644
--- a/python/vyos/ethtool.py
+++ b/python/vyos/ethtool.py
@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
-from vyos.util import cmd
+from vyos.util import popen
 
 class Ethtool:
     """
@@ -37,9 +37,9 @@ class Ethtool:
 
     def __init__(self, ifname):
         # Now populate features dictionaty
-        tmp = cmd(f'ethtool -k {ifname}')
+        out, err = popen(f'ethtool -k {ifname}')
         # skip the first line, it only says: "Features for eth0":
-        for line in tmp.splitlines()[1:]:
+        for line in out.splitlines()[1:]:
             if ":" in line:
                 key, value = [s.strip() for s in line.strip().split(":", 1)]
                 fixed = "fixed" in value
@@ -50,10 +50,10 @@ class Ethtool:
                     "fixed": fixed
                 }
 
-        tmp = cmd(f'ethtool -g {ifname}')
+        out, err = popen(f'ethtool -g {ifname}')
         # We are only interested in line 2-5 which contains the device maximum
         # ringbuffers
-        for line in tmp.splitlines()[2:6]:
+        for line in out.splitlines()[2:6]:
             if ':' in line:
                 key, value = [s.strip() for s in line.strip().split(":", 1)]
                 key = key.lower().replace(' ', '_')
@@ -91,11 +91,11 @@ class Ethtool:
         return self.features.get('udp-fragmentation-offload', True).get('fixed', True)
 
     def get_rx_buffer(self):
-        # in case of a missing configuration rather return a "small"
-        # buffer of only 512 bytes.
-        return self.ring_buffers.get('rx', '512')
+        # Configuration of RX ring-buffers is not supported on every device,
+        # thus when it's impossible return None
+        return self.ring_buffers.get('rx', None)
 
     def get_tx_buffer(self):
-        # in case of a missing configuration rather return a "small"
-        # buffer of only 512 bytes.
-        return self.ring_buffers.get('tx', '512')
+        # Configuration of RX ring-buffers is not supported on every device,
+        # thus when it's impossible return None
+        return self.ring_buffers.get('tx', None)
diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py
index e82a3e0f1..378f400b8 100755
--- a/src/conf_mode/interfaces-ethernet.py
+++ b/src/conf_mode/interfaces-ethernet.py
@@ -91,7 +91,12 @@ def verify(ethernet):
     ethtool = Ethtool(ifname)
     if 'ring_buffer' in ethernet:
         max_rx = ethtool.get_rx_buffer()
+        if not max_rx:
+            raise ConfigError('Driver does not support RX ring-buffer configuration!')
+
         max_tx = ethtool.get_tx_buffer()
+        if not max_tx:
+            raise ConfigError('Driver does not support TX ring-buffer configuration!')
 
         rx = dict_search('ring_buffer.rx', ethernet)
         if rx and int(rx) > int(max_rx):
diff --git a/src/tests/test_ethtool.py b/src/tests/test_ethtool.py
new file mode 100644
index 000000000..bc53b938a
--- /dev/null
+++ b/src/tests/test_ethtool.py
@@ -0,0 +1,33 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2021 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from unittest import TestCase
+from vyos.ethtool import Ethtool
+
+class TestVyOSEthtool(TestCase):
+    def test_ring_buffer(self):
+        tmp = Ethtool('lo')
+        self.assertEqual(tmp.get_rx_buffer(), None)
+        self.assertEqual(tmp.get_tx_buffer(), None)
+
+    def test_fixed_settings(self):
+        tmp = Ethtool('lo')
+        self.assertTrue(tmp.is_fixed_lro())
+        self.assertFalse(tmp.is_fixed_gro())
+        self.assertFalse(tmp.is_fixed_gso())
+        self.assertFalse(tmp.is_fixed_sg())
+
+
diff --git a/src/tests/test_find_device_file.py b/src/tests/test_find_device_file.py
index 43c80dc76..43c80dc76 100755..100644
--- a/src/tests/test_find_device_file.py
+++ b/src/tests/test_find_device_file.py
diff --git a/src/tests/test_util.py b/src/tests/test_util.py
index f7405cbde..22bc085c5 100644
--- a/src/tests/test_util.py
+++ b/src/tests/test_util.py
@@ -17,11 +17,7 @@
 from unittest import TestCase
 from vyos.util import mangle_dict_keys
 
-
 class TestVyOSUtil(TestCase):
-    def setUp(self):
-        pass
-
     def test_key_mangline(self):
         data = {"foo-bar": {"baz-quux": None}}
         expected_data = {"foo_bar": {"baz_quux": None}}