Re: [PATCH v2] nvmet-tcp: report a bounded MDTS instead of "no limit"

On 29/08/2026 4:39, Alfonso Kuen wrote:
> nvmet-tcp does not implement .get_mdts, so nvmet_ctrl_mdts() falls back
> to the port value (0 by default) and identify-controller advertises "no
> maximum data transfer size". The initiator believes it:
> nvme_init_ctrl_finish() sets max_hw_sectors to UINT_MAX.
>
> What the initiator then issues is decided by the block layer. The
> generic cap is 4 MiB (BLK_DEF_MAX_SECTORS_CAP), but a namespace that
> advertises a large NOWS raises it: blk_validate_limits() takes
> max_sectors from io_opt once io_opt exceeds that cap. On the array we
> measured, NOWS is 65535, so io_opt is 32 MiB and the initiator emitted
> 32 MiB commands -- from a controller that advertised no MDTS at all.
>
> The target cannot serve those, in one of two ways depending on the
> kernel.
>
> Since 4a3f00262a04 ("nvmet-tcp: bound SGL data length before allocating
> command buffers"), nvmet_tcp_map_data() rejects any len above
> NVMET_TCP_MAXH2CDATA (4 MiB) with NVME_SC_SGL_INVALID_DATA | DNR, so the
> command fails hard and deterministically. That is the right response to
> an oversized command, but the initiator had no way to avoid sending it:
> nothing advertised the limit it was exceeding.
>
> Before that commit the command reached sgl_alloc(), which for 32 MiB
> needs 8192 scatterlist entries -- 256 KiB, an order-6 kmalloc. Under
> fragmentation that fails, and it surfaces as NVME_SC_INTERNAL, a generic
> status, so NVMe multipath does not fail the command over to another
> path. A 1 MiB command needs 256 entries, 8 KiB, served from the
> kmalloc-8k slab, which does not fail. That failure mode is intermittent
> and load-dependent rather than deterministic; on a vendor kernel
> predating the bound it cost us roughly 98 GiB of a 2 TiB image copy,
> with the copy tool exiting 0.
>
> nvmet-rdma has advertised a bounded MDTS since "nvmet-rdma: Implement
> get_mdts controller op" (Max Gurtovoy, Mar 2020), which left the other
> transports untouched.

Nit - instead of this patch reference - you should simply say:

>   Do the same for TCP, using the same 1 MiB value,
> so initiators size their commands to something the target accepts
> instead of discovering the limit by failing.
>
> Signed-off-by: Alfonso Kuen <[email protected]>
> ---
> Changes in v2:
>   - Rewrite the rationale. v1 described the sgl_alloc() order-6 failure as
>     current behaviour; since 4a3f00262a04 that path is unreachable and an
>     oversized command is rejected earlier with SGL_INVALID_DATA | DNR. Both
>     failure modes are now described, and which kernels see which.
>   - v1 attributed the 32 MiB command size to a "generic 32 MiB ceiling" in
>     the block layer. The generic cap is 4 MiB; the 32 MiB came from the
>     target's own NOWS raising io_opt. Corrected -- and it sharpens the point,
>     since the same controller advertises an optimal I/O size it cannot serve.
>   - v1 called the 1 MiB SGL an order-0 allocation; it is 8 KiB.
>   - No functional change: the diff is byte-identical to v1.
>   drivers/nvme/target/tcp.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index e4f603b2a..414ba896e 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -23,6 +23,9 @@
>   #include "nvmet.h"
>   
>   #define NVMET_TCP_DEF_INLINE_DATA_SIZE	(4 * PAGE_SIZE)
> +
> +/* Assume mpsmin == device_page_size == 4KB */
> +#define NVMET_TCP_MAX_MDTS		8
>   #define NVMET_TCP_MAXH2CDATA		0x400000 /* 16M arbitrary limit */
>   #define NVMET_TCP_BACKLOG 128
>   
> @@ -2243,6 +2246,11 @@ static ssize_t nvmet_tcp_host_port_addr(struct nvmet_ctrl *ctrl,
>   			(struct sockaddr *)&queue->sockaddr_peer);
>   }
>   
> +static u8 nvmet_tcp_get_mdts(const struct nvmet_ctrl *ctrl)
> +{
> +	return NVMET_TCP_MAX_MDTS;
> +}
> +
>   static const struct nvmet_fabrics_ops nvmet_tcp_ops = {
>   	.owner			= THIS_MODULE,
>   	.type			= NVMF_TRTYPE_TCP,
> @@ -2254,6 +2262,7 @@ static const struct nvmet_fabrics_ops nvmet_tcp_ops = {
>   	.install_queue		= nvmet_tcp_install_queue,
>   	.disc_traddr		= nvmet_tcp_disc_port_addr,
>   	.host_traddr		= nvmet_tcp_host_port_addr,
> +	.get_mdts		= nvmet_tcp_get_mdts,
>   };
>   
>   static int __init nvmet_tcp_init(void)