nvmf: introduce listener opts

With new spdk_nvmf_transport_listen function it should be possible to
add generic options without breaking API/ABI. For now it only delivers
json parameters which can be decoded on a transport specific layer.
This is similar to what was done for spdk_nvmf_transport_create and opts there.

Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: Iaf576248a0b10b408c4a3182785270be3e32ebe4
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5570
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

include/spdk/nvmf.h

@@ -98,6 +98,18 @@ struct spdk_nvmf_transport_opts {
 	size_t opts_size;
 };
 
+struct spdk_nvmf_listen_opts {
+	/**
+	 * The size of spdk_nvmf_listen_opts according to the caller of this library is used for ABI
+	 * compatibility. The library uses this field to know how many fields in this
+	 * structure are valid. And the library will populate any remaining fields with default values.
+	 * New added fields should be put at the end of the struct.
+	 */
+	size_t opts_size;
+
+	const struct spdk_json_val *transport_specific;
+};
+
 struct spdk_nvmf_poll_group_stat {
 	uint32_t admin_qpairs;
 	uint32_t io_qpairs;
@@ -209,7 +221,7 @@ struct spdk_nvmf_tgt *spdk_nvmf_get_next_tgt(struct spdk_nvmf_tgt *prev);
 void spdk_nvmf_tgt_write_config_json(struct spdk_json_write_ctx *w, struct spdk_nvmf_tgt *tgt);
 
 /**
- * Begin accepting new connections at the address provided.
+ * Begin accepting new connections at the address provided (deprecated, please use spdk_nvmf_tgt_listen_ext).
  *
  * The connections will be matched with a subsystem, which may or may not allow
  * the connection based on a subsystem-specific list of allowed hosts. See
@@ -223,6 +235,22 @@ void spdk_nvmf_tgt_write_config_json(struct spdk_json_write_ctx *w, struct spdk_
 int spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
 			 struct spdk_nvme_transport_id *trid);
 
+/**
+ * Begin accepting new connections at the address provided.
+ *
+ * The connections will be matched with a subsystem, which may or may not allow
+ * the connection based on a subsystem-specific list of allowed hosts. See
+ * spdk_nvmf_subsystem_add_host() and spdk_nvmf_subsystem_add_listener()
+ *
+ * \param tgt The target associated with this listen address.
+ * \param trid The address to listen at.
+ * \param opts Listener options.
+ *
+ * \return 0 on success or a negated errno on failure.
+ */
+int spdk_nvmf_tgt_listen_ext(struct spdk_nvmf_tgt *tgt, const struct spdk_nvme_transport_id *trid,
+			     struct spdk_nvmf_listen_opts *opts);
+
 /**
  * Stop accepting new connections at the provided address.
  *
@@ -1034,14 +1062,15 @@ void spdk_nvmf_tgt_add_transport(struct spdk_nvmf_tgt *tgt,
 /**
  * Add listener to transport and begin accepting new connections.
  *
- * \param transport The transport to add listener to
+ * \param transport The transport to add listener to.
- * \param trid Address to listen at
+ * \param trid The address to listen at.
+ * \param opts Listener options.
  *
  * \return int. 0 if it completed successfully, or negative errno if it failed.
  */
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid);
+			   const struct spdk_nvme_transport_id *trid, struct spdk_nvmf_listen_opts *opts);
 
 /**
  * Remove listener from transport and stop accepting new connections.

include/spdk/nvmf_transport.h

@@ -241,8 +241,8 @@ struct spdk_nvmf_transport_ops {
 	  * Instruct the transport to accept new connections at the address
 	  * provided. This may be called multiple times.
 	  */
