From d8843dccc583edc283a71795d5ecdb955d2f898e Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Tue, 25 May 2021 16:53:20 +0800 Subject: [PATCH] nvmf/reservation: register new registrant when IEKEY is enabled The specification says: "A host may replace its reservation key without regard to its registration status or current reservation key value by setting the Ignore Existing Key (IEKEY) bit to '1' in the Reservation Register command." So for this case we treat it as a new registrant, also add UT to cover the added cases. Change-Id: I5990f15da36706063a35565d110ed4c6eb30a3f3 Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8024 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Ziye Yang Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk Reviewed-by: Shuhei Matsumoto --- lib/nvmf/subsystem.c | 32 +++++++++++++++---- test/unit/lib/nvmf/subsystem.c/subsystem_ut.c | 23 +++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index d6fbd75bd9..e38a308677 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -2399,18 +2399,36 @@ nvmf_ns_reservation_register(struct spdk_nvmf_ns *ns, update_sgroup = true; break; case SPDK_NVME_RESERVE_REPLACE_KEY: - if (!reg || (!iekey && reg->rkey != key.crkey)) { - SPDK_ERRLOG("No registrant or current key doesn't match " - "with existing registrant key\n"); - status = SPDK_NVME_SC_RESERVATION_CONFLICT; - goto exit; - } if (key.nrkey == 0) { SPDK_ERRLOG("Can't register zeroed new key\n"); status = SPDK_NVME_SC_INVALID_FIELD; goto exit; } - reg->rkey = key.nrkey; + /* Registrant exists */ + if (reg) { + if (!iekey && reg->rkey != key.crkey) { + SPDK_ERRLOG("Current key doesn't match " + "existing registrant key\n"); + status = SPDK_NVME_SC_RESERVATION_CONFLICT; + goto exit; + } + if (reg->rkey == key.nrkey) { + goto exit; + } + reg->rkey = key.nrkey; + } else if (iekey) { /* No registrant but IEKEY is set */ + /* new registrant */ + rc = nvmf_ns_reservation_add_registrant(ns, ctrlr, key.nrkey); + if (rc < 0) { + status = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR; + goto exit; + } + } else { /* No registrant */ + SPDK_ERRLOG("No registrant\n"); + status = SPDK_NVME_SC_RESERVATION_CONFLICT; + goto exit; + + } update_sgroup = true; break; default: diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c index bc396be119..55f7572956 100644 --- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +++ b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c @@ -784,6 +784,29 @@ test_reservation_register(void) reg = nvmf_ns_reservation_get_registrant(&g_ns, &g_ctrlr_B.hostid); SPDK_CU_ASSERT_FATAL(reg == NULL); + /* TEST CASE: No registrant now, g_ctrlr_B replace new key with IEKEY disabled */ + ut_reservation_build_register_request(req, SPDK_NVME_RESERVE_REPLACE_KEY, + 0, 0, 0, 0xb1); + nvmf_ns_reservation_register(&g_ns, &g_ctrlr_B, req); + SPDK_CU_ASSERT_FATAL(rsp->status.sc != SPDK_NVME_SC_SUCCESS); + + /* TEST CASE: No registrant now, g_ctrlr_B replace new key with IEKEY enabled */ + ut_reservation_build_register_request(req, SPDK_NVME_RESERVE_REPLACE_KEY, + 1, 0, 0, 0xb1); + nvmf_ns_reservation_register(&g_ns, &g_ctrlr_B, req); + SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); + reg = nvmf_ns_reservation_get_registrant(&g_ns, &g_ctrlr_B.hostid); + SPDK_CU_ASSERT_FATAL(reg != NULL); + + /* TEST CASE: g_ctrlr_B replace new key with IEKEY enabled and wrong crkey */ + ut_reservation_build_register_request(req, SPDK_NVME_RESERVE_REPLACE_KEY, + 1, 0, 0xff, 0xb2); + nvmf_ns_reservation_register(&g_ns, &g_ctrlr_B, req); + SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); + reg = nvmf_ns_reservation_get_registrant(&g_ns, &g_ctrlr_B.hostid); + SPDK_CU_ASSERT_FATAL(reg != NULL); + SPDK_CU_ASSERT_FATAL(reg->rkey == 0xb2); + /* TEST CASE: g_ctrlr1_A unregister with correct key, * reservation should be removed as well. */