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

Groups > linux.kernel > #1658825 > unrolled thread

[PATCH v5 0/9] Implement NVMe Namespace Descriptor Identification

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 0/9] Implement NVMe Namespace Descriptor Identification Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-06 16:40 +0200
    [PATCH v5 2/9] nvmet: use NVME_IDENTIFY_DATA_SIZE Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-06 16:40 +0200
    [PATCH v5 5/9] nvmet: implement namespace identify descriptor list Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-06 16:40 +0200
    Re: [PATCH v5 0/9] Implement NVMe Namespace Descriptor Identification Sagi Grimberg <sagi@grimberg.me> - 2017-06-06 20:30 +0200
      Re: [PATCH v5 0/9] Implement NVMe Namespace Descriptor Identification Johannes Thumshirn <jthumshirn@suse.de> - 2017-06-07 09:00 +0200

#1658825 — [PATCH v5 0/9] 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 (in
addition to 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.

The code is tested using the nvme-loop loopback target and cut against
Christoph's uuid/for-next branch.

A patch for nvmetcli and nvme-cli will follow shortly.

Changes to v4:
* Add nvmet_copy_ns_identifier() helper (Christoph)
* Move 'nvmet: use NVME_IDENTIFY_DATA_SIZE' to beginning of series (Christoph)
* Protect subsystem version with nvmet_config_sem (Christoph)
* Get rid of local variables in nvmet_subsys_version_show (Christoph)
* Included dots in subsystem version (Sagi)
* Merge nvme_parse_ns_descs() into nvme_identify_ns_descs() (Christoph)
* Skip unknown NS descriptors in nvme_identify_ns_descs() (Christoph)
* Add sg_zeroout_area() helper to lib/scatterlist.c (Christoph)
* I'm not sure if I shall use nvme_ns_identifier_hdr or nvme_ns_id_desc
* Rebased on Christoph's uuid/for-next branch
* Collected Acks

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 (9):
  scatterlist: add sg_zeroout_area() helper
  nvmet: use NVME_IDENTIFY_DATA_SIZE
  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

 drivers/nvme/host/core.c        | 110 ++++++++++++++++++++++++++++++++++++++--
 drivers/nvme/host/nvme.h        |   1 +
 drivers/nvme/target/admin-cmd.c |  65 +++++++++++++++++++++++-
 drivers/nvme/target/configfs.c  |  68 +++++++++++++++++++++++++
 drivers/nvme/target/core.c      |   3 +-
 drivers/nvme/target/discovery.c |   2 +-
 drivers/nvme/target/nvmet.h     |   1 +
 include/linux/nvme.h            |  23 +++++++++
 include/linux/scatterlist.h     |   3 ++
 lib/scatterlist.c               |  37 ++++++++++++++
 10 files changed, 305 insertions(+), 8 deletions(-)

-- 
2.12.3

[toc] | [next] | [standalone]

#1658828 — [PATCH v5 2/9] 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>
Reviewed-by: Max Gurtovoy <maxg@mellanox.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
 drivers/nvme/target/admin-cmd.c | 4 ++--
 drivers/nvme/target/discovery.c | 2 +-
 include/linux/nvme.h            | 2 ++
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index ff1f97006322..96c144325443 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);
@@ -504,7 +504,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 =
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index e400a69fa1d3..c004af5faa48 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -659,6 +659,8 @@ struct nvme_identify {
 	__u32			rsvd11[5];
 };
 
+#define NVME_IDENTIFY_DATA_SIZE 4096
+
 struct nvme_features {
 	__u8			opcode;
 	__u8			flags;
-- 
2.12.3

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

#1658831 — [PATCH v5 5/9] nvmet: implement namespace identify descriptor list

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>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
 drivers/nvme/target/admin-cmd.c | 61 +++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/core.c      |  3 +-
 drivers/nvme/target/nvmet.h     |  1 +
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 96c144325443..f15aa72e28b4 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -367,6 +367,64 @@ static void nvmet_execute_identify_nslist(struct nvmet_req *req)
 	nvmet_req_complete(req, status);
 }
 
+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;
+}
+
+static void nvmet_execute_identify_desclist(struct nvmet_req *req)
+{
+	struct nvmet_ns *ns;
+	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))) {
+		status = nvmet_copy_ns_identifier(req, NVME_NIDT_UUID,
+						  NVME_NIDT_UUID_LEN,
+						  &ns->uuid, &off);
+		if (status)
+			goto out_put_ns;
+	}
+	if (memchr_inv(ns->nguid, 0, sizeof(ns->nguid))) {
+		status = nvmet_copy_ns_identifier(req, NVME_NIDT_NGUID,
+						  NVME_NIDT_NGUID_LEN,
+						  &ns->nguid, &off);
+		if (status)
+			goto out_put_ns;
+	}
+
+	if (sg_zeroout_area(req->sg, req->sg_cnt, NVME_IDENTIFY_DATA_SIZE, off)
+			!= NVME_IDENTIFY_DATA_SIZE - off)
+		status = NVME_SC_INTERNAL | NVME_SC_DNR;
+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 +573,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..b5b4ac103748 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -380,6 +380,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
 
 	ns->nsid = nsid;
 	ns->subsys = subsys;
+	uuid_gen(&ns->uuid);
 
 	return ns;
 }
@@ -926,7 +927,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 8ff6e430b30a..747bbdb4f9c6 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -47,6 +47,7 @@ struct nvmet_ns {
 	u32			blksize_shift;
 	loff_t			size;
 	u8			nguid[16];
+	uuid_t			uuid;
 
 	bool			enabled;
 	struct nvmet_subsys	*subsys;
-- 
2.12.3

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

#1659040

> * I'm not sure if I shall use nvme_ns_identifier_hdr or nvme_ns_id_desc

I was kinda hoping you will :(

I won't insist if people are fine with that as is...

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

#1659415

On 06/06/2017 08:27 PM, Sagi Grimberg wrote:
> 
>> * I'm not sure if I shall use nvme_ns_identifier_hdr or nvme_ns_id_desc
> 
> I was kinda hoping you will :(
> 
> I won't insist if people are fine with that as is...

OK, I'll change it for ya :-)

-- 
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web