-	int (*listen)(struct spdk_nvmf_transport *transport,
+	int (*listen)(struct spdk_nvmf_transport *transport, const struct spdk_nvme_transport_id *trid,
-		      const struct spdk_nvme_transport_id *trid);
+		      struct spdk_nvmf_listen_opts *opts);
 
 	/**
 	  * Stop accepting new connections at the given address.

lib/nvmf/fc.c

@@ -1879,8 +1879,8 @@ nvmf_fc_destroy(struct spdk_nvmf_transport *transport,
 }
 
 static int
-nvmf_fc_listen(struct spdk_nvmf_transport *transport,
+nvmf_fc_listen(struct spdk_nvmf_transport *transport, const struct spdk_nvme_transport_id *trid,
-	       const struct spdk_nvme_transport_id *trid)
+	       struct spdk_nvmf_listen_opts *listen_opts)
 {
 	return 0;
 }

lib/nvmf/nvmf.c

@@ -598,12 +598,45 @@ spdk_nvmf_tgt_write_config_json(struct spdk_json_write_ctx *w, struct spdk_nvmf_
 	}
 }
 
+static void
+nvmf_listen_opts_copy(struct spdk_nvmf_listen_opts *opts,
+		      const struct spdk_nvmf_listen_opts *opts_src, size_t opts_size)
+{
+	assert(opts);
+	assert(opts_src);
+
+	opts->opts_size = opts_size;
+
+#define SET_FIELD(field) \
+    if (offsetof(struct spdk_nvmf_listen_opts, field) + sizeof(opts->field) <= opts_size) { \
+                 opts->field = opts_src->field; \
+    } \
+
+	SET_FIELD(transport_specific);
+#undef SET_FIELD
+
+	/* Do not remove this statement, you should always update this statement when you adding a new field,
+	 * and do not forget to add the SET_FIELD statement for your added field. */
+	SPDK_STATIC_ASSERT(sizeof(struct spdk_nvmf_listen_opts) == 16, "Incorrect size");
+}
+
 int
-spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
+spdk_nvmf_tgt_listen_ext(struct spdk_nvmf_tgt *tgt, const struct spdk_nvme_transport_id *trid,
-		     struct spdk_nvme_transport_id *trid)
+			 struct spdk_nvmf_listen_opts *opts)
 {
 	struct spdk_nvmf_transport *transport;
 	int rc;
+	struct spdk_nvmf_listen_opts opts_local = {};
+
+	if (!opts) {
+		SPDK_ERRLOG("opts should not be NULL\n");
+		return -EINVAL;
+	}
+
+	if (!opts->opts_size) {
+		SPDK_ERRLOG("The opts_size in opts structure should not be zero\n");
+		return -EINVAL;
+	}
 
 	transport = spdk_nvmf_tgt_get_transport(tgt, trid->trstring);
 	if (!transport) {
@@ -612,7 +645,8 @@ spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
 		return -EINVAL;
 	}
 
-	rc = spdk_nvmf_transport_listen(transport, trid);
+	nvmf_listen_opts_copy(&opts_local, opts, opts->opts_size);
+	rc = spdk_nvmf_transport_listen(transport, trid, &opts_local);
 	if (rc < 0) {
 		SPDK_ERRLOG("Unable to listen on address '%s'\n", trid->traddr);
 	}
@@ -620,6 +654,14 @@ spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
 	return rc;
 }
 
+int
+spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt, struct spdk_nvme_transport_id *trid)
+{
+	struct spdk_nvmf_listen_opts opts = {.opts_size = sizeof(opts)};
+
+	return spdk_nvmf_tgt_listen_ext(tgt, trid, &opts);
+}
+
 int
 spdk_nvmf_tgt_stop_listen(struct spdk_nvmf_tgt *tgt,
 			  struct spdk_nvme_transport_id *trid)

lib/nvmf/nvmf_rpc.c

