Merge pull request #366 from martin-belanger/various-fixes

Various fixes

staslib/service.py

@@ -873,6 +873,6 @@ def _nvme_cli_interop(self, udev_obj):
         options = r'\x09'.join(
             [fr'{option}\x3d{value}' for option, value in cnf if value not in (None, 'none', 'None', '')]
         )
-        logging.info('Invoking: systemctl start nvmf-connect@%s.service', options)
-        cmd = [defs.SYSTEMCTL, '--quiet', '--no-block', 'start', fr'nvmf-connect@{options}.service']
+        logging.info('Invoking: systemctl restart nvmf-connect@%s.service', options)
+        cmd = [defs.SYSTEMCTL, '--quiet', '--no-block', 'restart', fr'nvmf-connect@{options}.service']
         subprocess.run(cmd, check=False)

staslib/trid.py

@@ -61,7 +61,6 @@ def __init__(self, cid: dict):
         self._host_traddr = cid.get('host-traddr', '')
         self._host_iface = '' if conf.SvcConf().ignore_iface else cid.get('host-iface', '')
         self._subsysnqn = cid.get('subsysnqn', '')
-        self._shortkey = (self._transport, self._traddr, self._trsvcid, self._subsysnqn, self._host_traddr)
         self._key = (self._transport, self._traddr, self._trsvcid, self._subsysnqn, self._host_traddr, self._host_iface)
         self._hash = int.from_bytes(
             hashlib.md5(''.join(self._key).encode('utf-8')).digest(), 'big'
@@ -121,22 +120,10 @@ def __repr__(self):
         return self._id
 
     def __eq__(self, other):
-        if not isinstance(other, self.__class__):
-            return False
-
-        if self._host_iface and other._host_iface:
-            return self._key == other._key
-
-        return self._shortkey == other._shortkey
+        return isinstance(other, self.__class__) and self._key == other._key
 
     def __ne__(self, other):
-        if not isinstance(other, self.__class__):
-            return True
-
-        if self._host_iface and other._host_iface:
-            return self._key != other._key
-
-        return self._shortkey != other._shortkey
+        return not isinstance(other, self.__class__) or self._key != other._key
 
     def __hash__(self):
         return self._hash

staslib/udev.py

@@ -154,7 +154,7 @@ def is_ioc_device(device):
         return False
 
     @staticmethod
-    def _cid_matches_tid_legacy(cid, tid):  # pylint: disable=too-many-return-statements
+    def _cid_matches_tcp_tid_legacy(cid, tid):  # pylint: disable=too-many-return-statements
         '''On kernels older than 6.1, the src_addr parameter is not available
         from the sysfs. Therefore, we need to infer a match based on other
         parameters. And there are a few cases where we're simply not sure
@@ -172,7 +172,7 @@ def _cid_matches_tid_legacy(cid, tid):  # pylint: disable=too-many-return-statem
                 # was made. In this case, we can only declare a match if both
                 # tid.host_iface and tid.host_traddr are undefined as well.
                 logging.debug(
-                    'Udev._cid_matches_tid_legacy() - cid=%s, tid=%s - Not enough info. Assume "match" but this could be wrong.'
+                    'Udev._cid_matches_tcp_tid_legacy() - cid=%s, tid=%s - Not enough info. Assume "match" but this could be wrong.'
                 )
                 return True
 
@@ -235,7 +235,7 @@ def _cid_matches_tid_legacy(cid, tid):  # pylint: disable=too-many-return-statem
         return iputil.get_ipaddress_obj(tid.host_traddr) == host_traddr
 
     @staticmethod
-    def _cid_matches_tid(cid, tid):
+    def _cid_matches_tid(cid, tid):  #  pylint: disable=too-many-return-statements
         '''Check if existing controller's cid matches candidate controller's tid.
         @param cid: The Connection ID of an existing controller (from the sysfs).
         @param tid: The Transport ID of a candidate controller.
@@ -247,39 +247,73 @@ def _cid_matches_tid(cid, tid):
         tid.trsvcid, and tid.subsysnqn are not identical to those of the cid.
         These 4 parameters are mandatory for a match.
 
-        With regards to the candidate's tid.host_traddr and tid.host_iface, if
-        those are defined but do not match the existing cid.host_traddr and
-        cid.host_iface, we may still be able to find a match by taking the
-        existing cid.src_addr into consideration since that parameter identifies
-        the actual source address of the connection and therefore can be used
-        to infer the interface of the connection. However, the cid.src_addr can
-        only be read from the sysfs starting with kernel 6.1.
+        The tid.host_traddr and tid.host_iface depend on the transport type.
+        These parameters may not apply or have a different syntax/meaning
+        depending on the transport type.
+
+        For TCP only:
+            With regards to the candidate's tid.host_traddr and tid.host_iface,
+            if those are defined but do not match the existing cid.host_traddr
+            and cid.host_iface, we may still be able to find a match by taking
+            the existing cid.src_addr into consideration since that parameter
+            identifies the actual source address of the connection and therefore
+            can be used to infer the interface of the connection. However, the
+            cid.src_addr can only be read from the sysfs starting with kernel
+            6.1.
         '''
+        if tid.transport in ('tcp', 'rdma'):
+            # Need to convert to ipaddress objects to properly
+            # handle all variations of IPv6 addresses.
+            cid_traddr = iputil.get_ipaddress_obj(cid['traddr'], ipv4_mapped_convert=True)
+            tid_traddr = iputil.get_ipaddress_obj(tid.traddr, ipv4_mapped_convert=True)
+        else:
+            cid_traddr = cid['traddr']
+            tid_traddr = tid.traddr
+
+        # 'transport', 'traddr', 'trsvcid', and 'subsysnqn' must exactly match.
         if (
             cid['transport'] != tid.transport
-            or cid['traddr'] != tid.traddr
+            or cid_traddr != tid_traddr
             or cid['trsvcid'] != tid.trsvcid
             or cid['subsysnqn'] != tid.subsysnqn
         ):
             return False
 
-        src_addr = iputil.get_ipaddress_obj(cid['src-addr'], ipv4_mapped_convert=True)
-        if not src_addr:
-            # For legacy kernels (i.e. older than 6.1), the existing cid.src_addr
-            # is always undefined. We need to use advanced logic to determine
-            # whether cid and tid match.
-            return Udev._cid_matches_tid_legacy(cid, tid)
-
-        # The existing controller's cid.src_addr is always defined for kernel
-        # 6.1 and later. We can use the existing controller's cid.src_addr to
-        # find the interface on which the connection was made and therefore
-        # match it to the candidate's tid.host_iface. And the cid.src_addr
-        # can also be used to match the candidate's tid.host_traddr.
-        if tid.host_traddr and src_addr != iputil.get_ipaddress_obj(tid.host_traddr):
-            return False
+        # We need to know the type of transport to compare 'host-traddr' and
+        # 'host-iface'. These parameters don't apply to all transport types
+        # and may have a different meaning/syntax.
+        if tid.transport == 'tcp':
+            src_addr = iputil.get_ipaddress_obj(cid['src-addr'], ipv4_mapped_convert=True)
+            if not src_addr:
+                # For legacy kernels (i.e. older than 6.1), the existing cid.src_addr
+                # is always undefined. We need to use advanced logic to determine
+                # whether cid and tid match.
+                return Udev._cid_matches_tcp_tid_legacy(cid, tid)
+
+            # The existing controller's cid.src_addr is always defined for kernel
+            # 6.1 and later. We can use the existing controller's cid.src_addr to
+            # find the interface on which the connection was made and therefore
+            # match it to the candidate's tid.host_iface. And the cid.src_addr
+            # can also be used to match the candidate's tid.host_traddr.
+            if tid.host_traddr and src_addr != iputil.get_ipaddress_obj(tid.host_traddr):
+                return False
 
-        if tid.host_iface and tid.host_iface != iputil.get_interface(str(src_addr)):
-            return False
+            # host-iface is an optional tcp-only parameter.
+            if tid.host_iface and tid.host_iface != iputil.get_interface(str(src_addr)):
+                return False
+
+        elif tid.transport == 'fc':
+            # host-traddr is mandatory for FC.
+            if tid.host_traddr != cid['host-traddr']:
+                return False
+
+        elif tid.transport == 'rdma':
+            # host-traddr is optional for RDMA and is expressed as an IP address.
+            if tid.host_traddr:
+                tid_host_traddr = iputil.get_ipaddress_obj(tid.host_traddr, ipv4_mapped_convert=True)
+                cid_host_traddr = iputil.get_ipaddress_obj(cid['host-traddr'], ipv4_mapped_convert=True)
+                if tid_host_traddr != cid_host_traddr:
+                    return False
 
         return True
 
@@ -435,12 +469,13 @@ def get_key_from_attr(device, attr, key, delim=','):
     def get_tid(device):
         '''@brief return the Transport ID associated with a udev device'''
         cid = Udev.get_cid(device)
-        src_addr = cid['src-addr']
-        if not cid['host-iface'] and src_addr:
-            # We'll try to find the interface from the source address on
-            # the connection. Only available if kernel exposes the source
-            # address (src_addr) in the "address" attribute.
-            cid['host-iface'] = iputil.get_interface(src_addr)
+        if cid['transport'] == 'tcp':
+            src_addr = cid['src-addr']
+            if not cid['host-iface'] and src_addr:
+                # We'll try to find the interface from the source address on
+                # the connection. Only available if kernel exposes the source
+                # address (src_addr) in the "address" attribute.
+                cid['host-iface'] = iputil.get_interface(src_addr)
 
         return trid.TID(cid)
 

test/test-iputil.py

@@ -42,7 +42,9 @@ def test_get_interface(self):
 
     def test_mac2iface(self):
         for iface in self.ifaces:
-            self.assertEqual(iface['ifname'], iputil.mac2iface(iface['address']))
+            address = iface.get('address', None)
+            if address:
+                self.assertEqual(iface['ifname'], iputil.mac2iface(address))
 
     def test_remove_invalid_addresses(self):
         good_tcp = trid.TID({'transport': 'tcp', 'traddr': '1.1.1.1', 'subsysnqn': '', 'trsvcid': '8009'})

utils/nvmet/nvmet.conf

@@ -6,10 +6,10 @@
     'ports': [
         {
             'id': 1,
-            #'adrfam': 'ipv6',
-            #'traddr': '::',
-            'adrfam': 'ipv4',
-            'traddr': '0.0.0.0',
+            'adrfam': 'ipv6',
+            'traddr': '::',
+            #'adrfam': 'ipv4',
+            #'traddr': '0.0.0.0',
             'trsvcid': 8009,
             'trtype': 'tcp',
         }