* [PATCH V3 1/3] nvmet-auth: use common helper to check secp/spsp
2023-06-05 9:19 [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup Chaitanya Kulkarni
@ 2023-06-05 9:19 ` Chaitanya Kulkarni
2023-06-05 21:56 ` Sagi Grimberg
2023-06-07 10:59 ` Max Gurtovoy
2023-06-05 9:19 ` [PATCH V3 2/3] nvmet_auth: use common helper for buffer alloc Chaitanya Kulkarni
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-06-05 9:19 UTC (permalink / raw)
To: hare; +Cc: hch, sagi, linux-nvme, kbusch, Chaitanya Kulkarni
Add a common helper to factor out secp/spsp values check in
nvmet_execute_auth_send() and nvmet_execute_auth_receive().
No functional change in this patch.
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
drivers/nvme/target/fabrics-cmd-auth.c | 60 +++++++++++---------------
1 file changed, 24 insertions(+), 36 deletions(-)
diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index 586458f765f1..847aa12d2915 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -12,6 +12,23 @@
#include <crypto/kpp.h>
#include "nvmet.h"
+static u16 nvmet_auth_common_prep(struct nvmet_req *req)
+{
+ if (req->cmd->auth_send.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
+ req->error_loc = offsetof(struct nvmf_auth_send_command, secp);
+ return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+ }
+ if (req->cmd->auth_send.spsp0 != 0x01) {
+ req->error_loc = offsetof(struct nvmf_auth_send_command, spsp0);
+ return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+ }
+ if (req->cmd->auth_send.spsp1 != 0x01) {
+ req->error_loc = offsetof(struct nvmf_auth_send_command, spsp1);
+ return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+ }
+ return NVME_SC_SUCCESS;
+}
+
static void nvmet_auth_expired_work(struct work_struct *work)
{
struct nvmet_sq *sq = container_of(to_delayed_work(work),
@@ -185,26 +202,12 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
struct nvmf_auth_dhchap_success2_data *data;
void *d;
u32 tl;
- u16 status = 0;
+ u16 status;
- if (req->cmd->auth_send.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
- status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
- req->error_loc =
- offsetof(struct nvmf_auth_send_command, secp);
- goto done;
- }
- if (req->cmd->auth_send.spsp0 != 0x01) {
- status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
- req->error_loc =
- offsetof(struct nvmf_auth_send_command, spsp0);
- goto done;
- }
- if (req->cmd->auth_send.spsp1 != 0x01) {
- status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
- req->error_loc =
- offsetof(struct nvmf_auth_send_command, spsp1);
+ status = nvmet_auth_common_prep(req);
+ if (status)
goto done;
- }
+
tl = le32_to_cpu(req->cmd->auth_send.tl);
if (!tl) {
status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
@@ -432,26 +435,11 @@ void nvmet_execute_auth_receive(struct nvmet_req *req)
struct nvmet_ctrl *ctrl = req->sq->ctrl;
void *d;
u32 al;
- u16 status = 0;
+ u16 status;
- if (req->cmd->auth_receive.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
- status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
- req->error_loc =
- offsetof(struct nvmf_auth_receive_command, secp);
- goto done;
- }
- if (req->cmd->auth_receive.spsp0 != 0x01) {
- status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
- req->error_loc =
- offsetof(struct nvmf_auth_receive_command, spsp0);
- goto done;
- }
- if (req->cmd->auth_receive.spsp1 != 0x01) {
- status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
- req->error_loc =
- offsetof(struct nvmf_auth_receive_command, spsp1);
+ status = nvmet_auth_common_prep(req);
+ if (status)
goto done;
- }
al = le32_to_cpu(req->cmd->auth_receive.al);
if (!al) {
status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH V3 1/3] nvmet-auth: use common helper to check secp/spsp
2023-06-05 9:19 ` [PATCH V3 1/3] nvmet-auth: use common helper to check secp/spsp Chaitanya Kulkarni
@ 2023-06-05 21:56 ` Sagi Grimberg
2023-06-07 10:59 ` Max Gurtovoy
1 sibling, 0 replies; 11+ messages in thread
From: Sagi Grimberg @ 2023-06-05 21:56 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: hch, linux-nvme, kbusch
OK,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 1/3] nvmet-auth: use common helper to check secp/spsp
2023-06-05 9:19 ` [PATCH V3 1/3] nvmet-auth: use common helper to check secp/spsp Chaitanya Kulkarni
2023-06-05 21:56 ` Sagi Grimberg
@ 2023-06-07 10:59 ` Max Gurtovoy
1 sibling, 0 replies; 11+ messages in thread
From: Max Gurtovoy @ 2023-06-07 10:59 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: hch, sagi, linux-nvme, kbusch
On 05/06/2023 12:19, Chaitanya Kulkarni wrote:
> Add a common helper to factor out secp/spsp values check in
> nvmet_execute_auth_send() and nvmet_execute_auth_receive().
>
> No functional change in this patch.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
> drivers/nvme/target/fabrics-cmd-auth.c | 60 +++++++++++---------------
> 1 file changed, 24 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
> index 586458f765f1..847aa12d2915 100644
> --- a/drivers/nvme/target/fabrics-cmd-auth.c
> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
> @@ -12,6 +12,23 @@
> #include <crypto/kpp.h>
> #include "nvmet.h"
>
> +static u16 nvmet_auth_common_prep(struct nvmet_req *req)
The naming of the function is misleading a bit.
Function name should reflect better the logic of it.
This function is not preparing anything but just checking and validating
fields.
maybe call it nvmet_auth_security_protocol_validate(struct nvmet_req *req)
> +{
> + if (req->cmd->auth_send.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
> + req->error_loc = offsetof(struct nvmf_auth_send_command, secp);
> + return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> + }
> + if (req->cmd->auth_send.spsp0 != 0x01) {
> + req->error_loc = offsetof(struct nvmf_auth_send_command, spsp0);
> + return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> + }
> + if (req->cmd->auth_send.spsp1 != 0x01) {
> + req->error_loc = offsetof(struct nvmf_auth_send_command, spsp1);
> + return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> + }
> + return NVME_SC_SUCCESS;
> +}
> +
> static void nvmet_auth_expired_work(struct work_struct *work)
> {
> struct nvmet_sq *sq = container_of(to_delayed_work(work),
> @@ -185,26 +202,12 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
> struct nvmf_auth_dhchap_success2_data *data;
> void *d;
> u32 tl;
> - u16 status = 0;
> + u16 status;
>
> - if (req->cmd->auth_send.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
> - status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> - req->error_loc =
> - offsetof(struct nvmf_auth_send_command, secp);
> - goto done;
> - }
> - if (req->cmd->auth_send.spsp0 != 0x01) {
> - status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> - req->error_loc =
> - offsetof(struct nvmf_auth_send_command, spsp0);
> - goto done;
> - }
> - if (req->cmd->auth_send.spsp1 != 0x01) {
> - status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> - req->error_loc =
> - offsetof(struct nvmf_auth_send_command, spsp1);
> + status = nvmet_auth_common_prep(req);
> + if (status)
> goto done;
> - }
> +
> tl = le32_to_cpu(req->cmd->auth_send.tl);
> if (!tl) {
> status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> @@ -432,26 +435,11 @@ void nvmet_execute_auth_receive(struct nvmet_req *req)
> struct nvmet_ctrl *ctrl = req->sq->ctrl;
> void *d;
> u32 al;
> - u16 status = 0;
> + u16 status;
>
> - if (req->cmd->auth_receive.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
> - status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> - req->error_loc =
> - offsetof(struct nvmf_auth_receive_command, secp);
> - goto done;
> - }
> - if (req->cmd->auth_receive.spsp0 != 0x01) {
> - status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> - req->error_loc =
> - offsetof(struct nvmf_auth_receive_command, spsp0);
> - goto done;
> - }
> - if (req->cmd->auth_receive.spsp1 != 0x01) {
> - status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> - req->error_loc =
> - offsetof(struct nvmf_auth_receive_command, spsp1);
> + status = nvmet_auth_common_prep(req);
> + if (status)
> goto done;
> - }
> al = le32_to_cpu(req->cmd->auth_receive.al);
> if (!al) {
> status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V3 2/3] nvmet_auth: use common helper for buffer alloc
2023-06-05 9:19 [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup Chaitanya Kulkarni
2023-06-05 9:19 ` [PATCH V3 1/3] nvmet-auth: use common helper to check secp/spsp Chaitanya Kulkarni
@ 2023-06-05 9:19 ` Chaitanya Kulkarni
2023-06-05 22:02 ` Sagi Grimberg
2023-06-05 9:19 ` [PATCH V3 3/3] nvmet-auth: use correct type for status variable Chaitanya Kulkarni
2023-06-05 9:24 ` [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup Chaitanya Kulkarni
3 siblings, 1 reply; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-06-05 9:19 UTC (permalink / raw)
To: hare; +Cc: hch, sagi, linux-nvme, kbusch, Chaitanya Kulkarni
Add a common helper to factor out buffer allocation in
nvmet_execute_auth_send() and nvmet_execute_auth_receive() and call it
from nvmet_auth_common_prep() once we done with the secp/spsp0/spsp1
check.
Only functional change in this patch is transfer buffer allocation is
moved before nvmet_check_transfer_len() and it is freed if when
nvmet_check_transfer_len() fails. But similar allocation and free is
used in error unwind path in nvme code and it is not in fast path, so
it shuold be fine.
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
drivers/nvme/target/fabrics-cmd-auth.c | 37 ++++++++++++++------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index 847aa12d2915..dbcae93bd25c 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -12,7 +12,20 @@
#include <crypto/kpp.h>
#include "nvmet.h"
-static u16 nvmet_auth_common_prep(struct nvmet_req *req)
+static u16 nvmet_auth_alloc_transfer_buffer(struct nvmet_req *req, void **buf,
+ u32 *len)
+{
+ *len = le32_to_cpu(req->cmd->auth_receive.al);
+ if (!*len) {
+ req->error_loc = offsetof(struct nvmf_auth_receive_command, al);
+ return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+ }
+ *buf = kmalloc(*len, GFP_KERNEL);
+ return *buf ? NVME_SC_SUCCESS : NVME_SC_INTERNAL;
+}
+
+static u16 nvmet_auth_common_prep(struct nvmet_req *req, void **buf,
+ u32 *len)
{
if (req->cmd->auth_send.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
req->error_loc = offsetof(struct nvmf_auth_send_command, secp);
@@ -26,7 +39,8 @@ static u16 nvmet_auth_common_prep(struct nvmet_req *req)
req->error_loc = offsetof(struct nvmf_auth_send_command, spsp1);
return NVME_SC_INVALID_FIELD | NVME_SC_DNR;
}
- return NVME_SC_SUCCESS;
+
+ return nvmet_auth_alloc_transfer_buffer(req, buf, len);
}
static void nvmet_auth_expired_work(struct work_struct *work)
@@ -204,28 +218,16 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
u32 tl;
u16 status;
- status = nvmet_auth_common_prep(req);
+ status = nvmet_auth_common_prep(req, &d, &tl);
if (status)
goto done;
- tl = le32_to_cpu(req->cmd->auth_send.tl);
- if (!tl) {
- status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
- req->error_loc =
- offsetof(struct nvmf_auth_send_command, tl);
- goto done;
- }
if (!nvmet_check_transfer_len(req, tl)) {
pr_debug("%s: transfer length mismatch (%u)\n", __func__, tl);
+ kfree(d);
return;
}
- d = kmalloc(tl, GFP_KERNEL);
- if (!d) {
- status = NVME_SC_INTERNAL;
- goto done;
- }
-
status = nvmet_copy_from_sgl(req, 0, d, tl);
if (status)
goto done_kfree;
@@ -437,7 +439,7 @@ void nvmet_execute_auth_receive(struct nvmet_req *req)
u32 al;
u16 status;
- status = nvmet_auth_common_prep(req);
+ status = nvmet_auth_common_prep(req, &d, &al);
if (status)
goto done;
al = le32_to_cpu(req->cmd->auth_receive.al);
@@ -449,6 +451,7 @@ void nvmet_execute_auth_receive(struct nvmet_req *req)
}
if (!nvmet_check_transfer_len(req, al)) {
pr_debug("%s: transfer length mismatch (%u)\n", __func__, al);
+ kfree(d);
return;
}
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH V3 2/3] nvmet_auth: use common helper for buffer alloc
2023-06-05 9:19 ` [PATCH V3 2/3] nvmet_auth: use common helper for buffer alloc Chaitanya Kulkarni
@ 2023-06-05 22:02 ` Sagi Grimberg
2023-06-08 12:41 ` Max Gurtovoy
0 siblings, 1 reply; 11+ messages in thread
From: Sagi Grimberg @ 2023-06-05 22:02 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: hch, linux-nvme, kbusch
On 6/5/23 12:19, Chaitanya Kulkarni wrote:
> Add a common helper to factor out buffer allocation in
> nvmet_execute_auth_send() and nvmet_execute_auth_receive() and call it
> from nvmet_auth_common_prep() once we done with the secp/spsp0/spsp1
> check.
>
> Only functional change in this patch is transfer buffer allocation is
> moved before nvmet_check_transfer_len() and it is freed if when
> nvmet_check_transfer_len() fails. But similar allocation and free is
> used in error unwind path in nvme code and it is not in fast path, so
> it shuold be fine.
This is asking for a future memory leak. It kinda makes sense to
allocate after we run sanity checks on the request...
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 2/3] nvmet_auth: use common helper for buffer alloc
2023-06-05 22:02 ` Sagi Grimberg
@ 2023-06-08 12:41 ` Max Gurtovoy
0 siblings, 0 replies; 11+ messages in thread
From: Max Gurtovoy @ 2023-06-08 12:41 UTC (permalink / raw)
To: Sagi Grimberg, Chaitanya Kulkarni, hare; +Cc: hch, linux-nvme, kbusch
On 06/06/2023 1:02, Sagi Grimberg wrote:
>
>
> On 6/5/23 12:19, Chaitanya Kulkarni wrote:
>> Add a common helper to factor out buffer allocation in
>> nvmet_execute_auth_send() and nvmet_execute_auth_receive() and call it
>> from nvmet_auth_common_prep() once we done with the secp/spsp0/spsp1
>> check.
>>
>> Only functional change in this patch is transfer buffer allocation is
>> moved before nvmet_check_transfer_len() and it is freed if when
>> nvmet_check_transfer_len() fails. But similar allocation and free is
>> used in error unwind path in nvme code and it is not in fast path, so
>> it shuold be fine.
>
> This is asking for a future memory leak. It kinda makes sense to
> allocate after we run sanity checks on the request...
>
Also I don't understand why do we need helper function for malloc ?
Allocating using nvmet_auth_alloc_transfer_buffer() and freeing using
free() is just make the driver maintenance harder.
Another problem in this patch is that the len in the recv buffer is
le32_to_cpu(req->cmd->auth_receive.al) and in the send is
le32_to_cpu(req->cmd->auth_send.tl) so it's logically wrong to unify
them. Same for error_loc.
maybe just don't include this patch 2/3 in V4 ?
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V3 3/3] nvmet-auth: use correct type for status variable
2023-06-05 9:19 [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup Chaitanya Kulkarni
2023-06-05 9:19 ` [PATCH V3 1/3] nvmet-auth: use common helper to check secp/spsp Chaitanya Kulkarni
2023-06-05 9:19 ` [PATCH V3 2/3] nvmet_auth: use common helper for buffer alloc Chaitanya Kulkarni
@ 2023-06-05 9:19 ` Chaitanya Kulkarni
2023-06-05 22:27 ` Sagi Grimberg
2023-06-05 9:24 ` [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup Chaitanya Kulkarni
3 siblings, 1 reply; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-06-05 9:19 UTC (permalink / raw)
To: hare; +Cc: hch, sagi, linux-nvme, kbusch, Chaitanya Kulkarni
The dhchap_step member of structure nvmet_sq holds the following values:
NVME_AUTH_DHCHAP_FAILURE_FAILED = 0x01,
NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE = 0x02,
NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH = 0x03,
NVME_AUTH_DHCHAP_FAILURE_HASH_UNUSABLE = 0x04,
NVME_AUTH_DHCHAP_FAILURE_DHGROUP_UNUSABLE = 0x05,
NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD = 0x06,
NVME_AUTH_DHCHAP_FAILURE_INCORRECT_MESSAGE = 0x07,
These values can never be negative, hence change int type of
dhchap_step to u16 in the nvmet_sq struct.
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
drivers/nvme/target/nvmet.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 6cf723bc664e..66d8673c3ebf 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -113,7 +113,7 @@ struct nvmet_sq {
struct delayed_work auth_expired_work;
u16 dhchap_tid;
u16 dhchap_status;
- int dhchap_step;
+ u16 dhchap_step;
u8 *dhchap_c1;
u8 *dhchap_c2;
u32 dhchap_s1;
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH V3 3/3] nvmet-auth: use correct type for status variable
2023-06-05 9:19 ` [PATCH V3 3/3] nvmet-auth: use correct type for status variable Chaitanya Kulkarni
@ 2023-06-05 22:27 ` Sagi Grimberg
0 siblings, 0 replies; 11+ messages in thread
From: Sagi Grimberg @ 2023-06-05 22:27 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: hch, linux-nvme, kbusch
On 6/5/23 12:19, Chaitanya Kulkarni wrote:
> The dhchap_step member of structure nvmet_sq holds the following values:
>
> NVME_AUTH_DHCHAP_FAILURE_FAILED = 0x01,
> NVME_AUTH_DHCHAP_FAILURE_NOT_USABLE = 0x02,
> NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH = 0x03,
> NVME_AUTH_DHCHAP_FAILURE_HASH_UNUSABLE = 0x04,
> NVME_AUTH_DHCHAP_FAILURE_DHGROUP_UNUSABLE = 0x05,
> NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD = 0x06,
> NVME_AUTH_DHCHAP_FAILURE_INCORRECT_MESSAGE = 0x07,
No, these are the failure reason codes. You probably meant
NVME_AUTH_DHCHAP_MESSAGE_xxx ?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup
2023-06-05 9:19 [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup Chaitanya Kulkarni
` (2 preceding siblings ...)
2023-06-05 9:19 ` [PATCH V3 3/3] nvmet-auth: use correct type for status variable Chaitanya Kulkarni
@ 2023-06-05 9:24 ` Chaitanya Kulkarni
2023-06-05 22:27 ` Sagi Grimberg
3 siblings, 1 reply; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-06-05 9:24 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: hch, sagi, linux-nvme, kbusch
On 6/5/23 02:19, Chaitanya Kulkarni wrote:
> Hi,
>
> nvmet_execute_auth_send() and nvmet_exeucte_auth_receive() share a lot
> of common functionality such as :-
>
> 1. Checking secp/spsp values and its error handling.
> 2. Initializing transfer buffer len and its error handling.
> 2. Allocating transfer buffer and its error handling.
>
> This code is repeated in both the functions.
>
> Add common helpers with very small restructring of code to remove
> duplication of above functionality in the nvmet_exeucte_auth_receive()
> and nvmet_execute_auth_send(), it also makes code easy to read as both
> the functions are doing substantial work.
>
> Please note that this series is generated on the top of this :-
>
> commit 01cff945c026f1e245ba6401f7df2336ddbae11d (origin/nvme-6.5)
> Author: Chaitanya Kulkarni <kch@nvidia.com>
> Date: Fri May 19 02:40:52 2023 -0700
>
> nvme-fcloop: no need to return from void function
>
> Remove return at the end of void function.
>
> -ck
>
>
Please disregard this, I'll send V4, apologies for noise..
-ck
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup
2023-06-05 9:24 ` [PATCH V3 0/3] nvmet-auth: auth send / receive cleanup Chaitanya Kulkarni
@ 2023-06-05 22:27 ` Sagi Grimberg
0 siblings, 0 replies; 11+ messages in thread
From: Sagi Grimberg @ 2023-06-05 22:27 UTC (permalink / raw)
To: Chaitanya Kulkarni, hare; +Cc: hch, linux-nvme, kbusch
> On 6/5/23 02:19, Chaitanya Kulkarni wrote:
>> Hi,
>>
>> nvmet_execute_auth_send() and nvmet_exeucte_auth_receive() share a lot
>> of common functionality such as :-
>>
>> 1. Checking secp/spsp values and its error handling.
>> 2. Initializing transfer buffer len and its error handling.
>> 2. Allocating transfer buffer and its error handling.
>>
>> This code is repeated in both the functions.
>>
>> Add common helpers with very small restructring of code to remove
>> duplication of above functionality in the nvmet_exeucte_auth_receive()
>> and nvmet_execute_auth_send(), it also makes code easy to read as both
>> the functions are doing substantial work.
>>
>> Please note that this series is generated on the top of this :-
>>
>> commit 01cff945c026f1e245ba6401f7df2336ddbae11d (origin/nvme-6.5)
>> Author: Chaitanya Kulkarni <kch@nvidia.com>
>> Date: Fri May 19 02:40:52 2023 -0700
>>
>> nvme-fcloop: no need to return from void function
>>
>> Remove return at the end of void function.
>>
>> -ck
>>
>>
>
> Please disregard this, I'll send V4, apologies for noise..
:/
^ permalink raw reply [flat|nested] 11+ messages in thread