All of lore.kernel.org
 help / color / mirror / Atom feed
* NVMe over Fabrics: NQN UUID byte order
@ 2016-06-24 18:22 Verkamp, Daniel
  2016-06-28  8:45 ` Christoph Hellwig
  0 siblings, 1 reply; 8+ messages in thread
From: Verkamp, Daniel @ 2016-06-24 18:22 UTC (permalink / raw)


The NVMe 1.2.1 specification, section 7.9 (NVMe Qualified Names), says
that the UUID format of NQN is based on RFC 4122, which explicitly
requires all fields to be in big-endian/network byte order (section
4.1.2, Layout and Byte Order).

However, the current NVMe over Fabrics host code generates and formats
the Host Identifier UUID in little-endian byte order:

>static struct nvmf_host *nvmf_host_default(void)
>{
[...]
>	uuid_le_gen(&host->id);
>	snprintf(host->nqn, NVMF_NQN_SIZE,
>		"nqn.2014-08.org.nvmexpress:NVMf:uuid:%pUl", &host-
>id);

Is this intentional or an oversight?

This could probably use some clarification on the NVMe specification
side either way.

Thanks,
-- Daniel Verkamp

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

* NVMe over Fabrics: NQN UUID byte order
  2016-06-24 18:22 NVMe over Fabrics: NQN UUID byte order Verkamp, Daniel
@ 2016-06-28  8:45 ` Christoph Hellwig
  2016-06-28 18:20   ` [PATCH] nvme-fabrics: change NQN UUID to big-endian format Daniel Verkamp
  0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2016-06-28  8:45 UTC (permalink / raw)


On Fri, Jun 24, 2016@06:22:11PM +0000, Verkamp, Daniel wrote:
> The NVMe 1.2.1 specification, section 7.9 (NVMe Qualified Names), says
> that the UUID format of NQN is based on RFC 4122, which explicitly
> requires all fields to be in big-endian/network byte order (section
> 4.1.2, Layout and Byte Order).
> 
> However, the current NVMe over Fabrics host code generates and formats
> the Host Identifier UUID in little-endian byte order:
> 
> >static struct nvmf_host *nvmf_host_default(void)
> >{
> [...]
> >	uuid_le_gen(&host->id);
> >	snprintf(host->nqn, NVMF_NQN_SIZE,
> >		"nqn.2014-08.org.nvmexpress:NVMf:uuid:%pUl", &host-
> >id);
> 
> Is this intentional or an oversight?

This isn't intentional.  Can you send a patch to use uuid_be and
uuid_be_gen instead?

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

* [PATCH] nvme-fabrics: change NQN UUID to big-endian format
  2016-06-28  8:45 ` Christoph Hellwig
@ 2016-06-28 18:20   ` Daniel Verkamp
  2016-06-28 19:55     ` Freyensee, James P
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Daniel Verkamp @ 2016-06-28 18:20 UTC (permalink / raw)


NVM Express 1.2.1 section 7.9, NVMe Qualified Names, specifies that the
UUID format of NQN uses a UUID based on RFC 4122.

RFC 4122 specifies that the UUID is encoded in big-endian byte order.

Switch the NVMe over Fabrics host ID field from little-endian UUID to
big-endian UUID to match the specification.

Signed-off-by: Daniel Verkamp <daniel.verkamp at intel.com>
---
 drivers/nvme/host/fabrics.c | 10 +++++-----
 drivers/nvme/host/fabrics.h |  2 +-
 include/linux/nvme.h        |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index b86b637..918310c 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -56,7 +56,7 @@ static struct nvmf_host *nvmf_host_add(const char *hostnqn)
 
 	kref_init(&host->ref);
 	memcpy(host->nqn, hostnqn, NVMF_NQN_SIZE);
-	uuid_le_gen(&host->id);
+	uuid_be_gen(&host->id);
 
 	list_add_tail(&host->list, &nvmf_hosts);
 out_unlock:
