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.

(cherry picked from commit 159899ed9ba5661e4bdcfdadf1292e237f117063)

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}}