Commit 5c9a8a3c authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

lib/nvmf: Use qpair_get_listen_trid() to find a subsystem listener 



There is a public API spdk_nvmf_qpair_get_listen_trid() and it can
get trid of the specified qpair safely for any transport including
pluggable transports.

The API was overlooked when implementing the multipath feature for
NVMe-oF target. trid pointer was added to struct spdk_nvmf_qpair and
was used to find a subsystem listener.

However, pluggable transports got seg. fault because trid of the
qpair was not set.

To avoid such segmentation fault for any transport, change
nvmf_ctrlr_create() to use spdk_nvmf_qpair_get_listen_trid().

The struct spdk_nvmf_qpair is located in the public header file,
and so leave the added trid for now. It will be deprecated eventually.

Reported-by: default avatarJacek Kalwas <jacek.kalwas@intel.com>
Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5e0bd24bd58b6ffdf1352332a179a82682f1589f
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5323


Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarJacek Kalwas <jacek.kalwas@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent 92e541b5

lib/nvmf/ctrlr.c

+17 −3
Original line number Diff line number Diff line
@@ -317,6 +317,7 @@ nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, 
{

	struct spdk_nvmf_ctrlr	*ctrlr;

	struct spdk_nvmf_transport *transport;

	struct spdk_nvme_transport_id listen_trid = {};



	ctrlr = calloc(1, sizeof(*ctrlr));

	if (ctrlr == NULL) {
@@ -425,8 +426,13 @@ nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, 
	ctrlr->dif_insert_or_strip = transport->opts.dif_insert_or_strip;



	if (ctrlr->subsys->subtype == SPDK_NVMF_SUBTYPE_NVME) {

		ctrlr->listener = nvmf_subsystem_find_listener(ctrlr->subsys,

				  req->qpair->trid);

		if (spdk_nvmf_qpair_get_listen_trid(req->qpair, &listen_trid) != 0) {

			SPDK_ERRLOG("Could not get listener transport ID\n");

			free(ctrlr);

			return NULL;

		}



		ctrlr->listener = nvmf_subsystem_find_listener(ctrlr->subsys, &listen_trid);

		if (!ctrlr->listener) {

			SPDK_ERRLOG("Listener was not found\n");

			free(ctrlr);
@@ -527,6 +533,7 @@ _nvmf_ctrlr_add_io_qpair(void *ctx) 
	struct spdk_nvmf_qpair *admin_qpair;

	struct spdk_nvmf_tgt *tgt = qpair->transport->tgt;

	struct spdk_nvmf_subsystem *subsystem;

	struct spdk_nvme_transport_id listen_trid = {};

	const struct spdk_nvmf_subsystem_listener *listener;



	SPDK_DEBUGLOG(nvmf, "Connect I/O Queue for controller id 0x%x\n", data->cntlid);
@@ -553,7 +560,14 @@ _nvmf_ctrlr_add_io_qpair(void *ctx) 


	/* If ANA reporting is enabled, check if I/O connect is on the same listener. */

	if (subsystem->flags.ana_reporting) {

		listener = nvmf_subsystem_find_listener(subsystem, qpair->trid);

		if (spdk_nvmf_qpair_get_listen_trid(req->qpair, &listen_trid) != 0) {

			SPDK_ERRLOG("Could not get listener transport ID\n");

			SPDK_NVMF_INVALID_CONNECT_CMD(rsp, qid);

			spdk_nvmf_request_complete(req);

			return;

		}



		listener = nvmf_subsystem_find_listener(subsystem, &listen_trid);

		if (listener != ctrlr->listener) {

			SPDK_ERRLOG("I/O connect is on a listener different from admin connect\n");

			SPDK_NVMF_INVALID_CONNECT_CMD(rsp, qid);

lib/nvmf/nvmf_rpc.c

+12 −10
Original line number Diff line number Diff line
@@ -2276,7 +2276,7 @@ nvmf_qpair_state_str(enum spdk_nvmf_qpair_state state) 
static void

dump_nvmf_qpair(struct spdk_json_write_ctx *w, struct spdk_nvmf_qpair *qpair)

{

	const struct spdk_nvme_transport_id *trid = qpair->trid;

	struct spdk_nvme_transport_id listen_trid = {};

	const char *adrfam;



	spdk_json_write_object_begin(w);
@@ -2285,16 +2285,18 @@ dump_nvmf_qpair(struct spdk_json_write_ctx *w, struct spdk_nvmf_qpair *qpair) 
	spdk_json_write_named_uint32(w, "qid", qpair->qid);

	spdk_json_write_named_string(w, "state", nvmf_qpair_state_str(qpair->state));



	if (spdk_nvmf_qpair_get_listen_trid(qpair, &listen_trid) == 0) {

		spdk_json_write_named_object_begin(w, "listen_address");

	adrfam = spdk_nvme_transport_id_adrfam_str(trid->adrfam);

		adrfam = spdk_nvme_transport_id_adrfam_str(listen_trid.adrfam);

		if (adrfam == NULL) {

			adrfam = "unknown";

		}

	spdk_json_write_named_string(w, "trtype", trid->trstring);

		spdk_json_write_named_string(w, "trtype", listen_trid.trstring);

		spdk_json_write_named_string(w, "adrfam", adrfam);

	spdk_json_write_named_string(w, "traddr", trid->traddr);

	spdk_json_write_named_string(w, "trsvcid", trid->trsvcid);

		spdk_json_write_named_string(w, "traddr", listen_trid.traddr);

		spdk_json_write_named_string(w, "trsvcid", listen_trid.trsvcid);

		spdk_json_write_object_end(w);

	}



	spdk_json_write_object_end(w);

}