* [PATCH] nvme: check for dynamic controller model before verifying cntlid @ 2019-10-16 20:23 David Milburn 2019-10-17 14:50 ` Christoph Hellwig 0 siblings, 1 reply; 4+ messages in thread From: David Milburn @ 2019-10-16 20:23 UTC (permalink / raw) To: linux-nvme; +Cc: kbusch, hch, sagi If controller supports dynamic controller model the cntlid returned from the identify command maybe FFFFh causing check in nvme_init_identify() to fail which results in "nvme discover" failure. nvme discover -t rdma -a <target_IP> Failed to write to /dev/nvme-fabrics: Invalid argument Signed-off-by: David Milburn <dmilburn@redhat.com> --- drivers/nvme/host/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index fd7dea36c3b6..235aa1811446 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2812,7 +2812,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl) * In fabrics we need to verify the cntlid matches the * admin connect */ - if (ctrl->cntlid != le16_to_cpu(id->cntlid)) { + if (id->ctrattr && (ctrl->cntlid != le16_to_cpu(id->cntlid))) { ret = -EINVAL; goto out_free; } -- 2.18.1 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nvme: check for dynamic controller model before verifying cntlid 2019-10-16 20:23 [PATCH] nvme: check for dynamic controller model before verifying cntlid David Milburn @ 2019-10-17 14:50 ` Christoph Hellwig 2019-10-17 15:08 ` David Milburn 0 siblings, 1 reply; 4+ messages in thread From: Christoph Hellwig @ 2019-10-17 14:50 UTC (permalink / raw) To: David Milburn; +Cc: kbusch, hch, linux-nvme, sagi On Wed, Oct 16, 2019 at 03:23:22PM -0500, David Milburn wrote: > - if (ctrl->cntlid != le16_to_cpu(id->cntlid)) { > + if (id->ctrattr && (ctrl->cntlid != le16_to_cpu(id->cntlid))) { Where do get the all-Fs cntrlid? Identify Controller should return the actual controller id, and the fabrics setup code also sets it up to the return value of the connect command, which shouldn't be all-Fs either. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nvme: check for dynamic controller model before verifying cntlid 2019-10-17 14:50 ` Christoph Hellwig @ 2019-10-17 15:08 ` David Milburn 2019-10-17 15:10 ` Christoph Hellwig 0 siblings, 1 reply; 4+ messages in thread From: David Milburn @ 2019-10-17 15:08 UTC (permalink / raw) To: Christoph Hellwig; +Cc: kbusch, sagi, linux-nvme Hi Christoph, On 10/17/2019 09:50 AM, Christoph Hellwig wrote: > On Wed, Oct 16, 2019 at 03:23:22PM -0500, David Milburn wrote: >> - if (ctrl->cntlid != le16_to_cpu(id->cntlid)) { >> + if (id->ctrattr && (ctrl->cntlid != le16_to_cpu(id->cntlid))) { > > Where do get the all-Fs cntrlid? Identify Controller should return > the actual controller id, and the fabrics setup code also sets it up > to the return value of the connect command, which shouldn't be all-Fs > either. I see it in nvme_init_identify() after doing the nvme_identify_ctrl(), id->cntlid is FFFF causing the check to fail. There is a discussion in the NVMe over Fabrics spec, not specifically about identify, but with discover log page concerning dynamic vs static model and the value of cntlid, the target is a storage array. Thanks, David _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nvme: check for dynamic controller model before verifying cntlid 2019-10-17 15:08 ` David Milburn @ 2019-10-17 15:10 ` Christoph Hellwig 0 siblings, 0 replies; 4+ messages in thread From: Christoph Hellwig @ 2019-10-17 15:10 UTC (permalink / raw) To: David Milburn; +Cc: kbusch, Christoph Hellwig, linux-nvme, sagi On Thu, Oct 17, 2019 at 10:08:30AM -0500, David Milburn wrote: > Hi Christoph, > > On 10/17/2019 09:50 AM, Christoph Hellwig wrote: >> On Wed, Oct 16, 2019 at 03:23:22PM -0500, David Milburn wrote: >>> - if (ctrl->cntlid != le16_to_cpu(id->cntlid)) { >>> + if (id->ctrattr && (ctrl->cntlid != le16_to_cpu(id->cntlid))) { >> >> Where do get the all-Fs cntrlid? Identify Controller should return >> the actual controller id, and the fabrics setup code also sets it up >> to the return value of the connect command, which shouldn't be all-Fs >> either. > > I see it in nvme_init_identify() after doing the nvme_identify_ctrl(), > id->cntlid is FFFF causing the check to fail. There is a discussion > in the NVMe over Fabrics spec, not specifically about identify, but > with discover log page concerning dynamic vs static model and the > value of cntlid, the target is a storage array. Please trace down where it comes from, because I suspect your array is buggy. I guess it returns 0xffff from the connect command response, which is wrong as that should contain the actual controller id, and not 0xffff which is passed in the connect command to request the dynamic controller model. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-17 15:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-16 20:23 [PATCH] nvme: check for dynamic controller model before verifying cntlid David Milburn 2019-10-17 14:50 ` Christoph Hellwig 2019-10-17 15:08 ` David Milburn 2019-10-17 15:10 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).