Commit a5cbb2b1 authored by Marcin Spiewak's avatar Marcin Spiewak Committed by Konrad Sztyber
Browse files

bdev/nvme: ctrl config consistency check 



All controllers created with the same name shall be configured either
for multipath or for failover. Otherwise we have configuration mismatch.
This patch adds cofiguration consistency check, and all controllers
created with the same name will be forced to have consistent setting,
either '-x multipath' or '-x failover'. No mixing of '-x' options is
allowed.
Deprecation warning has been removed as we are now implementing the
advertised change.

Change-Id: I1bc472f95950e4326ca67bed33a1bf9523683843
Signed-off-by: default avatarMarcin Spiewak <marcin.spiewak@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/25028


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
parent 3950cd1b

module/bdev/nvme/bdev_nvme.c

+22 −26
Original line number Diff line number Diff line
@@ -5574,8 +5574,6 @@ bdev_nvme_check_multipath(struct nvme_bdev_ctrlr *nbdev_ctrlr, struct spdk_nvme_ 
	return true;

}



SPDK_LOG_DEPRECATION_REGISTER(multipath_config,

			      "bdev_nvme_attach_controller.multipath configuration mismatch", "v25.01", 0);



static int

nvme_bdev_ctrlr_create(const char *name, struct nvme_ctrlr *nvme_ctrlr)
@@ -5595,17 +5593,12 @@ nvme_bdev_ctrlr_create(const char *name, struct nvme_ctrlr *nvme_ctrlr) 
		}

		TAILQ_FOREACH(nctrlr, &nbdev_ctrlr->ctrlrs, tailq) {

			if (nctrlr->opts.multipath != nvme_ctrlr->opts.multipath) {

				/* All controllers created with the same name must be configured either

				 * for multipath or for failover. Otherwise we have configuration mismatch.

				 * While this is currently still supported, support for configuration where some

				 * controllers with the same name are configured for multipath, while others

				 * are configured for failover will be removed in release 25.01.

				 * Default mode change: starting from SPDK 25.01, if the user will not provide

				 * '-x <mode>' parameter in the bdev_nvme_attach_controller RPC call, default

				 * mode assigned to the controller will be 'multipath'

				/* All controllers with the same name must be configured the same

				 * way, either for multipath or failover. If the configuration doesn't

				 * match - report error.

				 */

				SPDK_LOG_DEPRECATED(multipath_config);

				break;

				rc = -EINVAL;

				goto exit;

			}

		}

	} else {
@@ -6223,8 +6216,6 @@ connect_attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, 
	}

}



SPDK_LOG_DEPRECATION_REGISTER(failover_config,

			      "bdev_nvme_attach_controller.failover configuration mismatch", "v25.01", 0);



static void

