nvme_gw: implicitly create transports when creating listener

Also support transport_{name}_options config param in spdk section
to specify transport specific options (space separated key=val)

Fixes: #12
Signed-off-by: Mykola Golub <mykola.golub@clyso.com>

README.md

@@ -61,12 +61,12 @@ Run the tool with the -h flag to see a list of available commands:
 
 	$ python3 ./nvme_gw_cli.py -h
 	usage: python3 ./nvme_gw_cli.py [-h] [-c CONFIG]
-			{create_bdev,delete_bdev,create_subsystem,delete_subsystem,create_namespace,delete_namespace,add_host,delete_host,create_transport,create_listener,delete_listener,get_subsystems} ...
+			{create_bdev,delete_bdev,create_subsystem,delete_subsystem,create_namespace,delete_namespace,add_host,delete_host,create_listener,delete_listener,get_subsystems} ...
 
 	CLI to manage NVMe gateways
 
 	positional arguments:
-	{create_bdev,delete_bdev,create_subsystem,delete_subsystem,create_namespace,delete_namespace,add_host,delete_host,create_transport,create_listener,delete_listener,get_subsystems}
+	{create_bdev,delete_bdev,create_subsystem,delete_subsystem,create_namespace,delete_namespace,add_host,delete_host,create_listener,delete_listener,get_subsystems}
 
 	optional arguments:
 	-h, --help            			show this help message and exit
@@ -136,9 +136,6 @@ Indicate the location of the keys and certificates in the config file:
 		$ python3 ./nvme_gw_cli.py add_host -n nqn.2016-06.io.spdk:cnode1 -t *
 		INFO:root:Allow open host access to nqn.2016-06.io.spdk:cnode1: True
 		
-		$ python3 ./nvme_gw_cli.py create_transport -t TCP
-		INFO:root:Created TCP transport: True
-		
 		$ python3 ./nvme_gw_cli.py create_listener -n nqn.2016-06.io.spdk:cnode1 -a 192.168.50.4 -s 5001
 		INFO:root:Created nqn.2016-06.io.spdk:cnode1 listener: True
 

nvme_gw_cli.py

@@ -273,17 +273,6 @@ def delete_host(self, args):
         except Exception as error:
             self.logger.error(f"Failed to remove host: \n {error}")
 
