On 21/10/2024 14:00, Hannes Reinecke wrote:
On 10/20/24 23:04, Sagi Grimberg wrote:
On 18/10/2024 9:33, Hannes Reinecke wrote:
Add a fabrics option 'concat' to request secure channel concatenation.
When secure channel concatenation is enabled a 'generated PSK' is
inserted
into the keyring such that it's available after reset.
Signed-off-by: Hannes Reinecke <hare@xxxxxxxxxx>
---
drivers/nvme/host/auth.c | 108
+++++++++++++++++++++++++++++++++++-
drivers/nvme/host/fabrics.c | 34 +++++++++++-
drivers/nvme/host/fabrics.h | 3 +
drivers/nvme/host/nvme.h | 2 +
drivers/nvme/host/sysfs.c | 4 +-
drivers/nvme/host/tcp.c | 47 ++++++++++++++--
include/linux/nvme.h | 7 +++
7 files changed, 191 insertions(+), 14 deletions(-)
[ .. ]
@@ -2314,6 +2345,8 @@ static void
nvme_tcp_error_recovery_work(struct work_struct *work)
struct nvme_tcp_ctrl, err_work);
struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl;
+ if (nvme_tcp_key_revoke_needed(ctrl))
+ nvme_auth_revoke_tls_key(ctrl);
Having this sprayed in various places in the code is really confusing.
Can you please add a small comment on each call-site? just for our
future selves
reading this code?
Outside of that, patch looks good.
Weelll ...
We need to reset the negotiated PSK exactly in three places:
- reset
- error recovery
- teardown
Much like we need to do for every other queue-related resource.
And in one of your previous reviews you stated that you do _not_
want to have 'nvme_auth_revoke_tls_key()' checking if the key
needs to be revoked, but rather have a check function.
Sounds right.
Otherwise I could just move the check into nvme_auth_revoke_tls_key()'
and drop the 'revoke needed' call.
I prefer that we don't
Furthermore we don't need to check if the key needs to be revoked
during teardown (answer will always be 'yes').
won't it also be revoked in setup?
So I'm quite unsure what to do now ... document that we need
to release the key when doing a reset or error recovery?
Move the check back into nvme_auth_tls_revoke_key()?
Hmm?
Just a little documentation to why we like to revoke the key so many
times ;)
