more nvmet Identify fixups

more nvmet Identify fixups

[Christoph Hellwig]

Hi all,

here is two more patches on top of the work from Damien that I've
applied to nvme-6.4.

[PATCH 1/2] nvmet: fix Identify Identification Descriptor List handling

[Christoph Hellwig]

The Identification Descriptor List CNS value does not check the CSI
value, so remove the code trying to handle it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/admin-cmd.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 3c4f2ddc09604e..c0908031ce25ee 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -668,23 +668,6 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)
 	nvmet_req_complete(req, status);
 }
 
-static bool nvmet_handle_identify_desclist(struct nvmet_req *req)
-{
-	switch (req->cmd->identify.csi) {
-	case NVME_CSI_NVM:
-		nvmet_execute_identify_desclist(req);
-		return true;
-	case NVME_CSI_ZNS:
-		if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
-			nvmet_execute_identify_desclist(req);
-			return true;
-		}
-		return false;
-	default:
-		return false;
-	}
-}
-
 static void nvmet_execute_identify_ctrl_nvm(struct nvmet_req *req)
 {
 	/* Not supported: return zeroes */
@@ -708,8 +691,7 @@ static void nvmet_execute_identify(struct nvmet_req *req)
 		nvmet_execute_identify_nslist(req);
 		return;
 	case NVME_ID_CNS_NS_DESC_LIST:
-		if (nvmet_handle_identify_desclist(req) == true)
-			return;
+		nvmet_execute_identify_desclist(req);
 		break;
 	case NVME_ID_CNS_CS_NS:
 		switch (req->cmd->identify.csi) {
-- 
2.39.2

[PATCH 2/2] nvmet: rename nvmet_execute_identify_cns_cs_ns

[Christoph Hellwig]

nvmet_execute_identify_ns_zns is a more descriptive name for the
function handling the "I/O Command Set Specific Identify Namespace
Data Structure for the Zoned Namespace Command Set".

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/admin-cmd.c | 2 +-
 drivers/nvme/target/nvmet.h     | 2 +-
 drivers/nvme/target/zns.c       | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index c0908031ce25ee..4ee30a4da50990 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -700,7 +700,7 @@ static void nvmet_execute_identify(struct nvmet_req *req)
 			break;
 		case NVME_CSI_ZNS:
 			if (IS_ENABLED(CONFIG_BLK_DEV_ZONED)) {
-				nvmet_execute_identify_cns_cs_ns(req);
+				nvmet_execute_identify_ns_zns(req);
 				return;
 			}
 			break;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index ed3786140965f0..2c1522d4063bde 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -582,7 +582,7 @@ u16 blk_to_nvme_status(struct nvmet_req *req, blk_status_t blk_sts);
 
 bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
 void nvmet_execute_identify_ctrl_zns(struct nvmet_req *req);
-void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req);
+void nvmet_execute_identify_ns_zns(struct nvmet_req *req);
 void nvmet_bdev_execute_zone_mgmt_recv(struct nvmet_req *req);
 void nvmet_bdev_execute_zone_mgmt_send(struct nvmet_req *req);
 void nvmet_bdev_execute_zone_append(struct nvmet_req *req);
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index ae4d9d35e46ae4..5b5c1e48172213 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -95,7 +95,7 @@ void nvmet_execute_identify_ctrl_zns(struct nvmet_req *req)
 	nvmet_req_complete(req, status);
 }
 
-void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
+void nvmet_execute_identify_ns_zns(struct nvmet_req *req)
 {
 	struct nvme_id_ns_zns *id_zns = NULL;
 	u64 zsze;
-- 
2.39.2

Re: [PATCH 1/2] nvmet: fix Identify Identification Descriptor List handling

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 2/2] nvmet: rename nvmet_execute_identify_cns_cs_ns

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: more nvmet Identify fixups

[Chaitanya Kulkarni]

On 3/15/23 07:20, Christoph Hellwig wrote:
> Hi all,
>
> here is two more patches on top of the work from Damien that I've
> applied to nvme-6.4.

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH 1/2] nvmet: fix Identify Identification Descriptor List handling

[Damien Le Moal]

On 2023/03/15 23:20, Christoph Hellwig wrote:
> The Identification Descriptor List CNS value does not check the CSI
> value, so remove the code trying to handle it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] nvmet: rename nvmet_execute_identify_cns_cs_ns

