On 6/10/21 00:45, Daniel Wagner wrote:

Hi Chaitanya,

On Thu, Jun 10, 2021 at 02:55:08AM +0000, Chaitanya Kulkarni wrote:

quoted

On 6/9/21 19:45, Chaitanya Kulkarni wrote:

quoted

This reverts commit 8872c159c7a83daf633768cee7a7ef7154010341. This is
needed to move forward with the blktests for now, without this patch
all the testcases result in the error :-

[ 3502.072798] nvme nvme1: Invalid MNAN value 1024

Thinking about the code again I think following should work :-

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 23573fe3fc7d..4277f1554bd5 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c

@@ -813,7 +813,7 @@ int nvme_mpath_init_identify(struct nvme_ctrl *ctrl,

struct nvme_id_ctrl *id)
            !(ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA))
                return 0;
 
-       if (!ctrl->max_namespaces ||
+       if (ctrl->max_namespaces &&
            ctrl->max_namespaces > le32_to_cpu(id->nn)) {
                dev_err(ctrl->device,
                        "Invalid MNAN value %u\n", ctrl->max_namespaces);

'!ctrl->max_namespace' could also be written as
'ctrl->max_namespace != 0' which makes it more obvious what the intend
is here:

  If the controller supports Asymmetric Namespace Access Reporting, then
  this field shall be set to a non-zero value that is less than or equal
  to the NN value.

Let us look into the host side issue first then we can move to target side.

Consider a scenario where ANA enabled subsys with 0 namespaces on the target
side. When host issues connect command to such a controller ctrl->mnan
should be 0 and ctrl->nn should be 0 which should be valid.

With original check in the code :-

if (!ctrl->max_namespaces ||
    ctrl->max_namespaces > le32_to_cpu(id->nn))

!ctrl->max_namespaces will return in 1 and due to || condition will be
true and code will report the error. The change proposed in this patch
with above mentioned scenario :- if (ctrl->max_namespaces &&
ctrl->max_namespaces > le32_to_cpu(id->nn)) { ctrl->max_namespaces will
return 0 and due to && condition will be false and new code will not
report zero. So I think above suggested patch is needed irrespective of
the target fix.


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme