nvmf: Allow transport specific statistics dumping into JSON

This change refactors the way nvmf_get_stats RPC works.
The RPC layer passes JSON write context to custom dump function defined within transport ops.
The RPC layer no longer needs to know the structure of transport poll group statictics.
Functions and structures used in the previous flow have been deprecated and will be removed.
JSON returned for RDMA transport should be the same as before this change.

Signed-off-by: Maciej Szulik <maciej.szulik@intel.com>
Change-Id: I03308c45be120793d316bf79814a1295afd9fb95
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6681
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>

CHANGELOG.md

@@ -58,6 +58,24 @@ the new SSD's PCI address.
 
 Removed the `spdk_nvmf_tgt_listen` and `spdk_nvmf_subsystem_add_ns` API.
 
+Added new APIs:
+- `spdk_nvmf_poll_group_dump_stat` (function in `nvmf.h`).
+- `poll_group_dump_stat` (transport op in `nvmf_transport.h`).
+
+The following APIs have been deprecated and will be removed in a future release:
+- `spdk_nvmf_poll_group_get_stat` (function in `nvmf.h`),
+- `spdk_nvmf_transport_poll_group_get_stat` (function in `nvmf.h`),
+- `spdk_nvmf_transport_poll_group_free_stat`(function in `nvmf.h`),
+- `spdk_nvmf_rdma_device_stat ` (struct in `nvmf.h`),
+- `spdk_nvmf_transport_poll_group_stat` (struct in `nvmf.h`),
+- `poll_group_get_stat` (transport op in `nvmf_transport.h`),
+- `poll_group_free_stat` (transport op in `nvmf_transport.h`).
+
+See header files for details.
+
+The `trtype` field in JSON returned by `nvmf_get_stats` RPC contains now the name of the transport,
+which is the same as the type for defined transports and more informative for a custom transport.
+
 ### opal
 
 Removed the `spdk_opal_supported` API.

include/spdk/nvmf.h

@@ -124,6 +124,10 @@ struct spdk_nvmf_poll_group_stat {
 	uint64_t pending_bdev_io;
 };
 
+/* Deprecated.
+ * Please use the flow with spdk_nvmf_poll_group_dump_stat,
+ * which hides statistics structures within the transport.
+ */
 struct spdk_nvmf_rdma_device_stat {
 	const char *name;
 	uint64_t polls;
@@ -140,6 +144,10 @@ struct spdk_nvmf_rdma_device_stat {
 	uint64_t recv_doorbell_updates;
 };
 
+/* Deprecated.
+ * Please use the flow with spdk_nvmf_poll_group_dump_stat,
+ * which hides statistics structures within the transport.
+ */
 struct spdk_nvmf_transport_poll_group_stat {
 	spdk_nvme_transport_type_t trtype;
 	union {
@@ -305,7 +313,7 @@ int spdk_nvmf_poll_group_add(struct spdk_nvmf_poll_group *group,
 			     struct spdk_nvmf_qpair *qpair);
 
 /**
- * Get current poll group statistics.
+ * Get current poll group statistics. (deprecated)
  *
  * \param tgt The NVMf target.
  * \param stat Pointer to allocated statistics structure to fill with values.
@@ -1096,8 +1104,11 @@ int spdk_nvmf_transport_stop_listen_async(struct spdk_nvmf_transport *transport,
 		spdk_nvmf_tgt_subsystem_listen_done_fn cb_fn,
 		void *cb_arg);
 
+
 /**
- * \brief Get current transport poll group statistics.
+ * \brief Get current transport poll group statistics. (deprecated)
+ *
+ * Please use the flow with spdk_nvmf_poll_group_dump_stat.
  *
  * This function allocates memory for statistics and returns it
  * in \p stat parameter. Caller must free this memory with
@@ -1120,7 +1131,9 @@ spdk_nvmf_transport_poll_group_get_stat(struct spdk_nvmf_tgt *tgt,
 					struct spdk_nvmf_transport_poll_group_stat **stat);
 
 /**
- * Free statistics memory previously allocated with spdk_nvmf_transport_poll_group_get_stat().
+ * Free statistics memory previously allocated with spdk_nvmf_transport_poll_group_get_stat(). (deprecated)
+ *
+ * Please use the flow with spdk_nvmf_poll_group_dump_stat.
  *
  * \param transport The NVMf transport.
  * \param stat Pointer to transport poll group statistics structure.
@@ -1129,6 +1142,15 @@ void
 spdk_nvmf_transport_poll_group_free_stat(struct spdk_nvmf_transport *transport,
 		struct spdk_nvmf_transport_poll_group_stat *stat);
 
+/**
+ * Dump poll group statistics into JSON.
+ *
+ * \param group The group which statistics should be dumped.
+ * \param w The JSON write context to which statistics should be dumped.
+ */
+void spdk_nvmf_poll_group_dump_stat(struct spdk_nvmf_poll_group *group,
+				    struct spdk_json_write_ctx *w);
+
 /**
  * \brief Set the global hooks for the RDMA transport, if necessary.
  *

include/spdk/nvmf_transport.h

@@ -368,15 +368,23 @@ struct spdk_nvmf_transport_ops {
 				    struct spdk_nvmf_request *req);
 
 	/*
-	 * Get transport poll group statistics
+	 * Get transport poll group statistics. (deprecated)
+	 * Please use the flow with spdk_nvmf_poll_group_dump_stat.
 	 */
 	int (*poll_group_get_stat)(struct spdk_nvmf_tgt *tgt,
 				   struct spdk_nvmf_transport_poll_group_stat **stat);
 
 	/*
-	 * Free transport poll group statistics previously allocated with poll_group_get_stat()
+	 * Free transport poll group statistics previously allocated with poll_group_get_stat(). (deprecated)
+	 * Please use the flow with spdk_nvmf_poll_group_dump_stat.
 	 */
 	void (*poll_group_free_stat)(struct spdk_nvmf_transport_poll_group_stat *stat);
+
+	/*
+	 * Dump transport poll group statistics into JSON.
+	 */
+	void (*poll_group_dump_stat)(struct spdk_nvmf_transport_poll_group *group,
+				     struct spdk_json_write_ctx *w);
 };
 
 /**

lib/nvmf/nvmf.c

@@ -1564,6 +1564,8 @@ spdk_nvmf_poll_group_get_stat(struct spdk_nvmf_tgt *tgt,
 	struct spdk_io_channel *ch;
 	struct spdk_nvmf_poll_group *group;
 
+	SPDK_ERRLOG("spdk_nvmf_poll_group_get_stat is deprecated and will be removed\n");
+
 	if (tgt == NULL || stat == NULL) {
 		return -EINVAL;
 	}
@@ -1574,3 +1576,36 @@ spdk_nvmf_poll_group_get_stat(struct spdk_nvmf_tgt *tgt,
 	spdk_put_io_channel(ch);
 	return 0;
 }
+
+void
+spdk_nvmf_poll_group_dump_stat(struct spdk_nvmf_poll_group *group, struct spdk_json_write_ctx *w)
+{
+	struct spdk_nvmf_transport_poll_group *tgroup;
+
+	spdk_json_write_object_begin(w);
+
+	spdk_json_write_named_string(w, "name", spdk_thread_get_name(spdk_get_thread()));
+	spdk_json_write_named_uint32(w, "admin_qpairs", group->stat.admin_qpairs);
+	spdk_json_write_named_uint32(w, "io_qpairs", group->stat.io_qpairs);
+	spdk_json_write_named_uint64(w, "pending_bdev_io", group->stat.pending_bdev_io);
+
+	spdk_json_write_named_array_begin(w, "transports");
+
+	TAILQ_FOREACH(tgroup, &group->tgroups, link) {
+		spdk_json_write_object_begin(w);
+		/*
+		 * The trtype field intentionally contains a transport name as this is more informative.
+		 * The field has not been renamed for backward compatibility.
+		 */
+		spdk_json_write_named_string(w, "trtype", spdk_nvmf_get_transport_name(tgroup->transport));
+
+		if (tgroup->transport->ops->poll_group_dump_stat) {
+			tgroup->transport->ops->poll_group_dump_stat(tgroup, w);
+		}
+
+		spdk_json_write_object_end(w);
+	}
+
+	spdk_json_write_array_end(w);
+	spdk_json_write_object_end(w);
+}

