From 2cce56eec41e0c8e069033e90c253c8b60f5dedb Mon Sep 17 00:00:00 2001 From: Gil Bregman Date: Tue, 15 Oct 2024 15:03:53 +0300 Subject: [PATCH] Fix problems using IPv6 addresses in connect and discover. Fixes #903 Signed-off-by: Gil Bregman --- control/discovery.py | 16 +++++++++++- control/grpc.py | 12 +++++---- control/utils.py | 8 +++++- tests/ha/demo_test.sh | 57 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 7 deletions(-) diff --git a/control/discovery.py b/control/discovery.py index 768dd8b80..eae1a5048 100644 --- 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 index 59b17c352..fc838073b 100644 --- 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 index 00aadb78b..08a893037 100644 --- 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 index ebb81ed5f..edf1ec69f 100755 --- 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 $? }