All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] Implement NVMe Namespace Descriptor Identification
@ 2017-06-02  9:45 ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

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 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      |   2 +-
 drivers/nvme/target/discovery.c |   2 +-
 drivers/nvme/target/nvmet.h     |   1 +
 include/linux/nvme.h            |  23 ++++++++
 8 files changed, 261 insertions(+), 8 deletions(-)

-- 
2.12.0

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 0/8] Implement NVMe Namespace Descriptor Identification
@ 2017-06-02  9:45 ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


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 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      |   2 +-
 drivers/nvme/target/discovery.c |   2 +-
 drivers/nvme/target/nvmet.h     |   1 +
 include/linux/nvme.h            |  23 ++++++++
 8 files changed, 261 insertions(+), 8 deletions(-)

-- 
2.12.0

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 1/8] nvme: introduce NVMe Namespace Identification Descriptor structures
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
---
 include/linux/nvme.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b625bacf37ef..afa6ef484e50 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -288,6 +288,7 @@ enum {
 	NVME_ID_CNS_NS			= 0x00,
 	NVME_ID_CNS_CTRL		= 0x01,
 	NVME_ID_CNS_NS_ACTIVE_LIST	= 0x02,
+	NVME_ID_CNS_NS_DESC_LIST	= 0x03,
 	NVME_ID_CNS_NS_PRESENT_LIST	= 0x10,
 	NVME_ID_CNS_NS_PRESENT		= 0x11,
 	NVME_ID_CNS_CTRL_NS_LIST	= 0x12,
@@ -314,6 +315,22 @@ enum {
 	NVME_NS_DPS_PI_TYPE3	= 3,
 };
 
+struct nvme_ns_identifier_hdr {
+	__u8 nidt;
+	__u8 nidl;
+	__le16 reserved;
+};
+
+#define NVME_NIDT_EUI64_LEN	8
+#define NVME_NIDT_NGUID_LEN	16
+#define NVME_NIDT_UUID_LEN	16
+
+enum {
+	NVME_NIDT_EUI64		= 0x01,
+	NVME_NIDT_NGUID		= 0x02,
+	NVME_NIDT_UUID		= 0x03,
+};
+
 struct nvme_smart_log {
 	__u8			critical_warning;
 	__u8			temperature[2];
@@ -658,6 +675,8 @@ struct nvme_identify {
 	__u32			rsvd11[5];
 };
 
+#define NVME_IDENTIFY_DATA_SIZE 4096
+
 struct nvme_features {
 	__u8			opcode;
 	__u8			flags;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 1/8] nvme: introduce NVMe Namespace Identification Descriptor structures
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
---
 include/linux/nvme.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b625bacf37ef..afa6ef484e50 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -288,6 +288,7 @@ enum {
 	NVME_ID_CNS_NS			= 0x00,
 	NVME_ID_CNS_CTRL		= 0x01,
 	NVME_ID_CNS_NS_ACTIVE_LIST	= 0x02,
+	NVME_ID_CNS_NS_DESC_LIST	= 0x03,
 	NVME_ID_CNS_NS_PRESENT_LIST	= 0x10,
 	NVME_ID_CNS_NS_PRESENT		= 0x11,
 	NVME_ID_CNS_CTRL_NS_LIST	= 0x12,
@@ -314,6 +315,22 @@ enum {
 	NVME_NS_DPS_PI_TYPE3	= 3,
 };
 
+struct nvme_ns_identifier_hdr {
+	__u8 nidt;
+	__u8 nidl;
+	__le16 reserved;
+};
+
+#define NVME_NIDT_EUI64_LEN	8
+#define NVME_NIDT_NGUID_LEN	16
+#define NVME_NIDT_UUID_LEN	16
+
+enum {
+	NVME_NIDT_EUI64		= 0x01,
+	NVME_NIDT_NGUID		= 0x02,
+	NVME_NIDT_UUID		= 0x03,
+};
+
 struct nvme_smart_log {
 	__u8			critical_warning;
 	__u8			temperature[2];
@@ -658,6 +675,8 @@ struct nvme_identify {
 	__u32			rsvd11[5];
 };
 
+#define NVME_IDENTIFY_DATA_SIZE 4096
+
 struct nvme_features {
 	__u8			opcode;
 	__u8			flags;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 2/8] nvme: rename uuid to nguid in nvme_ns
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

The uuid field in the nvme_ns structure represents the nguid field
from the identify namespace command. And as NVMe 1.3 introduced an
UUID in the NVMe Namespace Identification Descriptor this will
collide.

So rename the uuid to nguid to prevent any further
confusion. Unfortunately we export the nguid to sysfs in the uuid
sysfs attribute, but this can't be changed anymore without possibly
breaking existing userspace.

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 | 10 +++++-----
 drivers/nvme/host/nvme.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a60926410438..7b254be16887 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1016,7 +1016,7 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
 	if (ns->ctrl->vs >= NVME_VS(1, 1, 0))
 		memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
 	if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
-		memcpy(ns->uuid, (*id)->nguid, sizeof(ns->uuid));
+		memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
 
 	return 0;
 }
@@ -1784,8 +1784,8 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
 	int serial_len = sizeof(ctrl->serial);
 	int model_len = sizeof(ctrl->model);
 
-	if (memchr_inv(ns->uuid, 0, sizeof(ns->uuid)))
-		return sprintf(buf, "eui.%16phN\n", ns->uuid);
+	if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
+		return sprintf(buf, "eui.%16phN\n", ns->nguid);
 
 	if (memchr_inv(ns->eui, 0, sizeof(ns->eui)))
 		return sprintf(buf, "eui.%8phN\n", ns->eui);
@@ -1804,7 +1804,7 @@ 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->uuid);
+	return sprintf(buf, "%pU\n", ns->nguid);
 }
 static DEVICE_ATTR(uuid, S_IRUGO, uuid_show, NULL);
 
@@ -1839,7 +1839,7 @@ 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)))
+		if (!memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
 			return 0;
 	}
 	if (a == &dev_attr_eui.attr) {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9d6a070d4391..5004f0c41397 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -189,7 +189,7 @@ struct nvme_ns {
 	int instance;
 
 	u8 eui[8];
-	u8 uuid[16];
+	u8 nguid[16];
 
 	unsigned ns_id;
 	int lba_shift;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 2/8] nvme: rename uuid to nguid in nvme_ns
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


The uuid field in the nvme_ns structure represents the nguid field
from the identify namespace command. And as NVMe 1.3 introduced an
UUID in the NVMe Namespace Identification Descriptor this will
collide.

So rename the uuid to nguid to prevent any further
confusion. Unfortunately we export the nguid to sysfs in the uuid
sysfs attribute, but this can't be changed anymore without possibly
breaking existing userspace.

Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Reviewed-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Hannes Reinecke <hare at suse.com>
---
 drivers/nvme/host/core.c | 10 +++++-----
 drivers/nvme/host/nvme.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a60926410438..7b254be16887 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1016,7 +1016,7 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
 	if (ns->ctrl->vs >= NVME_VS(1, 1, 0))
 		memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
 	if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
-		memcpy(ns->uuid, (*id)->nguid, sizeof(ns->uuid));
+		memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
 
 	return 0;
 }