lib/nvmf/nvmf_rpc.c

@@ -2035,87 +2035,19 @@ rpc_nvmf_get_stats_done(struct spdk_io_channel_iter *i, int status)
 	free_get_stats_ctx(ctx);
 }
 
-static void
-write_nvmf_transport_stats(struct spdk_json_write_ctx *w,
-			   struct spdk_nvmf_transport_poll_group_stat *stat)
-{
-	uint64_t i;
-
-	spdk_json_write_object_begin(w);
-	spdk_json_write_named_string(w, "trtype",
-				     spdk_nvme_transport_id_trtype_str(stat->trtype));
-	switch (stat->trtype) {
-	case SPDK_NVME_TRANSPORT_RDMA:
-		spdk_json_write_named_uint64(w, "pending_data_buffer", stat->rdma.pending_data_buffer);
-		spdk_json_write_named_array_begin(w, "devices");
-		for (i = 0; i < stat->rdma.num_devices; ++i) {
-			spdk_json_write_object_begin(w);
-			spdk_json_write_named_string(w, "name", stat->rdma.devices[i].name);
-			spdk_json_write_named_uint64(w, "polls", stat->rdma.devices[i].polls);
-			spdk_json_write_named_uint64(w, "idle_polls", stat->rdma.devices[i].idle_polls);
-			spdk_json_write_named_uint64(w, "completions", stat->rdma.devices[i].completions);
-			spdk_json_write_named_uint64(w, "requests",
-						     stat->rdma.devices[i].requests);
-			spdk_json_write_named_uint64(w, "request_latency",
-						     stat->rdma.devices[i].request_latency);
-			spdk_json_write_named_uint64(w, "pending_free_request",
-						     stat->rdma.devices[i].pending_free_request);
-			spdk_json_write_named_uint64(w, "pending_rdma_read",
-						     stat->rdma.devices[i].pending_rdma_read);
-			spdk_json_write_named_uint64(w, "pending_rdma_write",
-						     stat->rdma.devices[i].pending_rdma_write);
-			spdk_json_write_named_uint64(w, "total_send_wrs",
-						     stat->rdma.devices[i].total_send_wrs);
-			spdk_json_write_named_uint64(w, "send_doorbell_updates",
-						     stat->rdma.devices[i].send_doorbell_updates);
-			spdk_json_write_named_uint64(w, "total_recv_wrs",
-						     stat->rdma.devices[i].total_recv_wrs);
-			spdk_json_write_named_uint64(w, "recv_doorbell_updates",
-						     stat->rdma.devices[i].recv_doorbell_updates);
-			spdk_json_write_object_end(w);
-		}
-		spdk_json_write_array_end(w);
-		break;
-	default:
-		break;
-	}
-	spdk_json_write_object_end(w);
-}
-
 static void
 _rpc_nvmf_get_stats(struct spdk_io_channel_iter *i)
 {
 	struct rpc_nvmf_get_stats_ctx *ctx = spdk_io_channel_iter_get_ctx(i);
-	struct spdk_nvmf_transport *transport;
-	struct spdk_nvmf_poll_group_stat stat;
-	struct spdk_nvmf_transport_poll_group_stat *trstat;
-	int rc;
+	struct spdk_io_channel *ch;
+	struct spdk_nvmf_poll_group *group;
 
-	if (0 == spdk_nvmf_poll_group_get_stat(ctx->tgt, &stat)) {
-		spdk_json_write_object_begin(ctx->w);
-		spdk_json_write_named_string(ctx->w, "name", spdk_thread_get_name(spdk_get_thread()));
-		spdk_json_write_named_uint32(ctx->w, "admin_qpairs", stat.admin_qpairs);
-		spdk_json_write_named_uint32(ctx->w, "io_qpairs", stat.io_qpairs);
-		spdk_json_write_named_uint64(ctx->w, "pending_bdev_io", stat.pending_bdev_io);
+	ch = spdk_get_io_channel(ctx->tgt);
+	group = spdk_io_channel_get_ctx(ch);
 
-		spdk_json_write_named_array_begin(ctx->w, "transports");
-		transport = spdk_nvmf_transport_get_first(ctx->tgt);
-		while (transport) {
-			rc = spdk_nvmf_transport_poll_group_get_stat(ctx->tgt, transport, &trstat);
-			if (0 == rc) {
-				write_nvmf_transport_stats(ctx->w, trstat);
-				spdk_nvmf_transport_poll_group_free_stat(transport, trstat);
-			} else if (-ENOTSUP != rc) {
-				SPDK_ERRLOG("Failed to get poll group statistics for transport %s, errno %d\n",
-					    spdk_nvme_transport_id_trtype_str(spdk_nvmf_get_transport_type(transport)),
-					    rc);
-			}
-			transport = spdk_nvmf_transport_get_next(transport);
-		}
-		spdk_json_write_array_end(ctx->w);
-		spdk_json_write_object_end(ctx->w);
-	}
+	spdk_nvmf_poll_group_dump_stat(group, ctx->w);
 
+	spdk_put_io_channel(ch);
 	spdk_for_each_channel_continue(i, 0);
 }
 

