Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1657049 > unrolled thread

[PATCH v4 0/8] Implement NVMe Namespace Descriptor Identification

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/8] Implement NVMe Namespace Descriptor Identification Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-04 12:40 +0200
    [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-04 12:40 +0200
      Re: [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE Sagi Grimberg <sagi@grimberg.me> - 2017-06-04 17:10 +0200
      Re: [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE Christoph Hellwig <hch@lst.de> - 2017-06-05 10:40 +0200
      Re: [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE Hannes Reinecke <hare@suse.de> - 2017-06-06 08:30 +0200
    [PATCH v4 6/8] nvme: provide UUID value to userspace Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-04 12:40 +0200
      Re: [PATCH v4 6/8] nvme: provide UUID value to userspace Sagi Grimberg <sagi@grimberg.me> - 2017-06-04 17:10 +0200
    Re: [PATCH v4 0/8] Implement NVMe Namespace Descriptor         Identification Christoph Hellwig <hch@lst.de> - 2017-06-05 07:40 +0200

#1657049 — [PATCH v4 0/8] Implement NVMe Namespace Descriptor Identification

This patchset implemets NVMe Namespace Descriptor Identification as of
NVMe 1.3. The Namespace Descriptor Identification allows a NVMe host
to query several Namespace Identification mechanisms, such as EUI-64,
NGUID and UUID from the target. If more than one value is set by the
target, it can transmit all set values to the host.

The Namespace Identification Descriptor list is the only way a target
can identify itself via the newly introduced UUID to the host (instead
of the EUI-64 or NGUID).

Both the Host and Target side are implemented. In order to get the
Linux Host to send the Linux target implementation a Namespace
Descriptor Identification command, you have to change the target's
announced version code to at least 1.3. 

Unfortunately the host side already did have a sysfs attribute called
'uuid' which represented the NGUID, so precautions have been taken to
not break any existing userspace.

While I was already touching the relevant code paths, I decided to
also include the EUI-64 in the 'Identify Namespace' command response.

The code is tested using the nvme-loop loopback target and cut against
the nvme tree's nvme-4.12 branch.

A patch for nvmetcli will follow shortly.

Changes to v3:
* Autogenerate UUID on target NS allocation (Sagi)

Changes to v2:
* Added Max's Reviewed-by
* Make series bisectable

Changes to v1:
* Added Reviewed-by tags from Christoph and Hannes for unchanged patches
* Added patch introducing new structs at the beginning (Christoph)
* Dropped SZ_4K patch
* Got rid of dynamic memory allocation on the target side (Christoph)
* Reworked host side parser (Christoph)
* Check length inside type check in the host parser (Max)

Changes to v0:
* Fixed wrong size of 4069 and replaced it with SZ_4K (Max)
* Add constants for UUID, NGUID and EUI-64 length (Max)
* Drop EUI-64 Support on target side (Christoph)
* Use uuid_be instead of u8[] (Christoph)
* Add ability to override target's version (Hannes)
* Change hard coded magic 4096 and 0x1000 to SZ_4k in drivers/nvme/*

Johannes Thumshirn (8):
  nvme: introduce NVMe Namespace Identification Descriptor structures
  nvme: rename uuid to nguid in nvme_ns
  nvmet: implement namespace identify descriptor list
  nvmet: add uuid field to nvme_ns and populate via configfs
  nvme: get list of namespace descriptors
  nvme: provide UUID value to userspace
  nvmet: allow overriding the NVMe VS via configfs
  nvmet: use NVME_IDENTIFY_DATA_SIZE

 drivers/nvme/host/core.c        | 118 ++++++++++++++++++++++++++++++++++++++--
 drivers/nvme/host/nvme.h        |   1 +
 drivers/nvme/target/admin-cmd.c |  57 ++++++++++++++++++-
 drivers/nvme/target/configfs.c  |  65 ++++++++++++++++++++++
 drivers/nvme/target/core.c      |   3 +-
 drivers/nvme/target/discovery.c |   2 +-
 drivers/nvme/target/nvmet.h     |   1 +
 include/linux/nvme.h            |  23 ++++++++
 8 files changed, 262 insertions(+), 8 deletions(-)

-- 
2.12.0

[toc] | [next] | [standalone]

#1657050 — [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE

Use NVME_IDENTIFY_DATA_SIZE define instead of hard coding the magic
4096 value.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/nvme/target/admin-cmd.c | 4 ++--
 drivers/nvme/target/discovery.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 833b5ef935c3..def6cde1e714 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -336,7 +336,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 
 static void nvmet_execute_identify_nslist(struct nvmet_req *req)
 {
-	static const int buf_size = 4096;
+	static const int buf_size = NVME_IDENTIFY_DATA_SIZE;
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvmet_ns *ns;
 	u32 min_nsid = le32_to_cpu(req->cmd->identify.nsid);
@@ -554,7 +554,7 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 		}
 		break;
 	case nvme_admin_identify:
-		req->data_len = 4096;
+		req->data_len = NVME_IDENTIFY_DATA_SIZE;
 		switch (cmd->identify.cns) {
 		case NVME_ID_CNS_NS:
 			req->execute = nvmet_execute_identify_ns;
diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
index 1aaf597e81fc..c7a90384dd75 100644
--- a/drivers/nvme/target/discovery.c
+++ b/drivers/nvme/target/discovery.c
@@ -185,7 +185,7 @@ u16 nvmet_parse_discovery_cmd(struct nvmet_req *req)
 		return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
 		}
 	case nvme_admin_identify:
-		req->data_len = 4096;
+		req->data_len = NVME_IDENTIFY_DATA_SIZE;
 		switch (cmd->identify.cns) {
 		case NVME_ID_CNS_CTRL:
 			req->execute =
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1657095 — Re: [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE

Looks fine,

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

[toc] | [prev] | [next] | [standalone]

#1657393 — Re: [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE

On Sun, Jun 04, 2017 at 12:36:49PM +0200, Johannes Thumshirn wrote:
> Use NVME_IDENTIFY_DATA_SIZE define instead of hard coding the magic
> 4096 value.

Can you bump this to the front of the series?

[toc] | [prev] | [next] | [standalone]

#1658385 — Re: [PATCH v4 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE

On 06/04/2017 12:36 PM, Johannes Thumshirn wrote:
> Use NVME_IDENTIFY_DATA_SIZE define instead of hard coding the magic
> 4096 value.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  drivers/nvme/target/admin-cmd.c | 4 ++--
>  drivers/nvme/target/discovery.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1657051 — [PATCH v4 6/8] nvme: provide UUID value to userspace

Now that we have a way for getting the UUID from a target, provide it
to userspace as well.

Unfortunately there is already a sysfs attribute called UUID which is
a misnomer as it holds the NGUID value. So instead of creating yet
another wrong name, create a new 'nguid' sysfs attribute for the
NGUID. For the UUID attribute add a check wheter the namespace has a
UUID assigned to it and return this or return the NGUID to maintain
backwards compatibility. This should give userspace a chance to catch
up.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
 drivers/nvme/host/core.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 37047841da0e..3aa5b12680e5 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1887,11 +1887,28 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR(wwid, S_IRUGO, wwid_show, NULL);
 
+static ssize_t nguid_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
+	return sprintf(buf, "%pU\n", ns->nguid);
+}
+static DEVICE_ATTR(nguid, S_IRUGO, nguid_show, NULL);
+
 static ssize_t uuid_show(struct device *dev, struct device_attribute *attr,
 								char *buf)
 {
 	struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
-	return sprintf(buf, "%pU\n", ns->nguid);
+
+	/* For backward compatibility expose the NGUID to userspace if
+	 * we have no UUID set
+	 */
+	if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid))) {
+		printk_ratelimited(KERN_WARNING
+				   "No UUID available providing old NGUID\n");
+		return sprintf(buf, "%pU\n", ns->nguid);
+	}
+	return sprintf(buf, "%pU\n", ns->uuid);
 }
 static DEVICE_ATTR(uuid, S_IRUGO, uuid_show, NULL);
 