@@ -1784,8 +1784,8 @@ static ssize_t wwid_show(struct device *dev, struct device_attribute *attr,
 	int serial_len = sizeof(ctrl->serial);
 	int model_len = sizeof(ctrl->model);
 
-	if (memchr_inv(ns->uuid, 0, sizeof(ns->uuid)))
-		return sprintf(buf, "eui.%16phN\n", ns->uuid);
+	if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
+		return sprintf(buf, "eui.%16phN\n", ns->nguid);
 
 	if (memchr_inv(ns->eui, 0, sizeof(ns->eui)))
 		return sprintf(buf, "eui.%8phN\n", ns->eui);
@@ -1804,7 +1804,7 @@ 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->uuid);
+	return sprintf(buf, "%pU\n", ns->nguid);
 }
 static DEVICE_ATTR(uuid, S_IRUGO, uuid_show, NULL);
 
@@ -1839,7 +1839,7 @@ 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)))
+		if (!memchr_inv(ns->nguid, 0, sizeof(ns->nguid)))
 			return 0;
 	}
 	if (a == &dev_attr_eui.attr) {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9d6a070d4391..5004f0c41397 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -189,7 +189,7 @@ struct nvme_ns {
 	int instance;
 
 	u8 eui[8];
-	u8 uuid[16];
+	u8 nguid[16];
 
 	unsigned ns_id;
 	int lba_shift;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

A NVMe Identify NS command with a CNS value of '3' is expecting a list
of Namespace Identification Descriptor structures to be returned to
the host for the namespace requested in the namespace identify
command.

This Namespace Identification Descriptor structure consists of the
type of the namespace identifier, the length of the identifier and the
actual identifier.

Valid types are NGUID and UUID which we have saved in our nvme_ns
structure if they have been configured via configfs. If no value has
been assigened to one of these we return an "invalid opcode" back to
the host to maintain backward compatibiliy with older implementations
without Namespace Identify Descriptor list support.

Also as the Namespace Identify Descriptor list is the only mandatory
feature change between 1.2.1 and 1.3.0 we can bump the advertised
version as well.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/nvme/target/admin-cmd.c | 53 +++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/core.c      |  2 +-
 drivers/nvme/target/nvmet.h     |  1 +
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index ff1f97006322..833b5ef935c3 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -367,6 +367,56 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req)
 	nvmet_req_complete(req, status);
 }
 
+static void nvmet_execute_identify_desclist(struct nvmet_req *req)
+{
+	struct nvmet_ns *ns;
+	struct nvme_ns_identifier_hdr hdr;
+	u16 status = 0;
+	off_t off = 0;
+
+	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+	if (!ns) {
+		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+		goto out;
+	}
+
+	if (memchr_inv(&ns->uuid, 0, sizeof(ns->uuid))) {
+		memset(&hdr, 0, sizeof(hdr));
+		hdr.nidt = NVME_NIDT_UUID;
+		hdr.nidl = NVME_NIDT_UUID_LEN;
+		status = nvmet_copy_to_sgl(req, off, &hdr, sizeof(hdr));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(hdr);
+
+		status = nvmet_copy_to_sgl(req, off, &ns->uuid,
+					   sizeof(ns->uuid));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(ns->uuid);
+	}
+	if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
+		memset(&hdr, 0, sizeof(hdr));
+		hdr.nidt = NVME_NIDT_NGUID;
+		hdr.nidl = NVME_NIDT_NGUID_LEN;
+		status = nvmet_copy_to_sgl(req, off, &hdr, sizeof(hdr));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(hdr);
+
+		status = nvmet_copy_to_sgl(req, off, &ns->nguid,
+					   sizeof(ns->nguid));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(ns->nguid);
+	}
+
+out_put_ns:
+	nvmet_put_namespace(ns);
+out:
+	nvmet_req_complete(req, status);
+}
+
 /*
  * A "mimimum viable" abort implementation: the command is mandatory in the
  * spec, but we are not required to do any useful work.  We couldn't really
@@ -515,6 +565,9 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 		case NVME_ID_CNS_NS_ACTIVE_LIST:
 			req->execute = nvmet_execute_identify_nslist;
 			return 0;
+		case NVME_ID_CNS_NS_DESC_LIST:
+			req->execute = nvmet_execute_identify_desclist;
+			return 0;
 		}
 		break;
 	case nvme_admin_abort_cmd:
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index eb9399ac97cf..d4eeee4f2fab 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -926,7 +926,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 	if (!subsys)
 		return NULL;
 
-	subsys->ver = NVME_VS(1, 2, 1); /* NVMe 1.2.1 */
+	subsys->ver = NVME_VS(1, 3, 0); /* NVMe 1.3.0 */
 
 	switch (type) {
 	case NVME_NQN_NVME:
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index cfc5c7fb0ab7..4c6cb5ea1186 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -46,6 +46,7 @@ struct nvmet_ns {
 	u32			blksize_shift;
 	loff_t			size;
 	u8			nguid[16];
+	uuid_be			uuid;
 
 	bool			enabled;
 	struct nvmet_subsys	*subsys;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


A NVMe Identify NS command with a CNS value of '3' is expecting a list
of Namespace Identification Descriptor structures to be returned to
the host for the namespace requested in the namespace identify
command.

This Namespace Identification Descriptor structure consists of the
type of the namespace identifier, the length of the identifier and the
actual identifier.

Valid types are NGUID and UUID which we have saved in our nvme_ns
structure if they have been configured via configfs. If no value has
been assigened to one of these we return an "invalid opcode" back to
the host to maintain backward compatibiliy with older implementations
without Namespace Identify Descriptor list support.

Also as the Namespace Identify Descriptor list is the only mandatory
feature change between 1.2.1 and 1.3.0 we can bump the advertised
version as well.

Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
---
 drivers/nvme/target/admin-cmd.c | 53 +++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/core.c      |  2 +-
 drivers/nvme/target/nvmet.h     |  1 +
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index ff1f97006322..833b5ef935c3 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -367,6 +367,56 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req)
 	nvmet_req_complete(req, status);
 }
 
