From 2e56512236c050c49d54bba85ce6dfd460ece05f Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Wed, 26 Apr 2023 20:53:47 +0000 Subject: [PATCH] nvmf: fix comparison in nvmf_stop_listen_disconnect_qpairs This function disconnects any qpairs that match both the listen trid and the subsystem pointer. If the specified subsystem is NULL, it will just disconnect all qpairs matching the listen trid. But there are cases where a qpair doesn't yet have an associated subsystem - for example, before a CONNECT is received. Currently we would always disconnect such a qpair, even if a subsystem pointer is passed. Presumably this check was added to ensure we don't dereference qpair->ctrlr when it is NULL but it was added incorrectly. Also while here, move and improve the comment about skipping qpairs. Signed-off-by: Jim Harris Change-Id: I8b7988b22799de2a069be692f4a5b4da59c2bad4 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17854 Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot --- lib/nvmf/transport.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/nvmf/transport.c b/lib/nvmf/transport.c index 75895fd90..e87e1e7e1 100644 --- a/lib/nvmf/transport.c +++ b/lib/nvmf/transport.c @@ -493,14 +493,16 @@ nvmf_stop_listen_disconnect_qpairs(struct spdk_io_channel_iter *i) group = spdk_io_channel_get_ctx(ch); TAILQ_FOREACH_SAFE(qpair, &group->qpairs, link, tmp_qpair) { - /* skip qpairs that don't match the TRID. */ if (spdk_nvmf_qpair_get_listen_trid(qpair, &tmp_trid)) { continue; } + /* Skip qpairs that don't match the listen trid and subsystem pointer. If + * the ctx->subsystem is NULL, it means disconnect all qpairs that match + * the listen trid. */ if (!spdk_nvme_transport_id_compare(&ctx->trid, &tmp_trid)) { - if (ctx->subsystem == NULL || qpair->ctrlr == NULL || - ctx->subsystem == qpair->ctrlr->subsys) { + if (ctx->subsystem == NULL || + (qpair->ctrlr != NULL && ctx->subsystem == qpair->ctrlr->subsys)) { spdk_nvmf_qpair_disconnect(qpair, NULL, NULL); qpair_found = true; }