Rss ladder warn (#127)

strizhechenko · web-flow · commit 052f919e45f5 · 2017-07-09T14:29:48.000+05:00
* Added(rss-ladder): warning about using few queues on same cpu. #113 * Added(rss-ladder): colorized output #117 * Added(rss-ladder): automatic socket detection #123 * Fixed(autorps): explicit passing --socket 0 didn't work and called socket_detection logic. * Fixed: reorder tests by time they take because they should fail as soon as possible. * Added(rss-ladder): docstrings everywhere * Changes(pylint): disabled C0111. * Fixed: pylint violations.
diff --git a/.pylintrc b/.pylintrc
@@ -1,2 +1,7 @@
 [FORMAT]
+# why: because checks should be consistent with default pycharm settings
 max-line-length=120
+
+[MESSAGES CONTROL]
+# why: because I think module/class docstrings are generally useless, naming should be the documentation
+disable=C0111
diff --git a/Makefile b/Makefile
@@ -1,9 +1,9 @@
 test:
+	pytest netutils_linux_*/
+	./tests/utils_runnable
 	./tests/rss-ladder-test
 	./tests/server-info-show
 	./tests/link_rate_units.sh
-	./tests/utils_runnable
-	pytest netutils_linux_*/
 
 env:
 	rm -rf env
diff --git a/netutils_linux_hardware/assessor_math.py b/netutils_linux_hardware/assessor_math.py
@@ -5,9 +5,14 @@
 import math
 
 
-def round_(x, d=0):
-    p = 10 ** d
-    return float(math.floor((x * p) + math.copysign(0.5, x))) / p
+def round_(value, precision=0):
+    """
+    :param value: float value
+    :param precision: how much digits after ',' we need
+    :return: rounded float value
+    """
+    precision = 10 ** precision
+    return float(math.floor((value * precision) + math.copysign(0.5, value))) / precision
 
 
 def extract(dictionary, key_sequence):
@@ -25,9 +30,9 @@ def any2int(value):
     elif value is None:
         return 0
     elif isinstance(value, str):
-        v = re.sub(r'[^0-9]', '', value)
-        if v.isdigit():
-            return int(v)
+        value = re.sub(r'[^0-9]', '', value)
+        if value.isdigit():
+            return int(value)
     elif isinstance(value, float):
         return int(value)
     return 0
diff --git a/netutils_linux_hardware/grade_test.py b/netutils_linux_hardware/grade_test.py
@@ -33,16 +33,16 @@ def test_grade_str(self):
             "Dlink": 1,
             "Realtek": 1,
         }
-        for k, v in iteritems(expected):
-            self.assertEqual(Grade.str(k, good, bad), v)
+        for k, value in iteritems(expected):
+            self.assertEqual(Grade.str(k, good, bad), value)
 
     def test_grade_fact(self):
         self.assertEqual(Grade.fact(None, True), 1)
-        self.assertEqual(Grade.fact(None, False), 10)
+        self.assertEqual(Grade.fact(None), 10)
         self.assertEqual(Grade.fact("Anything", True), 10)
-        self.assertEqual(Grade.fact("Anything", False), 1)
+        self.assertEqual(Grade.fact("Anything"), 1)
         self.assertEqual(Grade.fact(15, True), 10)
-        self.assertEqual(Grade.fact(15, False), 1)
+        self.assertEqual(Grade.fact(15), 1)
         self.assertEqual(Grade.fact({'x': 'y'}, True), 10)
         self.assertEqual(Grade.fact({}, True), 10)
 
diff --git a/netutils_linux_monitoring/layout.py b/netutils_linux_monitoring/layout.py
@@ -8,21 +8,21 @@
 
 def make_table(header, align_map=None, rows=None):
     """ Wrapper for pretty table """
-    t = PrettyTable()
-    t.horizontal_char = t.vertical_char = t.junction_char = ' '
-    t.field_names = header
+    table = PrettyTable()
+    table.horizontal_char = table.vertical_char = table.junction_char = ' '
+    table.field_names = header
     if align_map:
         for field, align in zip(header, align_map):
-            t.align[field] = align
+            table.align[field] = align
     if rows:
         for row in rows:
-            if len(row) < len(t.field_names):
+            if len(row) < len(table.field_names):
                 continue
             try:
-                t.add_row(row)
+                table.add_row(row)
             except Exception as err:
-                print_('fields:', t.field_names)
+                print_('fields:', table.field_names)
                 print_('row:', row)
                 print_('rows:', rows)
                 raise err
-    return t
+    return table
diff --git a/netutils_linux_monitoring/link_rate.py b/netutils_linux_monitoring/link_rate.py
@@ -135,7 +135,7 @@ def devices_list(self):
             devices = self.options.devices.split(',')
         else:
             devices = self.devices_list_regex()