@@ -685,6 +685,7 @@ struct nvmf_rpc_listener_ctx {
 	struct spdk_nvme_transport_id	trid;
 	enum nvmf_rpc_listen_op		op;
 	bool				response_sent;
+	struct spdk_nvmf_listen_opts	opts;
 };
 
 static const struct spdk_json_object_decoder nvmf_rpc_listener_decoder[] = {
@@ -799,7 +800,7 @@ nvmf_rpc_listen_paused(struct spdk_nvmf_subsystem *subsystem,
 
 	if (ctx->op == NVMF_RPC_LISTEN_ADD) {
 		if (!nvmf_subsystem_find_listener(subsystem, &ctx->trid)) {
-			rc = spdk_nvmf_tgt_listen(ctx->tgt, &ctx->trid);
+			rc = spdk_nvmf_tgt_listen_ext(ctx->tgt, &ctx->trid, &ctx->opts);
 			if (rc == 0) {
 				spdk_nvmf_subsystem_add_listener(ctx->subsystem, &ctx->trid, nvmf_rpc_subsystem_listen, ctx);
 				return;
@@ -900,10 +901,10 @@ rpc_nvmf_subsystem_add_listener(struct spdk_jsonrpc_request *request,
 
 	ctx->request = request;
 
-	if (spdk_json_decode_object(params, nvmf_rpc_listener_decoder,
+	if (spdk_json_decode_object_relaxed(params, nvmf_rpc_listener_decoder,
-				    SPDK_COUNTOF(nvmf_rpc_listener_decoder),
+					    SPDK_COUNTOF(nvmf_rpc_listener_decoder),
-				    ctx)) {
+					    ctx)) {
-		SPDK_ERRLOG("spdk_json_decode_object failed\n");
+		SPDK_ERRLOG("spdk_json_decode_object_relaxed failed\n");
 		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters");
 		nvmf_rpc_listener_ctx_free(ctx);
 		return;
@@ -937,6 +938,8 @@ rpc_nvmf_subsystem_add_listener(struct spdk_jsonrpc_request *request,
 	}
 
 	ctx->op = NVMF_RPC_LISTEN_ADD;
+	ctx->opts.transport_specific = params;
+	ctx->opts.opts_size = sizeof(ctx->opts);
 
 	rc = spdk_nvmf_subsystem_pause(subsystem, nvmf_rpc_listen_paused, ctx);
 	if (rc != 0) {

lib/nvmf/rdma.c

@@ -2643,8 +2643,8 @@ nvmf_rdma_trid_from_cm_id(struct rdma_cm_id *id,
 			  bool peer);
 
 static int
-nvmf_rdma_listen(struct spdk_nvmf_transport *transport,
+nvmf_rdma_listen(struct spdk_nvmf_transport *transport, const struct spdk_nvme_transport_id *trid,
-		 const struct spdk_nvme_transport_id *trid)
+		 struct spdk_nvmf_listen_opts *listen_opts)
 {
 	struct spdk_nvmf_rdma_transport	*rtransport;
 	struct spdk_nvmf_rdma_device	*device;
@@ -2962,7 +2962,7 @@ nvmf_rdma_handle_cm_event_addr_change(struct spdk_nvmf_transport *transport,
 		nvmf_rdma_disconnect_qpairs_on_port(rtransport, port);
 
 		nvmf_rdma_stop_listen(transport, trid);
-		nvmf_rdma_listen(transport, trid);
+		nvmf_rdma_listen(transport, trid, NULL);
 	}
 
 	return event_acked;

lib/nvmf/spdk_nvmf.map

@@ -10,6 +10,7 @@
 	spdk_nvmf_get_next_tgt;
 	spdk_nvmf_tgt_write_config_json;
 	spdk_nvmf_tgt_listen;
+	spdk_nvmf_tgt_listen_ext;
 	spdk_nvmf_tgt_stop_listen;
 	spdk_nvmf_poll_group_create;
 	spdk_nvmf_get_optimal_poll_group;

lib/nvmf/tcp.c

@@ -657,8 +657,8 @@ nvmf_tcp_find_port(struct spdk_nvmf_tcp_transport *ttransport,
 }
 
 static int
-nvmf_tcp_listen(struct spdk_nvmf_transport *transport,
+nvmf_tcp_listen(struct spdk_nvmf_transport *transport, const struct spdk_nvme_transport_id *trid,
-		const struct spdk_nvme_transport_id *trid)
+		struct spdk_nvmf_listen_opts *listen_opts)
 {
 	struct spdk_nvmf_tcp_transport *ttransport;
 	struct spdk_nvmf_tcp_port *port;

lib/nvmf/transport.c

@@ -255,7 +255,7 @@ nvmf_transport_find_listener(struct spdk_nvmf_transport *transport,
 
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid)
+			   const struct spdk_nvme_transport_id *trid, struct spdk_nvmf_listen_opts *opts)
 {
 	struct spdk_nvmf_listener *listener;
 	int rc;
@@ -270,8 +270,7 @@ spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
 		listener->ref = 1;
 		listener->trid = *trid;
 		TAILQ_INSERT_TAIL(&transport->listeners, listener, link);
-
+		rc = transport->ops->listen(transport, &listener->trid, opts);
-		rc = transport->ops->listen(transport, &listener->trid);
 		if (rc != 0) {
 			TAILQ_REMOVE(&transport->listeners, listener, link);
 			free(listener);

test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c

@@ -107,7 +107,7 @@ spdk_bdev_get_uuid(const struct spdk_bdev *bdev)
 
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid)
+			   const struct spdk_nvme_transport_id *trid, struct spdk_nvmf_listen_opts *opts)
 {
 	return 0;
 }

test/unit/lib/nvmf/subsystem.c/subsystem_ut.c

@@ -90,7 +90,7 @@ DEFINE_STUB(spdk_nvme_transport_id_trtype_str,
 
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid)
+			   const struct spdk_nvme_transport_id *trid, struct spdk_nvmf_listen_opts *opts)
 {
 	return 0;
 }