* [PATCH V2 1/2] nvmet: make ctrl-id configurable
2019-11-22 7:41 [PATCH 0/2] nvmet: make model and ctrl-id configurable Chaitanya Kulkarni
@ 2019-11-22 7:41 ` Chaitanya Kulkarni
2019-11-22 13:43 ` Christoph Hellwig
2019-11-22 7:41 ` [PATCH V3 2/2] nvmet: make ctrl model configurable Chaitanya Kulkarni
1 sibling, 1 reply; 5+ messages in thread
From: Chaitanya Kulkarni @ 2019-11-22 7:41 UTC (permalink / raw)
To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi
This patch adds a new target subsys attribute which allows user to
optionally specify target controller IDs which then used in the
nvmet_execute_identify_ctrl() to fill up the nvme_id_ctrl structure.
For example, when using a cluster setup with two nodes, with a dual
ported NVMe drive and exporting the drive from both the nodes,
The connection to the host fails due to the same controller ID and
results in the following error message:-
"nvme nvmeX: Duplicate cntlid XXX with nvmeX, rejecting"
With this patch now user can partition the controller IDs for each
subsystem by setting up the cntlid_min and cntlid_max. These values
will be used at the time of the controller ID creation. By partitioning
the ctrl-ids for each subsystem results in the unique ctrl-id space
which avoids the collision.
When new attribute is not specified target will fall back to original
cntlid calculation method.
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
Changes from V1:-
1. Add cntlid max and min configfs attributes.
2. Use simple if .. else statements instead of ternary operators.
---
drivers/nvme/target/configfs.c | 65 ++++++++++++++++++++++++++++++++++
drivers/nvme/target/core.c | 8 +++--
drivers/nvme/target/nvmet.h | 2 ++
3 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 98613a45bd3b..5316cdb1b271 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -862,10 +862,75 @@ static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
}
CONFIGFS_ATTR(nvmet_subsys_, attr_serial);
+static bool nvmet_subsys_cntlid_store(struct nvmet_subsys *s, const char *page,
+ bool min)
+{
+ bool ret = true;
+ u16 cid;
+
+ down_write(&nvmet_config_sem);
+ if (sscanf(page, "%hu\n", &cid) != 1) {
+ ret = false;
+ goto out;
+ }
+
+ if (min)
+ s->cntlid_min = cid;
+ else
+ s->cntlid_max = cid;
+out:
+ up_write(&nvmet_config_sem);
+
+ return ret;
+}
+
+static ssize_t nvmet_subsys_cntlid_show(struct nvmet_subsys *s, char *page,
+ bool min)
+{
+ if (min)
+ return snprintf(page, PAGE_SIZE, "%u\n", s->cntlid_min);
+
+ return snprintf(page, PAGE_SIZE, "%u\n", s->cntlid_max);
+}
+
+static ssize_t nvmet_subsys_attr_cntlid_min_show(struct config_item *item,
+ char *page)
+{
+ return nvmet_subsys_cntlid_show(to_subsys(item), page, true);
+}
+
+static ssize_t nvmet_subsys_attr_cntlid_min_store(struct config_item *item,
+ const char *page, size_t cnt)
+{
+ if (!nvmet_subsys_cntlid_store(to_subsys(item), page, true))
+ return -EINVAL;
+
+ return cnt;
+}
+CONFIGFS_ATTR(nvmet_subsys_, attr_cntlid_min);
+
+static ssize_t nvmet_subsys_attr_cntlid_max_show(struct config_item *item,
+ char *page)
+{
+ return nvmet_subsys_cntlid_show(to_subsys(item), page, false);
+}
+
+static ssize_t nvmet_subsys_attr_cntlid_max_store(struct config_item *item,
+ const char *page, size_t cnt)
+{
+ if (!nvmet_subsys_cntlid_store(to_subsys(item), page, false))
+ return -EINVAL;
+
+ return cnt;
+}
+CONFIGFS_ATTR(nvmet_subsys_, attr_cntlid_max);
+
static struct configfs_attribute *nvmet_subsys_attrs[] = {
&nvmet_subsys_attr_attr_allow_any_host,
&nvmet_subsys_attr_attr_version,
&nvmet_subsys_attr_attr_serial,
+ &nvmet_subsys_attr_attr_cntlid_min,
+ &nvmet_subsys_attr_attr_cntlid_max,
NULL,
};
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 28438b833c1b..990ad4c7bdfd 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1267,8 +1267,11 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
if (!ctrl->sqs)
goto out_free_cqs;
+ if (subsys->cntlid_min > subsys->cntlid_max)
+ goto out_free_cqs;
+
ret = ida_simple_get(&cntlid_ida,
- NVME_CNTLID_MIN, NVME_CNTLID_MAX,
+ subsys->cntlid_min, subsys->cntlid_max,
GFP_KERNEL);
if (ret < 0) {
status = NVME_SC_CONNECT_CTRL_BUSY | NVME_SC_DNR;
@@ -1416,7 +1419,8 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
kfree(subsys);
return ERR_PTR(-ENOMEM);
}
-
+ subsys->cntlid_min = NVME_CNTLID_MIN;
+ subsys->cntlid_max = NVME_CNTLID_MAX;
kref_init(&subsys->ref);
mutex_init(&subsys->lock);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 46df45e837c9..6492d12e626a 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -211,6 +211,8 @@ struct nvmet_subsys {
struct list_head namespaces;
unsigned int nr_namespaces;
unsigned int max_nsid;
+ u16 cntlid_min;
+ u16 cntlid_max;
struct list_head ctrls;
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V3 2/2] nvmet: make ctrl model configurable
2019-11-22 7:41 [PATCH 0/2] nvmet: make model and ctrl-id configurable Chaitanya Kulkarni
2019-11-22 7:41 ` [PATCH V2 1/2] nvmet: make " Chaitanya Kulkarni
@ 2019-11-22 7:41 ` Chaitanya Kulkarni
2019-11-22 13:45 ` Christoph Hellwig
1 sibling, 1 reply; 5+ messages in thread
From: Chaitanya Kulkarni @ 2019-11-22 7:41 UTC (permalink / raw)
To: linux-nvme; +Cc: Mark Ruijter, hch, Chaitanya Kulkarni, sagi
From: Mark Ruijter <MRuijter@onestopsystems.com>
From: Mark Ruijter <MRuijter@onestopsystems.com>
This patch adds a new target subsys attribute which allows user to
optionally specify model name which then used in the
nvmet_execute_identify_ctrl() to fill up the nvme_id_ctrl structure.
The default value for the model is set to "Linux" for backward
compatibility.
Signed-off-by: Mark Ruijter <MRuijter@onestopsystems.com>
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
Changes from V2:-
1. Use if else pattern than ternary operators.
2. Change the name nvmet_subsys_model() -> nvmet_model_number().
3. Introduce nvmet_is_ascii() to filter the model characters.
4. Change tmp -> tmp_model in nvmet_subsys_attr_model_store().
Changes from V1:-
1. Don't allocate memory for default subsys model,
2. Use helper to get the default model string from ctrl->subsys in the
nvmet_execute_identify_ctrl() and nvmet_subsys_attr_model()_show.
Later is needed so that nvmetcli can display default value Linux
when the model is not set from the user.
3. Get rid of the extra variable c in the nvmet_subsys_attr_model_store()
and replace for with while loop without loosing the code redability.
---
drivers/nvme/target/admin-cmd.c | 12 ++++++--
drivers/nvme/target/configfs.c | 50 +++++++++++++++++++++++++++++++++
drivers/nvme/target/core.c | 2 ++
drivers/nvme/target/nvmet.h | 4 +++
4 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 56c21b501185..613e715dc7d3 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -312,12 +312,20 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
nvmet_req_complete(req, NVME_SC_INVALID_FIELD | NVME_SC_DNR);
}
+const char *nvmet_model_number(struct nvmet_subsys *subsys)
+{
+ if (subsys->model)
+ return subsys->model;
+
+ return NVMET_DEFAULT_CTRL_MODEL;
+}
+
static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
{
+ const char *model = nvmet_model_number(req->sq->ctrl->subsys);
struct nvmet_ctrl *ctrl = req->sq->ctrl;
struct nvme_id_ctrl *id;
u16 status = 0;
- const char model[] = "Linux";
id = kzalloc(sizeof(*id), GFP_KERNEL);
if (!id) {
@@ -332,7 +340,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
memset(id->sn, ' ', sizeof(id->sn));
bin2hex(id->sn, &ctrl->subsys->serial,
min(sizeof(ctrl->subsys->serial), sizeof(id->sn) / 2));
- memcpy_and_pad(id->mn, sizeof(id->mn), model, sizeof(model) - 1, ' ');
+ memcpy_and_pad(id->mn, sizeof(id->mn), model, strlen(model), ' ');
memcpy_and_pad(id->fr, sizeof(id->fr),
UTS_RELEASE, strlen(UTS_RELEASE), ' ');
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 5316cdb1b271..00869b805c6b 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -925,12 +925,62 @@ static ssize_t nvmet_subsys_attr_cntlid_max_store(struct config_item *item,
}
CONFIGFS_ATTR(nvmet_subsys_, attr_cntlid_max);
+static ssize_t nvmet_subsys_attr_model_show(struct config_item *item,
+ char *page)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+
+ return snprintf(page, PAGE_SIZE, "%s\n",
+ nvmet_model_number(subsys));
+}
+
+/* See Section 1.5 of NVMe 1.4 */
+static bool nvmet_is_ascii(const char c)
+{
+ return c >= 0x20 && c <= 0x7e;
+}
+
+static ssize_t nvmet_subsys_attr_model_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+ int ret = -EINVAL, pos = 0, len;
+ char *tmp_model;
+
+ down_write(&nvmet_config_sem);
+ len = strcspn(page, "\n");
+ if (!len)
+ goto out_unlock;
+
+ for (pos = 0; pos < len; pos++) {
+ if (!nvmet_is_ascii(page[pos]))
+ goto out_unlock;
+ }
+
+ tmp_model = kstrndup(page, len, GFP_KERNEL);
+ if (!tmp_model) {
+ ret = -ENOMEM;
+ goto out_unlock;
+ }
+
+ ret = count;
+ kfree(subsys->model);
+ subsys->model = tmp_model;
+
+out_unlock:
+ up_write(&nvmet_config_sem);
+ return ret;
+}
+
+CONFIGFS_ATTR(nvmet_subsys_, attr_model);
+
static struct configfs_attribute *nvmet_subsys_attrs[] = {
&nvmet_subsys_attr_attr_allow_any_host,
&nvmet_subsys_attr_attr_version,
&nvmet_subsys_attr_attr_serial,
&nvmet_subsys_attr_attr_cntlid_min,
&nvmet_subsys_attr_attr_cntlid_max,
+ &nvmet_subsys_attr_attr_model,
NULL,
};
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 990ad4c7bdfd..a985ca7febd5 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1419,6 +1419,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
kfree(subsys);
return ERR_PTR(-ENOMEM);
}
+ subsys->model = NULL;
subsys->cntlid_min = NVME_CNTLID_MIN;
subsys->cntlid_max = NVME_CNTLID_MAX;
kref_init(&subsys->ref);
@@ -1439,6 +1440,7 @@ static void nvmet_subsys_free(struct kref *ref)
WARN_ON_ONCE(!list_empty(&subsys->namespaces));
kfree(subsys->subsysnqn);
+ kfree(subsys->model);
kfree(subsys);
}
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 6492d12e626a..c54ce5f66f09 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -23,6 +23,7 @@
#define NVMET_ASYNC_EVENTS 4
#define NVMET_ERROR_LOG_SLOTS 128
#define NVMET_NO_ERROR_LOC ((u16)-1)
+#define NVMET_DEFAULT_CTRL_MODEL "Linux"
/*
* Supported optional AENs:
@@ -224,6 +225,7 @@ struct nvmet_subsys {
u64 ver;
u64 serial;
char *subsysnqn;
+ char *model;
struct config_group group;
@@ -489,6 +491,8 @@ u16 nvmet_bdev_flush(struct nvmet_req *req);
u16 nvmet_file_flush(struct nvmet_req *req);
void nvmet_ns_changed(struct nvmet_subsys *subsys, u32 nsid);
+const char *nvmet_model_number(struct nvmet_subsys *subsys);
+
static inline u32 nvmet_rw_len(struct nvmet_req *req)
{
return ((u32)le16_to_cpu(req->cmd->rw.length) + 1) <<
--
2.22.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 5+ messages in thread