Fix problems using IPv6 addresses in connect and discover.

Fixes ceph#903 Signed-off-by: Gil Bregman <[email protected]>
gbregman · Oct 15, 2024 · 2cce56e · 2cce56e
1 parent ccb8af9
commit 2cce56e
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 7 deletions.
diff --git a/control/discovery.py b/control/discovery.py
@@ -1105,7 +1105,21 @@ def update_log_level(self):
     def start_service(self):
         """Enable listening on the server side."""
 
-        self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        family = socket.AF_INET
+        try:
+            addr_info = socket.getaddrinfo(self.discovery_addr, None, 0, socket.SOCK_STREAM)
+            if len(addr_info) == 1:
+                family = addr_info[0][0]
+            else:
+                self.logger.warning(f"Can't get information for address {self.discovery_addr}")
+                if ":" in self.discovery_addr:
+                    family = socket.AF_INET6
+        except Exception:
+            self.logger.exception(f"error trying to get the info for address {self.discovery_addr}")
+            if ":" in self.discovery_addr:
+                family = socket.AF_INET6
+
+        self.sock = socket.socket(family, socket.SOCK_STREAM)
         self.sock.bind((self.discovery_addr, int(self.discovery_port)))
         self.sock.listen(MAX_CONNECTION)
         self.sock.setblocking(False)

diff --git a/control/grpc.py b/control/grpc.py
@@ -2755,6 +2755,8 @@ def create_listener_safe(self, request, context):
                          f" TCP {adrfam} listener for {request.nqn} at"
                          f" {request.traddr}:{request.trsvcid}, secure: {request.secure}, context: {context}{peer_msg}")
 
+        traddr = GatewayUtils.unescape_address_if_ipv6(request.traddr, adrfam)
+
         if GatewayUtils.is_discovery_nqn(request.nqn):
             errmsg=f"{create_listener_error_prefix}: Can't create a listener for a discovery subsystem"
             self.logger.error(f"{errmsg}")
@@ -2773,7 +2775,7 @@ def create_listener_safe(self, request, context):
         add_listener_args = {}
         add_listener_args["nqn"] = request.nqn
         add_listener_args["trtype"] = "TCP"
-        add_listener_args["traddr"] = request.traddr
+        add_listener_args["traddr"] = traddr
         add_listener_args["trsvcid"] = str(request.trsvcid)
         add_listener_args["adrfam"] = adrfam
         if request.secure:
@@ -2783,13 +2785,13 @@ def create_listener_safe(self, request, context):
         with omap_lock:
             try:
                 if request.host_name == self.host_name:
-                    if (adrfam, request.traddr, request.trsvcid, False) in self.subsystem_listeners[request.nqn] or (adrfam, request.traddr, request.trsvcid, True) in self.subsystem_listeners[request.nqn]:
+                    if (adrfam, traddr, request.trsvcid, False) in self.subsystem_listeners[request.nqn] or (adrfam, traddr, request.trsvcid, True) in self.subsystem_listeners[request.nqn]:
                         self.logger.error(f"{request.nqn} already listens on address {request.traddr}:{request.trsvcid}")
                         return pb2.req_status(status=errno.EEXIST,
                                   error_message=f"{create_listener_error_prefix}: Subsystem already listens on this address")
                     ret = rpc_nvmf.nvmf_subsystem_add_listener(self.spdk_rpc_client, **add_listener_args)
                     self.logger.debug(f"create_listener: {ret}")
-                    self.subsystem_listeners[request.nqn].add((adrfam, request.traddr, request.trsvcid, request.secure))
+                    self.subsystem_listeners[request.nqn].add((adrfam, traddr, request.trsvcid, request.secure))
                 else:
                     if context:
                         errmsg=f"{create_listener_error_prefix}: Gateway's host name must match current host ({self.host_name})"
@@ -2822,7 +2824,7 @@ def create_listener_safe(self, request, context):
                   nqn=request.nqn,
                   ana_state=_ana_state,
                   trtype="TCP",
-                  traddr=request.traddr,
+                  traddr=traddr,
                   trsvcid=str(request.trsvcid),
                   adrfam=adrfam)
                 self.logger.debug(f"create_listener nvmf_subsystem_listener_set_ana_state response {ret=}")
@@ -2840,7 +2842,7 @@ def create_listener_safe(self, request, context):
                               nqn=request.nqn,
                               ana_state=_ana_state,
                               trtype="TCP",
-                              traddr=request.traddr,
+                              traddr=traddr,
                               trsvcid=str(request.trsvcid),
                               adrfam=adrfam,
                               anagrpid=ana_grp )

diff --git a/control/utils.py b/control/utils.py
@@ -53,12 +53,18 @@ class GatewayUtils:
     DISCOVERY_NQN = "nqn.2014-08.org.nvmexpress.discovery"
 
     # We need to enclose IPv6 addresses in brackets before concatenating a colon and port number to it
-    def escape_address_if_ipv6(addr) -> str:
+    def escape_address_if_ipv6(addr : str) -> str:
         ret_addr = addr
         if ":" in addr and not addr.strip().startswith("["):
             ret_addr = f"[{addr}]"
         return ret_addr
 
+    def unescape_address_if_ipv6(addr : str, adrfam : str) -> str:
+        ret_addr = addr.strip()
+        if adrfam.lower() == "ipv6":
+            ret_addr = ret_addr.removeprefix("[").removesuffix("]")
+        return ret_addr
+
     def is_discovery_nqn(nqn) -> bool:
         return nqn == GatewayUtils.DISCOVERY_NQN
 

