From 9ed56bb019ff2b3dac8b17e47b8e233bb5fbf7a7 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Fri, 21 Apr 2023 13:43:40 -0400 Subject: [PATCH] nbft: Add NbftConf() object to retrieve and cache NBFT data Signed-off-by: Martin Belanger --- coverage.sh.in | 1 + staslib/conf.py | 84 +++++++++++++++++++++++++++++++++++++++++- staslib/iputil.py | 65 +++++++------------------------- staslib/meson.build | 1 + staslib/nbft.py | 28 ++++++++++++++ staslib/service.py | 6 +-- staslib/stas.py | 48 +++++++++++++++++------- test/meson.build | 1 + test/test-iputil.py | 4 +- test/test-nbft.py | 6 +-- test/test-nbft_conf.py | 46 +++++++++++++++++++++++ 11 files changed, 217 insertions(+), 73 deletions(-) create mode 100644 staslib/nbft.py create mode 100755 test/test-nbft_conf.py diff --git a/coverage.sh.in b/coverage.sh.in index 8fd5898..9d225e6 100755 --- a/coverage.sh.in +++ b/coverage.sh.in @@ -659,6 +659,7 @@ run_unit_test ${TEST_DIR}/test-gtimer.py run_unit_test ${TEST_DIR}/test-iputil.py run_unit_test ${TEST_DIR}/test-log.py run_unit_test ${TEST_DIR}/test-nbft.py +run_unit_test ${TEST_DIR}/test-nbft_conf.py run_unit_test sudo ${TEST_DIR}/test-nvme_options.py # Test both with super user... run_unit_test ${TEST_DIR}/test-nvme_options.py # ... and with regular user run_unit_test ${TEST_DIR}/test-service.py diff --git a/staslib/conf.py b/staslib/conf.py index 8b6c3d7..b4b55ac 100644 --- a/staslib/conf.py +++ b/staslib/conf.py @@ -14,7 +14,8 @@ import logging import functools import configparser -from staslib import defs, singleton, timeparse +from urllib.parse import urlparse +from staslib import defs, iputil, nbft, singleton, timeparse __TOKEN_RE = re.compile(r'\s*;\s*') __OPTION_RE = re.compile(r'\s*=\s*') @@ -702,3 +703,84 @@ def dhchap_hostkey_supp(self): def dhchap_ctrlkey_supp(self): '''This option allows specifying the controller DHCHAP key used for authentication.''' return self._supported_options['dhchap_ctrl_secret'] + + +# ****************************************************************************** +class NbftConf(metaclass=singleton.Singleton): # pylint: disable=too-few-public-methods + '''Read and cache configuration file.''' + + def __init__(self, root_dir=defs.NBFT_SYSFS_PATH): + self._disc_ctrls = [] + self._subs_ctrls = [] + + for data in nbft.get_nbft_files(root_dir).values(): + hfis = data.get('hfi', []) + discovery = data.get('discovery', []) + subsystem = data.get('subsystem', []) + + self._disc_ctrls.extend(NbftConf.__nbft_disc_to_cids(discovery, hfis)) + self._subs_ctrls.extend(NbftConf.__nbft_subs_to_cids(subsystem, hfis)) + + dcs = property(lambda self: self._disc_ctrls) + iocs = property(lambda self: self._subs_ctrls) + + @staticmethod + def __nbft_disc_to_cids(discovery, hfis): + cids = [] + + for ctrl in discovery: + cid = NbftConf.__uri2cid(ctrl['uri']) + cid['subsysnqn'] = ctrl['nqn'] + + host_iface = NbftConf.__get_host_iface(ctrl.get('hfi_index'), hfis) + if host_iface: + cid['host-iface'] = host_iface + + cids.append(cid) + + return cids + + @staticmethod + def __nbft_subs_to_cids(subsystem, hfis): + cids = [] + + for ctrl in subsystem: + cid = { + 'transport': ctrl['trtype'], + 'traddr': ctrl['traddr'], + 'trsvcid': ctrl['trsvcid'], + 'subsysnqn': ctrl['subsys_nqn'], + 'hdr-digest': ctrl['pdu_header_digest_required'], + 'data-digest': ctrl['data_digest_required'], + } + + indexes = ctrl.get('hfi_indexes') + if isinstance(indexes, list) and len(indexes) > 0: + host_iface = NbftConf.__get_host_iface(indexes[0], hfis) + if host_iface: + cid['host-iface'] = host_iface + + cids.append(cid) + + return cids + + @staticmethod + def __get_host_iface(indx, hfis): + if indx is None or indx >= len(hfis): + return None + + mac = hfis[indx].get('mac_addr') + if mac is None: + return None + + return iputil.mac2iface(mac) + + @staticmethod + def __uri2cid(uri: str): + '''Convert a URI of the form "nvme+tcp://100.71.103.50:8009/" to a Controller ID''' + obj = urlparse(uri) + return { + 'transport': obj.scheme.split('+')[1], + 'traddr': obj.hostname, + 'trsvcid': str(obj.port), + } diff --git a/staslib/iputil.py b/staslib/iputil.py index 7e04847..8b4a141 100644 --- a/staslib/iputil.py +++ b/staslib/iputil.py @@ -10,9 +10,7 @@ import struct import socket -import logging import ipaddress -from staslib import conf RTM_BASE = 16 RTM_GETLINK = 18 @@ -21,26 +19,29 @@ NLM_F_REQUEST = 0x01 NLM_F_ROOT = 0x100 NLMSG_DONE = 3 -IFLA_ADDRESS = 1 -NLMSGHDR_SZ = 16 +NLMSG_HDRLEN = 16 IFADDRMSG_SZ = 8 IFINFOMSG_SZ = 16 -RTATTR_SZ = 4 ARPHRD_ETHER = 1 ARPHRD_LOOPBACK = 772 +NLMSG_LENGTH = lambda msg_len: msg_len + NLMSG_HDRLEN # pylint: disable=unnecessary-lambda-assignment + +RTATTR_SZ = 4 +RTA_ALIGN = lambda length: ((length + 3) & ~3) # pylint: disable=unnecessary-lambda-assignment +IFLA_ADDRESS = 1 def _nlmsghdr(nlmsg_type, nlmsg_flags, nlmsg_seq, nlmsg_pid, msg_len: int): '''Implement this C struct: struct nlmsghdr { - __u32 nlmsg_len; /* Length of message including header, but excluding length of nlmsg_len itself */ + __u32 nlmsg_len; /* Length of message including header */ __u16 nlmsg_type; /* Message content */ __u16 nlmsg_flags; /* Additional flags */ __u32 nlmsg_seq; /* Sequence number */ __u32 nlmsg_pid; /* Sending process port ID */ }; ''' - return struct.pack('= len(nlmsg): nlmsg += sock.recv(8192) - nlmsghdr = nlmsg[nlmsg_idx : nlmsg_idx + NLMSGHDR_SZ] + nlmsghdr = nlmsg[nlmsg_idx : nlmsg_idx + NLMSG_HDRLEN] nlmsg_len, nlmsg_type, _, _, _ = struct.unpack('= len(nlmsg): nlmsg += sock.recv(8192) - nlmsghdr = nlmsg[nlmsg_idx : nlmsg_idx + NLMSGHDR_SZ] + nlmsghdr = nlmsg[nlmsg_idx : nlmsg_idx + NLMSG_HDRLEN] nlmsg_len, nlmsg_type, _, _, _ = struct.unpack(' +# +'''Code used to access the NVMe Boot Firmware Tables''' + +import os +import glob +import logging +from libnvme import nvme +from staslib import defs + + +def get_nbft_files(root_dir=defs.NBFT_SYSFS_PATH): + """Return a dictionary containing the NBFT data for all the NBFT binary files located in @root_dir""" + if not defs.HAS_NBFT_SUPPORT: + logging.warning( + "libnvme-%s does not have NBFT support. Please upgrade libnvme.", + defs.LIBNVME_VERSION, + ) + return {} + + pathname = os.path.join(root_dir, defs.NBFT_SYSFS_FILENAME) + return {fname: nvme.nbft_get(fname) for fname in glob.iglob(pathname=pathname)} # pylint: disable=no-member diff --git a/staslib/service.py b/staslib/service.py index ce4769d..c70619b 100644 --- a/staslib/service.py +++ b/staslib/service.py @@ -20,7 +20,7 @@ from gi.repository import GLib from systemd.daemon import notify as sd_notify -from staslib import avahi, conf, ctrl, defs, gutil, iputil, stas, timeparse, trid, udev +from staslib import avahi, conf, ctrl, defs, gutil, stas, timeparse, trid, udev # ****************************************************************************** @@ -402,7 +402,7 @@ def _config_ctrls_finish(self, configured_ctrl_list: list): # pylint: disable=t logging.debug('Stac._config_ctrls_finish() - discovered_ctrl_list = %s', discovered_ctrl_list) controllers = stas.remove_excluded(configured_ctrl_list + discovered_ctrl_list) - controllers = iputil.remove_invalid_addresses(controllers) + controllers = stas.remove_invalid_addresses(controllers) new_controller_tids = set(controllers) cur_controller_tids = set(self._controllers.keys()) @@ -752,7 +752,7 @@ def _config_ctrls_finish(self, configured_ctrl_list: list): all_ctrls = configured_ctrl_list + discovered_ctrl_list + referral_ctrl_list controllers = stas.remove_excluded(all_ctrls) - controllers = iputil.remove_invalid_addresses(controllers) + controllers = stas.remove_invalid_addresses(controllers) new_controller_tids = set(controllers) cur_controller_tids = set(self._controllers.keys()) diff --git a/staslib/stas.py b/staslib/stas.py index f0f7fa6..a0cb089 100644 --- a/staslib/stas.py +++ b/staslib/stas.py @@ -12,15 +12,13 @@ import os import sys import abc -import glob import signal import pickle import logging import dasbus.connection -from libnvme import nvme from gi.repository import Gio, GLib from systemd.daemon import notify as sd_notify -from staslib import conf, defs, gutil, log, trid +from staslib import conf, defs, gutil, iputil, log, trid try: # Python 3.9 or later @@ -89,17 +87,41 @@ def check_if_allowed_to_continue(): # ****************************************************************************** -def get_nbft_files(root_dir=defs.NBFT_SYSFS_PATH): - """Return a dictionary containing the NBFT data for all the NBFT binary files located in @root_dir""" - if not defs.HAS_NBFT_SUPPORT: - logging.warning( - "libnvme-%s does not have NBFT support. Please upgrade libnvme.", - defs.LIBNVME_VERSION, - ) - return {} +def remove_invalid_addresses(controllers: list): + '''@brief Remove controllers with invalid addresses from the list of controllers. + @param controllers: List of TIDs + ''' + service_conf = conf.SvcConf() + valid_controllers = list() + for controller in controllers: + if controller.transport in ('tcp', 'rdma'): + # Let's make sure that traddr is + # syntactically a valid IPv4 or IPv6 address. + ip = iputil.get_ipaddress_obj(controller.traddr) + if ip is None: + logging.warning('%s IP address is not valid', controller) + continue + + # Let's make sure the address family is enabled. + if ip.version not in service_conf.ip_family: + logging.debug( + '%s ignored because IPv%s is disabled in %s', + controller, + ip.version, + service_conf.conf_file, + ) + continue + + valid_controllers.append(controller) + + elif controller.transport in ('fc', 'loop'): + # At some point, need to validate FC addresses as well... + valid_controllers.append(controller) + + else: + logging.warning('Invalid transport %s', controller.transport) - pathname = os.path.join(root_dir, defs.NBFT_SYSFS_FILENAME) - return {fname: nvme.nbft_get(fname) for fname in glob.iglob(pathname=pathname)} # pylint: disable=no-member + return valid_controllers # ****************************************************************************** diff --git a/test/meson.build b/test/meson.build index 42bba24..1d41dee 100644 --- a/test/meson.build +++ b/test/meson.build @@ -102,6 +102,7 @@ else ['Test KernelVersion', [], [srce_dir / 'test-version.py', ]], ['Test log', ['pyfakefs'], [srce_dir / 'test-log.py', ]], ['Test NBFT', [], [srce_dir / 'test-nbft.py', ]], + ['Test NbftConf', [], [srce_dir / 'test-nbft_conf.py', ]], ['Test NvmeOptions', ['pyfakefs'], [srce_dir / 'test-nvme_options.py', ]], ['Test Service', ['pyfakefs'], [srce_dir / 'test-service.py', ]], ['Test TID', [], [srce_dir / 'test-transport_id.py', ]], diff --git a/test/test-iputil.py b/test/test-iputil.py index 2fe95da..9f65f92 100755 --- a/test/test-iputil.py +++ b/test/test-iputil.py @@ -6,7 +6,7 @@ import unittest import ipaddress import subprocess -from staslib import iputil, log, trid +from staslib import iputil, log, stas, trid IP = shutil.which('ip') @@ -56,7 +56,7 @@ def test_remove_invalid_addresses(self): any_fc, bad_trtype, ] - l2 = iputil.remove_invalid_addresses(l1) + l2 = stas.remove_invalid_addresses(l1) self.assertNotEqual(l1, l2) diff --git a/test/test-nbft.py b/test/test-nbft.py index 0a55c46..b5af744 100755 --- a/test/test-nbft.py +++ b/test/test-nbft.py @@ -1,7 +1,7 @@ #!/usr/bin/python3 import os import unittest -from staslib import defs, stas +from staslib import defs, nbft from libnvme import nvme from argparse import ArgumentParser @@ -89,11 +89,11 @@ def setUp(self): def test_dir_with_nbft_files(self): """Make sure we get expected data when reading from binary NBFT file""" - actual_nbft = stas.get_nbft_files(TEST_DIR) + actual_nbft = nbft.get_nbft_files(TEST_DIR) self.assertEqual(actual_nbft, self.expected_nbft) def test_dir_without_nbft_files(self): - actual_nbft = stas.get_nbft_files("/tmp") + actual_nbft = nbft.get_nbft_files("/tmp") self.assertEqual(actual_nbft, {}) diff --git a/test/test-nbft_conf.py b/test/test-nbft_conf.py new file mode 100755 index 0000000..a91f325 --- /dev/null +++ b/test/test-nbft_conf.py @@ -0,0 +1,46 @@ +#!/usr/bin/python3 +import os +import unittest +from staslib import conf + +TEST_DIR = os.path.dirname(__file__) +EXPECTED_DCS = [ + { + 'subsysnqn': 'nqn.2014-08.org.nvmexpress.discovery', + 'traddr': '100.71.103.50', + 'transport': 'tcp', + 'trsvcid': '8009', + } +] +EXPECTED_IOCS = [ + { + 'data-digest': False, + 'hdr-digest': False, + 'subsysnqn': 'nqn.1988-11.com.dell:powerstore:00:2a64abf1c5b81F6C4549', + 'traddr': '100.71.103.48', + 'transport': 'tcp', + 'trsvcid': '4420', + }, + { + 'data-digest': False, + 'hdr-digest': False, + 'subsysnqn': 'nqn.1988-11.com.dell:powerstore:00:2a64abf1c5b81F6C4549', + 'traddr': '100.71.103.49', + 'transport': 'tcp', + 'trsvcid': '4420', + }, +] + + +class Test(unittest.TestCase): + """Unit tests for class NbftConf""" + + def test_nbft_matches(self): + conf.NbftConf.destroy() # Make sure singleton does not exist + nbft_conf = conf.NbftConf(TEST_DIR) + self.assertEqual(nbft_conf.dcs, EXPECTED_DCS) + self.assertEqual(nbft_conf.iocs, EXPECTED_IOCS) + + +if __name__ == "__main__": + unittest.main()