-        return list(filter(self.devices_list_validate, devices))
+        return [dev for dev in devices if self.devices_list_validate(dev)]
 
     def post_optparse(self):
         """ Asserting and applying parsing options """
diff --git a/netutils_linux_monitoring/numa.py b/netutils_linux_monitoring/numa.py
@@ -58,8 +58,8 @@ def dev_node(self, dev, fake=False):
         filename = '/sys/class/net/{0}/device/numa_node'.format(dev)
         if not os.path.isfile(filename):
             return -1
-        with open(filename) as fd:
-            return int(fd.read().strip())
+        with open(filename) as dev_file:
+            return int(dev_file.read().strip())
 
     def detect_layouts_fallback(self):
         """
@@ -96,7 +96,7 @@ def detect_layouts(self, lscpu_output=None):
         """ Determine NUMA and sockets layout """
         stdout = self.detect_layout_lscpu(lscpu_output)
         rows = [row for row in stdout.strip().split('\n') if not row.startswith('#')]
-        layouts = [list(map(any2int, row.split(',')[2:4])) for row in rows]
+        layouts = [[any2int(value) for value in row.split(',')][2:4] for row in rows]
         numa_layout, socket_layout = zip(*layouts)
         self.numa_layout = dict(enumerate(numa_layout))
         self.socket_layout = dict(enumerate(socket_layout))
diff --git a/netutils_linux_monitoring/softirqs.py b/netutils_linux_monitoring/softirqs.py
@@ -30,10 +30,9 @@ def post_optparse(self):
             self.numa = Numa(fake=self.options.random)
 
     def parse(self):
-        with open(self.options.softirqs_file) as fd:
-            metrics = [line.strip().split(':')
-                       for line in fd.readlines() if ':' in line]
-            return dict((k, list(map(int, v.strip().split()))) for k, v in metrics)
+        with open(self.options.softirqs_file) as softirq_file:
+            metrics = [line.strip().split(':') for line in softirq_file.readlines() if ':' in line]
+            return dict((k, [int(d) for d in v.strip().split()]) for k, v in metrics)
 
     @staticmethod
     def __active_cpu_count__(data):
diff --git a/netutils_linux_monitoring/softnet_stat.py b/netutils_linux_monitoring/softnet_stat.py
@@ -93,9 +93,7 @@ def make_rows(self):
             colorize(stat.time_squeeze, self.time_squeeze_warning, self.time_squeeze_error),
             colorize(stat.cpu_collision, self.cpu_collision_warning, self.cpu_collision_error),
             stat.received_rps
-        ]
-            for stat in self.repr_source()
-        ]
+        ] for stat in self.repr_source()]
 
     def __repr__(self):
         table = make_table(self.make_header(), self.align, list(self.make_rows()))
diff --git a/netutils_linux_tuning/__init__.py b/netutils_linux_tuning/__init__.py
@@ -1,3 +1,5 @@
 from netutils_linux_tuning.rx_buffers import RxBuffersIncreaser
+from netutils_linux_tuning.rss_ladder import RSSLadder
+from netutils_linux_tuning.autorps import AutoRPS
 
-__all__ = ['RxBuffersIncreaser']
+__all__ = ['RxBuffersIncreaser', 'AutoRPS', 'RSSLadder']
diff --git a/netutils_linux_tuning/autorps.py b/netutils_linux_tuning/autorps.py
@@ -14,10 +14,11 @@ def __init__(self):
         self.options = self.parse_options()
         self.numa = self.make_numa()
         self.process_options()
-        self.mask_apply()
+        queues = self.detect_queues()
+        self.mask_apply(queues)
 
     def socket_detect(self):
-        if any([self.options.socket, self.options.cpus, self.options.cpu_mask]):
+        if any([self.options.socket is not None, self.options.cpus, self.options.cpu_mask]):
             return
         socket = self.numa.node_dev_dict([self.options.dev], True).get(self.options.dev)
         self.options.socket = 0 if socket == -1 else socket
@@ -42,7 +43,7 @@ def cpus2mask(cpus, cpus_count):
         bitmap = [0] * cpus_count
         for cpu in cpus:
             bitmap[cpu] = 1
-        return hex(int("".join(map(str, bitmap)), 2))[2:]
+        return hex(int("".join([str(cpu) for cpu in bitmap]), 2))[2:]  # no need to write 0x
 
     def mask_detect(self):
         if self.options.cpu_mask:
@@ -55,16 +56,19 @@ def process_options(self):
         self.socket_detect()
         self.mask_detect()
 
-    def detect_queues(self, queue_dir):
+    def detect_queues_real(self):
+        queue_dir = "/sys/class/net/{0}/queues/".format(self.options.dev)
         return [queue for queue in os.listdir(queue_dir) if queue.startswith('rx')]
 
-    def mask_apply(self):
+    def detect_queues(self):
         if self.options.test_dir:
-            self.mask_apply_test()
-        queue_dir = "/sys/class/net/{0}/queues/".format(self.options.dev)
-        queues = self.detect_queues(queue_dir)
+            return ['rx-0']
+        return self.detect_queues_real()
+
+    def mask_apply(self, queues):
         if len(queues) > 1 and not self.options.force:
             raise OSError("Refuse to use RPS on multiqueue NIC. You may use --force flag to apply RPS for all queues")
+        queue_dir = "/sys/class/net/{0}/queues/".format(self.options.dev)
         for queue in queues:
             print_("Using mask '{0}' for {1}-{2}".format(self.options.cpu_mask, self.options.dev, queue))
             if self.options.dry_run:
diff --git a/netutils_linux_tuning/rss_ladder.py b/netutils_linux_tuning/rss_ladder.py
@@ -3,6 +3,7 @@
 """ Receive Side Scaling tuning utility """
 
 import re
+import sys
 from argparse import ArgumentParser
 from os.path import join, exists
 
@@ -11,23 +12,25 @@
 
 from netutils_linux_hardware.assessor_math import any2int
 from netutils_linux_monitoring.numa import Numa
+from netutils_linux_monitoring.colors import wrap, YELLOW, cpu_color, COLORS_NODE
 
 MAX_QUEUE_PER_DEVICE = 16
 
 
 class RSSLadder(object):
     """ Distributor of queues' interrupts by multiple CPUs """
 