[Damien Le Moal]

On 2023/03/15 23:20, Christoph Hellwig wrote:
> nvmet_execute_identify_ns_zns is a more descriptive name for the
> function handling the "I/O Command Set Specific Identify Namespace
> Data Structure for the Zoned Namespace Command Set".
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

-- 
Damien Le Moal
Western Digital Research

Re: more nvmet Identify fixups

[Chaitanya Kulkarni]

On 3/15/23 15:52, Chaitanya Kulkarni wrote:
> On 3/15/23 07:20, Christoph Hellwig wrote:
>> Hi all,
>>
>> here is two more patches on top of the work from Damien that I've
>> applied to nvme-6.4.
> Looks good.
>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
>
> -ck
>
>

I think you need this on the top of this,
I believe it will result in error, please check:-

diff --git a/drivers/nvme/target/admin-cmd.c 
b/drivers/nvme/target/admin-cmd.c
index 4ee30a4da509..afdf69f92599 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -692,7 +692,7 @@ static void nvmet_execute_identify(struct nvmet_req 
*req)
                 return;
         case NVME_ID_CNS_NS_DESC_LIST:
                 nvmet_execute_identify_desclist(req);
-               break;
+               return;
         case NVME_ID_CNS_CS_NS:
                 switch (req->cmd->identify.csi) {
                 case NVME_CSI_NVM:

-ck

Re: more nvmet Identify fixups

[Chaitanya Kulkarni]

On 3/16/23 00:14, Chaitanya Kulkarni wrote:
> On 3/15/23 15:52, Chaitanya Kulkarni wrote:
>> On 3/15/23 07:20, Christoph Hellwig wrote:
>>> Hi all,
>>>
>>> here is two more patches on top of the work from Damien that I've
>>> applied to nvme-6.4.
>> Looks good.
>>
>> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
>>
>> -ck
>>
>>
>
> I think you need this on the top of this,
> I believe it will result in error, please check:-
>
> diff --git a/drivers/nvme/target/admin-cmd.c 
> b/drivers/nvme/target/admin-cmd.c
> index 4ee30a4da509..afdf69f92599 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -692,7 +692,7 @@ static void nvmet_execute_identify(struct 
> nvmet_req *req)
>                 return;
>         case NVME_ID_CNS_NS_DESC_LIST:
>                 nvmet_execute_identify_desclist(req);
> -               break;
> +               return;
>         case NVME_ID_CNS_CS_NS:
>                 switch (req->cmd->identify.csi) {
>                 case NVME_CSI_NVM:
>
> -ck
>
>

Indeed is resulting in error, without above fix, unless I messed up
applying patch :-

nvme (nvme-6.4) # git diff
diff --git a/drivers/nvme/target/admin-cmd.c 
b/drivers/nvme/target/admin-cmd.c
index 4ee30a4da509..4ede1e4db7eb 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -692,6 +692,7 @@ static void nvmet_execute_identify(struct nvmet_req 
*req)
                 return;
         case NVME_ID_CNS_NS_DESC_LIST:
                 nvmet_execute_identify_desclist(req);
+               pr_info("%s %d\n", __func__, __LINE__);
                 break;
         case NVME_ID_CNS_CS_NS:
                 switch (req->cmd->identify.csi) {
@@ -720,7 +721,7 @@ static void nvmet_execute_identify(struct nvmet_req 
*req)
                 }
                 break;
         }
-
+       pr_info("%s %d\n", __func__, __LINE__);
         nvmet_req_cns_error_complete(req);
  }

[  802.029489] nvmet: creating nvm controller 1 for subsystem testnqn 
for NQN 
nqn.2014-08.org.nvmexpress:uuid:b65cece3-7e60-4d3a-9fd9-581db201b191.
[  802.029598] nvme nvme1: creating 48 I/O queues.
[  802.033240] nvme nvme1: new ctrl: "testnqn"
[  802.033320] nvmet: nvmet_execute_identify 695
[  802.033327] nvmet: nvmet_execute_identify 724
[  802.033331] nvme nvme1: request 0x7 genctr mismatch (got 0x0 expected 
0x1)
[  802.034677] nvme nvme1: got bad command_id 0x7 on queue 0

-ck