From: Hannes Reinecke <hare@suse.de>
To: Sagi Grimberg <sagi@grimberg.me>,
Hannes Reinecke <hare@kernel.org>, Christoph Hellwig <hch@lst.de>
Cc: Keith Busch <kbusch@kernel.org>, linux-nvme@lists.infradead.org
Subject: Re: [PATCH] nvme-tcp: strict pdu pacing to avoid send stalls on TLS
Date: Thu, 18 Apr 2024 11:05:54 +0200 [thread overview]
Message-ID: <0c9c08ba-e03d-4d9d-87ab-fef04ef10a65@suse.de> (raw)
In-Reply-To: <8a52ce1e-3cdb-468a-87a5-3cf8a118cad6@grimberg.me>
On 4/18/24 10:01, Sagi Grimberg wrote:
>
>
> On 17/04/2024 18:39, Hannes Reinecke wrote:
>> TLS requires a strict pdu pacing via MSG_EOR to signal the end
>> of a record and subsequent encryption. If we do not set MSG_EOR
>> at the end of a sequence the record won't be closed, encryption
>> doesn't start, and we end up with a send stall as the message
>> will never be passed on to the TCP layer.
>> So do not check for the queue status when figuring out whether
>> MSG_MORE should be set but rather make it dependent on the current
>> command only.
>
> How about making nvme_tcp_queue_more take into account nvme_tcp_tls()?
> so we preserve the behavior without tls.
>
> i.e. something like:
> --
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 0ba62fc647b3..bbffc67f8a1e 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -360,12 +360,18 @@ static inline void nvme_tcp_send_all(struct
> nvme_tcp_queue *queue)
> } while (ret > 0);
> }
>
> -static inline bool nvme_tcp_queue_more(struct nvme_tcp_queue *queue)
> +static inline bool nvme_tcp_queue_has_pending(struct nvme_tcp_queue
> *queue)
> {
> return !list_empty(&queue->send_list) ||
> !llist_empty(&queue->req_list);
> }
>
> +static inline bool nvme_tcp_queue_more(struct nvme_tcp_queue *queue)
> +{
> + return !nvme_tcp_tls(queue->ctrl) &&
> + nvme_tcp_queue_has_pending(queue);
> +}
> +
> static inline void nvme_tcp_queue_request(struct nvme_tcp_request *req,
> bool sync, bool last)
> {
> @@ -386,7 +392,7 @@ static inline void nvme_tcp_queue_request(struct
> nvme_tcp_request *req,
> mutex_unlock(&queue->send_mutex);
> }
>
> - if (last && nvme_tcp_queue_more(queue))
> + if (last && nvme_tcp_queue_has_pending(queue))
> queue_work_on(queue->io_cpu, nvme_tcp_wq,
> &queue->io_work);
> }
> --
>
Would work as well, I guess.
I'll give it a go.
Cheers,
Hannes
prev parent reply other threads:[~2024-04-18 9:06 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-17 15:39 [PATCH] nvme-tcp: strict pdu pacing to avoid send stalls on TLS Hannes Reinecke
2024-04-18 8:01 ` Sagi Grimberg
2024-04-18 9:05 ` Hannes Reinecke [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0c9c08ba-e03d-4d9d-87ab-fef04ef10a65@suse.de \
--to=hare@suse.de \
--cc=hare@kernel.org \
--cc=hch@lst.de \
--cc=kbusch@kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=sagi@grimberg.me \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).