Commit 1a158640 authored by Konrad Sztyber's avatar Konrad Sztyber
Browse files

nvmf: use bdev's nsid for admin command passthru 



The ID of a namespace exposed by an NVMe-oF subsystem can differ from
the nsid of the underlying namespace that backs a given bdev.  For
instance, the following:

  $ scripts/rpc.py bdev_nvme_attach_controller -b nvme0 ...
  nvme0n1
  nvme0n2
  $ scripts/rpc.py nvmf_subsystem_add_ns -n 2 nqn nvme0n1
  $ scripts/rpc.py nvmf_subsystem_add_ns -n 1 nqn nvme0n2

results in nvme0n1 being exposed with nsid=2, while nvme0n2 with nsid=1.
In this scenario, admin commands targeting a namespace (e.g. identify
namespace) would get sent to the wrong namespace.

Fixes: 05632afd ("nvmf: large IU and atomic write unit reporting")
Reported-by: default avatarMaciej Szulik <maciej.szulik@intel.com>
Signed-off-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Change-Id: Ib4f7071afb10691c88def880fadc5f4d7e134b07
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/25364


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarMaciej Szulik <maciej.szulik@intel.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Reviewed-by: default avatarChangpeng Liu <changpeliu@tencent.com>
Community-CI: Community CI Samsung <spdk.community.ci.samsung@gmail.com>
Community-CI: Mellanox Build Bot
parent 892c29f4

include/spdk/nvmf_transport.h

+2 −1
Original line number Diff line number Diff line
@@ -127,8 +127,9 @@ struct spdk_nvmf_request { 


	/* Timeout tracked for connect and abort flows. */

	uint64_t timeout_tsc;

	uint32_t			orig_nsid;

};

SPDK_STATIC_ASSERT(sizeof(struct spdk_nvmf_request) == 808, "Incorrect size");

SPDK_STATIC_ASSERT(sizeof(struct spdk_nvmf_request) == 816, "Incorrect size");



enum spdk_nvmf_qpair_state {

	SPDK_NVMF_QPAIR_UNINITIALIZED = 0,

lib/nvmf/ctrlr.c

+6 −2
Original line number Diff line number Diff line
@@ -2839,8 +2839,7 @@ identify_ns_passthru_cb(struct spdk_nvmf_request *req) 
	/* This is the identify data from the NVMe drive */

	datalen = spdk_nvmf_request_copy_to_buf(req, &nvme_nsdata,

						sizeof(nvme_nsdata));



	nvmf_ctrlr_identify_ns(ctrlr, cmd, rsp, &nvmf_nsdata, cmd->nsid);

	nvmf_ctrlr_identify_ns(ctrlr, cmd, rsp, &nvmf_nsdata, req->orig_nsid);



	/* Update fabric's namespace according to SSD's namespace */