-    def __init__(self):
+    def __init__(self, argv=None):
         interrupts_file = '/proc/interrupts'
+        if argv:
+            sys.argv = [sys.argv[0]] + argv
         self.options = self.parse_options()
         lscpu_output = None
         if self.options.test_dir:
             interrupts_file = join(self.options.test_dir, "interrupts")
             lscpu_output_filename = join(self.options.test_dir, "lscpu_output")
             lscpu_output = open(lscpu_output_filename).read()
-            # Popen.stdout in python 2.7 returns <str>
-            # Popen.stdout in python 3.6 returns <bytes>
+            # Popen.stdout: python 2.7 returns <str>, 3.6 returns <bytes>
             # read() in both cases return <str>
             if isinstance(lscpu_output, bytes):
                 lscpu_output = str(lscpu_output)
@@ -36,24 +39,37 @@ def __init__(self):
         self.interrupts = open(interrupts_file).readlines()
         if not self.options.cpus:  # no need to detect topology if user gave us cpu list
             self.numa = Numa(lscpu_output=lscpu_output)
+            self.socket_detect()
+            self.numa.devices = self.numa.node_dev_dict([self.options.dev], False)
         for postfix in sorted(self.queue_postfixes_detect()):
             self.smp_affinity_list_apply(self.smp_affinity_list_make(postfix))
 
+    def socket_detect(self):
+        if any([self.options.socket is not None, self.options.cpus]):
+            return
+        socket = self.numa.node_dev_dict([self.options.dev], True).get(self.options.dev)
+        self.options.socket = 0 if socket == -1 else socket
+
     @staticmethod
     def parse_options():
+        """
+        :return: parsed arguments
+        """
         parser = ArgumentParser()
         parser.add_argument('-t', '--test-dir', type=str,
                             help="Use prepared test dataset in TEST_DIR directory instead of /proc/interrupts.")
         parser.add_argument('-d', '--dry-run', help="Don't write anything to smp_affinity_list.",
                             action='store_true', default=False)
+        parser.add_argument('--no-color', help='Disable all highlights', dest='color', action='store_false',
+                            default=True)
         parser.add_argument('-o', '--offset', type=int, default=0,
                             help="If you have 2 NICs with 4 queues and 1 socket with 8 cpus, you may be want "
                                  "distribution like this: eth0: [0, 1, 2, 3]; eth1: [4, 5, 6, 7]; "
                                  "so run: rss-ladder-test eth0; rss-ladder-test --offset=4 eth1")
         parser.add_argument('-c', '--cpus', help='Explicitly define list of CPUs for binding NICs queues', type=int,
                             nargs='+')
         parser.add_argument('dev', type=str)
-        parser.add_argument('socket', nargs='?', type=int, default=0)
+        parser.add_argument('socket', nargs='?', type=int)
         return parser.parse_args()
 
     def queue_postfix_extract(self, line):
@@ -96,13 +112,40 @@ def smp_affinity_list_make(self, postfix):
             if queue_name:
                 yield any2int(line.split()[0]), queue_name[0], rss_cpus.pop()
 