-    @cli.cmd([argument("-t", "--trtype", help="Transport type", default="TCP")])
-    def create_transport(self, args):
-        """Sets a transport type."""
-
-        try:
-            create_req = pb2.create_transport_req(trtype=args.trtype)
-            ret = self.stub.nvmf_create_transport(create_req)
-            self.logger.info(f"Created {args.trtype} transport: {ret.status}")
-        except Exception as error:
-            self.logger.error(f"Failed to create transport: \n {error}")
-
     @cli.cmd([
         argument("-n", "--subnqn", help="Subsystem NQN", required=True),
         argument("-a", "--traddr", help="NVMe host IP", required=True),

nvme_gw_persistence.py

@@ -58,14 +58,6 @@ def add_listener(self, subsystem_nqn: str, traddr: str, trsvcid: str,
     def delete_listener(self, subsystem_nqn: str, traddr: str, trsvcid: str):
         pass
 
-    @abstractmethod
-    def set_transport(self, trtype: str, val: str):
-        pass
-
-    @abstractmethod
-    def delete_transport(self, trtype: str):
-        pass
-
     @abstractmethod
     def delete_config(self):
         pass
@@ -100,7 +92,6 @@ class OmapPersistentConfig(PersistentConfig):
     NAMESPACE_PREFIX = "namespace_"
     SUBSYSTEM_PREFIX = "subsystem_"
     HOST_PREFIX = "host_"
-    TRANSPORT_PREFIX = "transport_"
     LISTENER_PREFIX = "listener_"
 
     def __init__(self, nvme_config):
@@ -275,29 +266,6 @@ def _restore_listeners(self, omap_dict, callback):
                 req = json_format.Parse(val, pb2.subsystem_add_listener_req())
                 callback(req)
 
-    def set_transport(self, trtype: str, val: str):
-        """Sets transport type in the persistent config."""
-        key = self.TRANSPORT_PREFIX + trtype
-        self._write_key(key, val)
-
-    def delete_transport(self, trtype: str):
-        """Delete transport type in the persistent config."""
-        key = self.TRANSPORT_PREFIX + trtype
-        self._delete_key(key)
-
-    def get_transport(self, trtype: str):
-        """Read existing transport type from the persistent config."""
-        key = self.TRANSPORT_PREFIX + trtype
-        return self._read_key(key)
-
-    def _restore_transports(self, omap_dict, callback):
-        """Restores a transport from the persistent config."""
-
-        for (key, val) in omap_dict.items():
-            if key.startswith(self.TRANSPORT_PREFIX):
-                req = json_format.Parse(val, pb2.create_transport_req())
-                callback(req)
-
     def _read_key(self, key) -> Optional[str]:
         """Reads single key from persistent config and returns its value."""
 
@@ -343,8 +311,6 @@ def restore(self, callbacks):
             self._restore_namespaces(omap_dict,
                                      callbacks[self.NAMESPACE_PREFIX])
             self._restore_hosts(omap_dict, callbacks[self.HOST_PREFIX])
-            self._restore_transports(omap_dict,
-                                     callbacks[self.TRANSPORT_PREFIX])
             self._restore_listeners(omap_dict, callbacks[self.LISTENER_PREFIX])
             self.version = int(omap_dict[self.OMAP_VERSION_KEY])
             self.logger.info("Restore complete.")

nvme_gw_server.py

@@ -16,6 +16,7 @@
 import subprocess
 import grpc
 from concurrent import futures
+from distutils.util import strtobool
 import nvme_gw_pb2_grpc as pb2_grpc
 import nvme_gw_pb2 as pb2
 import nvme_gw_config
@@ -49,6 +50,7 @@ class GWService(pb2_grpc.NVMEGatewayServicer):
         spdk_rpc: Module methods for SPDK
         spdk_rpc_client: Client of SPDK RPC server
         spdk_process: Subprocess running SPDK NVMEoF target application
+        transports: List (set) of created transports
     """
 
     def __init__(self, nvme_config):
@@ -58,6 +60,7 @@ def __init__(self, nvme_config):
         self.persistent_config = OmapPersistentConfig(nvme_config)
         self.spdk_process = None
         self.server = None
+        self.transports = set()
 
     def __enter__(self):
         return self
@@ -196,15 +199,65 @@ def start_spdk(self):
         except Exception as ex:
             self.logger.error(f"Unable to initialize SPDK: \n {ex}")
             raise
-        return
+
+    def create_transport(self, trtype):
+        if trtype in self.transports:
+            self.logger.debug(
+                f"create_transport: transport {trtype} already exists"
+            )
+            return
+
+        args = {'trtype' : trtype}
+        name = "transport_" + trtype + "_options"
+
+        options = self.nvme_config.get("spdk", name, "")
+
+        SUPPORTED_OPTIONS = {
+            'max_queue_depth' : int,
+            'max_io_qpairs_per_ctrlr' : int,
+            'in_capsule_data_size' : int,
+            'max_io_size' : int,
+            'io_unit_size' : int,
+            'max_aq_depth' : int,
+            'num_shared_buffers' : int,
+            'buf_cache_size' : int,
+            'dif_insert_or_strip' : lambda x : bool(strtobool(x)),
+            'abort_timeout_sec' : int,
+            'zcopy' : lambda x : bool(strtobool(x)),
+            'tgt_name' : str,
+            'acceptor_poll_rate' : int,
+        }
+
+        self.logger.debug(f"create_transport: {trtype} options: {options}")
+
+        for opt in options.split():
+            try:
+                k, v =opt.split('=')
+                if k in SUPPORTED_OPTIONS:
+                    args[k] = SUPPORTED_OPTIONS[k](v)
+                else:
+                    self.logger.error(f"Ignoring unknown {name} option {k}={v}")
+            except ValueError as ex:
+                self.logger.error(
+                    f"Failed to parse spkd {name} ({options}): \n {ex}"
+                )
+        try:
+            status = self.spdk_rpc.nvmf.nvmf_create_transport(
+                self.spdk_rpc_client, **args)
+        except Exception as ex:
+            self.logger.error(
+                f"Create Transport {trtype} returned with error: \n {ex}"
+            )
+            raise
+
+        self.transports.add(trtype)
 
     def restore_config(self):
         callbacks = {
             self.persistent_config.BDEV_PREFIX: self.bdev_rbd_create,
             self.persistent_config.SUBSYSTEM_PREFIX: self.nvmf_create_subsystem,
             self.persistent_config.NAMESPACE_PREFIX: self.nvmf_subsystem_add_ns,
             self.persistent_config.HOST_PREFIX: self.nvmf_subsystem_add_host,
-            self.persistent_config.TRANSPORT_PREFIX: self.nvmf_create_transport,
             self.persistent_config.LISTENER_PREFIX: self.nvmf_subsystem_add_listener
         }
         self.persistent_config.restore(callbacks)
@@ -485,55 +538,15 @@ def nvmf_subsystem_remove_host(self, request, context=None):
 
         return pb2.req_status(status=return_string)
 
-    def nvmf_create_transport(self, request, context=None):
-        """Sets a transport type for device access."""
-        self.logger.info({f"Setting transport type to: {request.trtype}"})
-
-        # Check if transport type has already been created
-        if context:
-            trtype = self.persistent_config.get_transport(request.trtype)
-            if trtype is not None:
-                self.logger.info(
-                    f"Create Transport {trtype} already created.\n")
-                return pb2.req_status(status=True)
-
-        try:
-            status = self.spdk_rpc.nvmf.nvmf_create_transport(
-                self.spdk_rpc_client, trtype=request.trtype)
-        except Exception as ex:
-            self.logger.error(
-                f"Create Transport {request.trtype} returned with error: \n {ex}"
-            )
-            if context:
-                context.set_code(grpc.StatusCode.INTERNAL)
-                context.set_details(f"{ex}")
-            return pb2.req_status()
-
-        if context:
-            # Update persistent configuration
-            try:
-                json_req = json_format.MessageToJson(
-                    request, preserving_proto_field_name=True)
-                self.persistent_config.set_transport(request.trtype,
-                                                     json_req)
-            except Exception as ex:
-                self.terminate(
-                    f"Error persisting transport {request.trtype}: {ex}")
-
-        return pb2.req_status(status=status)
-
     def nvmf_subsystem_add_listener(self, request, context=None):
         """Adds a listener at the given TCP/IP address for the given subsystem."""
         self.logger.info({
             f"Adding listener at {request.traddr} : {request.trsvcid} for {request.nqn}"
         })
 
-        # Create transport if needed
-        if context:
-            self.nvmf_create_transport(pb2.create_transport_req(trtype='TCP'),
-                                       context)
-
         try:
+            self.create_transport(request.trtype)
+
             return_string = self.spdk_rpc.nvmf.nvmf_subsystem_add_listener(
                 self.spdk_rpc_client,
                 nqn=request.nqn,

proto/nvme_gw.proto

@@ -14,9 +14,6 @@ service NVMEGateway {
 	// start the SPDK target
 	rpc start_spdk(spdk_start_req) returns(spdk_status) {}
 
-	// Create Transport
-	rpc nvmf_create_transport(create_transport_req) returns(req_status) {}
-
 	// Create bdev/ NVMe Namespace from an RBD image
 	rpc bdev_rbd_create(bdev_create_req) returns (bdev_info) {}
 
@@ -131,10 +128,6 @@ message req_status {
 	bool status = 1;
 }
 
-message create_transport_req {
-	string trtype = 1; //NVMe Transport Type (eg: tcp. rdma, etc)
-}
-
 message subsystem_add_listener_req {
 	string nqn = 1; //subsystem for which listener is being added
 	optional string tgt_name = 2;

test_cli.py

@@ -36,10 +36,6 @@ def test_add_host(self, caplog, host):
         cli(["-c", config, "add_host", "-n", subsystem, "-t", host])
         assert "Failed to add" not in caplog.text
 
-    def test_create_transport(self, caplog):
-        cli(["-c", config, "create_transport", "-t", trtype])
-        assert "Failed to create" not in caplog.text
-
     def test_create_listener(self, caplog):
         cli(["-c", config, "create_listener", "-n", subsystem, "-a", addr, "-s", port])
         assert "Failed to create" not in caplog.text