mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] nvme-tcp: fix request PDU size when header digest is enabled
@ 2026-10-27 19:47 Christian Aurich Zanettini Martins
  2026-10-27 23:04 ` Keith Busch
  0 siblings, 1 reply; 2+ messages in thread
From: Christian Aurich Zanettini Martins @ 2026-10-27 19:47 UTC (permalink / raw)
  To: linux-nvme
  Cc: kbusch, axboe, hch, sagi, hare, sgogte, wagi, linux-kernel,
	Christian Aurich Zanettini Martins

Since commit 61b0ed3be81b ("nvme-tcp: parallelize I/O queue allocation
and startup") the I/O tag set is allocated before the I/O queues.
nvme_tcp_init_request() sizes the PDU with nvme_tcp_hdgst_len(queue),
but queue->hdr_digest is only set later by nvme_tcp_alloc_queue(), so
every I/O PDU is allocated without room for the header digest.

With hdr_digest enabled, the digest is then written 4 bytes past the
PDU, over the header of the next PDU in the page_frag cache, and the
target drops the connection on the first I/O:

  nvmet_tcp: unexpected pdu type 159
  nvmet: ctrl 1 fatal error occurred!

Queues that a later reconnect adds for the first time have the same
problem.

Set queue->hdr_digest when allocating the controller, together with
pf_cache_lock, so it is valid before any request is initialized.

Fixes: 61b0ed3be81b ("nvme-tcp: parallelize I/O queue allocation and startup")
Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
---
Changes in v2:
- Set queue->hdr_digest when allocating the controller, next to the
  pf_cache_lock init, instead of reading the option in
  nvme_tcp_init_request() (Keith)

v1: https://lore.kernel.org/linux-nvme/20261026184621.29276-1-christian.aurichzm@gmail.com/

Based on nvme-7.4 (aaedcff90148); needs a504cd79b167 from there.

Tested in QEMU with nvmet-tcp over loopback and parallel I/O: with
hdr_digest the connection failed on the first I/O before the fix; after
it, no errors and the data matched, also with data_digest and with a
reconnect growing the queues from 2 to 4 (attr_qid_max).

 drivers/nvme/host/tcp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index f876375dffd2..2209586f3809 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1948,7 +1948,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
 		}
 	}
 
-	queue->hdr_digest = nctrl->opts->hdr_digest;
 	queue->data_digest = nctrl->opts->data_digest;
 
 	rcv_pdu_size = sizeof(struct nvme_tcp_rsp_pdu) +
@@ -3098,10 +3097,13 @@ static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
 	/*
 	 * The I/O tag set is allocated before the I/O queues, and initializing
 	 * its requests already allocates the PDUs from the page frag caches of
-	 * the queues, so initialize their locks here.
+	 * the queues, sized by their header digest setting, so set up the
+	 * locks and hdr_digest here.
 	 */
-	for (i = 0; i < ctrl->ctrl.queue_count; i++)
+	for (i = 0; i < ctrl->ctrl.queue_count; i++) {
 		mutex_init(&ctrl->queues[i].pf_cache_lock);
+		ctrl->queues[i].hdr_digest = opts->hdr_digest;
+	}
 
 	ctrl->net = get_net(current->nsproxy->net_ns);
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] nvme-tcp: fix request PDU size when header digest is enabled
  2026-10-27 19:47 [PATCH v2] nvme-tcp: fix request PDU size when header digest is enabled Christian Aurich Zanettini Martins
@ 2026-10-27 23:04 ` Keith Busch
  0 siblings, 0 replies; 2+ messages in thread
From: Keith Busch @ 2026-10-27 23:04 UTC (permalink / raw)
  To: Christian Aurich Zanettini Martins
  Cc: linux-nvme, axboe, hch, sagi, hare, sgogte, wagi, linux-kernel

On Tue, Oct 27, 2026 at 04:47:10PM -0300, Christian Aurich Zanettini Martins wrote:
> Since commit 61b0ed3be81b ("nvme-tcp: parallelize I/O queue allocation
> and startup") the I/O tag set is allocated before the I/O queues.
> nvme_tcp_init_request() sizes the PDU with nvme_tcp_hdgst_len(queue),
> but queue->hdr_digest is only set later by nvme_tcp_alloc_queue(), so
> every I/O PDU is allocated without room for the header digest.
> 
> With hdr_digest enabled, the digest is then written 4 bytes past the
> PDU, over the header of the next PDU in the page_frag cache, and the
> target drops the connection on the first I/O:
> 
>   nvmet_tcp: unexpected pdu type 159
>   nvmet: ctrl 1 fatal error occurred!
> 
> Queues that a later reconnect adds for the first time have the same
> problem.
> 
> Set queue->hdr_digest when allocating the controller, together with
> pf_cache_lock, so it is valid before any request is initialized.

Thanks, applied to nvme-7.4.

> @@ -1948,7 +1948,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
>  		}
>  	}
>  
> -	queue->hdr_digest = nctrl->opts->hdr_digest;
>  	queue->data_digest = nctrl->opts->data_digest;
>  
>  	rcv_pdu_size = sizeof(struct nvme_tcp_rsp_pdu) +
> @@ -3098,10 +3097,13 @@ static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
>  	/*
>  	 * The I/O tag set is allocated before the I/O queues, and initializing
>  	 * its requests already allocates the PDUs from the page frag caches of
> -	 * the queues, so initialize their locks here.
> +	 * the queues, sized by their header digest setting, so set up the
> +	 * locks and hdr_digest here.
>  	 */
> -	for (i = 0; i < ctrl->ctrl.queue_count; i++)
> +	for (i = 0; i < ctrl->ctrl.queue_count; i++) {
>  		mutex_init(&ctrl->queues[i].pf_cache_lock);
> +		ctrl->queues[i].hdr_digest = opts->hdr_digest;
> +	}

A follow on bonus patch can clean this up a bit by introducing a helper
function, maybe nvme_tcp_init_queue(), and initialize all the basic
fields there.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-27 23:04 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-27 19:47 [PATCH v2] nvme-tcp: fix request PDU size when header digest is enabled Christian Aurich Zanettini Martins
2026-10-27 23:04 ` Keith Busch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®