+    def dev_colorize(self):
+        """
+        :return: highlighted by NUMA-node name of the device
+        """
+        if not self.numa or not self.options.color:
+            return self.options.dev
+        color = COLORS_NODE.get(self.numa.devices.get(self.options.dev))
+        return wrap(self.options.dev, color)
+
+    def cpu_colorize(self, cpu):
+        """
+        :param cpu: cpu number (0)
+        :return: highlighted by NUMA-node cpu number.
+        """
+        if not self.numa or not self.options.color:
+            return cpu
+        return wrap(cpu, cpu_color(cpu, numa=self.numa))
+
     def smp_affinity_list_apply(self, smp_affinity):
         """
         '* 4' is in case of NIC has more queues than socket has CPUs
         :param smp_affinity: list of tuples(irq, queue_name, socket)
         """
-        for irq, queue_name, socket_cpu in smp_affinity:
-            print_("  - {0}: irq {1} {2} -> {3}".format(self.options.dev, irq, queue_name, socket_cpu))
+        affinity = list(smp_affinity)
+        cpus = [socket_cpu for irq, queue, socket_cpu in affinity]
+        if len(set(cpus)) != len(cpus):
+            warning = "WARNING: some CPUs process multiple queues, consider reduce queue count for this network device"
+            if self.options.color:
+                print_(wrap(warning, YELLOW))
+            else:
+                print_(warning)
+        for irq, queue_name, socket_cpu in affinity:
+            print_("  - {0}: irq {1} {2} -> {3}".format(
+                self.dev_colorize(), irq, queue_name, self.cpu_colorize(socket_cpu)))
             if self.options.dry_run:
                 continue
             filename = "/proc/irq/{0}/smp_affinity_list".format(irq)
diff --git a/netutils_linux_tuning/rss_ladder_test.py b/netutils_linux_tuning/rss_ladder_test.py
@@ -7,8 +7,7 @@
 class RSSLadderTests(TestCase):
 
     def setUp(self):
-        self.rss = RSSLadder()
-        self.rss.options.dev = 'eth0'
+        self.rss = RSSLadder(argv=['--dry-run', 'eth0'])
 
     def test_queue_postfix_extract(self):
         line = "31312 0 0 0 blabla eth0-TxRx-0"
diff --git a/netutils_linux_tuning/rx_buffers.py b/netutils_linux_tuning/rx_buffers.py
@@ -27,14 +27,14 @@ def __str__(self):
 
     def investigate(self):
         """ get maximum and current rx ring buffers values via ethtool """
-        def extract_value(s):
-            return int(s.strip('RX:\t\n'))
+        def extract_value(line):
+            return int(line.strip('RX:\t\n'))
 
         # We need to be sure that on RHEL system we don't automatically tune buffers
         # that have been already manually tuned. In non RHEL system we skip this checks.
-        ns = '/etc/sysconfig/network-scripts/'
-        if path.exists(ns):  # don't check ifcfg on non RHEL-based systems.
-            config_file = path.join(ns, 'ifcfg-' + self.dev)
+        network_scripts = '/etc/sysconfig/network-scripts/'
+        if path.exists(network_scripts):  # don't check ifcfg on non RHEL-based systems.
+            config_file = path.join(network_scripts, 'ifcfg-' + self.dev)
             if path.exists(config_file):
                 with open(config_file) as config:
                     if any(line for line in config.readlines() if 'ETHTOOL_OPTS' in line):
diff --git a/tests/rss-ladder-test b/tests/rss-ladder-test
@@ -12,7 +12,7 @@ run_test() {
 	cp $testdata/$name/interrupts ./interrupts || return 1
 	cp $testdata/$name/lscpu_output ./lscpu_output || return 2
 	read -a params < $testdata/$name/params
-	rss-ladder --dry-run --test-dir $testdata/$name "${params[@]}" > ./output || return 3
+	rss-ladder --no-color --dry-run --test-dir $testdata/$name "${params[@]}" > ./output || return 3
 	cmp -s $testdata/$name/expected output || rc=$?
 	if [ "$rc" = '0' ]; then
 		echo OK
diff --git a/tests/rss-ladder.tests/ixgbe.E5645/expected b/tests/rss-ladder.tests/ixgbe.E5645/expected
@@ -1,4 +1,5 @@
 - distribute interrupts of eth1 (-TxRx-) on socket 1
+WARNING: some CPUs process multiple queues, consider reduce queue count for this network device
   - eth1: irq 75 eth1-TxRx-0 -> 6
   - eth1: irq 76 eth1-TxRx-1 -> 7
   - eth1: irq 77 eth1-TxRx-2 -> 8
diff --git a/utils/detect_virt.py b/utils/detect_virt.py