+static void nvmet_execute_identify_desclist(struct nvmet_req *req)
+{
+	struct nvmet_ns *ns;
+	struct nvme_ns_identifier_hdr hdr;
+	u16 status = 0;
+	off_t off = 0;
+
+	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+	if (!ns) {
+		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+		goto out;
+	}
+
+	if (memchr_inv(&ns->uuid, 0, sizeof(ns->uuid))) {
+		memset(&hdr, 0, sizeof(hdr));
+		hdr.nidt = NVME_NIDT_UUID;
+		hdr.nidl = NVME_NIDT_UUID_LEN;
+		status = nvmet_copy_to_sgl(req, off, &hdr, sizeof(hdr));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(hdr);
+
+		status = nvmet_copy_to_sgl(req, off, &ns->uuid,
+					   sizeof(ns->uuid));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(ns->uuid);
+	}
+	if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
+		memset(&hdr, 0, sizeof(hdr));
+		hdr.nidt = NVME_NIDT_NGUID;
+		hdr.nidl = NVME_NIDT_NGUID_LEN;
+		status = nvmet_copy_to_sgl(req, off, &hdr, sizeof(hdr));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(hdr);
+
+		status = nvmet_copy_to_sgl(req, off, &ns->nguid,
+					   sizeof(ns->nguid));
+		if (status)
+			goto out_put_ns;
+		off += sizeof(ns->nguid);
+	}
+
+out_put_ns:
+	nvmet_put_namespace(ns);
+out:
+	nvmet_req_complete(req, status);
+}
+
 /*
  * A "mimimum viable" abort implementation: the command is mandatory in the
  * spec, but we are not required to do any useful work.  We couldn't really
@@ -515,6 +565,9 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 		case NVME_ID_CNS_NS_ACTIVE_LIST:
 			req->execute = nvmet_execute_identify_nslist;
 			return 0;
+		case NVME_ID_CNS_NS_DESC_LIST:
+			req->execute = nvmet_execute_identify_desclist;
+			return 0;
 		}
 		break;
 	case nvme_admin_abort_cmd:
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index eb9399ac97cf..d4eeee4f2fab 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -926,7 +926,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 	if (!subsys)
 		return NULL;
 
-	subsys->ver = NVME_VS(1, 2, 1); /* NVMe 1.2.1 */
+	subsys->ver = NVME_VS(1, 3, 0); /* NVMe 1.3.0 */
 
 	switch (type) {
 	case NVME_NQN_NVME:
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index cfc5c7fb0ab7..4c6cb5ea1186 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -46,6 +46,7 @@ struct nvmet_ns {
 	u32			blksize_shift;
 	loff_t			size;
 	u8			nguid[16];
+	uuid_be			uuid;
 
 	bool			enabled;
 	struct nvmet_subsys	*subsys;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 4/8] nvmet: add uuid field to nvme_ns and populate via configfs
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

Add the UUID field from the NVMe Namespace Identification Descriptor
to the nvmet_ns structure and allow it's population via configfs.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/nvme/target/configfs.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index be8c800078e2..16f9f6e3a084 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -305,11 +305,41 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_ns_, device_path);
 
+static ssize_t nvmet_ns_device_uuid_show(struct config_item *item, char *page)
+{
+	return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->uuid);
+}
+
+static ssize_t nvmet_ns_device_uuid_store(struct config_item *item,
+					  const char *page, size_t count)
+{
+	struct nvmet_ns *ns = to_nvmet_ns(item);
+	struct nvmet_subsys *subsys = ns->subsys;
+	int ret = 0;
+
+
+	mutex_lock(&subsys->lock);
+	if (ns->enabled) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+
+	if (uuid_be_to_bin(page, &ns->uuid))
+		ret = -EINVAL;
+
+out_unlock:
+	mutex_unlock(&subsys->lock);
+	return ret ? ret : count;
+}
+
 static ssize_t nvmet_ns_device_nguid_show(struct config_item *item, char *page)
 {
 	return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->nguid);
 }
 
+CONFIGFS_ATTR(nvmet_ns_, device_uuid);
+
 static ssize_t nvmet_ns_device_nguid_store(struct config_item *item,
 		const char *page, size_t count)
 {
@@ -379,6 +409,7 @@ CONFIGFS_ATTR(nvmet_ns_, enable);
 static struct configfs_attribute *nvmet_ns_attrs[] = {
 	&nvmet_ns_attr_device_path,
 	&nvmet_ns_attr_device_nguid,
+	&nvmet_ns_attr_device_uuid,
 	&nvmet_ns_attr_enable,
 	NULL,
 };
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 4/8] nvmet: add uuid field to nvme_ns and populate via configfs
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


Add the UUID field from the NVMe Namespace Identification Descriptor
to the nvmet_ns structure and allow it's population via configfs.

Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
---
 drivers/nvme/target/configfs.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index be8c800078e2..16f9f6e3a084 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -305,11 +305,41 @@ static ssize_t nvmet_ns_device_path_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_ns_, device_path);
 
+static ssize_t nvmet_ns_device_uuid_show(struct config_item *item, char *page)
+{
+	return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->uuid);
+}
+
+static ssize_t nvmet_ns_device_uuid_store(struct config_item *item,
+					  const char *page, size_t count)
+{
+	struct nvmet_ns *ns = to_nvmet_ns(item);
+	struct nvmet_subsys *subsys = ns->subsys;
+	int ret = 0;
+
+
+	mutex_lock(&subsys->lock);
+	if (ns->enabled) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+
+	if (uuid_be_to_bin(page, &ns->uuid))
+		ret = -EINVAL;
+
+out_unlock:
+	mutex_unlock(&subsys->lock);
+	return ret ? ret : count;
+}
+
 static ssize_t nvmet_ns_device_nguid_show(struct config_item *item, char *page)
 {
 	return sprintf(page, "%pUb\n", &to_nvmet_ns(item)->nguid);
 }
 