@@ -1914,6 +1931,7 @@ static DEVICE_ATTR(nsid, S_IRUGO, nsid_show, NULL);
 static struct attribute *nvme_ns_attrs[] = {
 	&dev_attr_wwid.attr,
 	&dev_attr_uuid.attr,
+	&dev_attr_nguid.attr,
 	&dev_attr_eui.attr,
 	&dev_attr_nsid.attr,
 	NULL,
@@ -1926,6 +1944,11 @@ static umode_t nvme_ns_attrs_are_visible(struct kobject *kobj,
 	struct nvme_ns *ns = nvme_get_ns_from_dev(dev);
 
 	if (a == &dev_attr_uuid.attr) {
+		if (!memchr_inv(ns->uuid, 0, sizeof(ns->uuid)) ||
+		    !memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
+			return 0;
+	}
+	if (a == &dev_attr_nguid.attr) {
 		if (!memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
 			return 0;
 	}
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1657086 — Re: [PATCH v4 6/8] nvme: provide UUID value to userspace

Looks good,

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

[toc] | [prev] | [next] | [standalone]

#1657278 — Re: [PATCH v4 0/8] Implement NVMe Namespace Descriptor Identification

On Sun, Jun 04, 2017 at 12:36:41PM +0200, Johannes Thumshirn wrote:
> The Namespace Identification Descriptor list is the only way a target
> can identify itself via the newly introduced UUID to the host (instead
> of the EUI-64 or NGUID).

s/instead/in addition/

> While I was already touching the relevant code paths, I decided to
> also include the EUI-64 in the 'Identify Namespace' command response.

Not anymore :)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web