Change in vdsm[ovirt-3.5]: ethtool_opts: provide a way to apply to all slaves

asegurap at redhat.com asegurap at redhat.com
Wed Sep 10 15:09:00 UTC 2014


Antoni Segura Puimedon has uploaded a new change for review.

Change subject: ethtool_opts: provide a way to apply to all slaves
......................................................................

ethtool_opts: provide a way to apply to all slaves

There was a hole in the feature that this hook services in that it
was not possible to apply some opts to all the slaves in the
cases when the combination of settings was not appliable to all
the slaves in a single ethtool command.

This patch solves the bug by adding a special device name that
the admin can use when defining the custom network opts, i.e.,
'*' that will mean that the special device name will be replaced
with each of the slave names, one at a time, and run as many ethtool
procs as there are slaves in the bond.

Bug-Url: https://bugzilla.redhat.com/1126478
Change-Id: Idddbc167be4ac8ba7c4b4ab10da0275b4caf2a58
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
---
M vdsm/hooking.py
M vdsm_hooks/ethtool_options/README
M vdsm_hooks/ethtool_options/ethtool_options.py
3 files changed, 60 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/32784/1

diff --git a/vdsm/hooking.py b/vdsm/hooking.py
index 6b17ae7..4712ca3 100644
--- a/vdsm/hooking.py
+++ b/vdsm/hooking.py
@@ -69,6 +69,10 @@
         f.write(json.dumps(data))
 
 