@@ -73,9 +73,9 @@ static struct nvmf_host *nvmf_host_default(void)
 		return NULL;
 
 	kref_init(&host->ref);
-	uuid_le_gen(&host->id);
+	uuid_be_gen(&host->id);
 	snprintf(host->nqn, NVMF_NQN_SIZE,
-		"nqn.2014-08.org.nvmexpress:NVMf:uuid:%pUl", &host->id);
+		"nqn.2014-08.org.nvmexpress:NVMf:uuid:%pUb", &host->id);
 
 	mutex_lock(&nvmf_hosts_mutex);
 	list_add_tail(&host->list, &nvmf_hosts);
@@ -371,7 +371,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 	if (!data)
 		return -ENOMEM;
 
-	memcpy(&data->hostid, &ctrl->opts->host->id, sizeof(uuid_le));
+	memcpy(&data->hostid, &ctrl->opts->host->id, sizeof(uuid_be));
 	data->cntlid = cpu_to_le16(0xffff);
 	strncpy(data->subsysnqn, ctrl->opts->subsysnqn, NVMF_NQN_SIZE);
 	strncpy(data->hostnqn, ctrl->opts->host->nqn, NVMF_NQN_SIZE);
@@ -430,7 +430,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
 	if (!data)
 		return -ENOMEM;
 
-	memcpy(&data->hostid, &ctrl->opts->host->id, sizeof(uuid_le));
+	memcpy(&data->hostid, &ctrl->opts->host->id, sizeof(uuid_be));
 	data->cntlid = cpu_to_le16(ctrl->cntlid);
 	strncpy(data->subsysnqn, ctrl->opts->subsysnqn, NVMF_NQN_SIZE);
 	strncpy(data->hostnqn, ctrl->opts->host->nqn, NVMF_NQN_SIZE);
diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index b540674..9d31466 100644
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -34,7 +34,7 @@ struct nvmf_host {
 	struct kref		ref;
 	struct list_head	list;
 	char			nqn[NVMF_NQN_SIZE];
-	uuid_le			id;
+	uuid_be			id;
 };
 
 /**
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index d8b37ba..7676557 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -794,7 +794,7 @@ struct nvmf_connect_command {
 };
 
 struct nvmf_connect_data {
-	uuid_le		hostid;
+	uuid_be		hostid;
 	__le16		cntlid;
 	char		resv4[238];
 	char		subsysnqn[NVMF_NQN_FIELD_LEN];
-- 
2.7.4

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

* [PATCH] nvme-fabrics: change NQN UUID to big-endian format
  2016-06-28 18:20   ` [PATCH] nvme-fabrics: change NQN UUID to big-endian format Daniel Verkamp
@ 2016-06-28 19:55     ` Freyensee, James P
  2016-06-30  6:41     ` Christoph Hellwig
  2016-07-13 10:12     ` Sagi Grimberg
  2 siblings, 0 replies; 8+ messages in thread
From: Freyensee, James P @ 2016-06-28 19:55 UTC (permalink / raw)


On Tue, 2016-06-28@11:20 -0700, Daniel Verkamp wrote:
> NVM Express 1.2.1 section 7.9, NVMe Qualified Names, specifies that
> the
> UUID format of NQN uses a UUID based on RFC 4122.
> 
> RFC 4122 specifies that the UUID is encoded in big-endian byte order.
> 
> Switch the NVMe over Fabrics host ID field from little-endian UUID to
> big-endian UUID to match the specification.

Good catch.

Reviewed-by: Jay Freyensee <james_p_freyensee at linux.intel.com>

> 
> Signed-off-by: Daniel Verkamp <daniel.verkamp at intel.com>

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

* [PATCH] nvme-fabrics: change NQN UUID to big-endian format
  2016-06-28 18:20   ` [PATCH] nvme-fabrics: change NQN UUID to big-endian format Daniel Verkamp
  2016-06-28 19:55     ` Freyensee, James P
@ 2016-06-30  6:41     ` Christoph Hellwig
  2016-08-18 19:48       ` Verkamp, Daniel
  2016-07-13 10:12     ` Sagi Grimberg
  2 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2016-06-30  6:41 UTC (permalink / raw)


On Tue, Jun 28, 2016@11:20:23AM -0700, Daniel Verkamp wrote:
> NVM Express 1.2.1 section 7.9, NVMe Qualified Names, specifies that the
> UUID format of NQN uses a UUID based on RFC 4122.
> 
> RFC 4122 specifies that the UUID is encoded in big-endian byte order.
> 
> Switch the NVMe over Fabrics host ID field from little-endian UUID to
> big-endian UUID to match the specification.
> 
> Signed-off-by: Daniel Verkamp <daniel.verkamp at intel.com>

Thanks a lot Daniel, I'll send this on to Jens.

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

* [PATCH] nvme-fabrics: change NQN UUID to big-endian format
  2016-06-28 18:20   ` [PATCH] nvme-fabrics: change NQN UUID to big-endian format Daniel Verkamp
  2016-06-28 19:55     ` Freyensee, James P
  2016-06-30  6:41     ` Christoph Hellwig
@ 2016-07-13 10:12     ` Sagi Grimberg
  2 siblings, 0 replies; 8+ messages in thread
From: Sagi Grimberg @ 2016-07-13 10:12 UTC (permalink / raw)


Thanks Daniel,

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

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

* [PATCH] nvme-fabrics: change NQN UUID to big-endian format
  2016-06-30  6:41     ` Christoph Hellwig
@ 2016-08-18 19:48       ` Verkamp, Daniel
  2016-08-19  9:02         ` Sagi Grimberg
  0 siblings, 1 reply; 8+ messages in thread
From: Verkamp, Daniel @ 2016-08-18 19:48 UTC (permalink / raw)


On Wed, 2016-06-29@23:41 -0700, Christoph Hellwig wrote:
> On Tue, Jun 28, 2016@11:20:23AM -0700, Daniel Verkamp wrote:
> > 
> > NVM Express 1.2.1 section 7.9, NVMe Qualified Names, specifies that
> > the
> > UUID format of NQN uses a UUID based on RFC 4122.
> > 
> > RFC 4122 specifies that the UUID is encoded in big-endian byte
> > order.
> > 
> > Switch the NVMe over Fabrics host ID field from little-endian UUID
> > to
> > big-endian UUID to match the specification.
> > 
> > Signed-off-by: Daniel Verkamp <daniel.verkamp at intel.com>
> 
> Thanks a lot Daniel, I'll send this on to Jens.

Hi,

Just wanted to check on this and make sure it didn't get dropped.
It doesn't appear to be in Linus's tree or in any branches of nvme-
fabrics.git yet.

Thanks,
-- Daniel

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

* [PATCH] nvme-fabrics: change NQN UUID to big-endian format
  2016-08-18 19:48       ` Verkamp, Daniel
@ 2016-08-19  9:02         ` Sagi Grimberg
  0 siblings, 0 replies; 8+ messages in thread
From: Sagi Grimberg @ 2016-08-19  9:02 UTC (permalink / raw)



> Hi,
>
> Just wanted to check on this and make sure it didn't get dropped.
> It doesn't appear to be in Linus's tree or in any branches of nvme-
> fabrics.git yet.

Thanks for the reminder,

Queued to nvmf-4.8-rc.

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

end of thread, other threads:[~2016-08-19  9:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-24 18:22 NVMe over Fabrics: NQN UUID byte order Verkamp, Daniel
2016-06-28  8:45 ` Christoph Hellwig
2016-06-28 18:20   ` [PATCH] nvme-fabrics: change NQN UUID to big-endian format Daniel Verkamp
2016-06-28 19:55     ` Freyensee, James P
2016-06-30  6:41     ` Christoph Hellwig
2016-08-18 19:48       ` Verkamp, Daniel
2016-08-19  9:02         ` Sagi Grimberg
2016-07-13 10:12     ` Sagi Grimberg

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.