* [PATCH] Add new clear_ids attribute for passthru targets
@ 2022-06-22 22:47 Alan Adamson
2022-06-26 13:38 ` Sagi Grimberg
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Alan Adamson @ 2022-06-22 22:47 UTC (permalink / raw)
To: linux-nvme; +Cc: alan.adamson, kbusch, hch, sagi
If the clear_ids attribute is set to true, the EUI/GUID/UUID is
cleared for the passthru target. By default, loop targets will
set clear_ids to true.
This resolves an issue where a connect to a passthru target fails
when using a trtype of 'loop' because EUI/GUID/UUID is not unique.
Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
---
drivers/nvme/target/configfs.c | 20 ++++++++++++
drivers/nvme/target/core.c | 4 +++
drivers/nvme/target/nvmet.h | 1 +
drivers/nvme/target/passthru.c | 58 ++++++++++++++++++++++++++++++++++
4 files changed, 83 insertions(+)
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e44b2988759e..ff77c3d2354f 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -773,11 +773,31 @@ static ssize_t nvmet_passthru_io_timeout_store(struct config_item *item,
}
CONFIGFS_ATTR(nvmet_passthru_, io_timeout);
+static ssize_t nvmet_passthru_clear_ids_show(struct config_item *item,
+ char *page)
+{
+ return sprintf(page, "%u\n", to_subsys(item->ci_parent)->clear_ids);
+}
+
+static ssize_t nvmet_passthru_clear_ids_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_subsys *subsys = to_subsys(item->ci_parent);
+ unsigned int clear_ids;
+
+ if (kstrtouint(page, 0, &clear_ids))
+ return -EINVAL;
+ subsys->clear_ids = clear_ids;
+ return count;
+}
+CONFIGFS_ATTR(nvmet_passthru_, clear_ids);
+
static struct configfs_attribute *nvmet_passthru_attrs[] = {
&nvmet_passthru_attr_device_path,
&nvmet_passthru_attr_enable,
&nvmet_passthru_attr_admin_timeout,
&nvmet_passthru_attr_io_timeout,
+ &nvmet_passthru_attr_clear_ids,
NULL,
};
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 90e75324dae0..d4203d582343 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1374,6 +1374,10 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
ctrl->port = req->port;
ctrl->ops = req->ops;
+ /* By default, set loop targets to clear IDS by default */
+ if (ctrl->port->disc_addr.trtype == NVMF_TRTYPE_LOOP)
+ subsys->clear_ids = 1;
+
INIT_WORK(&ctrl->async_event_work, nvmet_async_event_work);
INIT_LIST_HEAD(&ctrl->async_events);
INIT_RADIX_TREE(&ctrl->p2p_ns_map, GFP_KERNEL);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 69818752a33a..2b3e5719f24e 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -249,6 +249,7 @@ struct nvmet_subsys {
struct config_group passthru_group;
unsigned int admin_timeout;
unsigned int io_timeout;
+ unsigned int clear_ids;
#endif /* CONFIG_NVME_TARGET_PASSTHRU */
#ifdef CONFIG_BLK_DEV_ZONED
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index b1f7efab3918..e8c033fbdb5c 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -30,6 +30,55 @@ void nvmet_passthrough_override_cap(struct nvmet_ctrl *ctrl)
ctrl->cap &= ~(1ULL << 43);
}
+static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
+{
+ struct nvmet_ctrl *ctrl = req->sq->ctrl;
+ void *data;
+ struct nvme_ns_id_desc *cur;
+ u16 status = NVME_SC_SUCCESS;
+ u8 csi;
+ int pos, len;
+ bool csi_seen;
+
+ if (!ctrl->subsys->clear_ids)
+ return status;
+
+ data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
+ if (!data)
+ return NVME_SC_INTERNAL;
+
+ status = nvmet_copy_from_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
+ if (status)
+ goto out_free;
+
+ for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
+ cur = data + pos;
+
+ if (cur->nidl == 0)
+ break;
+ len = cur->nidl;
+ if (cur->nidt == NVME_NIDT_CSI) {
+ memcpy(&csi, data + pos + sizeof(struct nvme_ns_id_desc),
+ NVME_NIDT_CSI_LEN);
+ csi_seen = 1;
+ break;
+ }
+ len += sizeof(struct nvme_ns_id_desc);
+ }
+
+ memset(data, 0, NVME_IDENTIFY_DATA_SIZE);
+ if (csi_seen) {
+ cur = data;
+ cur->nidt = NVME_NIDT_CSI;
+ cur->nidl = NVME_NIDT_CSI_LEN;
+ memcpy(data + sizeof(struct nvme_ns_id_desc), &csi, NVME_NIDT_CSI_LEN);
+ }
+ status = nvmet_copy_to_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
+out_free:
+ kfree(data);
+ return status;
+}
+
static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
{
struct nvmet_ctrl *ctrl = req->sq->ctrl;
@@ -127,6 +176,7 @@ static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
{
+ struct nvmet_ctrl *ctrl = req->sq->ctrl;
u16 status = NVME_SC_SUCCESS;
struct nvme_id_ns *id;
int i;
@@ -152,6 +202,11 @@ static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
*/
id->mc = 0;
+ if (ctrl->subsys->clear_ids) {
+ memset(id->nguid, 0, NVME_NIDT_NGUID_LEN);
+ memset(id->eui64, 0, NVME_NIDT_EUI64_LEN);
+ }
+
status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
out_free:
@@ -176,6 +231,9 @@ static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
case NVME_ID_CNS_NS:
nvmet_passthru_override_id_ns(req);
break;
+ case NVME_ID_CNS_NS_DESC_LIST:
+ nvmet_passthru_override_id_descs(req);
+ break;
}
} else if (status < 0)
status = NVME_SC_INTERNAL;
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Add new clear_ids attribute for passthru targets
2022-06-22 22:47 [PATCH] Add new clear_ids attribute for passthru targets Alan Adamson
@ 2022-06-26 13:38 ` Sagi Grimberg
2022-06-26 13:42 ` Sagi Grimberg
2022-06-27 15:16 ` Keith Busch
2022-06-27 21:00 ` Chaitanya Kulkarni
2 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2022-06-26 13:38 UTC (permalink / raw)
To: Alan Adamson, linux-nvme; +Cc: kbusch, hch
On 6/23/22 01:47, Alan Adamson wrote:
> If the clear_ids attribute is set to true, the EUI/GUID/UUID is
> cleared for the passthru target. By default, loop targets will
> set clear_ids to true.
>
> This resolves an issue where a connect to a passthru target fails
> when using a trtype of 'loop' because EUI/GUID/UUID is not unique.
>
> Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
> ---
> drivers/nvme/target/configfs.c | 20 ++++++++++++
> drivers/nvme/target/core.c | 4 +++
> drivers/nvme/target/nvmet.h | 1 +
> drivers/nvme/target/passthru.c | 58 ++++++++++++++++++++++++++++++++++
> 4 files changed, 83 insertions(+)
>
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index e44b2988759e..ff77c3d2354f 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -773,11 +773,31 @@ static ssize_t nvmet_passthru_io_timeout_store(struct config_item *item,
> }
> CONFIGFS_ATTR(nvmet_passthru_, io_timeout);
>
> +static ssize_t nvmet_passthru_clear_ids_show(struct config_item *item,
> + char *page)
> +{
> + return sprintf(page, "%u\n", to_subsys(item->ci_parent)->clear_ids);
> +}
> +
> +static ssize_t nvmet_passthru_clear_ids_store(struct config_item *item,
> + const char *page, size_t count)
> +{
> + struct nvmet_subsys *subsys = to_subsys(item->ci_parent);
> + unsigned int clear_ids;
> +
> + if (kstrtouint(page, 0, &clear_ids))
> + return -EINVAL;
> + subsys->clear_ids = clear_ids;
> + return count;
> +}
> +CONFIGFS_ATTR(nvmet_passthru_, clear_ids);
> +
> static struct configfs_attribute *nvmet_passthru_attrs[] = {
> &nvmet_passthru_attr_device_path,
> &nvmet_passthru_attr_enable,
> &nvmet_passthru_attr_admin_timeout,
> &nvmet_passthru_attr_io_timeout,
> + &nvmet_passthru_attr_clear_ids,
> NULL,
> };
>
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 90e75324dae0..d4203d582343 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1374,6 +1374,10 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
> ctrl->port = req->port;
> ctrl->ops = req->ops;
>
> + /* By default, set loop targets to clear IDS by default */
> + if (ctrl->port->disc_addr.trtype == NVMF_TRTYPE_LOOP)
> + subsys->clear_ids = 1;
> +
> INIT_WORK(&ctrl->async_event_work, nvmet_async_event_work);
> INIT_LIST_HEAD(&ctrl->async_events);
> INIT_RADIX_TREE(&ctrl->p2p_ns_map, GFP_KERNEL);
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 69818752a33a..2b3e5719f24e 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -249,6 +249,7 @@ struct nvmet_subsys {
> struct config_group passthru_group;
> unsigned int admin_timeout;
> unsigned int io_timeout;
> + unsigned int clear_ids;
> #endif /* CONFIG_NVME_TARGET_PASSTHRU */
>
> #ifdef CONFIG_BLK_DEV_ZONED
> diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
> index b1f7efab3918..e8c033fbdb5c 100644
> --- a/drivers/nvme/target/passthru.c
> +++ b/drivers/nvme/target/passthru.c
> @@ -30,6 +30,55 @@ void nvmet_passthrough_override_cap(struct nvmet_ctrl *ctrl)
> ctrl->cap &= ~(1ULL << 43);
> }
>
> +static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
> +{
> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
> + void *data;
> + struct nvme_ns_id_desc *cur;
> + u16 status = NVME_SC_SUCCESS;
> + u8 csi;
> + int pos, len;
> + bool csi_seen;
> +
> + if (!ctrl->subsys->clear_ids)
> + return status;
> +
> + data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
> + if (!data)
> + return NVME_SC_INTERNAL;
> +
> + status = nvmet_copy_from_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
> + if (status)
> + goto out_free;
> +
> + for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
> + cur = data + pos;
> +
> + if (cur->nidl == 0)
> + break;
> + len = cur->nidl;
> + if (cur->nidt == NVME_NIDT_CSI) {
> + memcpy(&csi, data + pos + sizeof(struct nvme_ns_id_desc),
> + NVME_NIDT_CSI_LEN);
> + csi_seen = 1;
> + break;
> + }
> + len += sizeof(struct nvme_ns_id_desc);
> + }
> +
> + memset(data, 0, NVME_IDENTIFY_DATA_SIZE);
> + if (csi_seen) {
> + cur = data;
> + cur->nidt = NVME_NIDT_CSI;
> + cur->nidl = NVME_NIDT_CSI_LEN;
> + memcpy(data + sizeof(struct nvme_ns_id_desc), &csi, NVME_NIDT_CSI_LEN);
> + }
> + status = nvmet_copy_to_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
> +out_free:
> + kfree(data);
> + return status;
> +}
I can't understand what this function does...
> +
> static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
> {
> struct nvmet_ctrl *ctrl = req->sq->ctrl;
> @@ -127,6 +176,7 @@ static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
>
> static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
> {
> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
> u16 status = NVME_SC_SUCCESS;
> struct nvme_id_ns *id;
> int i;
> @@ -152,6 +202,11 @@ static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
> */
> id->mc = 0;
>
> + if (ctrl->subsys->clear_ids) {
> + memset(id->nguid, 0, NVME_NIDT_NGUID_LEN);
> + memset(id->eui64, 0, NVME_NIDT_EUI64_LEN);
What about uuid?
> + }
> +
> status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
>
> out_free:
> @@ -176,6 +231,9 @@ static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
> case NVME_ID_CNS_NS:
> nvmet_passthru_override_id_ns(req);
> break;
> + case NVME_ID_CNS_NS_DESC_LIST:
> + nvmet_passthru_override_id_descs(req);
> + break;
> }
> } else if (status < 0)
> status = NVME_SC_INTERNAL;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add new clear_ids attribute for passthru targets
2022-06-26 13:38 ` Sagi Grimberg
@ 2022-06-26 13:42 ` Sagi Grimberg
0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2022-06-26 13:42 UTC (permalink / raw)
To: Alan Adamson, linux-nvme; +Cc: kbusch, hch
> I can't understand what this function does...
OK, just learned about it from a different thread. You can disregard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add new clear_ids attribute for passthru targets
2022-06-22 22:47 [PATCH] Add new clear_ids attribute for passthru targets Alan Adamson
2022-06-26 13:38 ` Sagi Grimberg
@ 2022-06-27 15:16 ` Keith Busch
2022-06-27 21:00 ` Chaitanya Kulkarni
2 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2022-06-27 15:16 UTC (permalink / raw)
To: Alan Adamson; +Cc: linux-nvme, hch, sagi
On Wed, Jun 22, 2022 at 03:47:24PM -0700, Alan Adamson wrote:
> If the clear_ids attribute is set to true, the EUI/GUID/UUID is
> cleared for the passthru target. By default, loop targets will
> set clear_ids to true.
>
> This resolves an issue where a connect to a passthru target fails
> when using a trtype of 'loop' because EUI/GUID/UUID is not unique.
This looks good to me.
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add new clear_ids attribute for passthru targets
2022-06-22 22:47 [PATCH] Add new clear_ids attribute for passthru targets Alan Adamson
2022-06-26 13:38 ` Sagi Grimberg
2022-06-27 15:16 ` Keith Busch
@ 2022-06-27 21:00 ` Chaitanya Kulkarni
2022-06-27 23:24 ` Alan Adamson
2 siblings, 1 reply; 6+ messages in thread
From: Chaitanya Kulkarni @ 2022-06-27 21:00 UTC (permalink / raw)
To: Alan Adamson; +Cc: linux-nvme
On 6/22/22 15:47, Alan Adamson wrote:
> If the clear_ids attribute is set to true, the EUI/GUID/UUID is
> cleared for the passthru target. By default, loop targets will
> set clear_ids to true.
>
> This resolves an issue where a connect to a passthru target fails
> when using a trtype of 'loop' because EUI/GUID/UUID is not unique.
>
> Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
> ---
> drivers/nvme/target/configfs.c | 20 ++++++++++++
> drivers/nvme/target/core.c | 4 +++
> drivers/nvme/target/nvmet.h | 1 +
> drivers/nvme/target/passthru.c | 58 ++++++++++++++++++++++++++++++++++
> 4 files changed, 83 insertions(+)
>
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index e44b2988759e..ff77c3d2354f 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -773,11 +773,31 @@ static ssize_t nvmet_passthru_io_timeout_store(struct config_item *item,
> }
> CONFIGFS_ATTR(nvmet_passthru_, io_timeout);
>
> +static ssize_t nvmet_passthru_clear_ids_show(struct config_item *item,
> + char *page)
> +{
> + return sprintf(page, "%u\n", to_subsys(item->ci_parent)->clear_ids);
should we use snprintf() ? (I don't think so just a thought.
> +}
> +
> +static ssize_t nvmet_passthru_clear_ids_store(struct config_item *item,
> + const char *page, size_t count)
> +{
> + struct nvmet_subsys *subsys = to_subsys(item->ci_parent);
> + unsigned int clear_ids;
> +
> + if (kstrtouint(page, 0, &clear_ids))
> + return -EINVAL;
> + subsys->clear_ids = clear_ids;
> + return count;
> +}
> +CONFIGFS_ATTR(nvmet_passthru_, clear_ids);
> +
> static struct configfs_attribute *nvmet_passthru_attrs[] = {
> &nvmet_passthru_attr_device_path,
> &nvmet_passthru_attr_enable,
> &nvmet_passthru_attr_admin_timeout,
> &nvmet_passthru_attr_io_timeout,
> + &nvmet_passthru_attr_clear_ids,
> NULL,
> };
>
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 90e75324dae0..d4203d582343 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1374,6 +1374,10 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
> ctrl->port = req->port;
> ctrl->ops = req->ops;
>
> + /* By default, set loop targets to clear IDS by default */
> + if (ctrl->port->disc_addr.trtype == NVMF_TRTYPE_LOOP)
> + subsys->clear_ids = 1;
> +
> INIT_WORK(&ctrl->async_event_work, nvmet_async_event_work);
> INIT_LIST_HEAD(&ctrl->async_events);
> INIT_RADIX_TREE(&ctrl->p2p_ns_map, GFP_KERNEL);
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 69818752a33a..2b3e5719f24e 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -249,6 +249,7 @@ struct nvmet_subsys {
> struct config_group passthru_group;
> unsigned int admin_timeout;
> unsigned int io_timeout;
> + unsigned int clear_ids;
> #endif /* CONFIG_NVME_TARGET_PASSTHRU */
>
> #ifdef CONFIG_BLK_DEV_ZONED
> diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
> index b1f7efab3918..e8c033fbdb5c 100644
> --- a/drivers/nvme/target/passthru.c
> +++ b/drivers/nvme/target/passthru.c
> @@ -30,6 +30,55 @@ void nvmet_passthrough_override_cap(struct nvmet_ctrl *ctrl)
> ctrl->cap &= ~(1ULL << 43);
> }
>
> +static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
> +{
> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
> + void *data;
> + struct nvme_ns_id_desc *cur;
> + u16 status = NVME_SC_SUCCESS;
> + u8 csi;
> + int pos, len;
> + bool csi_seen;
> +
consider followig if it works, totally untested :-
struct nvmet_ctrl *ctrl = req->sq->ctrl;
struct nvme_ns_id_desc *cur;
u16 status = NVME_SC_SUCCESS;
int pos, len;
bool csi_seen;
void *data;
u8 csi;
> + if (!ctrl->subsys->clear_ids)
> + return status;
> +
> + data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
> + if (!data)
> + return NVME_SC_INTERNAL;
> +
> + status = nvmet_copy_from_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
> + if (status)
> + goto out_free;
> +
> + for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
> + cur = data + pos;
> +
> + if (cur->nidl == 0)
> + break;
> + len = cur->nidl;
> + if (cur->nidt == NVME_NIDT_CSI) {
> + memcpy(&csi, data + pos + sizeof(struct nvme_ns_id_desc),
> + NVME_NIDT_CSI_LEN);
> + csi_seen = 1;
> + break;
> + }
> + len += sizeof(struct nvme_ns_id_desc);
> + }
> +
> + memset(data, 0, NVME_IDENTIFY_DATA_SIZE);
> + if (csi_seen) {
> + cur = data;
> + cur->nidt = NVME_NIDT_CSI;
> + cur->nidl = NVME_NIDT_CSI_LEN;
> + memcpy(data + sizeof(struct nvme_ns_id_desc), &csi, NVME_NIDT_CSI_LEN);
> + }
> + status = nvmet_copy_to_sgl(req, 0, data, NVME_IDENTIFY_DATA_SIZE);
> +out_free:
> + kfree(data);
> + return status;
> +}
> +
> static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
> {
> struct nvmet_ctrl *ctrl = req->sq->ctrl;
> @@ -127,6 +176,7 @@ static u16 nvmet_passthru_override_id_ctrl(struct nvmet_req *req)
>
> static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
> {
> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
since ctrl is only used once in this function do can we get away with
the req->sq->ctrl->subsys->clear_ids ?
> u16 status = NVME_SC_SUCCESS;
> struct nvme_id_ns *id;
> int i;
> @@ -152,6 +202,11 @@ static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
> */
> id->mc = 0;
>
> + if (ctrl->subsys->clear_ids) {
> + memset(id->nguid, 0, NVME_NIDT_NGUID_LEN);
> + memset(id->eui64, 0, NVME_NIDT_EUI64_LEN);
> + }
> +
> status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
>
> out_free:
> @@ -176,6 +231,9 @@ static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
> case NVME_ID_CNS_NS:
> nvmet_passthru_override_id_ns(req);
> break;
> + case NVME_ID_CNS_NS_DESC_LIST:
> + nvmet_passthru_override_id_descs(req);
> + break;
> }
> } else if (status < 0)
> status = NVME_SC_INTERNAL;
Irrespective of cosmetics, looks good.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Add new clear_ids attribute for passthru targets
2022-06-27 21:00 ` Chaitanya Kulkarni
@ 2022-06-27 23:24 ` Alan Adamson
0 siblings, 0 replies; 6+ messages in thread
From: Alan Adamson @ 2022-06-27 23:24 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: linux-nvme
> On Jun 27, 2022, at 2:00 PM, Chaitanya Kulkarni <chaitanyak@nvidia.com> wrote:
>
> On 6/22/22 15:47, Alan Adamson wrote:
>> If the clear_ids attribute is set to true, the EUI/GUID/UUID is
>> cleared for the passthru target. By default, loop targets will
>> set clear_ids to true.
>>
>> This resolves an issue where a connect to a passthru target fails
>> when using a trtype of 'loop' because EUI/GUID/UUID is not unique.
>>
>> Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
>> ---
>> drivers/nvme/target/configfs.c | 20 ++++++++++++
>> drivers/nvme/target/core.c | 4 +++
>> drivers/nvme/target/nvmet.h | 1 +
>> drivers/nvme/target/passthru.c | 58 ++++++++++++++++++++++++++++++++++
>> 4 files changed, 83 insertions(+)
>>
>> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
>> index e44b2988759e..ff77c3d2354f 100644
>> --- a/drivers/nvme/target/configfs.c
>> +++ b/drivers/nvme/target/configfs.c
>> @@ -773,11 +773,31 @@ static ssize_t nvmet_passthru_io_timeout_store(struct config_item *item,
>> }
>> CONFIGFS_ATTR(nvmet_passthru_, io_timeout);
>>
>> +static ssize_t nvmet_passthru_clear_ids_show(struct config_item *item,
>> + char *page)
>> +{
>> + return sprintf(page, "%u\n", to_subsys(item->ci_parent)->clear_ids);
>
> should we use snprintf() ? (I don't think so just a thought.
I used sprintf() to be consistent with the other passthru attributes.
>>
>> +static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
>> +{
>> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
>> + void *data;
>> + struct nvme_ns_id_desc *cur;
>> + u16 status = NVME_SC_SUCCESS;
>> + u8 csi;
>> + int pos, len;
>> + bool csi_seen;
>> +
>
> consider followig if it works, totally untested :-
>
> struct nvmet_ctrl *ctrl = req->sq->ctrl;
> struct nvme_ns_id_desc *cur;
> u16 status = NVME_SC_SUCCESS;
> int pos, len;
> bool csi_seen;
> void *data;
> u8 csi;
I'll include these changes in v2.
>>
>> static u16 nvmet_passthru_override_id_ns(struct nvmet_req *req)
>> {
>> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
>
> since ctrl is only used once in this function do can we get away with
> the req->sq->ctrl->subsys->clear_ids ?
I’ll include in v2.
>>
>
> Irrespective of cosmetics, looks good.
>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
>
> -ck
>
Thanks,
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-27 23:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-22 22:47 [PATCH] Add new clear_ids attribute for passthru targets Alan Adamson
2022-06-26 13:38 ` Sagi Grimberg
2022-06-26 13:42 ` Sagi Grimberg
2022-06-27 15:16 ` Keith Busch
2022-06-27 21:00 ` Chaitanya Kulkarni
2022-06-27 23:24 ` Alan Adamson
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.