Re: [PATCH] nvmet-passthru: allow nvme ns-id descriptor list to clear EUID/NGUID/UUID

[Nilay Shroff <nilay@linux.ibm.com>]

On 9/17/24 14:21, Nilay Shroff wrote:
> When nvme passthru is configured using loop target, the clear_ids
> attribute is, by default, set to true. This attribute would ensure that
> EUID/NGUID/UUID is cleared for the loop passthru target.
> 
> The newer NVMe disk supporting the NVMe spec 1.3 or higher, typically,
> implements the support for "Namespace Identification Descriptor list"
> command. This command when issued from host returns EUID/NGUID/UUID
> assigned to the inquired namespace. Not clearing these values, while
> using nvme passthru using loop target, would result in NVMe host driver
> rejecting the namespace. This check was implemented in the commit
> 2079f41ec6ff ("nvme: check that EUI/GUID/UUID are globally unique").
> 
> The fix implemented in this commit ensure that when host issues ns-id
> descriptor list command, the EUID/NGUID/UUID are cleared by passthru
> target. In fact, the function nvmet_passthru_override_id_descs() which
> clears those unique ids already exits, so we just need to ensure that
> ns-id descriptor list command falls through the corretc code path. And
> while we're at it, we also combines the three passthru admin command
> cases together which shares the same code.
> 
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
>  drivers/nvme/target/passthru.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
> index 24d0e2418d2e..d786d8dee131 100644
> --- a/drivers/nvme/target/passthru.c
> +++ b/drivers/nvme/target/passthru.c
> @@ -535,10 +535,6 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req)
>  		break;
>  	case nvme_admin_identify:
>  		switch (req->cmd->identify.cns) {
> -		case NVME_ID_CNS_CTRL:
> -			req->execute = nvmet_passthru_execute_cmd;
> -			req->p.use_workqueue = true;
> -			return NVME_SC_SUCCESS;
>  		case NVME_ID_CNS_CS_CTRL:
>  			switch (req->cmd->identify.csi) {
>  			case NVME_CSI_ZNS:
> @@ -547,7 +543,11 @@ u16 nvmet_parse_passthru_admin_cmd(struct nvmet_req *req)
>  				return NVME_SC_SUCCESS;
>  			}
>  			return NVME_SC_INVALID_OPCODE | NVME_STATUS_DNR;
> +		case NVME_ID_CNS_CTRL:
> +			fallthrough;
>  		case NVME_ID_CNS_NS:
> +			fallthrough;
> +		case NVME_ID_CNS_NS_DESC_LIST:
>  			req->execute = nvmet_passthru_execute_cmd;
>  			req->p.use_workqueue = true;
>  			return NVME_SC_SUCCESS;

On a sidenote: This issue could be reproduced (where we find passthru target namespeace being 
ignored on the host due to duplicate IDs) using blktest nvme/033. I have a system with NVMe disk
which supports NVMe spec 1.3. On this system, when I run this blktest I can always reproduce 
this issue:

# nvme list -v 
Subsystem        Subsystem-NQN                                                                                    Controllers
---------------- ------------------------------------------------------------------------------------------------ ----------------
nvme-subsys0     nqn.1994-11.com.samsung:nvme:PM1735:2.5-inch:S6EUNA0R500358                                      nvme0

Device   SN                   MN                                       FR       TxPort Asdress        Slot   Subsystem    Namespaces      
-------- -------------------- ---------------------------------------- -------- ------ -------------- ------ ------------ ----------------
nvme0    S6EUNA0R500358       1.6TB NVMe Gen4 U.2 SSD                  REV.SN49 pcie   0018:01:00.0          nvme-subsys0 nvme0n1

Device       Generic      NSID       Usage                      Format           Controllers     
------------ ------------ ---------- -------------------------- ---------------- ----------------
/dev/nvme0n1 /dev/ng0n1   0x1          5.75  GB /   5.75  GB      4 KiB +  0 B   nvme0

# nvme show-regs /dev/nvme0 -H 
<snip>
version : 10300
	NVMe specification 1.3
<snip>

# ./check tests/nvme/033 
nvme/033 => nvme0n1 (tr=loop) (create and connect to an NVMeOF target with a passthru controller)

Here, the above blktest hangs indefinitely. 

And the relevant kernel logs:
# dmesg
nvmet: creating nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:0f01fb42-9f7f-4856-b0b3-51e60b8de349.
nvme nvme1: D3 entry latency set to 10 seconds
nvme nvme1: creating 32 I/O queues.
nvme nvme1: new ctrl: "blktests-subsystem-1"
nvme nvme1: Failed to configure AEN (cfg 300)
nvme nvme1: VID:144d model:1.6TB NVMe Gen4 U.2 SSD firmware:REV.SN49
nvme nvme1: ignoring nsid 1 because of duplicate IDs

I have another system using an NVMe disk supporting NVMe spec 1.2 and on this system, running 
the above test would PASS.

Moreover, I think we should fix the blktest nvme/033 so that it would bail out 
after some time instead of hanging indefinitely. Also we don't have an nvme-cli 
command which supports ns-id descriptor list command. We may also want to add 
support of this new command in nvme-cli? 

Thanks,
--Nilay