lib/nvmf/rdma.c

@@ -4150,6 +4150,7 @@ nvmf_rdma_qpair_abort_request(struct spdk_nvmf_qpair *qpair,
 	_nvmf_rdma_qpair_abort_request(req);
 }
 
+/* Deprecated, please use the flow with nvmf_rdma_poll_group_dump_stat. */
 static int
 nvmf_rdma_poll_group_get_stat(struct spdk_nvmf_tgt *tgt,
 			      struct spdk_nvmf_transport_poll_group_stat **stat)
@@ -4162,6 +4163,8 @@ nvmf_rdma_poll_group_get_stat(struct spdk_nvmf_tgt *tgt,
 	struct spdk_nvmf_rdma_device_stat *device_stat;
 	uint64_t num_devices = 0;
 
+	SPDK_ERRLOG("nvmf_rdma_poll_group_get_stat is deprecated and will be removed\n");
+
 	if (tgt == NULL || stat == NULL) {
 		return -EINVAL;
 	}
@@ -4215,15 +4218,67 @@ nvmf_rdma_poll_group_get_stat(struct spdk_nvmf_tgt *tgt,
 	return -ENOENT;
 }
 
+/* Deprecated, please use the flow with nvmf_rdma_poll_group_dump_stat. */
 static void
 nvmf_rdma_poll_group_free_stat(struct spdk_nvmf_transport_poll_group_stat *stat)
 {
+	SPDK_ERRLOG("nvmf_rdma_poll_group_free_stat is deprecated and will be removed\n");
+
 	if (stat) {
 		free(stat->rdma.devices);
 	}
 	free(stat);
 }
 
+static void
+nvmf_rdma_poll_group_dump_stat(struct spdk_nvmf_transport_poll_group *group,
+			       struct spdk_json_write_ctx *w)
+{
+	struct spdk_nvmf_rdma_poll_group *rgroup;
+	struct spdk_nvmf_rdma_poller *rpoller;
+
+	assert(w != NULL);
+
+	rgroup = SPDK_CONTAINEROF(group, struct spdk_nvmf_rdma_poll_group, group);
+
+	spdk_json_write_named_uint64(w, "pending_data_buffer", rgroup->stat.pending_data_buffer);
+
+	spdk_json_write_named_array_begin(w, "devices");
+
+	TAILQ_FOREACH(rpoller, &rgroup->pollers, link) {
+		spdk_json_write_object_begin(w);
+		spdk_json_write_named_string(w, "name",
+					     ibv_get_device_name(rpoller->device->context->device));
+		spdk_json_write_named_uint64(w, "polls",
+					     rpoller->stat.polls);
+		spdk_json_write_named_uint64(w, "idle_polls",
+					     rpoller->stat.idle_polls);
+		spdk_json_write_named_uint64(w, "completions",
+					     rpoller->stat.completions);
+		spdk_json_write_named_uint64(w, "requests",
+					     rpoller->stat.requests);
+		spdk_json_write_named_uint64(w, "request_latency",
+					     rpoller->stat.request_latency);
+		spdk_json_write_named_uint64(w, "pending_free_request",
+					     rpoller->stat.pending_free_request);
+		spdk_json_write_named_uint64(w, "pending_rdma_read",
+					     rpoller->stat.pending_rdma_read);
+		spdk_json_write_named_uint64(w, "pending_rdma_write",
+					     rpoller->stat.pending_rdma_write);
+		spdk_json_write_named_uint64(w, "total_send_wrs",
+					     rpoller->stat.qp_stats.send.num_submitted_wrs);
+		spdk_json_write_named_uint64(w, "send_doorbell_updates",
+					     rpoller->stat.qp_stats.send.doorbell_updates);
+		spdk_json_write_named_uint64(w, "total_recv_wrs",
+					     rpoller->stat.qp_stats.recv.num_submitted_wrs);
+		spdk_json_write_named_uint64(w, "recv_doorbell_updates",
+					     rpoller->stat.qp_stats.recv.doorbell_updates);
+		spdk_json_write_object_end(w);
+	}
+
+	spdk_json_write_array_end(w);
+}
+
 const struct spdk_nvmf_transport_ops spdk_nvmf_transport_rdma = {
 	.name = "RDMA",
 	.type = SPDK_NVME_TRANSPORT_RDMA,
@@ -4257,6 +4312,7 @@ const struct spdk_nvmf_transport_ops spdk_nvmf_transport_rdma = {
 
 	.poll_group_get_stat = nvmf_rdma_poll_group_get_stat,
 	.poll_group_free_stat = nvmf_rdma_poll_group_free_stat,
+	.poll_group_dump_stat = nvmf_rdma_poll_group_dump_stat,
 };
 
 SPDK_NVMF_TRANSPORT_REGISTER(rdma, &spdk_nvmf_transport_rdma);

lib/nvmf/spdk_nvmf.map

@@ -79,6 +79,7 @@
 	spdk_nvmf_transport_stop_listen_async;
 	spdk_nvmf_transport_poll_group_get_stat;
 	spdk_nvmf_transport_poll_group_free_stat;
+	spdk_nvmf_poll_group_dump_stat;
 	spdk_nvmf_rdma_init_hooks;
 	spdk_nvmf_subsystem_set_ana_reporting;
 

lib/nvmf/transport.c

@@ -592,6 +592,8 @@ spdk_nvmf_transport_poll_group_get_stat(struct spdk_nvmf_tgt *tgt,
 					struct spdk_nvmf_transport *transport,
 					struct spdk_nvmf_transport_poll_group_stat **stat)
 {
+	SPDK_ERRLOG("spdk_nvmf_transport_poll_group_get_stat is deprecated and will be removed\n");
+
 	if (transport->ops->poll_group_get_stat) {
 		return transport->ops->poll_group_get_stat(tgt, stat);
 	} else {
@@ -603,6 +605,8 @@ void
 spdk_nvmf_transport_poll_group_free_stat(struct spdk_nvmf_transport *transport,
 		struct spdk_nvmf_transport_poll_group_stat *stat)
 {
+	SPDK_ERRLOG("spdk_nvmf_transport_poll_group_free_stat is deprecated and will be removed\n");
+
 	if (transport->ops->poll_group_free_stat) {
 		transport->ops->poll_group_free_stat(stat);
 	}