[PATCH] nvmet: return Invalid Log Page status code for unsupported lid

[Klaus Jensen]

On Sep  2 11:53, amit.engel at dell.com wrote:
> From: Amit Engel <amit.engel at dell.com>
> 
> In case of an invalid or not supported log page
> return NVME_SC_INVALID_LOG_PAGE
> 
> Signed-off-by: Amit Engel <amit.engel at dell.com>
> ---
>  drivers/nvme/target/admin-cmd.c | 2 +-
>  drivers/nvme/target/discovery.c | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index aa6d84d8848e..5dcda2c87fdd 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -339,7 +339,7 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
>  	pr_debug("unhandled lid %d on qid %d\n",
>  	       req->cmd->get_log_page.lid, req->sq->qid);
>  	req->error_loc = offsetof(struct nvme_get_log_page_command, lid);
> -	nvmet_req_complete(req, NVME_SC_INVALID_FIELD | NVME_SC_DNR);
> +	nvmet_req_complete(req, NVME_SC_INVALID_LOG_PAGE | NVME_SC_DNR);
>  }
>  
>  static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
> diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
> index 7aa62bc6ae84..fdd52dbdd00f 100644
> --- a/drivers/nvme/target/discovery.c
> +++ b/drivers/nvme/target/discovery.c
> @@ -176,9 +176,11 @@ static void nvmet_execute_disc_get_log_page(struct nvmet_req *req)
>  		return;
>  
>  	if (req->cmd->get_log_page.lid != NVME_LOG_DISC) {
> +		pr_err("unhandled get log page cmd with lid %#x from ctrlid %d\n"
> +			req->cmd->get_log_page.lid, ctrl->cntlid);
>  		req->error_loc =
>  			offsetof(struct nvme_get_log_page_command, lid);
> -		status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> +		status = NVME_SC_INVALID_LOG_PAGE | NVME_SC_DNR;
>  		goto out;
>  	}
>  

This should only be done if the target is reporting v1.4 or above.

But honestly I'm not sure if there is any benefit to potentially
breaking hosts here. It is perfectly ok for a v1.4 compliant controller
to continue using Invalid Field in Command in this case.

The spec actually says that the controller *should* abort the command
with a status code of Invalid Field in Command. But I acknowledge that
v1.4+ specs are a bit contradictory here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20210902/5e9864d3/attachment.sig>