On Tue, Feb 26, 2019 at 04:34:43PM +0100, csfakian(a)redhat.com wrote:
From: Christos Sfakianakis <csfakian(a)redhat.com>
Add methods for handling coalesce settings. Raise errors
when these settings cannot be read or modified correctly.
Signed-off-by: Christos Sfakianakis <csfakian(a)redhat.com>
---
lnst/Devices/Device.py | 47 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py
index 20cddcf..6a04d1b 100644
--- a/lnst/Devices/Device.py
+++ b/lnst/Devices/Device.py
@@ -17,10 +17,12 @@ import pyroute2
import logging
import pprint
from abc import ABCMeta
+from itertools import product
from pyroute2.netlink.rtnl import ifinfmsg
from lnst.Common.Logs import log_exc_traceback
from lnst.Common.NetUtils import normalize_hwaddr
from lnst.Common.ExecCmd import exec_cmd
+from lnst.Common.LnstError import LnstError
from lnst.Common.DeviceError import DeviceError, DeviceDeleted, DeviceDisabled
from lnst.Common.DeviceError import DeviceConfigError, DeviceConfigValueError
from lnst.Common.IpAddress import ipaddress
@@ -58,6 +60,7 @@ class Device(object):
self._deleted = False
self._ip_addrs = []
+ self._coalesce_settings = ['', '']
self._nl_update = {}
self._bulk_enabled = False
@@ -603,6 +606,50 @@ class Device(object):
"""disable automatic negotiation of speed for this
device"""
exec_cmd("ethtool -s %s autoneg off" % self.name)
+ def _read_coalesce(self):
+ try:
+ res = exec_cmd("ethtool -c %s" % self.name)
+ except:
+ raise DeviceError("Could not fetch coalesce settings of "
+ "%s." % self.name)
+ res = re.findall('\wX: (o[n|f]*)', res[0])
^^^^^^^
i think this can just be (on|off)
+ if not tuple(res) in product(['off', 'on'],
repeat=2):
+ raise LnstError("Invalid values for coalesce settings of "
+ "{} : {}".format(self.name, res))
^^^
use DeviceError for exceptions from the Device class
So if I understand this correctly, the condition checks if all the
strings found by the re.findall are "on" or "off", and that
there's
exactly two of them. But the regex already ensures that it's only "on"
or "off" (with my modification at least). And you can ensure the
"exactly two values" by modifying the regex a bit more:
"Adaptive RX: (on|off) TX: (on|off)"
then I'd just use re.search and return the result.groups().
In case the regex search fails, then throw the exception.
+ return res
+
+ def _store_coalesce(self):
+ coal = self._read_coalesce()
+ self._coalesce_settings = coal
+
+ def _write_coalesce(self, rx_val, tx_val):
+ if self._coalesce_settings == [rx_val, tx_val]:
Depending on if you store a tuple or a list into the _coalesce_settings,
this comparison might always fail, tuple never equals a list... just a
note in case you rewrite the read method with match.groups() which
returns a tuple...
+ return
+ try:
+ exec_cmd("ethtool -C %s adaptive-rx %s adaptive-tx %s" %
+ (self.name, rx_val, tx_val))
+ except:
+ raise DeviceConfigError("Not allowed to modify coalesce "
+ "settings for %s." % self.name)
+ if self._read_coalesce != [rx_val, tx_val]:
^^^
this is just a reference to a method, you're missing ()
to actually call it so this condition is always True
+ raise LnstError("Could not apply coalesce settings
for "
+ "%s effectively." % self.name)
DeviceError instead of LnstError, DeviceConfigError
is also appropriate here I think
+
+ def _disable_coalesce(self):
+ self._store_coalesce()
+ self._write_coalesce('off', 'off')
+
+ def _restore_coalesce(self):
+ rx_val = self._coalesce_settings[0]
+ tx_val = self._coalesce_settings[1]
+ self._write_coalesce(rx_val, tx_val)
+
+ def disable_coalesce(self):
+ self._disable_coalesce()
Does it make sense to also have an enable_coalesce method for
completeness?
> +
> + def restore_coalesce(self):
> + self._restore_coalesce()
> +
> #TODO implement proper Route objects
> #consider the same as with tc?
> # def route_add(self, dest):
> --
> 2.17.1
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct:
https://getfedora.org/code-of-conduct.html
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...