+def log(message):
+    sys.stderr.write(message + '\n')
+
+
 def exit_hook(message, return_code=2):
     """
     Exit the hook with a given message, which will be printed to the standard
diff --git a/vdsm_hooks/ethtool_options/README b/vdsm_hooks/ethtool_options/README
index e15b71b..db8272e 100644
--- a/vdsm_hooks/ethtool_options/README
+++ b/vdsm_hooks/ethtool_options/README
@@ -10,6 +10,18 @@
 In the oVirt UI edit custom network properties and, for the key 'ethtool_opts'
 set the command line parameters that one would pass to the ethtool command
 line application. E.g.:
-    '--coalesce ethX rx-usecs 14 sample_interval 3 --offload ethX rx on lro on tso off --change ethX speed 1000 duplex half'
-If it is for a bond with em1 and em2, it could look like:
+    '--coalesce ethX rx-usecs 14 sample-interval 3 --offload ethX rx on lro on tso off --change ethX speed 1000 duplex half'
+
+Note that it is possible to substitute the ethX name of the NIC with a '*' and
+the hook will fill in the right nic name for you.
+
+bonding
+--------
+For bondings there are two options:
+a) Pick which devices to apply something on (subject to the command actually
+   being appliable with a single ethtool call):
+   If it is for a bond with em1 and em2, it could look like:
     '--offload em2 rx on --offload em1 tx on'
+b) Apply to all the bond slaves:
+    '--coalesce * rx-usecs 14 sample_interval 3'
+   This would execute an ethtool process for each slave.
diff --git a/vdsm_hooks/ethtool_options/ethtool_options.py b/vdsm_hooks/ethtool_options/ethtool_options.py
index 58f19bf..e533719 100644
--- a/vdsm_hooks/ethtool_options/ethtool_options.py
+++ b/vdsm_hooks/ethtool_options/ethtool_options.py
@@ -32,7 +32,13 @@
     '/usr/bin/ethtool',  # Arch
 )
 
+ALL_SLAVES = '*'  # wildcard to make the hook resolve the nics to modify
+
 Subcommand = namedtuple('Subcommand', ('name', 'device', 'flags'))
+
+
+class EthtoolError(Exception):
+    pass
 
 
 def _test_cmd_with_nics(nics, ethtool_opts):
@@ -40,17 +46,25 @@
                  'custom': ethtool_opts,
                  'bootproto': 'dhcp', 'STP': 'no', 'bridged': 'true'}
 
-    _validate_dev_ownership(nics, 'test_net',
-                            (item for item in
-                             net_attrs['custom']['ethtool_opts'].split(' ') if
-                             item))
+    for subcommand in _parse_into_subcommands(
+            net_attrs['custom']['ethtool_opts'].split(' ')):
+        _validate_dev_ownership(nics, 'test_net', subcommand)
 
 
 def test():
     opts = {'ethtool_opts':
-            '--coalesce em1 rx-usecs 14 sample_interval 3 '
+            '--coalesce em1 rx-usecs 14 sample-interval 3 '
             '--offload em2 rx on lro on tso off '
             '--change em1 speed 1000 duplex half'}
+    # Test subcmd split
+    print(opts['ethtool_opts'])
+    print('splits into: ')
+    for subcmd in _parse_into_subcommands(opts['ethtool_opts'].split()):
+        command = ([ETHTOOL_BINARY.cmd] + [subcmd.name, subcmd.device] +
+                   subcmd.flags)
+        print('    '),
+        print(command)
+
     # Test with the correct nics
     nics = ('em1', 'em2')
     try:
@@ -87,9 +101,19 @@
     """Applies ethtool_options to the network if necessary"""
     options = attrs['custom'].get('ethtool_opts')
     if options is not None:
-        tokens = options.split(' ')
-        _validate_dev_ownership(_net_nics(attrs), network, tokens)
-        _set_ethtool_opts(network, tokens)
+        nics = _net_nics(attrs)
+        for subcmd in _parse_into_subcommands(options.split()):
+            if subcmd.device == ALL_SLAVES:
+                expanded_nics = nics
+            else:
+                _validate_dev_ownership(nics, network, subcmd)
+                expanded_nics = (subcmd.device,)
+            for nic in expanded_nics:
+                try:
+                    _set_ethtool_opts(network,
+                                      [subcmd.name, nic] + subcmd.flags)
+                except EthtoolError as ee:
+                    hooking.log(ee.message)
 
 
 def _net_nics(attrs):
@@ -99,27 +123,26 @@
         return [attrs.pop('nic')] if 'nic' in attrs else ()
 
 
-def _validate_dev_ownership(nics, name, tokens):
-    """Generator that takes ethtool cmdline arguments and raises an exception
-    if there is a device that does not belong to the network"""
+def _validate_dev_ownership(nics, name, subcommand):
+    """Takes ethtool subcommands and raises an exception if there is a device
+    that does not belong to the network"""
     if not nics:
         raise RuntimeError('Network %s has no nics.' % name)
 
-    for cmd in _parse_into_subcommands(tokens):
-        if cmd.device not in nics:
-            raise RuntimeError('Trying to apply ethtool opts for dev: %s, not '
-                               'in the net nics: %s' % (cmd.device, nics))
+    if subcommand.device not in nics:
+        raise RuntimeError('Trying to apply ethtool opts for dev: %s, not in '
+                           '%s nics: %s' % (subcommand.device, name, nics))
 
 
 def _parse_into_subcommands(tokens):
     current = []
     for token in tokens:
         if token.startswith('-') and current:
-            yield Subcommand(current[0], current[1], current[1:])
+            yield Subcommand(current[0], current[1], current[2:])
             current = []
         current.append(token)
     if current:
-        yield Subcommand(current[0], current[1], ' '.join(current[1:]))
+        yield Subcommand(current[0], current[1], current[2:])
 
 
 def _set_ethtool_opts(network, options):
@@ -128,8 +151,8 @@
     command = [ETHTOOL_BINARY.cmd] + options
     rc, _, err = hooking.execCmd(command, sudo=True)
     if rc != 0:
-        raise RuntimeError('Failed to set ethtool opts (%s) for network %s' %
-                           (' '.join(options), network))
+        raise EthtoolError('Failed to set ethtool opts (%s) for network %s. '
+                           'Err: %s' % (' '.join(options), network, err))
 
 
 if __name__ == '__main__':


-- 
To view, visit http://gerrit.ovirt.org/32784
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idddbc167be4ac8ba7c4b4ab10da0275b4caf2a58
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>


More information about the vdsm-patches mailing list