+CONFIGFS_ATTR(nvmet_ns_, device_uuid);
+
 static ssize_t nvmet_ns_device_nguid_store(struct config_item *item,
 		const char *page, size_t count)
 {
@@ -379,6 +409,7 @@ CONFIGFS_ATTR(nvmet_ns_, enable);
 static struct configfs_attribute *nvmet_ns_attrs[] = {
 	&nvmet_ns_attr_device_path,
 	&nvmet_ns_attr_device_nguid,
+	&nvmet_ns_attr_device_uuid,
 	&nvmet_ns_attr_enable,
 	NULL,
 };
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 5/8] nvme: get list of namespace descriptors
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

If a target identifies itself as NVMe 1.3 compliant, try to get the
list of Namespace Identification Descriptors and populate the UUID,
NGUID and EUI64 fileds in the NVMe namespace structure with these
values.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/nvme/host/core.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h |  1 +
 2 files changed, 88 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7b254be16887..37047841da0e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -643,6 +643,85 @@ int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
 	return error;
 }
 
+static void nvme_parse_ns_descs(struct nvme_ns *ns, void *data)
+{
+	int pos;
+	int len;
+
+	for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
+		struct nvme_ns_identifier_hdr *cur = data + pos;
+
+		if (cur->nidl == 0)
+			break;
+
+		switch (cur->nidt) {
+		case NVME_NIDT_EUI64:
+			if (cur->nidl != NVME_NIDT_EUI64_LEN) {
+				dev_warn(ns->ctrl->dev,
+					 "Target returned bogus length: %d for NVME_NIDT_EUI64\n",
+					 cur->nidl);
+				return;
+			}
+			len = NVME_NIDT_EUI64_LEN;
+			memcpy(ns->eui, data + pos + sizeof(*cur), len);
+			break;
+		case NVME_NIDT_NGUID:
+			if (cur->nidl != NVME_NIDT_NGUID_LEN) {
+				dev_warn(ns->ctrl->dev,
+					 "Target returned bogus length: %d for NVME_NIDT_NGUID\n",
+					 cur->nidl);
+				return;
+			}
+			len = NVME_NIDT_NGUID_LEN;
+			memcpy(ns->nguid, data + pos + sizeof(*cur), len);
+			break;
+		case NVME_NIDT_UUID:
+			if (cur->nidl != NVME_NIDT_UUID_LEN) {
+				dev_warn(ns->ctrl->dev,
+					 "Target returned bogus length: %d for NVME_NIDT_UUID\n",
+					 cur->nidl);
+				return;
+			}
+			len = NVME_NIDT_UUID_LEN;
+			memcpy(ns->uuid, data + pos + sizeof(*cur), len);
+			break;
+		default:
+			dev_warn(ns->ctrl->dev,
+				 "Invalid Namespace Identification Descriptor Type: %d\n",
+				 cur->nidt);
+			return;
+		}
+
+		len += sizeof(*cur);
+	}
+}
+
+static int nvme_identify_ns_descs(struct nvme_ns *ns, unsigned nsid)
+{
+	struct nvme_command c = { };
+	int status;
+	void *data;
+
+	c.identify.opcode = nvme_admin_identify;
+	c.identify.nsid = cpu_to_le32(nsid);
+	c.identify.cns = NVME_ID_CNS_NS_DESC_LIST;
+
+	data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	status = nvme_submit_sync_cmd(ns->ctrl->admin_q, &c, data,
+				      NVME_IDENTIFY_DATA_SIZE);
+	if (status)
+		goto free_data;
+
+	nvme_parse_ns_descs(ns, data);
+
+free_data:
+	kfree(data);
+	return status;
+}
+
 static int nvme_identify_ns_list(struct nvme_ctrl *dev, unsigned nsid, __le32 *ns_list)
 {
 	struct nvme_command c = { };
@@ -1017,6 +1096,14 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
 		memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
 	if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
 		memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
+	if (ns->ctrl->vs >= NVME_VS(1, 3, 0)) {
+		 /* Don't treat error as fatal we potentially
+		  * already have a NGUID or EUI-64
+		  */
+		if (nvme_identify_ns_descs(ns, ns->ns_id))
+			dev_warn(ns->ctrl->dev,
+				 "%s: Identify Descriptors failed\n", __func__);
+	}
 
 	return 0;
 }
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 5004f0c41397..7007521e8194 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -190,6 +190,7 @@ struct nvme_ns {
 
 	u8 eui[8];
 	u8 nguid[16];
+	u8 uuid[16];
 
 	unsigned ns_id;
 	int lba_shift;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 5/8] nvme: get list of namespace descriptors
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


If a target identifies itself as NVMe 1.3 compliant, try to get the
list of Namespace Identification Descriptors and populate the UUID,
NGUID and EUI64 fileds in the NVMe namespace structure with these
values.

Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
---
 drivers/nvme/host/core.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h |  1 +
 2 files changed, 88 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7b254be16887..37047841da0e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -643,6 +643,85 @@ int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
 	return error;
 }
 