connect_set_failover_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid,
@@ -6241,18 +6232,6 @@ connect_set_failover_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, 


	nvme_ctrlr = nvme_ctrlr_get_by_name(ctx->base_name);

	if (nvme_ctrlr) {

		if (nvme_ctrlr->opts.multipath) {

			/* All controllers created with the same name must be configured either

			 * for multipath or for failover. Otherwise we have configuration mismatch.

			 * While this is currently still supported, support for configuration where some

			 * controllers with the same name are configured for multipath, while others

			 * are configured for failover will be removed in release 25.01.

			 * Default mode change: starting from SPDK 25.01, if the user will not provide

			 * '-x <mode>' parameter in the bdev_nvme_attach_controller RPC call, default

			 * mode assigned to the controller will be 'multipath'

			 */

			SPDK_LOG_DEPRECATED(failover_config);

		}

		rc = bdev_nvme_add_secondary_trid(nvme_ctrlr, ctrlr, &ctx->trid);

	} else {

		rc = -ENODEV;
@@ -6347,6 +6326,7 @@ spdk_bdev_nvme_create(struct spdk_nvme_transport_id *trid, 
	struct nvme_probe_skip_entry *entry, *tmp;

	struct nvme_async_probe_ctx *ctx;

	spdk_nvme_attach_cb attach_cb;

	struct nvme_ctrlr *nvme_ctrlr;

	int len;



	/* TODO expand this check to include both the host and target TRIDs.
@@ -6444,6 +6424,16 @@ spdk_bdev_nvme_create(struct spdk_nvme_transport_id *trid, 
		attach_cb = connect_set_failover_cb;

	}



	nvme_ctrlr = nvme_ctrlr_get_by_name(ctx->base_name);

	if (nvme_ctrlr  && nvme_ctrlr->opts.multipath != multipath) {

		/* All controllers with the same name must be configured the same

		 * way, either for multipath or failover. If the configuration doesn't

		 * match - report error.

		 */

		free_nvme_async_probe_ctx(ctx);

		return -EINVAL;

	}



	ctx->probe_ctx = spdk_nvme_connect_async(trid, &ctx->drv_opts, attach_cb);

	if (ctx->probe_ctx == NULL) {

		SPDK_ERRLOG("No controller was found with provided trid (traddr: %s)\n", trid->traddr);
@@ -7218,6 +7208,12 @@ discovery_poller(void *arg) 
		ctx->entry_ctx_in_use = TAILQ_FIRST(&ctx->discovery_entry_ctxs);

		TAILQ_REMOVE(&ctx->discovery_entry_ctxs, ctx->entry_ctx_in_use, tailq);

		trid = &ctx->entry_ctx_in_use->trid;



		/* All controllers must be configured explicitely either for multipath or failover.

		 * While discovery use multipath mode, we need to set this in bdev options as well.

		 */

		ctx->bdev_opts.multipath = true;



		ctx->probe_ctx = spdk_nvme_connect_async(trid, &ctx->drv_opts, discovery_attach_cb);

		if (ctx->probe_ctx) {

			spdk_poller_unregister(&ctx->poller);

test/nvmf/host/failover.sh

+6 −6
Original line number Diff line number Diff line
@@ -32,8 +32,8 @@ bdevperf_pid=$! 


trap 'process_shm --id $NVMF_APP_SHM_ID; cat $testdir/try.txt; rm -f $testdir/try.txt; killprocess $bdevperf_pid; nvmftestfini; exit 1' SIGINT SIGTERM EXIT

waitforlisten $bdevperf_pid $bdevperf_rpc_sock

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -x failover

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -x failover



$rootdir/examples/bdev/bdevperf/bdevperf.py -s $bdevperf_rpc_sock perform_tests &

run_test_pid=$!
@@ -44,7 +44,7 @@ $rpc_py nvmf_subsystem_remove_listener nqn.2016-06.io.spdk:cnode1 -t $TEST_TRANS 


sleep 3



$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_THIRD_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_THIRD_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -x failover

$rpc_py nvmf_subsystem_remove_listener nqn.2016-06.io.spdk:cnode1 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT



sleep 3
@@ -75,9 +75,9 @@ bdevperf_pid=$! 
waitforlisten $bdevperf_pid $bdevperf_rpc_sock

$rpc_py nvmf_subsystem_add_listener nqn.2016-06.io.spdk:cnode1 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT

$rpc_py nvmf_subsystem_add_listener nqn.2016-06.io.spdk:cnode1 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_THIRD_PORT

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_THIRD_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -x failover

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -x failover

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_THIRD_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -x failover



$rpc_py -s $bdevperf_rpc_sock bdev_nvme_get_controllers | grep -q NVMe0

test/nvmf/host/multipath.sh

+1 −1
Original line number Diff line number Diff line
@@ -51,7 +51,7 @@ waitforlisten $bdevperf_pid $bdevperf_rpc_sock 
$rpc_py -s $bdevperf_rpc_sock bdev_nvme_set_options -r -1

# -l -1 ctrlr_loss_timeout_sec -1 means infinite reconnects

# -o 10 reconnect_delay_sec time to delay a reconnect retry is limited to 10 sec

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b Nvme0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n $NQN -l -1 -o 10

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b Nvme0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n $NQN -x multipath -l -1 -o 10

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b Nvme0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT -f ipv4 -n $NQN -x multipath -l -1 -o 10



function set_ANA_state() {

test/nvmf/host/multipath_status.sh

+1 −1
Original line number Diff line number Diff line
@@ -52,7 +52,7 @@ waitforlisten $bdevperf_pid $bdevperf_rpc_sock 
$rpc_py -s $bdevperf_rpc_sock bdev_nvme_set_options -r -1

# -l -1 ctrlr_loss_timeout_sec -1 means infinite reconnects

# -o 10 reconnect_delay_sec time to delay a reconnect retry is limited to 10 sec

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b Nvme0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n $NQN -l -1 -o 10

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b Nvme0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT -f ipv4 -n $NQN -x multipath -l -1 -o 10

$rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b Nvme0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_SECOND_PORT -f ipv4 -n $NQN -x multipath -l -1 -o 10



function set_ANA_state() {

test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c

+163 −51

File changed.

Preview size limit exceeded, changes collapsed.