[PATCH] nvmet: Add support for HOST_ID feature.

[PATCH] nvmet: Add support for HOST_ID feature.

[Omri Mann]

Add host_id feature to NVMeoF targets. This will be required in order to
support reservations.
Signed-off-by: Omri Mann <omri at excelero.com>
---
 drivers/nvme/target/admin-cmd.c | 7 +++++++
 drivers/nvme/target/nvmet.h     | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index a53bb66..071de0b 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -461,6 +461,10 @@ static void nvmet_execute_set_features(struct nvmet_req *req)
 		req->sq->ctrl->kato = DIV_ROUND_UP(val32, 1000);
 		nvmet_set_result(req, req->sq->ctrl->kato);
 		break;
+	case NVME_FEAT_HOST_ID:
+		status = nvmet_copy_from_sgl(req, 0, &req->sq->ctrl->host_id,
+						8);
+		break;
 	default:
 		status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
 		break;
@@ -509,6 +513,9 @@ static void nvmet_execute_get_features(struct nvmet_req *req)
 	case NVME_FEAT_KATO:
 		nvmet_set_result(req, req->sq->ctrl->kato * 1000);
 		break;
+	case NVME_FEAT_HOST_ID:
+		status = nvmet_copy_to_sgl(req, 0, &req->sq->ctrl->host_id, 8);
+		break;
 	default:
 		status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
 		break;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index e3b244c..841f35d 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -132,6 +132,7 @@ struct nvmet_ctrl {
 
 	char			subsysnqn[NVMF_NQN_FIELD_LEN];
 	char			hostnqn[NVMF_NQN_FIELD_LEN];
+	u64			host_id;
 };
 
 struct nvmet_subsys {
-- 
1.8.3.1

[PATCH] nvmet: Add support for HOST_ID feature.

[Sagi Grimberg]

> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index e3b244c..841f35d 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -132,6 +132,7 @@ struct nvmet_ctrl {
>   
>   	char			subsysnqn[NVMF_NQN_FIELD_LEN];
>   	char			hostnqn[NVMF_NQN_FIELD_LEN];
> +	u64			host_id;

I'd move it to the first cacheline given it will be accessed
in the hot path.

Other than that, looks fine,

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

[PATCH] nvmet: Add support for HOST_ID feature.

[Christoph Hellwig]

On Mon, Aug 28, 2017@03:07:37PM +0300, Omri Mann wrote:
> Add host_id feature to NVMeoF targets. This will be required in order to
> support reservations.

For Fabrics the host id is set in the connect command and Set Features
must return an error (see section 5.21.1.19.2 in NVMe 1.3).  But the
Get Features is still useful.

[PATCH] nvmet: Add support for HOST_ID feature.

[Verkamp, Daniel]

> -----Original Message-----
> From: Linux-nvme [mailto:linux-nvme-bounces at lists.infradead.org] On Behalf
> Of Sagi Grimberg
> Sent: Monday, August 28, 2017 6:44 AM
> To: Omri Mann <omri at excelero.com>; linux-nvme at lists.infradead.org
> Cc: Mike Christie <mchristi at redhat.com>
> Subject: Re: [PATCH] nvmet: Add support for HOST_ID feature.
> 
> > diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> > index e3b244c..841f35d 100644
> > --- a/drivers/nvme/target/nvmet.h
> > +++ b/drivers/nvme/target/nvmet.h
> > @@ -132,6 +132,7 @@ struct nvmet_ctrl {
> >
> >   	char			subsysnqn[NVMF_NQN_FIELD_LEN];
> >   	char			hostnqn[NVMF_NQN_FIELD_LEN];
> > +	u64			host_id;
> 
> I'd move it to the first cacheline given it will be accessed
> in the hot path.
> 
> Other than that, looks fine,
> 
> Reviewed-by: Sagi Grimberg <sagi at grimberg.me>

The specification says that NVMe-oF must use a 128-bit host ID - from NVMe 1.3, 5.21.1.19.2:

"The Host Identifier is a mandatory feature in NVMe over Fabrics.  The Host Identifier shall be an extended 128-bit Host Identifier." 

The target should also check the EXHID bit in the Get Features - Host Identifier command.

Thanks,
-- Daniel Verkamp

[PATCH] nvmet: Add support for HOST_ID feature.

[Sagi Grimberg]

> The specification says that NVMe-oF must use a 128-bit host ID - from NVMe 1.3, 5.21.1.19.2:
> 
> "The Host Identifier is a mandatory feature in NVMe over Fabrics.  The Host Identifier shall be an extended 128-bit Host Identifier."
> 
> The target should also check the EXHID bit in the Get Features - Host Identifier command.

That's true, We should work with uuid_t for hostids. thanks Daniel.

[PATCH] nvmet: Add support for HOST_ID feature.

[Christoph Hellwig]

On Mon, Aug 28, 2017@11:15:57PM +0300, Sagi Grimberg wrote:
> That's true, We should work with uuid_t for hostids. thanks Daniel.

And we already do on the host side, it's just that the target didn't
need to parse it yet.