+static void nvme_parse_ns_descs(struct nvme_ns *ns, void *data)
+{
+	int pos;
+	int len;
+
+	for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
+		struct nvme_ns_identifier_hdr *cur = data + pos;
+
+		if (cur->nidl == 0)
+			break;
+
+		switch (cur->nidt) {
+		case NVME_NIDT_EUI64:
+			if (cur->nidl != NVME_NIDT_EUI64_LEN) {
+				dev_warn(ns->ctrl->dev,
+					 "Target returned bogus length: %d for NVME_NIDT_EUI64\n",
+					 cur->nidl);
+				return;
+			}
+			len = NVME_NIDT_EUI64_LEN;
+			memcpy(ns->eui, data + pos + sizeof(*cur), len);
+			break;
+		case NVME_NIDT_NGUID:
+			if (cur->nidl != NVME_NIDT_NGUID_LEN) {
+				dev_warn(ns->ctrl->dev,
+					 "Target returned bogus length: %d for NVME_NIDT_NGUID\n",
+					 cur->nidl);
+				return;
+			}
+			len = NVME_NIDT_NGUID_LEN;
+			memcpy(ns->nguid, data + pos + sizeof(*cur), len);
+			break;
+		case NVME_NIDT_UUID:
+			if (cur->nidl != NVME_NIDT_UUID_LEN) {
+				dev_warn(ns->ctrl->dev,
+					 "Target returned bogus length: %d for NVME_NIDT_UUID\n",
+					 cur->nidl);
+				return;
+			}
+			len = NVME_NIDT_UUID_LEN;
+			memcpy(ns->uuid, data + pos + sizeof(*cur), len);
+			break;
+		default:
+			dev_warn(ns->ctrl->dev,
+				 "Invalid Namespace Identification Descriptor Type: %d\n",
+				 cur->nidt);
+			return;
+		}
+
+		len += sizeof(*cur);
+	}
+}
+
+static int nvme_identify_ns_descs(struct nvme_ns *ns, unsigned nsid)
+{
+	struct nvme_command c = { };
+	int status;
+	void *data;
+
+	c.identify.opcode = nvme_admin_identify;
+	c.identify.nsid = cpu_to_le32(nsid);
+	c.identify.cns = NVME_ID_CNS_NS_DESC_LIST;
+
+	data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	status = nvme_submit_sync_cmd(ns->ctrl->admin_q, &c, data,
+				      NVME_IDENTIFY_DATA_SIZE);
+	if (status)
+		goto free_data;
+
+	nvme_parse_ns_descs(ns, data);
+
+free_data:
+	kfree(data);
+	return status;
+}
+
 static int nvme_identify_ns_list(struct nvme_ctrl *dev, unsigned nsid, __le32 *ns_list)
 {
 	struct nvme_command c = { };
@@ -1017,6 +1096,14 @@ static int nvme_revalidate_ns(struct nvme_ns *ns, struct nvme_id_ns **id)
 		memcpy(ns->eui, (*id)->eui64, sizeof(ns->eui));
 	if (ns->ctrl->vs >= NVME_VS(1, 2, 0))
 		memcpy(ns->nguid, (*id)->nguid, sizeof(ns->nguid));
+	if (ns->ctrl->vs >= NVME_VS(1, 3, 0)) {
+		 /* Don't treat error as fatal we potentially
+		  * already have a NGUID or EUI-64
+		  */
+		if (nvme_identify_ns_descs(ns, ns->ns_id))
+			dev_warn(ns->ctrl->dev,
+				 "%s: Identify Descriptors failed\n", __func__);
+	}
 
 	return 0;
 }
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 5004f0c41397..7007521e8194 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -190,6 +190,7 @@ struct nvme_ns {
 
 	u8 eui[8];
 	u8 nguid[16];
+	u8 uuid[16];
 
 	unsigned ns_id;
 	int lba_shift;
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 6/8] nvme: provide UUID value to userspace
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 6/8] nvme: provide UUID value to userspace
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


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 at suse.de>
Reviewed-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Hannes Reinecke <hare at 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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 7/8] nvmet: allow overriding the NVMe VS via configfs
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

Allow overriding the announced NVMe Version of a via configfs.

This is particularly helpful when debugging new features for the host
or target side without bumping the hard coded version (as the target
might not be fully compliant to the announced version yet).

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/nvme.h           |  4 ++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 16f9f6e3a084..45421d4308a4 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -650,8 +650,42 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host);
 
+static ssize_t nvmet_subsys_version_show(struct config_item *item,
+					      char *page)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	int major, minor, tertiary;
+	u32 ver;
+
+	ver = subsys->ver;
+	major = NVME_MAJOR(ver);
+	minor = NVME_MINOR(ver);
+	tertiary = NVME_TERRIARY(ver);
+
+	return snprintf(page, PAGE_SIZE, "%d %d %d\n", major, minor, tertiary);
+}
+
+static ssize_t nvmet_subsys_version_store(struct config_item *item,
+					       const char *page, size_t count)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	int major, minor, tertiary;
+	int ret;
+
+
+	ret = sscanf(page, "%d %d %d\n", &major, &minor, &tertiary);
+	if (ret != 3)
+		return -EINVAL;
+
+	subsys->ver = NVME_VS(major, minor, tertiary);
+
+	return count;
+}
+CONFIGFS_ATTR(nvmet_subsys_, version);
+
 static struct configfs_attribute *nvmet_subsys_attrs[] = {
 	&nvmet_subsys_attr_attr_allow_any_host,
+	&nvmet_subsys_attr_version,
 	NULL,
 };
 
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index afa6ef484e50..0d6e307a7aa4 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -1069,4 +1069,8 @@ struct nvme_completion {
 #define NVME_VS(major, minor, tertiary) \
 	(((major) << 16) | ((minor) << 8) | (tertiary))
 
+#define NVME_MAJOR(ver)		((ver) >> 16)
+#define NVME_MINOR(ver)		(((ver) >> 8) & 0xff)
+#define NVME_TERRIARY(ver)	((ver) & 0xff)
+
 #endif /* _LINUX_NVME_H */
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 7/8] nvmet: allow overriding the NVMe VS via configfs
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


Allow overriding the announced NVMe Version of a via configfs.

This is particularly helpful when debugging new features for the host
or target side without bumping the hard coded version (as the target
might not be fully compliant to the announced version yet).

Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
---
 drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/nvme.h           |  4 ++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 16f9f6e3a084..45421d4308a4 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -650,8 +650,42 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host);
 
