[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DCKMSNY3N59V.3KQCDWA1VJQNQ@bsdbackstore.eu>
Date: Fri, 05 Sep 2025 07:52:41 +0200
From: "Maurizio Lombardi" <mlombard@...backstore.eu>
To: <alistair23@...il.com>, <chuck.lever@...cle.com>, <hare@...nel.org>,
<kernel-tls-handshake@...ts.linux.dev>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-nvme@...ts.infradead.org>, <linux-nfs@...r.kernel.org>
Cc: <kbusch@...nel.org>, <axboe@...nel.dk>, <hch@....de>,
<sagi@...mberg.me>, <kch@...dia.com>, "Alistair Francis"
<alistair.francis@....com>
Subject: Re: [PATCH v2 7/7] nvmet-tcp: Support KeyUpdate
On Fri Sep 5, 2025 at 4:46 AM CEST, alistair23 wrote:
> From: Alistair Francis <alistair.francis@....com>
>
> If the nvmet_tcp_try_recv() function return EKEYEXPIRED or if we receive
> a KeyUpdate handshake type then the underlying TLS keys need to be
> updated.
>
> If the NVMe Host (TLS client) initiates a KeyUpdate this patch will
> allow the NVMe layer to process the KeyUpdate request and forward the
> request to userspace. Userspace must then update the key to keep the
> connection alive.
>
> This patch allows us to handle the NVMe host sending a KeyUpdate
> request without aborting the connection. At this time we don't support
> initiating a KeyUpdate.
>
> Link: https://datatracker.ietf.org/doc/html/rfc8446#section-4.6.3
> Signed-off-by: Alistair Francis <alistair.francis@....com>
> ---
> v2:
> - Use a helper function for KeyUpdates
> - Ensure keep alive timer is stopped
> - Wait for TLS KeyUpdate to complete
>
> drivers/nvme/target/tcp.c | 90 ++++++++++++++++++++++++++++++++++++---
> 1 file changed, 84 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index bee0355195f5..dd09940e9635 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -175,6 +175,7 @@ struct nvmet_tcp_queue {
>
> /* TLS state */
> key_serial_t tls_pskid;
> + key_serial_t user_session_id;
> struct delayed_work tls_handshake_tmo_work;
>
> unsigned long poll_end;
> @@ -186,6 +187,8 @@ struct nvmet_tcp_queue {
> struct sockaddr_storage sockaddr_peer;
> struct work_struct release_work;
>
> + struct completion tls_complete;
> +
> int idx;
> struct list_head queue_list;
>
> @@ -836,6 +839,11 @@ static int nvmet_tcp_try_send_one(struct nvmet_tcp_queue *queue,
> return 1;
> }
>
> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> +static int nvmet_tcp_try_peek_pdu(struct nvmet_tcp_queue *queue);
> +static void nvmet_tcp_tls_handshake_timeout(struct work_struct *w);
> +#endif
> +
> static int nvmet_tcp_try_send(struct nvmet_tcp_queue *queue,
> int budget, int *sends)
> {
> @@ -844,6 +852,13 @@ static int nvmet_tcp_try_send(struct nvmet_tcp_queue *queue,
> for (i = 0; i < budget; i++) {
> ret = nvmet_tcp_try_send_one(queue, i == budget - 1);
> if (unlikely(ret < 0)) {
> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> + if (ret == -EKEYEXPIRED &&
> + queue->state != NVMET_TCP_Q_DISCONNECTING &&
> + queue->state != NVMET_TCP_Q_TLS_HANDSHAKE) {
> + goto done;
> + }
> +#endif
> nvmet_tcp_socket_error(queue, ret);
> goto done;
> } else if (ret == 0) {
> @@ -1110,11 +1125,52 @@ static inline bool nvmet_tcp_pdu_valid(u8 type)
> return false;
> }
>
> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> +static int update_tls_keys(struct nvmet_tcp_queue *queue)
> +{
> + int ret;
> +
> + cancel_work(&queue->io_work);
> + handshake_req_cancel(queue->sock->sk);
> + handshake_sk_destruct_req(queue->sock->sk);
> + queue->state = NVMET_TCP_Q_TLS_HANDSHAKE;
> +
> + /* Restore the default callbacks before starting upcall */
> + read_lock_bh(&queue->sock->sk->sk_callback_lock);
> + queue->sock->sk->sk_data_ready = queue->data_ready;
> + queue->sock->sk->sk_state_change = queue->state_change;
> + queue->sock->sk->sk_write_space = queue->write_space;
> + queue->sock->sk->sk_user_data = NULL;
Shouldn't "sk_user_data = NULL" be protected by a write lock?
Maurizio
Powered by blists - more mailing lists