NVMF: Add model number as parameter to construct_nvmf_subsystem (-d option).

Change-Id: Ia1a458a0ac1c5a17d2955a3f31c6dfe77538eb17
Signed-off-by: Gregory Shapiro <gregory.shapiro@kaminario.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/438562
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

CHANGELOG.md

@@ -88,6 +88,11 @@ is specified, use that specified nbd device. If it's not specified, pick availab
 Add Opal scan support for NVMe to check whether it supports SED Opal and dump
 device info. nvme_manage tool can be used to invoke this.
 
+### nvmf
+
+Add model number as parameter to construct_nvmf_subsystem (-d option),
+rather than using hardcoded define.
+
 ## v19.01:
 
 ### ocf bdev

doc/jsonrpc.md

@@ -3562,6 +3562,7 @@ Example response:
       ],
       "allow_any_host": false,
       "serial_number": "abcdef",
+      "model_number": "ghijklmnop",
       "namespaces": [
         {"nsid": 1, "name": "Malloc2"},
         {"nsid": 2, "name": "Nvme0n1"}
@@ -3581,6 +3582,7 @@ Name                    | Optional | Type        | Description
 ----------------------- | -------- | ----------- | -----------
 nqn                     | Required | string      | Subsystem NQN
 serial_number           | Optional | string      | Serial number of virtual controller
+model_number            | Optional | string      | Model number of virtual controller
 max_namespaces          | Optional | number      | Maximum number of namespaces that can be attached to the subsystem. Default: 0 (Unlimited)
 allow_any_host          | Optional | boolean     | Allow any host (`true`) or enforce allowed host whitelist (`false`). Default: `false`.
 
@@ -3597,6 +3599,7 @@ Example request:
     "nqn": "nqn.2016-06.io.spdk:cnode1",
     "allow_any_host": false,
     "serial_number": "abcdef",
+    "model_number": "ghijklmnop"
   }
 }
 ~~~

doc/nvmf.md

@@ -146,7 +146,7 @@ NQN, serial number, and IP address with RDMA transport to your own circumstances
 
 ~~~{.sh}
 scripts/rpc.py construct_malloc_bdev -b Malloc0 512 512
-scripts/rpc.py nvmf_subsystem_create nqn.2016-06.io.spdk:cnode1 -a -s SPDK00000000000001
+scripts/rpc.py nvmf_subsystem_create nqn.2016-06.io.spdk:cnode1 -a -s SPDK00000000000001 -d SPDK_Controller1
 scripts/rpc.py nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode1 Malloc0
 scripts/rpc.py nvmf_subsystem_add_listener nqn.2016-06.io.spdk:cnode1 -t rdma -a 192.168.100.8 -s 4420
 ~~~

etc/spdk/nvmf.conf.in

@@ -226,6 +226,7 @@
   AllowAnyHost No
   Host nqn.2016-06.io.spdk:init
   SN SPDK00000000000001
+  MN SPDK_Controller1
   MaxNamespaces 20
   Namespace Nvme0n1 1
   Namespace Nvme1n1 2
@@ -239,6 +240,7 @@
   AllowAnyHost No
   Host nqn.2016-06.io.spdk:init
   SN SPDK00000000000002
+  MN SPDK_Controller2
   Namespace Malloc0
   Namespace Malloc1
   Namespace AIO0

include/spdk/nvmf.h