	if (nvme_nsdata.nsfeat.optperf) {
@@ -2871,6 +2870,7 @@ spdk_nvmf_ctrlr_identify_ns_ext(struct spdk_nvmf_request *req) 
{

	struct spdk_nvme_cmd *cmd = spdk_nvmf_request_get_cmd(req);

	struct spdk_nvmf_ctrlr *ctrlr = spdk_nvmf_request_get_ctrlr(req);

	struct spdk_nvmf_ns *ns = nvmf_ctrlr_get_ns(ctrlr, cmd->nsid);

	struct spdk_nvme_cpl *rsp = spdk_nvmf_request_get_response(req);

	struct spdk_bdev *bdev;

	struct spdk_bdev_desc *desc;
@@ -2893,6 +2893,10 @@ spdk_nvmf_ctrlr_identify_ns_ext(struct spdk_nvmf_request *req) 
		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	assert(ns->passthrough_nsid != 0);

	req->orig_nsid = ns->nsid;

	cmd->nsid = ns->passthrough_nsid;



	return spdk_nvmf_bdev_ctrlr_nvme_passthru_admin(bdev, desc, ch, req, identify_ns_passthru_cb);

}

test/nvmf/nvmf_target_extra.sh

+1 −0
Original line number Diff line number Diff line
@@ -64,5 +64,6 @@ if [[ $NET_TYPE == phy ]]; then 
	fi

	run_test "nvmf_shutdown" $rootdir/test/nvmf/target/shutdown.sh "${TEST_ARGS[@]}"

fi

run_test "nvmf_nsid" "$rootdir/test/nvmf/target/nsid.sh" "${TEST_ARGS[@]}"



trap - SIGINT SIGTERM EXIT

test/nvmf/target/nsid.sh

0 → 100755
+104 −0
Original line number Diff line number Diff line 
#!/usr/bin/env bash

# SPDX-License-Identifier: BSD-3-Clause

# Copyright (c) 2024 Intel Corporation



testdir=$(readlink -f "$(dirname "$0")")

rootdir=$(readlink -f "$testdir/../../../")



source "$rootdir/test/common/autotest_common.sh"

source "$rootdir/test/nvmf/common.sh"



subnqn1="nqn.2024-10.io.spdk:cnode0"

subnqn2="nqn.2024-10.io.spdk:cnode1"

subnqn3="nqn.2024-10.io.spdk:cnode2"

tgt2sock="/var/tmp/tgt2.sock"

tgt2pid=



cleanup() {

	killprocess $tgt2pid || :

	nvmftestfini || :

}



nvme_connect() {

	local ctrlr



	nvme connect -t "$TEST_TRANSPORT" -a "$tgt2addr" -s "$NVMF_SECOND_PORT" -n "$subnqn3" \

		"${NVME_HOST[@]}" "$@"



	for ctrlr in /sys/class/nvme/nvme*; do

		# shellcheck disable=SC2053

		[[ -e $ctrlr/subsysnqn && $(< "$ctrlr/subsysnqn") == "$subnqn3" ]] || continue

		echo "${ctrlr##*/}"

		return

	done



	nvme disconnect -n "$subnqn3"

	false

}



nvme_get_nguid() {

	local ctrlr=$1 nsid=$2 nguid



	nguid=$(nvme id-ns "/dev/${ctrlr}n${nsid}" -o json | jq -r '.nguid')

	echo "${nguid^^}"

}



nvmftestinit

nvmfappstart -m 1



trap cleanup SIGINT SIGTERM EXIT



"$rootdir/build/bin/spdk_tgt" -m 2 -r "$tgt2sock" &

tgt2pid=$!



tgt1addr="$NVMF_FIRST_TARGET_IP"

tgt2addr="$(get_main_ns_ip)"

ns1uuid=$(uuidgen)

ns2uuid=$(uuidgen)

ns3uuid=$(uuidgen)



# Start up two targets, both exposing 3 namespaces. Target #2 connects to the subsystems from

# target #1 and configures its namespaces as follows: nvme0n1 -> nsid:2, nvme0n2 -> nsid:1,

# nvme1n1 -> nsid:3.

rpc_cmd <<- CONFIG

	bdev_null_create null0 100 4096 -u "$ns2uuid"

	bdev_null_create null1 100 4096 -u "$ns1uuid"

	bdev_null_create null2 100 4096 -u "$ns3uuid"

	nvmf_create_transport -t "$TEST_TRANSPORT"

	nvmf_create_subsystem "$subnqn1" -a

	nvmf_create_subsystem "$subnqn2" -a

	nvmf_subsystem_add_listener -t "$TEST_TRANSPORT" -a "$tgt1addr" \

		-s "$NVMF_PORT" "$subnqn1"

	nvmf_subsystem_add_listener -t "$TEST_TRANSPORT" -a "$tgt1addr" \

		-s "$NVMF_PORT" "$subnqn2"

	nvmf_subsystem_add_ns "$subnqn1" null0 -n 1

	nvmf_subsystem_add_ns "$subnqn1" null1 -n 2

	nvmf_subsystem_add_ns "$subnqn2" null2 -n 1

CONFIG



waitforlisten "$tgt2pid" "$tgt2sock"

"$rootdir/scripts/rpc.py" -s "$tgt2sock" <<- CONFIG

	bdev_nvme_attach_controller -t "$TEST_TRANSPORT" -a "$tgt1addr" \

		-s "$NVMF_PORT" -f ipv4 -n "$subnqn1" -b nvme0

	bdev_nvme_attach_controller -t "$TEST_TRANSPORT" -a "$tgt1addr" \

		-s "$NVMF_PORT" -f ipv4 -n "$subnqn2" -b nvme1

	nvmf_create_transport -t "$TEST_TRANSPORT"

	nvmf_create_subsystem "$subnqn3" -a

	nvmf_subsystem_add_listener -t "$TEST_TRANSPORT" -a "$tgt2addr" \

		-s "$NVMF_SECOND_PORT" "$subnqn3"

	nvmf_subsystem_add_ns "$subnqn3" nvme0n2 -n 1

	nvmf_subsystem_add_ns "$subnqn3" nvme0n1 -n 2

	nvmf_subsystem_add_ns "$subnqn3" nvme1n1 -n 3

CONFIG

# Ensure that the nguids returned in id-ns match target #1's configuration

ctrlr="$(nvme_connect)"

waitforblk "${ctrlr}n1"

[[ "$(uuid2nguid "$ns1uuid")" == "$(nvme_get_nguid "$ctrlr" 1)" ]]

waitforblk "${ctrlr}n2"

[[ "$(uuid2nguid "$ns2uuid")" == "$(nvme_get_nguid "$ctrlr" 2)" ]]

waitforblk "${ctrlr}n3"

[[ "$(uuid2nguid "$ns3uuid")" == "$(nvme_get_nguid "$ctrlr" 3)" ]]

nvme disconnect -d "/dev/$ctrlr"



trap - SIGINT SIGTERM EXIT

cleanup