diff --git a/.pylintrc b/.pylintrc index 7615b86..8867489 100644 --- 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 index fb5a2b2..752a5b0 100644 --- 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 index 37a003a..a4d51f7 100644 --- 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 index da820f2..d1df08d 100755 --- 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 index 6e4a4e4..ac7a784 100644 --- 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 index a451ea3..f5e886c 100644 --- 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 index e2ce118..6033a9f 100644 --- 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 index 20f982b..7755111 100644 --- 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 index d789926..fc6cc8e 100644 --- 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 index 8a1d068..1e4ae4e 100644 --- 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 index 6260fc4..3b2e66b 100644 --- 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 index dd54e7f..707cafa 100644 --- 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,6 +12,7 @@ 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 @@ -18,16 +20,17 @@ 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 - # Popen.stdout in python 3.6 returns + # Popen.stdout: python 2.7 returns , 3.6 returns # read() in both cases return if isinstance(lscpu_output, bytes): lscpu_output = str(lscpu_output) @@ -36,16 +39,29 @@ 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]; " @@ -53,7 +69,7 @@ def parse_options(): 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 index 0ef6fc2..b10b682 100644 --- 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 index e509144..639c60a 100644 --- 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 index 7141b77..8929404 100755 --- 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 index 6e2388c..8caa1f0 100644 --- 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 index 30ae4a0..c978ae4 100644 --- a/utils/detect_virt.py +++ b/utils/detect_virt.py @@ -4,7 +4,7 @@ from six import print_ -dmi_vendor_table = [ +DMI_VENDOR_TABLE = [ "KVM", "QEMU", "VMware", @@ -25,7 +25,7 @@ def detect_vm_dmi(): continue with open(dmi_file) as dmi_fd: dmi_data = dmi_fd.read() - if dmi_data in dmi_vendor_table: + if dmi_data in DMI_VENDOR_TABLE: return dmi_data @@ -49,9 +49,9 @@ def detect_vm_zvm(): return "ZVM" if 'z/VM' in zvm_fd.read() else "KVM" -def detect_by_file(filename, vm): +def detect_by_file(filename, vm_type): if os.path.exists(filename): - return vm + return vm_type def main():