diff --git a/tests/ha/demo_test.sh b/tests/ha/demo_test.sh
@@ -78,6 +78,9 @@ function demo_bdevperf_unsecured()
     [[ `echo $conns | jq -r '.connections[0].trtype'` == "TCP" ]]
     [[ `echo $conns | jq -r '.connections[0].connected'` == "true" ]]
     [[ `echo $conns | jq -r '.connections[0].qpairs_count'` == "1" ]]
+    [[ `echo $conns | jq -r '.connections[0].secure'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[0].use_psk'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[0].use_dhchap'` == "false" ]]
     [[ `echo $conns | jq -r '.connections[1]'` == "null" ]]
 
     echo "ℹ️  bdevperf perform_tests"
@@ -89,11 +92,65 @@ function demo_bdevperf_unsecured()
     echo "ℹ️  bdevperf detach controller"
     make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_detach_controller Nvme0"
 
+    echo "ℹ️  verify empty connection list"
+    conns=$(cephnvmf_func --output stdio --format json connection list --subsystem $NQN)
+    [[ `echo $conns | jq -r '.status'` == "0" ]]
+    [[ `echo $conns | jq -r '.subsystem_nqn'` == "${NQN}" ]]
+    [[ `echo $conns | jq -r '.connections[0]'` == "null" ]]
+
     echo "ℹ️  bdevperf tcp connect ip: $NVMEOF_IP_ADDRESS port: $NVMEOF_IO_PORT nqn: $NQN, host in namespace netmask"
     localhostnqn=`cat /etc/nvme/hostnqn`
     devs=`make exec -s SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme0 -t tcp -a $NVMEOF_IP_ADDRESS -s $NVMEOF_IO_PORT -f ipv4 -n $NQN -q $localhostnqn -l -1 -o 10"`
     [[ "$devs" == "Nvme0n1 Nvme0n2" ]]
 
+    echo "ℹ️  verify connection list"
+    conns=$(cephnvmf_func --output stdio --format json connection list --subsystem $NQN)
+    [[ `echo $conns | jq -r '.status'` == "0" ]]
+    [[ `echo $conns | jq -r '.subsystem_nqn'` == "${NQN}" ]]
+    [[ `echo $conns | jq -r '.connections[0].nqn'` == "${localhostnqn}" ]]
+    [[ `echo $conns | jq -r '.connections[0].trsvcid'` == "${NVMEOF_IO_PORT}" ]]
+    [[ `echo $conns | jq -r '.connections[0].traddr'` == "${NVMEOF_IP_ADDRESS}" ]]
+    [[ `echo $conns | jq -r '.connections[0].adrfam'` == "ipv4" ]]
+    [[ `echo $conns | jq -r '.connections[0].trtype'` == "TCP" ]]
+    [[ `echo $conns | jq -r '.connections[0].connected'` == "true" ]]
+    [[ `echo $conns | jq -r '.connections[0].qpairs_count'` == "1" ]]
+    [[ `echo $conns | jq -r '.connections[0].secure'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[0].use_psk'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[0].use_dhchap'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[1]'` == "null" ]]
+
+    echo "ℹ️  bdevperf detach controller"
+    make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_detach_controller Nvme0"
+
+    echo "ℹ️  bdevperf tcp connect ip: $NVMEOF_IPV6_ADDRESS port: $NVMEOF_IO_PORT nqn: $NQN, using IPv6"
+    make exec -s SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_attach_controller -b Nvme1 -t tcp -a $NVMEOF_IPV6_ADDRESS -s $NVMEOF_IO_PORT -f ipv6 -n $NQN -q $localhostnqn -l -1 -o 10"
+
+    echo "ℹ️  verify connection list with IPv6"
+    conns=$(cephnvmf_func --output stdio --format json connection list --subsystem $NQN)
+    echo $conns
+    [[ `echo $conns | jq -r '.status'` == "0" ]]
+    [[ `echo $conns | jq -r '.subsystem_nqn'` == "${NQN}" ]]
+    [[ `echo $conns | jq -r '.connections[0].nqn'` == "${localhostnqn}" ]]
+    [[ `echo $conns | jq -r '.connections[0].trsvcid'` == "${NVMEOF_IO_PORT}" ]]
+    [[ `echo $conns | jq -r '.connections[0].traddr'` == "${NVMEOF_IPV6_ADDRESS}" ]]
+    [[ `echo $conns | jq -r '.connections[0].adrfam'` == "ipv6" ]]
+    [[ `echo $conns | jq -r '.connections[0].trtype'` == "TCP" ]]
+    [[ `echo $conns | jq -r '.connections[0].connected'` == "true" ]]
+    [[ `echo $conns | jq -r '.connections[0].qpairs_count'` == "1" ]]
+    [[ `echo $conns | jq -r '.connections[0].secure'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[0].use_psk'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[0].use_dhchap'` == "false" ]]
+    [[ `echo $conns | jq -r '.connections[1]'` == "null" ]]
+
+    echo "ℹ️  bdevperf detach controller"
+    make exec SVC=bdevperf OPTS=-T CMD="$rpc -v -s $BDEVPERF_SOCKET bdev_nvme_detach_controller Nvme1"
+
+    echo "ℹ️  verify empty connection list"
+    conns=$(cephnvmf_func --output stdio --format json connection list --subsystem $NQN)
+    [[ `echo $conns | jq -r '.status'` == "0" ]]
+    [[ `echo $conns | jq -r '.subsystem_nqn'` == "${NQN}" ]]
+    [[ `echo $conns | jq -r '.connections[0]'` == "null" ]]
+
     return $?
 }