+static ssize_t nvmet_subsys_version_show(struct config_item *item,
+					      char *page)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	int major, minor, tertiary;
+	u32 ver;
+
+	ver = subsys->ver;
+	major = NVME_MAJOR(ver);
+	minor = NVME_MINOR(ver);
+	tertiary = NVME_TERRIARY(ver);
+
+	return snprintf(page, PAGE_SIZE, "%d %d %d\n", major, minor, tertiary);
+}
+
+static ssize_t nvmet_subsys_version_store(struct config_item *item,
+					       const char *page, size_t count)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	int major, minor, tertiary;
+	int ret;
+
+
+	ret = sscanf(page, "%d %d %d\n", &major, &minor, &tertiary);
+	if (ret != 3)
+		return -EINVAL;
+
+	subsys->ver = NVME_VS(major, minor, tertiary);
+
+	return count;
+}
+CONFIGFS_ATTR(nvmet_subsys_, version);
+
 static struct configfs_attribute *nvmet_subsys_attrs[] = {
 	&nvmet_subsys_attr_attr_allow_any_host,
+	&nvmet_subsys_attr_version,
 	NULL,
 };
 
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index afa6ef484e50..0d6e307a7aa4 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -1069,4 +1069,8 @@ struct nvme_completion {
 #define NVME_VS(major, minor, tertiary) \
 	(((major) << 16) | ((minor) << 8) | (tertiary))
 
+#define NVME_MAJOR(ver)		((ver) >> 16)
+#define NVME_MINOR(ver)		(((ver) >> 8) & 0xff)
+#define NVME_TERRIARY(ver)	((ver) & 0xff)
+
 #endif /* _LINUX_NVME_H */
-- 
2.12.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE
  2017-06-02  9:45 ` Johannes Thumshirn
@ 2017-06-02  9:45   ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist, Johannes Thumshirn

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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v3 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE
@ 2017-06-02  9:45   ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02  9:45 UTC (permalink / raw)


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

Signed-off-by: Johannes Thumshirn <jthumshirn at 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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-02  9:45   ` Johannes Thumshirn
@ 2017-06-02 13:46     ` Sagi Grimberg
  -1 siblings, 0 replies; 34+ messages in thread
From: Sagi Grimberg @ 2017-06-02 13:46 UTC (permalink / raw)
  To: Johannes Thumshirn, Christoph Hellwig, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist, Linux Kernel Mailinglist


>  	switch (type) {
>  	case NVME_NQN_NVME:
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index cfc5c7fb0ab7..4c6cb5ea1186 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -46,6 +46,7 @@ struct nvmet_ns {
>  	u32			blksize_shift;
>  	loff_t			size;
>  	u8			nguid[16];
> +	uuid_be			uuid;

This should be uuid_le to match the rest of NVMe.

Also, it would be useful to generate a random default with uuid_le_gen()
for a better out-of-the-box experience.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-02 13:46     ` Sagi Grimberg
  0 siblings, 0 replies; 34+ messages in thread
From: Sagi Grimberg @ 2017-06-02 13:46 UTC (permalink / raw)



>  	switch (type) {
>  	case NVME_NQN_NVME:
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index cfc5c7fb0ab7..4c6cb5ea1186 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -46,6 +46,7 @@ struct nvmet_ns {
>  	u32			blksize_shift;
>  	loff_t			size;
>  	u8			nguid[16];
> +	uuid_be			uuid;

This should be uuid_le to match the rest of NVMe.

Also, it would be useful to generate a random default with uuid_le_gen()
for a better out-of-the-box experience.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-02 13:46     ` Sagi Grimberg
@ 2017-06-02 13:54       ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02 13:54 UTC (permalink / raw)
  To: Sagi Grimberg, Christoph Hellwig, Keith Busch
  Cc: Hannes Reinecke, maxg, Linux NVMe Mailinglist, Linux Kernel Mailinglist

On 06/02/2017 03:46 PM, Sagi Grimberg wrote:
>> +    uuid_be            uuid;
> 
> This should be uuid_le to match the rest of NVMe.

Ahm, are you sure?
$ PAGER= git grep -E uuid_\(l\|b\)e drivers/nvme
drivers/nvme/host/fabrics.c:61:	uuid_be_gen(&host->id);
drivers/nvme/host/fabrics.c:78:	uuid_be_gen(&host->id);
drivers/nvme/host/fabrics.c:398:	memcpy(&data->hostid,
&ctrl->opts->host->id, sizeof(uuid_be));
drivers/nvme/host/fabrics.c:457:	memcpy(&data->hostid,
&ctrl->opts->host->id, sizeof(uuid_be));
drivers/nvme/host/fabrics.h:39:	uuid_be			id;
drivers/nvme/host/fc.c:882:		min_t(size_t, FCNVME_ASSOC_HOSTID_LEN,
sizeof(uuid_be)));
drivers/nvme/target/admin-cmd.c:324:	memcpy(&id->nguid, &ns->nguid,
sizeof(uuid_le));
drivers/nvme/target/configfs.c:328:	if (uuid_be_to_bin(page, &ns->uuid))
drivers/nvme/target/nvmet.h:49:	uuid_be			uuid;

And RFC4122 4.1.2 says MSB first as well.

> 
> Also, it would be useful to generate a random default with uuid_le_gen()
> for a better out-of-the-box experience.

Yep, sounds reasonable.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-02 13:54       ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-02 13:54 UTC (permalink / raw)


On 06/02/2017 03:46 PM, Sagi Grimberg wrote:
>> +    uuid_be            uuid;
> 
> This should be uuid_le to match the rest of NVMe.

Ahm, are you sure?
$ PAGER= git grep -E uuid_\(l\|b\)e drivers/nvme
drivers/nvme/host/fabrics.c:61:	uuid_be_gen(&host->id);
drivers/nvme/host/fabrics.c:78:	uuid_be_gen(&host->id);
drivers/nvme/host/fabrics.c:398:	memcpy(&data->hostid,
&ctrl->opts->host->id, sizeof(uuid_be));
drivers/nvme/host/fabrics.c:457:	memcpy(&data->hostid,
&ctrl->opts->host->id, sizeof(uuid_be));
drivers/nvme/host/fabrics.h:39:	uuid_be			id;
drivers/nvme/host/fc.c:882:		min_t(size_t, FCNVME_ASSOC_HOSTID_LEN,
sizeof(uuid_be)));
drivers/nvme/target/admin-cmd.c:324:	memcpy(&id->nguid, &ns->nguid,
sizeof(uuid_le));
drivers/nvme/target/configfs.c:328:	if (uuid_be_to_bin(page, &ns->uuid))
drivers/nvme/target/nvmet.h:49:	uuid_be			uuid;

And RFC4122 4.1.2 says MSB first as well.

> 
> Also, it would be useful to generate a random default with uuid_le_gen()
> for a better out-of-the-box experience.

Yep, sounds reasonable.

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-02 13:46     ` Sagi Grimberg
@ 2017-06-03  5:18       ` Christoph Hellwig
  -1 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2017-06-03  5:18 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: Johannes Thumshirn, Christoph Hellwig, Keith Busch,
	Hannes Reinecke, maxg, Linux NVMe Mailinglist,
	Linux Kernel Mailinglist

On Fri, Jun 02, 2017 at 04:46:40PM +0300, Sagi Grimberg wrote:
>
>>  	switch (type) {
>>  	case NVME_NQN_NVME:
>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>> index cfc5c7fb0ab7..4c6cb5ea1186 100644
>> --- a/drivers/nvme/target/nvmet.h
>> +++ b/drivers/nvme/target/nvmet.h
>> @@ -46,6 +46,7 @@ struct nvmet_ns {
>>  	u32			blksize_shift;
>>  	loff_t			size;
>>  	u8			nguid[16];
>> +	uuid_be			uuid;
>
> This should be uuid_le to match the rest of NVMe.

It shouldn't.  A RFC4122 uuid is always in "big endian" notation.
A little endiane uuid doesn't exist, it's called a GUID in Wintel
speak and isn't used in NVMe.  My new uuid tree tries to fix up
the confusion by renaming our uuid_be type to uuid_t and uuid_le
to guid_t.

> Also, it would be useful to generate a random default with uuid_le_gen()
> for a better out-of-the-box experience.

That's actually a good point.  Just needs to be uuid_be_gen/uuid_gen :)

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-03  5:18       ` Christoph Hellwig
  0 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2017-06-03  5:18 UTC (permalink / raw)


On Fri, Jun 02, 2017@04:46:40PM +0300, Sagi Grimberg wrote:
>
>>  	switch (type) {
>>  	case NVME_NQN_NVME:
>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>> index cfc5c7fb0ab7..4c6cb5ea1186 100644
>> --- a/drivers/nvme/target/nvmet.h
>> +++ b/drivers/nvme/target/nvmet.h
>> @@ -46,6 +46,7 @@ struct nvmet_ns {
>>  	u32			blksize_shift;
>>  	loff_t			size;
>>  	u8			nguid[16];
>> +	uuid_be			uuid;
>
> This should be uuid_le to match the rest of NVMe.

It shouldn't.  A RFC4122 uuid is always in "big endian" notation.
A little endiane uuid doesn't exist, it's called a GUID in Wintel
speak and isn't used in NVMe.  My new uuid tree tries to fix up
the confusion by renaming our uuid_be type to uuid_t and uuid_le
to guid_t.

> Also, it would be useful to generate a random default with uuid_le_gen()
> for a better out-of-the-box experience.

That's actually a good point.  Just needs to be uuid_be_gen/uuid_gen :)

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-03  5:18       ` Christoph Hellwig
@ 2017-06-03  8:03         ` Sagi Grimberg
  -1 siblings, 0 replies; 34+ messages in thread
From: Sagi Grimberg @ 2017-06-03  8:03 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Johannes Thumshirn, Keith Busch, Hannes Reinecke, maxg,
	Linux NVMe Mailinglist, Linux Kernel Mailinglist


>>>   	switch (type) {
>>>   	case NVME_NQN_NVME:
>>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>>> index cfc5c7fb0ab7..4c6cb5ea1186 100644
>>> --- a/drivers/nvme/target/nvmet.h
>>> +++ b/drivers/nvme/target/nvmet.h
>>> @@ -46,6 +46,7 @@ struct nvmet_ns {
>>>   	u32			blksize_shift;
>>>   	loff_t			size;
>>>   	u8			nguid[16];
>>> +	uuid_be			uuid;
>>
>> This should be uuid_le to match the rest of NVMe.
> 
> It shouldn't.  A RFC4122 uuid is always in "big endian" notation.
> A little endiane uuid doesn't exist, it's called a GUID in Wintel
> speak and isn't used in NVMe.  My new uuid tree tries to fix up
> the confusion by renaming our uuid_be type to uuid_t and uuid_le
> to guid_t.

Thanks for clarifying.

At one point we had a uuid before we allowed user-space to pass a nguid
and it was little endian, but it was a hack anyway.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-03  8:03         ` Sagi Grimberg
  0 siblings, 0 replies; 34+ messages in thread
From: Sagi Grimberg @ 2017-06-03  8:03 UTC (permalink / raw)



>>>   	switch (type) {
>>>   	case NVME_NQN_NVME:
>>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>>> index cfc5c7fb0ab7..4c6cb5ea1186 100644
>>> --- a/drivers/nvme/target/nvmet.h
>>> +++ b/drivers/nvme/target/nvmet.h
>>> @@ -46,6 +46,7 @@ struct nvmet_ns {
>>>   	u32			blksize_shift;
>>>   	loff_t			size;
>>>   	u8			nguid[16];
>>> +	uuid_be			uuid;
>>
>> This should be uuid_le to match the rest of NVMe.
> 
> It shouldn't.  A RFC4122 uuid is always in "big endian" notation.
> A little endiane uuid doesn't exist, it's called a GUID in Wintel
> speak and isn't used in NVMe.  My new uuid tree tries to fix up
> the confusion by renaming our uuid_be type to uuid_t and uuid_le
> to guid_t.

Thanks for clarifying.

At one point we had a uuid before we allowed user-space to pass a nguid
and it was little endian, but it was a hack anyway.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-02  9:45   ` Johannes Thumshirn
@ 2017-06-05  5:31     ` Christoph Hellwig
  -1 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2017-06-05  5:31 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
	maxg, Linux NVMe Mailinglist, Linux Kernel Mailinglist

What about a little helper like this: ?

static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
		void *id, off_t *off)
{
	struct nvme_ns_identifier_hdr hdr = {
		.nidt	= type,
		.nidl	= len,
	};
	u16 status;

	status = nvmet_copy_to_sgl(req, *off, &hdr, sizeof(hdr));
	if (status)
		return status;
	*off += sizeof(hdr);

	status = nvmet_copy_to_sgl(req, off, id, len);
	if (status)
		return status;
	*off += len;

	return 0;
}

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-05  5:31     ` Christoph Hellwig
  0 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2017-06-05  5:31 UTC (permalink / raw)


What about a little helper like this: ?

static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
		void *id, off_t *off)
{
	struct nvme_ns_identifier_hdr hdr = {
		.nidt	= type,
		.nidl	= len,
	};
	u16 status;

	status = nvmet_copy_to_sgl(req, *off, &hdr, sizeof(hdr));
	if (status)
		return status;
	*off += sizeof(hdr);

	status = nvmet_copy_to_sgl(req, off, id, len);
	if (status)
		return status;
	*off += len;

	return 0;
}

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-05  5:31     ` Christoph Hellwig
@ 2017-06-06  7:17       ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-06  7:17 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Sagi Grimberg, Keith Busch, Hannes Reinecke, maxg,
	Linux NVMe Mailinglist, Linux Kernel Mailinglist

On 06/05/2017 07:31 AM, Christoph Hellwig wrote:
> What about a little helper like this: ?
> 
> static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
> 		void *id, off_t *off)
> {
> 	struct nvme_ns_identifier_hdr hdr = {
> 		.nidt	= type,
> 		.nidl	= len,
> 	};
> 	u16 status;
> 
> 	status = nvmet_copy_to_sgl(req, *off, &hdr, sizeof(hdr));
> 	if (status)
> 		return status;
> 	*off += sizeof(hdr);
> 
> 	status = nvmet_copy_to_sgl(req, off, id, len);
> 	if (status)
> 		return status;
> 	*off += len;
> 
> 	return 0;
> }
> 

Yep sounds good. Sorry I didn't see this for the v4 submission, I'll
include in (the hopefully final) v5.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-06  7:17       ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-06  7:17 UTC (permalink / raw)


On 06/05/2017 07:31 AM, Christoph Hellwig wrote:
> What about a little helper like this: ?
> 
> static u16 nvmet_copy_ns_identifier(struct nvmet_req *req, u8 type, u8 len,
> 		void *id, off_t *off)
> {
> 	struct nvme_ns_identifier_hdr hdr = {
> 		.nidt	= type,
> 		.nidl	= len,
> 	};
> 	u16 status;
> 
> 	status = nvmet_copy_to_sgl(req, *off, &hdr, sizeof(hdr));
> 	if (status)
> 		return status;
> 	*off += sizeof(hdr);
> 
> 	status = nvmet_copy_to_sgl(req, off, id, len);
> 	if (status)
> 		return status;
> 	*off += len;
> 
> 	return 0;
> }
> 

Yep sounds good. Sorry I didn't see this for the v4 submission, I'll
include in (the hopefully final) v5.

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-06  7:17       ` Johannes Thumshirn
@ 2017-06-06  7:19         ` Christoph Hellwig
  -1 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2017-06-06  7:19 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, Hannes Reinecke,
	maxg, Linux NVMe Mailinglist, Linux Kernel Mailinglist

Can you rebase on top of the new uuid_t type and the plain uuid_*
prefixed helpers here:

http://git.infradead.org/users/hch/uuid.git/shortlog/refs/heads/for-next

I'll declare that branch stable soon and don't want to introduce
more users of the old types.  Thanks!

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-06  7:19         ` Christoph Hellwig
  0 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2017-06-06  7:19 UTC (permalink / raw)


Can you rebase on top of the new uuid_t type and the plain uuid_*
prefixed helpers here:

http://git.infradead.org/users/hch/uuid.git/shortlog/refs/heads/for-next

I'll declare that branch stable soon and don't want to introduce
more users of the old types.  Thanks!

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
  2017-06-06  7:19         ` Christoph Hellwig
@ 2017-06-06  7:20           ` Johannes Thumshirn
  -1 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-06  7:20 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Sagi Grimberg, Keith Busch, Hannes Reinecke, maxg,
	Linux NVMe Mailinglist, Linux Kernel Mailinglist

On 06/06/2017 09:19 AM, Christoph Hellwig wrote:
> Can you rebase on top of the new uuid_t type and the plain uuid_*
> prefixed helpers here:
> 
> http://git.infradead.org/users/hch/uuid.git/shortlog/refs/heads/for-next
> 
> I'll declare that branch stable soon and don't want to introduce
> more users of the old types.  Thanks!

Sure


-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v3 3/8] nvmet: implement namespace identify descriptor list
@ 2017-06-06  7:20           ` Johannes Thumshirn
  0 siblings, 0 replies; 34+ messages in thread
From: Johannes Thumshirn @ 2017-06-06  7:20 UTC (permalink / raw)


On 06/06/2017 09:19 AM, Christoph Hellwig wrote:
> Can you rebase on top of the new uuid_t type and the plain uuid_*
> prefixed helpers here:
> 
> http://git.infradead.org/users/hch/uuid.git/shortlog/refs/heads/for-next
> 
> I'll declare that branch stable soon and don't want to introduce
> more users of the old types.  Thanks!

Sure


-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2017-06-06  7:20 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-02  9:45 [PATCH v3 0/8] Implement NVMe Namespace Descriptor Identification Johannes Thumshirn
2017-06-02  9:45 ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 1/8] nvme: introduce NVMe Namespace Identification Descriptor structures Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 2/8] nvme: rename uuid to nguid in nvme_ns Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 3/8] nvmet: implement namespace identify descriptor list Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn
2017-06-02 13:46   ` Sagi Grimberg
2017-06-02 13:46     ` Sagi Grimberg
2017-06-02 13:54     ` Johannes Thumshirn
2017-06-02 13:54       ` Johannes Thumshirn
2017-06-03  5:18     ` Christoph Hellwig
2017-06-03  5:18       ` Christoph Hellwig
2017-06-03  8:03       ` Sagi Grimberg
2017-06-03  8:03         ` Sagi Grimberg
2017-06-05  5:31   ` Christoph Hellwig
2017-06-05  5:31     ` Christoph Hellwig
2017-06-06  7:17     ` Johannes Thumshirn
2017-06-06  7:17       ` Johannes Thumshirn
2017-06-06  7:19       ` Christoph Hellwig
2017-06-06  7:19         ` Christoph Hellwig
2017-06-06  7:20         ` Johannes Thumshirn
2017-06-06  7:20           ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 4/8] nvmet: add uuid field to nvme_ns and populate via configfs Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 5/8] nvme: get list of namespace descriptors Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 6/8] nvme: provide UUID value to userspace Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 7/8] nvmet: allow overriding the NVMe VS via configfs Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn
2017-06-02  9:45 ` [PATCH v3 8/8] nvmet: use NVME_IDENTIFY_DATA_SIZE Johannes Thumshirn
2017-06-02  9:45   ` Johannes Thumshirn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.