* [PATCH 2/4] nvmet: make sn stable once connection was established
2021-04-20 9:09 [PATCH 1/4] nvmet: change sn size and check validity Max Gurtovoy
@ 2021-04-20 9:09 ` Max Gurtovoy
2021-04-20 9:09 ` [PATCH 3/4] nvmet: allow mn change if subsys not discovered Max Gurtovoy
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Max Gurtovoy @ 2021-04-20 9:09 UTC (permalink / raw)
To: linux-nvme, sagi, kbusch, hch, chaitanya.kulkarni
Cc: oren, ngottlieb, mgurtovoy
From: Noam Gottlieb <ngottlieb@nvidia.com>
Once some host has connected to the target, make sure that the serial
number is stable and cannot be changed.
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Noam Gottlieb <ngottlieb@nvidia.com>
---
drivers/nvme/target/admin-cmd.c | 6 ++++++
drivers/nvme/target/configfs.c | 28 ++++++++++++++++++++++------
drivers/nvme/target/nvmet.h | 1 +
3 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 91eb7562a88a..712e09f8158b 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -337,6 +337,12 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
u32 cmd_capsule_size;
u16 status = 0;
+ if (!subsys->subsys_discovered) {
+ mutex_lock(&subsys->lock);
+ subsys->subsys_discovered = true;
+ mutex_unlock(&subsys->lock);
+ }
+
/*
* If there is no model number yet, set it now. It will then remain
* stable for the life time of the subsystem.
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 576540fdba73..7040d92bb31a 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1044,13 +1044,18 @@ static ssize_t nvmet_subsys_attr_serial_show(struct config_item *item,
return snprintf(page, PAGE_SIZE, "%s\n", subsys->serial);
}
-static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
- const char *page, size_t count)
+static ssize_t
+nvmet_subsys_attr_serial_store_locked(struct nvmet_subsys *subsys,
+ const char *page, size_t count)
{
- struct nvmet_subsys *subsys;
int pos, len;
- subsys = to_subsys(item);
+ if (subsys->subsys_discovered) {
+ pr_err("Can't set serial number. %s is already assigned\n",
+ subsys->serial);
+ return -EINVAL;
+ }
+
len = strcspn(page, "\n");
if (len == 0 || len > NVMET_SN_MAX_SIZE) {
@@ -1066,13 +1071,24 @@ static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
}
}
+ memcpy_and_pad(subsys->serial, NVMET_SN_MAX_SIZE, page, len, ' ');
+
+ return count;
+}
+
+static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+ ssize_t ret;
+
down_write(&nvmet_config_sem);
mutex_lock(&subsys->lock);
- memcpy_and_pad(subsys->serial, NVMET_SN_MAX_SIZE, page, len, ' ');
+ ret = nvmet_subsys_attr_serial_store_locked(subsys, page, count);
mutex_unlock(&subsys->lock);
up_write(&nvmet_config_sem);
- return count;
+ return ret;
}
CONFIGFS_ATTR(nvmet_subsys_, attr_serial);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 53999bd259ed..7a47ca00fb03 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -231,6 +231,7 @@ struct nvmet_subsys {
u64 ver;
char serial[NVMET_SN_MAX_SIZE];
+ bool subsys_discovered;
char *subsysnqn;
bool pi_support;
--
2.25.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/4] nvmet: allow mn change if subsys not discovered
2021-04-20 9:09 [PATCH 1/4] nvmet: change sn size and check validity Max Gurtovoy
2021-04-20 9:09 ` [PATCH 2/4] nvmet: make sn stable once connection was established Max Gurtovoy
@ 2021-04-20 9:09 ` Max Gurtovoy
2021-04-20 9:09 ` [PATCH 4/4] nvmet: make ver stable once connection established Max Gurtovoy
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Max Gurtovoy @ 2021-04-20 9:09 UTC (permalink / raw)
To: linux-nvme, sagi, kbusch, hch, chaitanya.kulkarni
Cc: oren, ngottlieb, mgurtovoy
From: Noam Gottlieb <ngottlieb@nvidia.com>
Currently, once the subsystem's model_number is set for the first time
there is no way to change it. However, as long as no connection was
established to nvmf target, there is no reason for such restriction and
we should allow to change the subsystem's model_number as many times as
needed.
In addition, in order to simplfy the changes and make the model number
flow more similar to the rest of the attributes in the Identify
Controller data structure, we set a default value for the model number
at the initiation of the subsystem.
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Noam Gottlieb <ngottlieb@nvidia.com>
---
drivers/nvme/target/admin-cmd.c | 26 --------------------------
drivers/nvme/target/configfs.c | 10 ++--------
drivers/nvme/target/core.c | 21 +++++++++++++++++----
drivers/nvme/target/discovery.c | 4 ++--
4 files changed, 21 insertions(+), 40 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 712e09f8158b..954de0d813d6 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -313,22 +313,6 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
nvmet_req_complete(req, NVME_SC_INVALID_FIELD | NVME_SC_DNR);
}
-static u16 nvmet_set_model_number(struct nvmet_subsys *subsys)
-{
- u16 status = 0;
-
- mutex_lock(&subsys->lock);
- if (!subsys->model_number) {
- subsys->model_number =
- kstrdup(NVMET_DEFAULT_CTRL_MODEL, GFP_KERNEL);
- if (!subsys->model_number)
- status = NVME_SC_INTERNAL;
- }
- mutex_unlock(&subsys->lock);
-
- return status;
-}
-
static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
{
struct nvmet_ctrl *ctrl = req->sq->ctrl;
@@ -343,16 +327,6 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
mutex_unlock(&subsys->lock);
}
- /*
- * If there is no model number yet, set it now. It will then remain
- * stable for the life time of the subsystem.
- */
- if (!subsys->model_number) {
- status = nvmet_set_model_number(subsys);
- if (status)
- goto out;
- }
-
id = kzalloc(sizeof(*id), GFP_KERNEL);
if (!id) {
status = NVME_SC_INTERNAL;
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 7040d92bb31a..61a9a52da30d 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1156,14 +1156,8 @@ static ssize_t nvmet_subsys_attr_model_show(struct config_item *item,
char *page)
{
struct nvmet_subsys *subsys = to_subsys(item);
- int ret;
-
- mutex_lock(&subsys->lock);
- ret = snprintf(page, PAGE_SIZE, "%s\n", subsys->model_number ?
- subsys->model_number : NVMET_DEFAULT_CTRL_MODEL);
- mutex_unlock(&subsys->lock);
- return ret;
+ return snprintf(page, PAGE_SIZE, "%s\n", subsys->model_number);
}
static ssize_t nvmet_subsys_attr_model_store_locked(struct nvmet_subsys *subsys,
@@ -1171,7 +1165,7 @@ static ssize_t nvmet_subsys_attr_model_store_locked(struct nvmet_subsys *subsys,
{
int pos = 0, len;
- if (subsys->model_number) {
+ if (subsys->subsys_discovered) {
pr_err("Can't set model number. %s is already assigned\n",
subsys->model_number);
return -EINVAL;
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 3efd48b0a34e..84e9a4b261b3 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1483,6 +1483,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
{
struct nvmet_subsys *subsys;
char serial[NVMET_SN_MAX_SIZE / 2];
+ int ret;
subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
if (!subsys)
@@ -1493,6 +1494,12 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
get_random_bytes(&serial, sizeof(serial));
bin2hex(subsys->serial, &serial, sizeof(serial));
+ subsys->model_number = kstrdup(NVMET_DEFAULT_CTRL_MODEL, GFP_KERNEL);
+ if (!subsys->model_number) {
+ ret = -ENOMEM;
+ goto free_subsys;
+ }
+
switch (type) {
case NVME_NQN_NVME:
subsys->max_qid = NVMET_NR_QUEUES;
@@ -1502,15 +1509,15 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
break;
default:
pr_err("%s: Unknown Subsystem type - %d\n", __func__, type);
- kfree(subsys);
- return ERR_PTR(-EINVAL);
+ ret = -EINVAL;
+ goto free_mn;
}
subsys->type = type;
subsys->subsysnqn = kstrndup(subsysnqn, NVMF_NQN_SIZE,
GFP_KERNEL);
if (!subsys->subsysnqn) {
- kfree(subsys);
- return ERR_PTR(-ENOMEM);
+ ret = -ENOMEM;
+ goto free_mn;
}
subsys->cntlid_min = NVME_CNTLID_MIN;
subsys->cntlid_max = NVME_CNTLID_MAX;
@@ -1522,6 +1529,12 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
INIT_LIST_HEAD(&subsys->hosts);
return subsys;
+
+free_mn:
+ kfree(subsys->model_number);
+free_subsys:
+ kfree(subsys);
+ return ERR_PTR(ret);
}
static void nvmet_subsys_free(struct kref *ref)
diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
index f39946615fd6..6689bd27d629 100644
--- a/drivers/nvme/target/discovery.c
+++ b/drivers/nvme/target/discovery.c
@@ -244,7 +244,6 @@ static void nvmet_execute_disc_identify(struct nvmet_req *req)
{
struct nvmet_ctrl *ctrl = req->sq->ctrl;
struct nvme_id_ctrl *id;
- const char model[] = "Linux";
u16 status = 0;
if (!nvmet_check_transfer_len(req, NVME_IDENTIFY_DATA_SIZE))
@@ -264,7 +263,8 @@ static void nvmet_execute_disc_identify(struct nvmet_req *req)
memcpy(id->sn, ctrl->subsys->serial, NVMET_SN_MAX_SIZE);
memset(id->fr, ' ', sizeof(id->fr));
- memcpy_and_pad(id->mn, sizeof(id->mn), model, sizeof(model) - 1, ' ');
+ memcpy_and_pad(id->mn, sizeof(id->mn), ctrl->subsys->model_number,
+ strlen(ctrl->subsys->model_number), ' ');
memcpy_and_pad(id->fr, sizeof(id->fr),
UTS_RELEASE, strlen(UTS_RELEASE), ' ');
--
2.25.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/4] nvmet: make ver stable once connection established
2021-04-20 9:09 [PATCH 1/4] nvmet: change sn size and check validity Max Gurtovoy
2021-04-20 9:09 ` [PATCH 2/4] nvmet: make sn stable once connection was established Max Gurtovoy
2021-04-20 9:09 ` [PATCH 3/4] nvmet: allow mn change if subsys not discovered Max Gurtovoy
@ 2021-04-20 9:09 ` Max Gurtovoy
2021-04-20 17:47 ` Chaitanya Kulkarni
2021-04-20 17:35 ` [PATCH 1/4] nvmet: change sn size and check validity Chaitanya Kulkarni
2021-04-23 15:51 ` Keith Busch
4 siblings, 1 reply; 13+ messages in thread
From: Max Gurtovoy @ 2021-04-20 9:09 UTC (permalink / raw)
To: linux-nvme, sagi, kbusch, hch, chaitanya.kulkarni
Cc: oren, ngottlieb, mgurtovoy
From: Noam Gottlieb <ngottlieb@nvidia.com>
Once some host has connected to the nvmf target, make sure that the
version number is stable and cannot be changed.
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Noam Gottlieb <ngottlieb@nvidia.com>
---
drivers/nvme/target/configfs.c | 36 +++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 61a9a52da30d..2c7aebd4d529 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1007,13 +1007,26 @@ static ssize_t nvmet_subsys_attr_version_show(struct config_item *item,
NVME_MINOR(subsys->ver));
}
-static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
- const char *page, size_t count)
+static ssize_t
+nvmet_subsys_attr_version_store_locked(struct nvmet_subsys *subsys,
+ const char *page, size_t count)
{
- struct nvmet_subsys *subsys = to_subsys(item);
int major, minor, tertiary = 0;
int ret;
+ if (subsys->subsys_discovered) {
+ if (NVME_TERTIARY(subsys->ver))
+ pr_err("Can't set version number. %llu.%llu.%llu is already assigned\n",
+ NVME_MAJOR(subsys->ver),
+ NVME_MINOR(subsys->ver),
+ NVME_TERTIARY(subsys->ver));
+ else
+ pr_err("Can't set version number. %llu.%llu is already assigned\n",
+ NVME_MAJOR(subsys->ver),
+ NVME_MINOR(subsys->ver));
+ return -EINVAL;
+ }
+
/* passthru subsystems use the underlying controller's version */
if (nvmet_passthru_ctrl(subsys))
return -EINVAL;
@@ -1022,12 +1035,25 @@ static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
if (ret != 2 && ret != 3)
return -EINVAL;
- down_write(&nvmet_config_sem);
subsys->ver = NVME_VS(major, minor, tertiary);
- up_write(&nvmet_config_sem);
return count;
}
+
+static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ struct nvmet_subsys *subsys = to_subsys(item);
+ int ret;
+
+ down_write(&nvmet_config_sem);
+ mutex_lock(&subsys->lock);
+ ret = nvmet_subsys_attr_version_store_locked(subsys, page, count);
+ mutex_unlock(&subsys->lock);
+ up_write(&nvmet_config_sem);
+
+ return ret;
+}
CONFIGFS_ATTR(nvmet_subsys_, attr_version);
/* See Section 1.5 of NVMe 1.4 */
--
2.25.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] nvmet: make ver stable once connection established
2021-04-20 9:09 ` [PATCH 4/4] nvmet: make ver stable once connection established Max Gurtovoy
@ 2021-04-20 17:47 ` Chaitanya Kulkarni
2021-04-22 8:43 ` Max Gurtovoy
0 siblings, 1 reply; 13+ messages in thread
From: Chaitanya Kulkarni @ 2021-04-20 17:47 UTC (permalink / raw)
To: Max Gurtovoy, linux-nvme, sagi, kbusch, hch; +Cc: oren, ngottlieb
On 4/20/21 02:09, Max Gurtovoy wrote:
> From: Noam Gottlieb <ngottlieb@nvidia.com>
>
> Once some host has connected to the nvmf target, make sure that the
> version number is stable and cannot be changed.
>
> Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> Signed-off-by: Noam Gottlieb <ngottlieb@nvidia.com>
> ---
> drivers/nvme/target/configfs.c | 36 +++++++++++++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index 61a9a52da30d..2c7aebd4d529 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -1007,13 +1007,26 @@ static ssize_t nvmet_subsys_attr_version_show(struct config_item *item,
> NVME_MINOR(subsys->ver));
> }
>
> -static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
> - const char *page, size_t count)
> +static ssize_t
> +nvmet_subsys_attr_version_store_locked(struct nvmet_subsys *subsys,
> + const char *page, size_t count)
> {
Seems like we lost the indentation in above line which was there previously.
> - struct nvmet_subsys *subsys = to_subsys(item);
> int major, minor, tertiary = 0;
> int ret;
>
> + if (subsys->subsys_discovered) {
> + if (NVME_TERTIARY(subsys->ver))
> + pr_err("Can't set version number. %llu.%llu.%llu is already assigned\n",
> + NVME_MAJOR(subsys->ver),
> + NVME_MINOR(subsys->ver),
> + NVME_TERTIARY(subsys->ver));
> + else
> + pr_err("Can't set version number. %llu.%llu is already assigned\n",
> + NVME_MAJOR(subsys->ver),
> + NVME_MINOR(subsys->ver));
> + return -EINVAL;
> + }
> +
Can you create a helper to for about error reporting so we can avoid
extra long lines whenever itis possible something like :-
if (subsys->subsys_discovered)
return nvmet_subsys_discover_err(subsys);
> /* passthru subsystems use the underlying controller's version */
> if (nvmet_passthru_ctrl(subsys))
> return -EINVAL;
> @@ -1022,12 +1035,25 @@ static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
> if (ret != 2 && ret != 3)
> return -EINVAL;
>
> - down_write(&nvmet_config_sem);
> subsys->ver = NVME_VS(major, minor, tertiary);
> - up_write(&nvmet_config_sem);
>
> return count;
> }
> +
> +static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
> + const char *page, size_t count)
> +{
> + struct nvmet_subsys *subsys = to_subsys(item);
> + int ret;
> +
nit:- ret variable type should be ssize_t
nvmet_subsys_attr_version_store_locke().
> + down_write(&nvmet_config_sem);
> + mutex_lock(&subsys->lock);
> + ret = nvmet_subsys_attr_version_store_locked(subsys, page, count);
> + mutex_unlock(&subsys->lock);
> + up_write(&nvmet_config_sem);
> +
> + return ret;
> +}
> CONFIGFS_ATTR(nvmet_subsys_, attr_version);
>
> /* See Section 1.5 of NVMe 1.4 */
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] nvmet: make ver stable once connection established
2021-04-20 17:47 ` Chaitanya Kulkarni
@ 2021-04-22 8:43 ` Max Gurtovoy
2021-04-22 19:01 ` Chaitanya Kulkarni
0 siblings, 1 reply; 13+ messages in thread
From: Max Gurtovoy @ 2021-04-22 8:43 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-nvme, sagi, kbusch, hch; +Cc: oren, ngottlieb
On 4/20/2021 8:47 PM, Chaitanya Kulkarni wrote:
> On 4/20/21 02:09, Max Gurtovoy wrote:
>> From: Noam Gottlieb <ngottlieb@nvidia.com>
>>
>> Once some host has connected to the nvmf target, make sure that the
>> version number is stable and cannot be changed.
>>
>> Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>> Signed-off-by: Noam Gottlieb <ngottlieb@nvidia.com>
>> ---
>> drivers/nvme/target/configfs.c | 36 +++++++++++++++++++++++++++++-----
>> 1 file changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
>> index 61a9a52da30d..2c7aebd4d529 100644
>> --- a/drivers/nvme/target/configfs.c
>> +++ b/drivers/nvme/target/configfs.c
>> @@ -1007,13 +1007,26 @@ static ssize_t nvmet_subsys_attr_version_show(struct config_item *item,
>> NVME_MINOR(subsys->ver));
>> }
>>
>> -static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
>> - const char *page, size_t count)
>> +static ssize_t
>> +nvmet_subsys_attr_version_store_locked(struct nvmet_subsys *subsys,
>> + const char *page, size_t count)
>> {
> Seems like we lost the indentation in above line which was there previously.
We added a new line since the line was > 80 chars.
And we follow the conventions.
>
>> - struct nvmet_subsys *subsys = to_subsys(item);
>> int major, minor, tertiary = 0;
>> int ret;
>>
>> + if (subsys->subsys_discovered) {
>> + if (NVME_TERTIARY(subsys->ver))
>> + pr_err("Can't set version number. %llu.%llu.%llu is already assigned\n",
>> + NVME_MAJOR(subsys->ver),
>> + NVME_MINOR(subsys->ver),
>> + NVME_TERTIARY(subsys->ver));
>> + else
>> + pr_err("Can't set version number. %llu.%llu is already assigned\n",
>> + NVME_MAJOR(subsys->ver),
>> + NVME_MINOR(subsys->ver));
>> + return -EINVAL;
>> + }
>> +
> Can you create a helper to for about error reporting so we can avoid
> extra long lines whenever itis possible something like :-
>
> if (subsys->subsys_discovered)
> return nvmet_subsys_discover_err(subsys);
We will create helper in the future in case this code will be re-used.
There is no need to create it now.
Long lines avoided by new lines.
>
>> /* passthru subsystems use the underlying controller's version */
>> if (nvmet_passthru_ctrl(subsys))
>> return -EINVAL;
>> @@ -1022,12 +1035,25 @@ static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
>> if (ret != 2 && ret != 3)
>> return -EINVAL;
>>
>> - down_write(&nvmet_config_sem);
>> subsys->ver = NVME_VS(major, minor, tertiary);
>> - up_write(&nvmet_config_sem);
>>
>> return count;
>> }
>> +
>> +static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
>> + const char *page, size_t count)
>> +{
>> + struct nvmet_subsys *subsys = to_subsys(item);
>> + int ret;
>> +
> nit:- ret variable type should be ssize_t
> nvmet_subsys_attr_version_store_locke().
Ok. good catch.
Thanks.
>
>> + down_write(&nvmet_config_sem);
>> + mutex_lock(&subsys->lock);
>> + ret = nvmet_subsys_attr_version_store_locked(subsys, page, count);
>> + mutex_unlock(&subsys->lock);
>> + up_write(&nvmet_config_sem);
>> +
>> + return ret;
>> +}
>> CONFIGFS_ATTR(nvmet_subsys_, attr_version);
>>
>> /* See Section 1.5 of NVMe 1.4 */
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] nvmet: make ver stable once connection established
2021-04-22 8:43 ` Max Gurtovoy
@ 2021-04-22 19:01 ` Chaitanya Kulkarni
0 siblings, 0 replies; 13+ messages in thread
From: Chaitanya Kulkarni @ 2021-04-22 19:01 UTC (permalink / raw)
To: Max Gurtovoy; +Cc: linux-nvme, sagi, kbusch, hch, oren, ngottlieb
On 4/22/21 01:43, Max Gurtovoy wrote:
> On 4/20/2021 8:47 PM, Chaitanya Kulkarni wrote:
>> On 4/20/21 02:09, Max Gurtovoy wrote:
>>> -static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
>>> - const char *page, size_t count)
>>> +static ssize_t
>>> +nvmet_subsys_attr_version_store_locked(struct nvmet_subsys *subsys,
>>> + const char *page, size_t count)
>>> {
>> Seems like we lost the indentation in above line which was there previously.
> We added a new line since the line was > 80 chars.
>
> And we follow the conventions.
Original line has 5 tabs + spaces :-
-static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,$
-^I^I^I^I^I const char *page, size_t count)$
This change has 2 tabs no spaces, breaks the alignment of args:-
+nvmet_subsys_attr_version_store_locked(struct nvmet_subsys *subsys,$
+^I^Iconst char *page, size_t count)$
Following fits in 70 char(note the tabs before spaces):-
static ssize_t$
nvmet_subsys_attr_version_store_locked(struct nvmet_subsys *subsys,$
^I^I^I^I const char *page, size_t count)$
It doesn't break convention, also keeps the alignment of args.
>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] nvmet: change sn size and check validity
2021-04-20 9:09 [PATCH 1/4] nvmet: change sn size and check validity Max Gurtovoy
` (2 preceding siblings ...)
2021-04-20 9:09 ` [PATCH 4/4] nvmet: make ver stable once connection established Max Gurtovoy
@ 2021-04-20 17:35 ` Chaitanya Kulkarni
2021-04-22 8:50 ` Max Gurtovoy
2021-04-23 15:51 ` Keith Busch
4 siblings, 1 reply; 13+ messages in thread
From: Chaitanya Kulkarni @ 2021-04-20 17:35 UTC (permalink / raw)
To: Max Gurtovoy, linux-nvme, sagi, kbusch, hch; +Cc: oren, ngottlieb
On 4/20/21 02:09, Max Gurtovoy wrote:
> From: Noam Gottlieb <ngottlieb@nvidia.com>
>
> According to the NVM specification, the serial_number should be 20 bytes
> (bytes 23:04 of the Identify Controller data structure), and should
> contain only ASCII characters.
>
> In accordance, the serial_number size is changed to 20 bytes and before
> any attempt to store a new value in serial_number we check that the
> input is valid - i.e. contains only ASCII characters, is not empty and
> does not exceed 20 bytes.
>
> Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> Signed-off-by: Noam Gottlieb <ngottlieb@nvidia.com>
> ---
> drivers/nvme/target/admin-cmd.c | 4 +---
> drivers/nvme/target/configfs.c | 36 ++++++++++++++++++++++++---------
> drivers/nvme/target/core.c | 4 +++-
> drivers/nvme/target/discovery.c | 4 +---
> drivers/nvme/target/nvmet.h | 3 ++-
> 5 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
> index d2a26ff3f7b3..91eb7562a88a 100644
> --- a/drivers/nvme/target/admin-cmd.c
> +++ b/drivers/nvme/target/admin-cmd.c
> @@ -357,9 +357,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
> id->vid = 0;
> id->ssvid = 0;
>
> - memset(id->sn, ' ', sizeof(id->sn));
> - bin2hex(id->sn, &ctrl->subsys->serial,
> - min(sizeof(ctrl->subsys->serial), sizeof(id->sn) / 2));
> + memcpy(id->sn, ctrl->subsys->serial, NVMET_SN_MAX_SIZE);
> memcpy_and_pad(id->mn, sizeof(id->mn), subsys->model_number,
> strlen(subsys->model_number), ' ');
> memcpy_and_pad(id->fr, sizeof(id->fr),
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index 65a0cf99f557..576540fdba73 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -1030,24 +1030,46 @@ static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
> }
> CONFIGFS_ATTR(nvmet_subsys_, attr_version);
>
> +/* 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_serial_show(struct config_item *item,
> char *page)
> {
> struct nvmet_subsys *subsys = to_subsys(item);
>
> - return snprintf(page, PAGE_SIZE, "%llx\n", subsys->serial);
> + return snprintf(page, PAGE_SIZE, "%s\n", subsys->serial);
> }
>
> static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
> const char *page, size_t count)
> {
> - u64 serial;
> + struct nvmet_subsys *subsys;
> + int pos, len;
> +
> + subsys = to_subsys(item);
> + len = strcspn(page, "\n");
>
nit:- we can avoid two extra initialization lines with :-
struct nvmet_subsys *subsys = to_subsys(item);
size_t pos, len = strcspn(page, "\n");
> - if (sscanf(page, "%llx\n", &serial) != 1)
> + if (len == 0 || len > NVMET_SN_MAX_SIZE) {
nit:- 's/len == 0/!len/' is pretty common in the code.
> + pr_err("Serial Number can not be empty or exceed %d Bytes\n",
> + NVMET_SN_MAX_SIZE);
> return -EINVAL;
> + }
> +
> + for (pos = 0; pos < len; pos++) {
> + if (!nvmet_is_ascii(page[pos])) {
> + pr_err("Serial Number must contain only ASCII strings\n");
> + return -EINVAL;
> + }
> + }
>
> down_write(&nvmet_config_sem);
> - to_subsys(item)->serial = serial;
> + mutex_lock(&subsys->lock);
> + memcpy_and_pad(subsys->serial, NVMET_SN_MAX_SIZE, page, len, ' ');
> + mutex_unlock(&subsys->lock);
> up_write(&nvmet_config_sem);
>
> return count;
> @@ -1128,12 +1150,6 @@ static ssize_t nvmet_subsys_attr_model_show(struct config_item *item,
> return ret;
> }
>
> -/* 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_locked(struct nvmet_subsys *subsys,
> const char *page, size_t count)
> {
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index adbede9ab7f3..3efd48b0a34e 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1482,6 +1482,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
> enum nvme_subsys_type type)
> {
> struct nvmet_subsys *subsys;
> + char serial[NVMET_SN_MAX_SIZE / 2];
This needs a comment why NVMET_SN_MAX_SIZE / 2.
>
> subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
> if (!subsys)
> @@ -1489,7 +1490,8 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
>
> subsys->ver = NVMET_DEFAULT_VS;
> /* generate a random serial number as our controllers are ephemeral: */
> - get_random_bytes(&subsys->serial, sizeof(subsys->serial));
> + get_random_bytes(&serial, sizeof(serial));
> + bin2hex(subsys->serial, &serial, sizeof(serial));
>
> switch (type) {
> case NVME_NQN_NVME:
> diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
> index 4845d12e374a..f39946615fd6 100644
> --- a/drivers/nvme/target/discovery.c
> +++ b/drivers/nvme/target/discovery.c
> @@ -262,9 +262,7 @@ static void nvmet_execute_disc_identify(struct nvmet_req *req)
> goto out;
> }
>
> - memset(id->sn, ' ', sizeof(id->sn));
> - bin2hex(id->sn, &ctrl->subsys->serial,
> - min(sizeof(ctrl->subsys->serial), sizeof(id->sn) / 2));
> + memcpy(id->sn, ctrl->subsys->serial, NVMET_SN_MAX_SIZE);
> memset(id->fr, ' ', sizeof(id->fr));
> memcpy_and_pad(id->mn, sizeof(id->mn), model, sizeof(model) - 1, ' ');
> memcpy_and_pad(id->fr, sizeof(id->fr),
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 5566ed403576..53999bd259ed 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -28,6 +28,7 @@
> #define NVMET_NO_ERROR_LOC ((u16)-1)
> #define NVMET_DEFAULT_CTRL_MODEL "Linux"
> #define NVMET_MN_MAX_SIZE 40
> +#define NVMET_SN_MAX_SIZE 20
>
> /*
> * Supported optional AENs:
> @@ -229,7 +230,7 @@ struct nvmet_subsys {
> u16 max_qid;
>
> u64 ver;
> - u64 serial;
> + char serial[NVMET_SN_MAX_SIZE];
> char *subsysnqn;
> bool pi_support;
>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] nvmet: change sn size and check validity
2021-04-20 17:35 ` [PATCH 1/4] nvmet: change sn size and check validity Chaitanya Kulkarni
@ 2021-04-22 8:50 ` Max Gurtovoy
2021-04-22 19:11 ` Chaitanya Kulkarni
0 siblings, 1 reply; 13+ messages in thread
From: Max Gurtovoy @ 2021-04-22 8:50 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-nvme, sagi, kbusch, hch; +Cc: oren, ngottlieb
On 4/20/2021 8:35 PM, Chaitanya Kulkarni wrote:
> On 4/20/21 02:09, Max Gurtovoy wrote:
>> From: Noam Gottlieb <ngottlieb@nvidia.com>
>>
>> According to the NVM specification, the serial_number should be 20 bytes
>> (bytes 23:04 of the Identify Controller data structure), and should
>> contain only ASCII characters.
>>
>> In accordance, the serial_number size is changed to 20 bytes and before
>> any attempt to store a new value in serial_number we check that the
>> input is valid - i.e. contains only ASCII characters, is not empty and
>> does not exceed 20 bytes.
>>
>> Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>> Signed-off-by: Noam Gottlieb <ngottlieb@nvidia.com>
>> ---
>> drivers/nvme/target/admin-cmd.c | 4 +---
>> drivers/nvme/target/configfs.c | 36 ++++++++++++++++++++++++---------
>> drivers/nvme/target/core.c | 4 +++-
>> drivers/nvme/target/discovery.c | 4 +---
>> drivers/nvme/target/nvmet.h | 3 ++-
>> 5 files changed, 33 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
>> index d2a26ff3f7b3..91eb7562a88a 100644
>> --- a/drivers/nvme/target/admin-cmd.c
>> +++ b/drivers/nvme/target/admin-cmd.c
>> @@ -357,9 +357,7 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
>> id->vid = 0;
>> id->ssvid = 0;
>>
>> - memset(id->sn, ' ', sizeof(id->sn));
>> - bin2hex(id->sn, &ctrl->subsys->serial,
>> - min(sizeof(ctrl->subsys->serial), sizeof(id->sn) / 2));
>> + memcpy(id->sn, ctrl->subsys->serial, NVMET_SN_MAX_SIZE);
>> memcpy_and_pad(id->mn, sizeof(id->mn), subsys->model_number,
>> strlen(subsys->model_number), ' ');
>> memcpy_and_pad(id->fr, sizeof(id->fr),
>> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
>> index 65a0cf99f557..576540fdba73 100644
>> --- a/drivers/nvme/target/configfs.c
>> +++ b/drivers/nvme/target/configfs.c
>> @@ -1030,24 +1030,46 @@ static ssize_t nvmet_subsys_attr_version_store(struct config_item *item,
>> }
>> CONFIGFS_ATTR(nvmet_subsys_, attr_version);
>>
>> +/* 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_serial_show(struct config_item *item,
>> char *page)
>> {
>> struct nvmet_subsys *subsys = to_subsys(item);
>>
>> - return snprintf(page, PAGE_SIZE, "%llx\n", subsys->serial);
>> + return snprintf(page, PAGE_SIZE, "%s\n", subsys->serial);
>> }
>>
>> static ssize_t nvmet_subsys_attr_serial_store(struct config_item *item,
>> const char *page, size_t count)
>> {
>> - u64 serial;
>> + struct nvmet_subsys *subsys;
>> + int pos, len;
>> +
>> + subsys = to_subsys(item);
>> + len = strcspn(page, "\n");
>>
> nit:- we can avoid two extra initialization lines with :-
>
> struct nvmet_subsys *subsys = to_subsys(item);
> size_t pos, len = strcspn(page, "\n");
>
>> - if (sscanf(page, "%llx\n", &serial) != 1)
>> + if (len == 0 || len > NVMET_SN_MAX_SIZE) {
> nit:- 's/len == 0/!len/' is pretty common in the code.
Ok.
>> + pr_err("Serial Number can not be empty or exceed %d Bytes\n",
>> + NVMET_SN_MAX_SIZE);
>> return -EINVAL;
>> + }
>> +
>> + for (pos = 0; pos < len; pos++) {
>> + if (!nvmet_is_ascii(page[pos])) {
>> + pr_err("Serial Number must contain only ASCII strings\n");
>> + return -EINVAL;
>> + }
>> + }
>>
>> down_write(&nvmet_config_sem);
>> - to_subsys(item)->serial = serial;
>> + mutex_lock(&subsys->lock);
>> + memcpy_and_pad(subsys->serial, NVMET_SN_MAX_SIZE, page, len, ' ');
>> + mutex_unlock(&subsys->lock);
>> up_write(&nvmet_config_sem);
>>
>> return count;
>> @@ -1128,12 +1150,6 @@ static ssize_t nvmet_subsys_attr_model_show(struct config_item *item,
>> return ret;
>> }
>>
>> -/* 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_locked(struct nvmet_subsys *subsys,
>> const char *page, size_t count)
>> {
>> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
>> index adbede9ab7f3..3efd48b0a34e 100644
>> --- a/drivers/nvme/target/core.c
>> +++ b/drivers/nvme/target/core.c
>> @@ -1482,6 +1482,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
>> enum nvme_subsys_type type)
>> {
>> struct nvmet_subsys *subsys;
>> + char serial[NVMET_SN_MAX_SIZE / 2];
> This needs a comment why NVMET_SN_MAX_SIZE / 2.
explain bin2hex ?
>>
>> subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
>> if (!subsys)
>> @@ -1489,7 +1490,8 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
>>
>> subsys->ver = NVMET_DEFAULT_VS;
>> /* generate a random serial number as our controllers are ephemeral: */
>> - get_random_bytes(&subsys->serial, sizeof(subsys->serial));
>> + get_random_bytes(&serial, sizeof(serial));
>> + bin2hex(subsys->serial, &serial, sizeof(serial));
>>
>> switch (type) {
>> case NVME_NQN_NVME:
>> diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
>> index 4845d12e374a..f39946615fd6 100644
>> --- a/drivers/nvme/target/discovery.c
>> +++ b/drivers/nvme/target/discovery.c
>> @@ -262,9 +262,7 @@ static void nvmet_execute_disc_identify(struct nvmet_req *req)
>> goto out;
>> }
>>
>> - memset(id->sn, ' ', sizeof(id->sn));
>> - bin2hex(id->sn, &ctrl->subsys->serial,
>> - min(sizeof(ctrl->subsys->serial), sizeof(id->sn) / 2));
>> + memcpy(id->sn, ctrl->subsys->serial, NVMET_SN_MAX_SIZE);
>> memset(id->fr, ' ', sizeof(id->fr));
>> memcpy_and_pad(id->mn, sizeof(id->mn), model, sizeof(model) - 1, ' ');
>> memcpy_and_pad(id->fr, sizeof(id->fr),
>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>> index 5566ed403576..53999bd259ed 100644
>> --- a/drivers/nvme/target/nvmet.h
>> +++ b/drivers/nvme/target/nvmet.h
>> @@ -28,6 +28,7 @@
>> #define NVMET_NO_ERROR_LOC ((u16)-1)
>> #define NVMET_DEFAULT_CTRL_MODEL "Linux"
>> #define NVMET_MN_MAX_SIZE 40
>> +#define NVMET_SN_MAX_SIZE 20
>>
>> /*
>> * Supported optional AENs:
>> @@ -229,7 +230,7 @@ struct nvmet_subsys {
>> u16 max_qid;
>>
>> u64 ver;
>> - u64 serial;
>> + char serial[NVMET_SN_MAX_SIZE];
>> char *subsysnqn;
>> bool pi_support;
>>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] nvmet: change sn size and check validity
2021-04-22 8:50 ` Max Gurtovoy
@ 2021-04-22 19:11 ` Chaitanya Kulkarni
0 siblings, 0 replies; 13+ messages in thread
From: Chaitanya Kulkarni @ 2021-04-22 19:11 UTC (permalink / raw)
To: Max Gurtovoy; +Cc: linux-nvme, sagi, kbusch, hch, oren, ngottlieb
On 4/22/21 01:50, Max Gurtovoy wrote:
>>> -/* 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_locked(struct nvmet_subsys *subsys,
>>> const char *page, size_t count)
>>> {
>>> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
>>> index adbede9ab7f3..3efd48b0a34e 100644
>>> --- a/drivers/nvme/target/core.c
>>> +++ b/drivers/nvme/target/core.c
>>> @@ -1482,6 +1482,7 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
>>> enum nvme_subsys_type type)
>>> {
>>> struct nvmet_subsys *subsys;
>>> + char serial[NVMET_SN_MAX_SIZE / 2];
>> This needs a comment why NVMET_SN_MAX_SIZE / 2.
> explain bin2hex ?
>
>
I'm not sure, I'll leave it to Christoph and Sagi.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] nvmet: change sn size and check validity
2021-04-20 9:09 [PATCH 1/4] nvmet: change sn size and check validity Max Gurtovoy
` (3 preceding siblings ...)
2021-04-20 17:35 ` [PATCH 1/4] nvmet: change sn size and check validity Chaitanya Kulkarni
@ 2021-04-23 15:51 ` Keith Busch
2021-04-26 13:15 ` Max Gurtovoy
4 siblings, 1 reply; 13+ messages in thread
From: Keith Busch @ 2021-04-23 15:51 UTC (permalink / raw)
To: Max Gurtovoy; +Cc: linux-nvme, sagi, hch, chaitanya.kulkarni, oren, ngottlieb
On Tue, Apr 20, 2021 at 09:09:00AM +0000, Max Gurtovoy wrote:
> +/* See Section 1.5 of NVMe 1.4 */
> +static bool nvmet_is_ascii(const char c)
> +{
> + return c >= 0x20 && c <= 0x7e;
> +}
There's a library function, "isprint()", that can provide this check.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] nvmet: change sn size and check validity
2021-04-23 15:51 ` Keith Busch
@ 2021-04-26 13:15 ` Max Gurtovoy
2021-04-29 10:47 ` Max Gurtovoy
0 siblings, 1 reply; 13+ messages in thread
From: Max Gurtovoy @ 2021-04-26 13:15 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-nvme, sagi, hch, chaitanya.kulkarni, oren, ngottlieb
On 4/23/2021 6:51 PM, Keith Busch wrote:
> On Tue, Apr 20, 2021 at 09:09:00AM +0000, Max Gurtovoy wrote:
>> +/* See Section 1.5 of NVMe 1.4 */
>> +static bool nvmet_is_ascii(const char c)
>> +{
>> + return c >= 0x20 && c <= 0x7e;
>> +}
> There's a library function, "isprint()", that can provide this check.
This is an existing function in the code.
We'll add a dedicated commit to replace it.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] nvmet: change sn size and check validity
2021-04-26 13:15 ` Max Gurtovoy
@ 2021-04-29 10:47 ` Max Gurtovoy
0 siblings, 0 replies; 13+ messages in thread
From: Max Gurtovoy @ 2021-04-29 10:47 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-nvme, sagi, hch, chaitanya.kulkarni, oren, ngottlieb
On 4/26/2021 4:15 PM, Max Gurtovoy wrote:
>
> On 4/23/2021 6:51 PM, Keith Busch wrote:
>> On Tue, Apr 20, 2021 at 09:09:00AM +0000, Max Gurtovoy wrote:
>>> +/* See Section 1.5 of NVMe 1.4 */
>>> +static bool nvmet_is_ascii(const char c)
>>> +{
>>> + return c >= 0x20 && c <= 0x7e;
>>> +}
>> There's a library function, "isprint()", that can provide this check.
>
> This is an existing function in the code.
>
> We'll add a dedicated commit to replace it.
>
Actually the isprint check is weaker than the nvmet_is_ascii so we'll
leave it as-is.
Thanks.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 13+ messages in thread