@@ -672,6 +672,26 @@ const char *spdk_nvmf_subsystem_get_sn(const struct spdk_nvmf_subsystem *subsyst
  */
 int spdk_nvmf_subsystem_set_sn(struct spdk_nvmf_subsystem *subsystem, const char *sn);
 
+/**
+ * Get the model number of the specified subsystem.
+ *
+ * \param subsystem Subsystem to query.
+ *
+ * \return model number of the specified subsystem.
+ */
+const char *spdk_nvmf_subsystem_get_mn(const struct spdk_nvmf_subsystem *subsystem);
+
+
+/**
+ * Set the model number for the specified subsystem.
+ *
+ * \param subsystem Subsystem to set for.
+ * \param mn model number to set.
+ *
+ * \return 0 on success, -1 on failure.
+ */
+int spdk_nvmf_subsystem_set_mn(struct spdk_nvmf_subsystem *subsystem, const char *mn);
+
 /**
  * Get the NQN of the specified subsystem.
  *

lib/event/subsystems/nvmf/conf.c

@@ -213,6 +213,7 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp)
 	int lcore;
 	bool allow_any_host;
 	const char *sn;
+	const char *mn;
 	struct spdk_nvmf_subsystem *subsystem;
 	int num_ns;
 
@@ -274,6 +275,22 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp)
 		goto done;
 	}
 
+	mn = spdk_conf_section_get_val(sp, "MN");
+	if (mn == NULL) {
+		SPDK_NOTICELOG(
+			"Subsystem %s: missing model number, will use default\n",
+			nqn);
+	}
+
+	if (mn != NULL) {
+		if (spdk_nvmf_subsystem_set_mn(subsystem, mn)) {
+			SPDK_ERRLOG("Subsystem %s: invalid model number '%s'\n", nqn, mn);
+			spdk_nvmf_subsystem_destroy(subsystem);
+			subsystem = NULL;
+			goto done;
+		}
+	}
+
 	for (i = 0; ; i++) {
 		struct spdk_nvmf_ns_opts ns_opts;
 		struct spdk_bdev *bdev;

lib/event/subsystems/nvmf/nvmf_rpc.c

@@ -246,6 +246,8 @@ dump_nvmf_subsystem(struct spdk_json_write_ctx *w, struct spdk_nvmf_subsystem *s
 
 		spdk_json_write_named_string(w, "serial_number", spdk_nvmf_subsystem_get_sn(subsystem));
 
+		spdk_json_write_named_string(w, "model_number", spdk_nvmf_subsystem_get_mn(subsystem));
+
 		max_namespaces = spdk_nvmf_subsystem_get_max_namespaces(subsystem);
 		if (max_namespaces != 0) {
 			spdk_json_write_named_uint32(w, "max_namespaces", max_namespaces);
@@ -319,6 +321,7 @@ SPDK_RPC_REGISTER("get_nvmf_subsystems", spdk_rpc_get_nvmf_subsystems, SPDK_RPC_
 struct rpc_subsystem_create {
 	char *nqn;
 	char *serial_number;
+	char *model_number;
 	uint32_t max_namespaces;
 	bool allow_any_host;
 };
@@ -326,6 +329,7 @@ struct rpc_subsystem_create {
 static const struct spdk_json_object_decoder rpc_subsystem_create_decoders[] = {
 	{"nqn", offsetof(struct rpc_subsystem_create, nqn), spdk_json_decode_string},
 	{"serial_number", offsetof(struct rpc_subsystem_create, serial_number), spdk_json_decode_string, true},
+	{"model_number", offsetof(struct rpc_subsystem_create, model_number), spdk_json_decode_string, true},
 	{"max_namespaces", offsetof(struct rpc_subsystem_create, max_namespaces), spdk_json_decode_uint32, true},
 	{"allow_any_host", offsetof(struct rpc_subsystem_create, allow_any_host), spdk_json_decode_bool, true},
 };
@@ -378,10 +382,18 @@ spdk_rpc_nvmf_subsystem_create(struct spdk_jsonrpc_request *request,
 		}
 	}
 
+	if (req->model_number) {
+		if (spdk_nvmf_subsystem_set_mn(subsystem, req->model_number)) {
+			SPDK_ERRLOG("Subsystem %s: invalid model number '%s'\n", req->nqn, req->model_number);
+			goto invalid;
+		}
+	}
+
 	spdk_nvmf_subsystem_set_allow_any_host(subsystem, req->allow_any_host);
 
 	free(req->nqn);
 	free(req->serial_number);
+	free(req->model_number);
 	free(req);
 
 	spdk_nvmf_subsystem_start(subsystem,
@@ -395,6 +407,7 @@ invalid:
 	if (req) {
 		free(req->nqn);
 		free(req->serial_number);
+		free(req->model_number);
 	}
 	free(req);
 }

lib/nvmf/ctrlr.c

@@ -52,8 +52,6 @@
 #define KAS_TIME_UNIT_IN_MS 100
 #define KAS_DEFAULT_VALUE (MIN_KEEP_ALIVE_TIMEOUT_IN_MS / KAS_TIME_UNIT_IN_MS)
 
-#define MODEL_NUMBER "SPDK bdev Controller"
-
 /*
  * Report the SPDK version as the firmware revision.
  * SPDK_VERSION_STRING won't fit into FR (only 8 bytes), so try to fit the most important parts.
@@ -1557,7 +1555,7 @@ spdk_nvmf_ctrlr_identify_ctrlr(struct spdk_nvmf_ctrlr *ctrlr, struct spdk_nvme_c
 	 * NVM subsystem fields (reserved for discovery subsystems)
 	 */
 	if (subsystem->subtype == SPDK_NVMF_SUBTYPE_NVME) {
-		spdk_strcpy_pad(cdata->mn, MODEL_NUMBER, sizeof(cdata->mn), ' ');
+		spdk_strcpy_pad(cdata->mn, spdk_nvmf_subsystem_get_mn(subsystem), sizeof(cdata->mn), ' ');
 		spdk_strcpy_pad(cdata->sn, spdk_nvmf_subsystem_get_sn(subsystem), sizeof(cdata->sn), ' ');
 		cdata->kas = KAS_DEFAULT_VALUE;
 

lib/nvmf/nvmf.c

@@ -327,6 +327,7 @@ spdk_nvmf_write_subsystem_config_json(struct spdk_json_write_ctx *w,
 	spdk_json_write_named_string(w, "nqn", spdk_nvmf_subsystem_get_nqn(subsystem));
 	spdk_json_write_named_bool(w, "allow_any_host", spdk_nvmf_subsystem_get_allow_any_host(subsystem));
 	spdk_json_write_named_string(w, "serial_number", spdk_nvmf_subsystem_get_sn(subsystem));
+	spdk_json_write_named_string(w, "model_number", spdk_nvmf_subsystem_get_mn(subsystem));
 
 	max_namespaces = spdk_nvmf_subsystem_get_max_namespaces(subsystem);
 	if (max_namespaces != 0) {

lib/nvmf/nvmf_internal.h

@@ -302,6 +302,7 @@ struct spdk_nvmf_subsystem {
 	struct spdk_nvmf_tgt			*tgt;
 
 	char sn[SPDK_NVME_CTRLR_SN_LEN + 1];
+	char mn[SPDK_NVME_CTRLR_MN_LEN + 1];
 
 	/* Array of pointers to namespaces of size max_nsid indexed by nsid - 1 */
 	struct spdk_nvmf_ns			**ns;

lib/nvmf/subsystem.c

@@ -47,6 +47,8 @@
 #include "spdk_internal/log.h"
 #include "spdk_internal/utf.h"
 
+#define MODEL_NUMBER_DEFAULT "SPDK bdev Controller"
+
 /*
  * States for parsing valid domains in NQNs according to RFC 1034
  */
@@ -292,6 +294,9 @@ spdk_nvmf_subsystem_create(struct spdk_nvmf_tgt *tgt,
 	memset(subsystem->sn, '0', sizeof(subsystem->sn) - 1);
 	subsystem->sn[sizeof(subsystem->sn) - 1] = '\0';
 
+	snprintf(subsystem->mn, sizeof(subsystem->mn), "%s",
+		 MODEL_NUMBER_DEFAULT);
+
 	tgt->subsystems[sid] = subsystem;
 	tgt->discovery_genctr++;
 
@@ -1203,6 +1208,39 @@ spdk_nvmf_subsystem_set_sn(struct spdk_nvmf_subsystem *subsystem, const char *sn
 	return 0;
 }
 
+const char *
+spdk_nvmf_subsystem_get_mn(const struct spdk_nvmf_subsystem *subsystem)
+{
+	return subsystem->mn;
+}
+
+int
+spdk_nvmf_subsystem_set_mn(struct spdk_nvmf_subsystem *subsystem, const char *mn)
+{
+	size_t len, max_len;
+
+	if (mn == NULL) {
+		mn = MODEL_NUMBER_DEFAULT;
+	}
+	max_len = sizeof(subsystem->mn) - 1;
+	len = strlen(mn);
+	if (len > max_len) {
+		SPDK_DEBUGLOG(SPDK_LOG_NVMF, "Invalid mn \"%s\": length %zu > max %zu\n",
+			      mn, len, max_len);
+		return -1;
+	}
+
+	if (!spdk_nvmf_valid_ascii_string(mn, len)) {
+		SPDK_DEBUGLOG(SPDK_LOG_NVMF, "Non-ASCII mn\n");
+		SPDK_LOGDUMP(SPDK_LOG_NVMF, "mn", mn, len);
+		return -1;
+	}
+
+	snprintf(subsystem->mn, sizeof(subsystem->mn), "%s", mn);
+
+	return 0;
+}
+
 const char *
 spdk_nvmf_subsystem_get_nqn(struct spdk_nvmf_subsystem *subsystem)
 {

scripts/config_converter.py

@@ -319,6 +319,7 @@ def get_nvmf_subsystem_json(config, section):
         ["NQN", "nqn", str, ""],
         ["AllowAnyHost", "allow_any_host", bool, False],
         ["SN", "serial_number", str, "00000000000000000000"],
+        ["MN", "model_number", str, "SPDK bdev Controller"],
         ["MaxNamespaces", "max_namespaces", str, ""],
     ]
     listen_address = []
@@ -355,7 +356,7 @@ def get_nvmf_subsystem_json(config, section):
                 })
     # Get parameters: nqn, allow_any_host, serial_number
     # for nvmf_subsystem_create rpc method
-    parameters = to_json_params(params[1:4])
+    parameters = to_json_params(params[1:5])
     nvmf_subsystem_methods.append({
         "params": parameters,
         "method": "nvmf_subsystem_create"
@@ -386,8 +387,8 @@ def get_nvmf_subsystem_json(config, section):
         })
 
     # Define max_namespaces if it is set in old config
-    if params[4][3]:
-        nvmf_subsystem_methods[0]['params']['max_namespaces'] = int(params[4][3])
+    if params[5][3]:
+        nvmf_subsystem_methods[0]['params']['max_namespaces'] = int(params[5][3])
 
     return nvmf_subsystem_methods
 

scripts/rpc.py

@@ -1436,6 +1436,7 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse
         rpc.nvmf.nvmf_subsystem_create(args.client,
                                        nqn=args.nqn,
                                        serial_number=args.serial_number,
+                                       model_number=args.model_number,
                                        allow_any_host=args.allow_any_host,
                                        max_namespaces=args.max_namespaces)
 
@@ -1444,6 +1445,9 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse
     p.add_argument("-s", "--serial-number", help="""
     Format:  'sn' etc
     Example: 'SPDK00000000000001'""", default='00000000000000000000')
+    p.add_argument("-d", "--model-number", help="""
+    Format:  'mn' etc
+    Example: 'SPDK Controller'""", default='SPDK bdev Controller')
     p.add_argument("-a", "--allow-any-host", action='store_true', help="Allow any host to connect (don't enforce host NQN whitelist)")
     p.add_argument("-m", "--max-namespaces", help="Maximum number of namespaces allowed",
                    type=int, default=0)

scripts/rpc/nvmf.py

@@ -108,6 +108,7 @@ def get_nvmf_subsystems(client):
 def nvmf_subsystem_create(client,
                           nqn,
                           serial_number,
+                          model_number='SPDK bdev Controller',
                           allow_any_host=False,
                           max_namespaces=0):
     """Construct an NVMe over Fabrics target subsystem.
@@ -115,6 +116,7 @@ def nvmf_subsystem_create(client,
     Args:
         nqn: Subsystem NQN.
         serial_number: Serial number of virtual controller.
+        model_number: Model number of virtual controller.
         allow_any_host: Allow any host (True) or enforce allowed host whitelist (False). Default: False.
         max_namespaces: Maximum number of namespaces that can be attached to the subsystem (optional). Default: 0 (Unlimited).
 
@@ -128,6 +130,9 @@ def nvmf_subsystem_create(client,
     if serial_number:
         params['serial_number'] = serial_number
 
+    if model_number:
+        params['model_number'] = model_number
+
     if allow_any_host:
         params['allow_any_host'] = True
 

test/config_converter/config.ini

@@ -99,6 +99,7 @@
   AllowAnyHost No
   Host nqn.2016-06.io.spdk:init
   SN SPDK00000000000001
+  MN SPDK_Controller1
   MaxNamespaces 20
   Namespace Nvme0n1p5 1
   Namespace Nvme0n1p6 2
@@ -109,6 +110,7 @@
   AllowAnyHost No
   Host nqn.2016-06.io.spdk:init
   SN SPDK00000000000002
+  MN SPDK_Controller2
   Namespace Malloc1
   Namespace Malloc2
   Namespace AIO0

test/config_converter/spdk_config.json

@@ -192,6 +192,7 @@
             "max_namespaces": 20,
             "allow_any_host": false,
             "serial_number": "SPDK00000000000001",
+            "model_number": "SPDK_Controller1",
             "nqn": "nqn.2016-06.io.spdk:cnode1"
           },
           "method": "nvmf_subsystem_create"
@@ -239,6 +240,7 @@
           "params": {
             "allow_any_host": false,
             "serial_number": "SPDK00000000000002",
+            "model_number": "SPDK_Controller2",
             "nqn": "nqn.2016-06.io.spdk:cnode2"
           },
           "method": "nvmf_subsystem_create"

test/nvmf/nvme_cli/nvme_cli.sh

@@ -46,7 +46,7 @@ bdevs+="$($rpc_py construct_malloc_bdev $MALLOC_BDEV_SIZE $MALLOC_BLOCK_SIZE)"
 
 modprobe -v nvme-rdma
 
-$rpc_py nvmf_subsystem_create nqn.2016-06.io.spdk:cnode1 -a -s SPDK00000000000001
+$rpc_py nvmf_subsystem_create nqn.2016-06.io.spdk:cnode1 -a -s SPDK00000000000001 -d SPDK_Controller1
 for bdev in $bdevs; do
 	$rpc_py nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode1 $bdev
 done
@@ -62,6 +62,11 @@ nvme list
 for ctrl in /dev/nvme?; do
 	nvme id-ctrl $ctrl
 	nvme smart-log $ctrl
+	nvme_model = $(nvme id-ctrl $ctrl | grep -w mn | sed 's/^.*: //')
+	if [ "$nvme_model" != "SPDK_Controller1" ]; then
+		echo "Wrong model number for controller" $nvme_model
+		exit 1
+	fi
 done
 
 for ns in /dev/nvme?n*; do

test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c

@@ -62,6 +62,11 @@ DEFINE_STUB(spdk_nvmf_subsystem_get_sn,
 	    (const struct spdk_nvmf_subsystem *subsystem),
 	    NULL);
 
+DEFINE_STUB(spdk_nvmf_subsystem_get_mn,
+	    const char *,
+	    (const struct spdk_nvmf_subsystem *subsystem),
+	    NULL);
+
 DEFINE_STUB(spdk_nvmf_subsystem_get_first_ns,
 	    struct spdk_nvmf_ns *,
 	    (struct spdk_nvmf_subsystem *subsystem),

test/unit/lib/nvmf/tcp.c/tcp_ut.c

@@ -220,6 +220,12 @@ spdk_nvmf_subsystem_get_sn(const struct spdk_nvmf_subsystem *subsystem)
 	return subsystem->sn;
 }
 
+const char *
+spdk_nvmf_subsystem_get_mn(const struct spdk_nvmf_subsystem *subsystem)
+{
+	return subsystem->mn;
+}
+
 void
 spdk_trace_add_register_fn(struct spdk_trace_register_fn *reg_fn)
 {