* [PATCH 1/3] nvme: add generic helper to store secret
2023-05-16 10:06 [PATCH 0/3] nvme-core: dhchap_secret code cleanup Chaitanya Kulkarni
@ 2023-05-16 10:06 ` Chaitanya Kulkarni
2023-05-17 7:31 ` Sagi Grimberg
2023-05-16 10:06 ` [PATCH 2/3] nvme: use generic helper to store ctrl secret Chaitanya Kulkarni
2023-05-16 10:06 ` [PATCH 3/3] nvme-core: use macro defination to define dev attr Chaitanya Kulkarni
2 siblings, 1 reply; 8+ messages in thread
From: Chaitanya Kulkarni @ 2023-05-16 10:06 UTC (permalink / raw)
To: hare; +Cc: kbusch, hch, sagi, linux-nvme, Chaitanya Kulkarni
Refactor code to avoid duplication and improve maintainability:
Consolidate the shared code between the functions
nvme_ctrl_dhchap_secret_store() and
nvme_ctrl_dhchap_ctrl_secret_store(). This duplication not only
increases the likelihood of bugs but also requires additional effort for
maintenance and testing.
Introduce a new generic helper function called
nvme_dhchap_secret_store_common() to handle the storage of the
dhchap secret. This helper function will be used by both
nvme_ctrl_dhchap_secret_store() and
nvme_ctrl_dhchap_ctrl_secret_store().
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
drivers/nvme/host/sysfs.c | 59 ++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 20 deletions(-)
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 796e1d373b7c..9ce3b16f06da 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -418,43 +418,53 @@ static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
return sysfs_emit(buf, "%s\n", opts->dhchap_secret);
}
-static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
+ const char *buf, size_t count, bool ctrl_secret)
{
- struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
- struct nvmf_ctrl_options *opts = ctrl->opts;
- char *dhchap_secret;
+ struct nvme_dhchap_key **orig_key;
+ char **dhchap_secret;
+ char *new_dhchap_secret;
+
+ if (ctrl_secret) {
+ if (!ctrl->opts->dhchap_ctrl_secret)
+ return -EINVAL;
+ dhchap_secret = &ctrl->opts->dhchap_ctrl_secret;
+ orig_key = &ctrl->ctrl_key;
+ } else {
+ if (!ctrl->opts->dhchap_secret)
+ return -EINVAL;
+ dhchap_secret = &ctrl->opts->dhchap_secret;
+ orig_key = &ctrl->host_key;
+ }
- if (!ctrl->opts->dhchap_secret)
- return -EINVAL;
if (count < 7)
return -EINVAL;
if (memcmp(buf, "DHHC-1:", 7))
return -EINVAL;
- dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
- if (!dhchap_secret)
+ new_dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
+ if (!new_dhchap_secret)
return -ENOMEM;
- memcpy(dhchap_secret, buf, count);
+ memcpy(new_dhchap_secret, buf, count);
nvme_auth_stop(ctrl);
- if (strcmp(dhchap_secret, opts->dhchap_secret)) {
- struct nvme_dhchap_key *key, *host_key;
+ if (strcmp(new_dhchap_secret, *dhchap_secret)) {
+ struct nvme_dhchap_key *new_key, *prev_host_key;
int ret;
- ret = nvme_auth_generate_key(dhchap_secret, &key);
+ ret = nvme_auth_generate_key(new_dhchap_secret, &new_key);
if (ret) {
- kfree(dhchap_secret);
+ kfree(new_dhchap_secret);
return ret;
}
- kfree(opts->dhchap_secret);
- opts->dhchap_secret = dhchap_secret;
- host_key = ctrl->host_key;
+ kfree(*dhchap_secret);
+ *dhchap_secret = new_dhchap_secret;
+ prev_host_key = *orig_key;
mutex_lock(&ctrl->dhchap_auth_mutex);
- ctrl->host_key = key;
+ *orig_key = new_key;
mutex_unlock(&ctrl->dhchap_auth_mutex);
- nvme_auth_free_key(host_key);
+ nvme_auth_free_key(prev_host_key);
} else
- kfree(dhchap_secret);
+ kfree(new_dhchap_secret);
/* Start re-authentication */
dev_info(ctrl->device, "re-authenticating controller\n");
queue_work(nvme_wq, &ctrl->dhchap_auth_work);
@@ -462,6 +472,15 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
return count;
}
+
+static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
+
+ return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
+}
+
static DEVICE_ATTR(dhchap_secret, S_IRUGO | S_IWUSR,
nvme_ctrl_dhchap_secret_show, nvme_ctrl_dhchap_secret_store);
--
2.40.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] nvme: add generic helper to store secret
2023-05-16 10:06 ` [PATCH 1/3] nvme: add generic helper to store secret Chaitanya Kulkarni
@ 2023-05-17 7:31 ` Sagi Grimberg
2023-05-17 21:05 ` Chaitanya Kulkarni
0 siblings, 1 reply; 8+ messages in thread
From: Sagi Grimberg @ 2023-05-17 7:31 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: kbusch, hch, linux-nvme
On 5/16/23 13:06, Chaitanya Kulkarni wrote:
> Refactor code to avoid duplication and improve maintainability:
>
> Consolidate the shared code between the functions
> nvme_ctrl_dhchap_secret_store() and
> nvme_ctrl_dhchap_ctrl_secret_store(). This duplication not only
> increases the likelihood of bugs but also requires additional effort for
> maintenance and testing.
>
> Introduce a new generic helper function called
> nvme_dhchap_secret_store_common() to handle the storage of the
> dhchap secret. This helper function will be used by both
> nvme_ctrl_dhchap_secret_store() and
> nvme_ctrl_dhchap_ctrl_secret_store().
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
> drivers/nvme/host/sysfs.c | 59 ++++++++++++++++++++++++++-------------
> 1 file changed, 39 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 796e1d373b7c..9ce3b16f06da 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -418,43 +418,53 @@ static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
> return sysfs_emit(buf, "%s\n", opts->dhchap_secret);
> }
>
> -static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t count)
> +static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
> + const char *buf, size_t count, bool ctrl_secret)
> {
> - struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> - struct nvmf_ctrl_options *opts = ctrl->opts;
> - char *dhchap_secret;
> + struct nvme_dhchap_key **orig_key;
> + char **dhchap_secret;
> + char *new_dhchap_secret;
> +
> + if (ctrl_secret) {
> + if (!ctrl->opts->dhchap_ctrl_secret)
> + return -EINVAL;
> + dhchap_secret = &ctrl->opts->dhchap_ctrl_secret;
> + orig_key = &ctrl->ctrl_key;
> + } else {
> + if (!ctrl->opts->dhchap_secret)
> + return -EINVAL;
> + dhchap_secret = &ctrl->opts->dhchap_secret;
> + orig_key = &ctrl->host_key;
> + }
>
> - if (!ctrl->opts->dhchap_secret)
> - return -EINVAL;
> if (count < 7)
> return -EINVAL;
> if (memcmp(buf, "DHHC-1:", 7))
> return -EINVAL;
>
> - dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
> - if (!dhchap_secret)
> + new_dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
> + if (!new_dhchap_secret)
> return -ENOMEM;
> - memcpy(dhchap_secret, buf, count);
> + memcpy(new_dhchap_secret, buf, count);
> nvme_auth_stop(ctrl);
> - if (strcmp(dhchap_secret, opts->dhchap_secret)) {
> - struct nvme_dhchap_key *key, *host_key;
> + if (strcmp(new_dhchap_secret, *dhchap_secret)) {
> + struct nvme_dhchap_key *new_key, *prev_host_key;
prev_host_key? or prev_key?
> int ret;
>
> - ret = nvme_auth_generate_key(dhchap_secret, &key);
> + ret = nvme_auth_generate_key(new_dhchap_secret, &new_key);
> if (ret) {
> - kfree(dhchap_secret);
> + kfree(new_dhchap_secret);
> return ret;
> }
> - kfree(opts->dhchap_secret);
> - opts->dhchap_secret = dhchap_secret;
> - host_key = ctrl->host_key;
> + kfree(*dhchap_secret);
> + *dhchap_secret = new_dhchap_secret;
> + prev_host_key = *orig_key;
> mutex_lock(&ctrl->dhchap_auth_mutex);
> - ctrl->host_key = key;
> + *orig_key = new_key;
> mutex_unlock(&ctrl->dhchap_auth_mutex);
> - nvme_auth_free_key(host_key);
> + nvme_auth_free_key(prev_host_key);
> } else
> - kfree(dhchap_secret);
> + kfree(new_dhchap_secret);
> /* Start re-authentication */
> dev_info(ctrl->device, "re-authenticating controller\n");
> queue_work(nvme_wq, &ctrl->dhchap_auth_work);
> @@ -462,6 +472,15 @@ static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
> return count;
> }
>
> +
> +static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> +
> + return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
> +}
> +
> static DEVICE_ATTR(dhchap_secret, S_IRUGO | S_IWUSR,
> nvme_ctrl_dhchap_secret_show, nvme_ctrl_dhchap_secret_store);
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] nvme: add generic helper to store secret
2023-05-17 7:31 ` Sagi Grimberg
@ 2023-05-17 21:05 ` Chaitanya Kulkarni
0 siblings, 0 replies; 8+ messages in thread
From: Chaitanya Kulkarni @ 2023-05-17 21:05 UTC (permalink / raw)
To: Sagi Grimberg, Chaitanya Kulkarni, hare@suse.de
Cc: kbusch@kernel.org, hch@lst.de, linux-nvme@lists.infradead.org
On 5/17/23 00:31, Sagi Grimberg wrote:
>
>
> On 5/16/23 13:06, Chaitanya Kulkarni wrote:
>> Refactor code to avoid duplication and improve maintainability:
>>
>> Consolidate the shared code between the functions
>> nvme_ctrl_dhchap_secret_store() and
>> nvme_ctrl_dhchap_ctrl_secret_store(). This duplication not only
>> increases the likelihood of bugs but also requires additional effort for
>> maintenance and testing.
>>
>> Introduce a new generic helper function called
>> nvme_dhchap_secret_store_common() to handle the storage of the
>> dhchap secret. This helper function will be used by both
>> nvme_ctrl_dhchap_secret_store() and
>> nvme_ctrl_dhchap_ctrl_secret_store().
>>
>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>> ---
>> drivers/nvme/host/sysfs.c | 59 ++++++++++++++++++++++++++-------------
>> 1 file changed, 39 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
>> index 796e1d373b7c..9ce3b16f06da 100644
>> --- a/drivers/nvme/host/sysfs.c
>> +++ b/drivers/nvme/host/sysfs.c
>> @@ -418,43 +418,53 @@ static ssize_t
>> nvme_ctrl_dhchap_secret_show(struct device *dev,
>> return sysfs_emit(buf, "%s\n", opts->dhchap_secret);
>> }
>> -static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
>> - struct device_attribute *attr, const char *buf, size_t count)
>> +static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
>> + const char *buf, size_t count, bool ctrl_secret)
>> {
>> - struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>> - struct nvmf_ctrl_options *opts = ctrl->opts;
>> - char *dhchap_secret;
>> + struct nvme_dhchap_key **orig_key;
>> + char **dhchap_secret;
>> + char *new_dhchap_secret;
>> +
>> + if (ctrl_secret) {
>> + if (!ctrl->opts->dhchap_ctrl_secret)
>> + return -EINVAL;
>> + dhchap_secret = &ctrl->opts->dhchap_ctrl_secret;
>> + orig_key = &ctrl->ctrl_key;
>> + } else {
>> + if (!ctrl->opts->dhchap_secret)
>> + return -EINVAL;
>> + dhchap_secret = &ctrl->opts->dhchap_secret;
>> + orig_key = &ctrl->host_key;
>> + }
>> - if (!ctrl->opts->dhchap_secret)
>> - return -EINVAL;
>> if (count < 7)
>> return -EINVAL;
>> if (memcmp(buf, "DHHC-1:", 7))
>> return -EINVAL;
>> - dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
>> - if (!dhchap_secret)
>> + new_dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
>> + if (!new_dhchap_secret)
>> return -ENOMEM;
>> - memcpy(dhchap_secret, buf, count);
>> + memcpy(new_dhchap_secret, buf, count);
>> nvme_auth_stop(ctrl);
>> - if (strcmp(dhchap_secret, opts->dhchap_secret)) {
>> - struct nvme_dhchap_key *key, *host_key;
>> + if (strcmp(new_dhchap_secret, *dhchap_secret)) {
>> + struct nvme_dhchap_key *new_key, *prev_host_key;
>
> prev_host_key? or prev_key?
will fix it in the next version ..
-ck
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] nvme: use generic helper to store ctrl secret
2023-05-16 10:06 [PATCH 0/3] nvme-core: dhchap_secret code cleanup Chaitanya Kulkarni
2023-05-16 10:06 ` [PATCH 1/3] nvme: add generic helper to store secret Chaitanya Kulkarni
@ 2023-05-16 10:06 ` Chaitanya Kulkarni
2023-05-17 7:32 ` Sagi Grimberg
2023-05-16 10:06 ` [PATCH 3/3] nvme-core: use macro defination to define dev attr Chaitanya Kulkarni
2 siblings, 1 reply; 8+ messages in thread
From: Chaitanya Kulkarni @ 2023-05-16 10:06 UTC (permalink / raw)
To: hare; +Cc: kbusch, hch, sagi, linux-nvme, Chaitanya Kulkarni
Use generic helper to store the dhchap ctrl secret.
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
drivers/nvme/host/sysfs.c | 40 +--------------------------------------
1 file changed, 1 insertion(+), 39 deletions(-)
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 9ce3b16f06da..515baf8d6402 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -498,45 +498,7 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
- struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
- struct nvmf_ctrl_options *opts = ctrl->opts;
- char *dhchap_secret;
-
- if (!ctrl->opts->dhchap_ctrl_secret)
- return -EINVAL;
- if (count < 7)
- return -EINVAL;
- if (memcmp(buf, "DHHC-1:", 7))
- return -EINVAL;
-
- dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
- if (!dhchap_secret)
- return -ENOMEM;
- memcpy(dhchap_secret, buf, count);
- nvme_auth_stop(ctrl);
- if (strcmp(dhchap_secret, opts->dhchap_ctrl_secret)) {
- struct nvme_dhchap_key *key, *ctrl_key;
- int ret;
-
- ret = nvme_auth_generate_key(dhchap_secret, &key);
- if (ret) {
- kfree(dhchap_secret);
- return ret;
- }
- kfree(opts->dhchap_ctrl_secret);
- opts->dhchap_ctrl_secret = dhchap_secret;
- ctrl_key = ctrl->ctrl_key;
- mutex_lock(&ctrl->dhchap_auth_mutex);
- ctrl->ctrl_key = key;
- mutex_unlock(&ctrl->dhchap_auth_mutex);
- nvme_auth_free_key(ctrl_key);
- } else
- kfree(dhchap_secret);
- /* Start re-authentication */
- dev_info(ctrl->device, "re-authenticating controller\n");
- queue_work(nvme_wq, &ctrl->dhchap_auth_work);
-
- return count;
+ return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
}
static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
--
2.40.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] nvme: use generic helper to store ctrl secret
2023-05-16 10:06 ` [PATCH 2/3] nvme: use generic helper to store ctrl secret Chaitanya Kulkarni
@ 2023-05-17 7:32 ` Sagi Grimberg
0 siblings, 0 replies; 8+ messages in thread
From: Sagi Grimberg @ 2023-05-17 7:32 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: kbusch, hch, linux-nvme
Can be squashed to the former.
On 5/16/23 13:06, Chaitanya Kulkarni wrote:
> Use generic helper to store the dhchap ctrl secret.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
> drivers/nvme/host/sysfs.c | 40 +--------------------------------------
> 1 file changed, 1 insertion(+), 39 deletions(-)
>
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 9ce3b16f06da..515baf8d6402 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -498,45 +498,7 @@ static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
> static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
> struct device_attribute *attr, const char *buf, size_t count)
> {
> - struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> - struct nvmf_ctrl_options *opts = ctrl->opts;
> - char *dhchap_secret;
> -
> - if (!ctrl->opts->dhchap_ctrl_secret)
> - return -EINVAL;
> - if (count < 7)
> - return -EINVAL;
> - if (memcmp(buf, "DHHC-1:", 7))
> - return -EINVAL;
> -
> - dhchap_secret = kzalloc(count + 1, GFP_KERNEL);
> - if (!dhchap_secret)
> - return -ENOMEM;
> - memcpy(dhchap_secret, buf, count);
> - nvme_auth_stop(ctrl);
> - if (strcmp(dhchap_secret, opts->dhchap_ctrl_secret)) {
> - struct nvme_dhchap_key *key, *ctrl_key;
> - int ret;
> -
> - ret = nvme_auth_generate_key(dhchap_secret, &key);
> - if (ret) {
> - kfree(dhchap_secret);
> - return ret;
> - }
> - kfree(opts->dhchap_ctrl_secret);
> - opts->dhchap_ctrl_secret = dhchap_secret;
> - ctrl_key = ctrl->ctrl_key;
> - mutex_lock(&ctrl->dhchap_auth_mutex);
> - ctrl->ctrl_key = key;
> - mutex_unlock(&ctrl->dhchap_auth_mutex);
> - nvme_auth_free_key(ctrl_key);
> - } else
> - kfree(dhchap_secret);
> - /* Start re-authentication */
> - dev_info(ctrl->device, "re-authenticating controller\n");
> - queue_work(nvme_wq, &ctrl->dhchap_auth_work);
> -
> - return count;
> + return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
> }
>
> static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] nvme-core: use macro defination to define dev attr
2023-05-16 10:06 [PATCH 0/3] nvme-core: dhchap_secret code cleanup Chaitanya Kulkarni
2023-05-16 10:06 ` [PATCH 1/3] nvme: add generic helper to store secret Chaitanya Kulkarni
2023-05-16 10:06 ` [PATCH 2/3] nvme: use generic helper to store ctrl secret Chaitanya Kulkarni
@ 2023-05-16 10:06 ` Chaitanya Kulkarni
2023-05-17 7:35 ` Sagi Grimberg
2 siblings, 1 reply; 8+ messages in thread
From: Chaitanya Kulkarni @ 2023-05-16 10:06 UTC (permalink / raw)
To: hare; +Cc: kbusch, hch, sagi, linux-nvme, Chaitanya Kulkarni
Insated of duplicating the code for dhchap_secret & dhchap_ctrl_secret,
define a macro to register device attribute to create device attribute
that will also define store and show helpers.
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
drivers/nvme/host/sysfs.c | 66 +++++++++++++++------------------------
1 file changed, 25 insertions(+), 41 deletions(-)
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 515baf8d6402..e0a2a041e595 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -407,17 +407,6 @@ static ssize_t dctype_show(struct device *dev,
static DEVICE_ATTR_RO(dctype);
#ifdef CONFIG_NVME_AUTH
-static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
- struct nvmf_ctrl_options *opts = ctrl->opts;
-
- if (!opts->dhchap_secret)
- return sysfs_emit(buf, "none\n");
- return sysfs_emit(buf, "%s\n", opts->dhchap_secret);
-}
-
static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
const char *buf, size_t count, bool ctrl_secret)
{
@@ -472,37 +461,32 @@ static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
return count;
}
+#define NVME_AUTH_DEVICE_ATTR(NAME, cs) \
+static ssize_t nvme_ctrl_##NAME##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ struct nvme_ctrl *ctrl = dev_get_drvdata(dev); \
+ struct nvmf_ctrl_options *opts = ctrl->opts; \
+ \
+ if (!opts->NAME) \
+ return sysfs_emit(buf, "none\n"); \
+ return sysfs_emit(buf, "%s\n", opts->NAME); \
+} \
+ \
+static ssize_t nvme_ctrl_##NAME##_store(struct device *dev, \
+ struct device_attribute *attr, const char *buf, \
+ size_t count) \
+{ \
+ struct nvme_ctrl *ctrl = dev_get_drvdata(dev); \
+ \
+ return nvme_dhchap_secret_store_common(ctrl, buf, count, cs); \
+} \
+ \
+static DEVICE_ATTR(NAME, S_IRUGO | S_IWUSR, \
+ nvme_ctrl_##NAME##_show, nvme_ctrl_##NAME##_store); \
-static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
-{
- struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
-
- return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
-}
-
-static DEVICE_ATTR(dhchap_secret, S_IRUGO | S_IWUSR,
- nvme_ctrl_dhchap_secret_show, nvme_ctrl_dhchap_secret_store);
-
-static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
- struct nvmf_ctrl_options *opts = ctrl->opts;
-
- if (!opts->dhchap_ctrl_secret)
- return sysfs_emit(buf, "none\n");
- return sysfs_emit(buf, "%s\n", opts->dhchap_ctrl_secret);
-}
-
-static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
- struct device_attribute *attr, const char *buf, size_t count)
-{
- return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
-}
-
-static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
- nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);
+NVME_AUTH_DEVICE_ATTR(dhchap_secret, false);
+NVME_AUTH_DEVICE_ATTR(dhchap_ctrl_secret, true);
#endif
static struct attribute *nvme_dev_attrs[] = {
--
2.40.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] nvme-core: use macro defination to define dev attr
2023-05-16 10:06 ` [PATCH 3/3] nvme-core: use macro defination to define dev attr Chaitanya Kulkarni
@ 2023-05-17 7:35 ` Sagi Grimberg
0 siblings, 0 replies; 8+ messages in thread
From: Sagi Grimberg @ 2023-05-17 7:35 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: kbusch, hch, linux-nvme
> Insated of duplicating the code for dhchap_secret & dhchap_ctrl_secret,
> define a macro to register device attribute to create device attribute
> that will also define store and show helpers.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
> drivers/nvme/host/sysfs.c | 66 +++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
> index 515baf8d6402..e0a2a041e595 100644
> --- a/drivers/nvme/host/sysfs.c
> +++ b/drivers/nvme/host/sysfs.c
> @@ -407,17 +407,6 @@ static ssize_t dctype_show(struct device *dev,
> static DEVICE_ATTR_RO(dctype);
>
> #ifdef CONFIG_NVME_AUTH
> -static ssize_t nvme_ctrl_dhchap_secret_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> - struct nvmf_ctrl_options *opts = ctrl->opts;
> -
> - if (!opts->dhchap_secret)
> - return sysfs_emit(buf, "none\n");
> - return sysfs_emit(buf, "%s\n", opts->dhchap_secret);
> -}
> -
> static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
> const char *buf, size_t count, bool ctrl_secret)
> {
> @@ -472,37 +461,32 @@ static ssize_t nvme_dhchap_secret_store_common(struct nvme_ctrl *ctrl,
> return count;
> }
>
> +#define NVME_AUTH_DEVICE_ATTR(NAME, cs) \
> +static ssize_t nvme_ctrl_##NAME##_show(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> +{ \
> + struct nvme_ctrl *ctrl = dev_get_drvdata(dev); \
> + struct nvmf_ctrl_options *opts = ctrl->opts; \
> + \
> + if (!opts->NAME) \
This is where these macros can really take things out of context...
The code just lost readability at this point.
> + return sysfs_emit(buf, "none\n"); \
> + return sysfs_emit(buf, "%s\n", opts->NAME); \
> +} \
> + \
> +static ssize_t nvme_ctrl_##NAME##_store(struct device *dev, \
> + struct device_attribute *attr, const char *buf, \
> + size_t count) \
> +{ \
> + struct nvme_ctrl *ctrl = dev_get_drvdata(dev); \
> + \
> + return nvme_dhchap_secret_store_common(ctrl, buf, count, cs); \
> +} \
> + \
> +static DEVICE_ATTR(NAME, S_IRUGO | S_IWUSR, \
> + nvme_ctrl_##NAME##_show, nvme_ctrl_##NAME##_store); \
>
> -static ssize_t nvme_ctrl_dhchap_secret_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t count)
> -{
> - struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> -
> - return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
> -}
> -
> -static DEVICE_ATTR(dhchap_secret, S_IRUGO | S_IWUSR,
> - nvme_ctrl_dhchap_secret_show, nvme_ctrl_dhchap_secret_store);
> -
> -static ssize_t nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
> - struct nvmf_ctrl_options *opts = ctrl->opts;
> -
> - if (!opts->dhchap_ctrl_secret)
> - return sysfs_emit(buf, "none\n");
> - return sysfs_emit(buf, "%s\n", opts->dhchap_ctrl_secret);
> -}
> -
> -static ssize_t nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t count)
> -{
> - return nvme_dhchap_secret_store_common(ctrl, buf, count, false);
> -}
> -
> -static DEVICE_ATTR(dhchap_ctrl_secret, S_IRUGO | S_IWUSR,
> - nvme_ctrl_dhchap_ctrl_secret_show, nvme_ctrl_dhchap_ctrl_secret_store);
> +NVME_AUTH_DEVICE_ATTR(dhchap_secret, false);
> +NVME_AUTH_DEVICE_ATTR(dhchap_ctrl_secret, true);
> #endif
>
> static struct attribute *nvme_dev_attrs[] = {
^ permalink raw reply [flat|nested] 8+ messages in thread