* [PATCH v2] nvmet-tcp: report a bounded MDTS instead of "no limit" [not found] <20260828222356.1264-1-gerencia@idkmanager.com> @ 2026-08-29 1:39 ` Alfonso Kuen 2026-08-30 21:00 ` Sagi Grimberg 2026-08-31 0:51 ` [PATCH v3] " Alfonso Kuen 0 siblings, 2 replies; 5+ messages in thread From: Alfonso Kuen @ 2026-08-29 1:39 UTC (permalink / raw) To: hch, sagi, kch; +Cc: linux-nvme, linux-kernel, Alfonso Kuen 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. 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 <gerencia@idkmanager.com> --- 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) -- 2.47.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nvmet-tcp: report a bounded MDTS instead of "no limit" 2026-08-29 1:39 ` [PATCH v2] nvmet-tcp: report a bounded MDTS instead of "no limit" Alfonso Kuen @ 2026-08-30 21:00 ` Sagi Grimberg 2026-08-30 21:02 ` Sagi Grimberg 2026-08-31 0:51 ` [PATCH v3] " Alfonso Kuen 1 sibling, 1 reply; 5+ messages in thread From: Sagi Grimberg @ 2026-08-30 21:00 UTC (permalink / raw) To: Alfonso Kuen, hch, kch; +Cc: linux-nvme, linux-kernel 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 <gerencia@idkmanager.com> > --- > 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) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nvmet-tcp: report a bounded MDTS instead of "no limit" 2026-08-30 21:00 ` Sagi Grimberg @ 2026-08-30 21:02 ` Sagi Grimberg 0 siblings, 0 replies; 5+ messages in thread From: Sagi Grimberg @ 2026-08-30 21:02 UTC (permalink / raw) To: Alfonso Kuen, hch, kch; +Cc: linux-nvme, linux-kernel On 31/08/2026 0:00, Sagi Grimberg wrote: > > > 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: Commit ec6d20e16c2d ("nvmet-rdma: Implement get_mdts controller op") left the other transports untouched... Other than that looks good Reviewed-by: Sagi Grimberg <sagi@grimberg.me> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3] nvmet-tcp: report a bounded MDTS instead of "no limit" 2026-08-29 1:39 ` [PATCH v2] nvmet-tcp: report a bounded MDTS instead of "no limit" Alfonso Kuen 2026-08-30 21:00 ` Sagi Grimberg @ 2026-08-31 0:51 ` Alfonso Kuen 2026-09-01 14:06 ` Maurizio Lombardi 1 sibling, 1 reply; 5+ messages in thread From: Alfonso Kuen @ 2026-08-31 0:51 UTC (permalink / raw) To: hch, sagi, kch; +Cc: linux-nvme, linux-kernel, Alfonso Kuen 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 commit 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. Commit ec6d20e16c2d ("nvmet-rdma: Implement get_mdts controller op") left the other transports untouched. 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 <gerencia@idkmanager.com> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> --- Changes in v3: - Cite the RDMA change as commit ec6d20e16c2d ("nvmet-rdma: Implement get_mdts controller op") rather than by patch title, per Sagi's review. - Pick up Sagi's Reviewed-by. - No functional change: the diff is byte-identical to v1 and v2. 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) -- 2.47.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] nvmet-tcp: report a bounded MDTS instead of "no limit" 2026-08-31 0:51 ` [PATCH v3] " Alfonso Kuen @ 2026-09-01 14:06 ` Maurizio Lombardi 0 siblings, 0 replies; 5+ messages in thread From: Maurizio Lombardi @ 2026-09-01 14:06 UTC (permalink / raw) To: Alfonso Kuen, hch, sagi, kch; +Cc: linux-nvme, linux-kernel On Mon Aug 31, 2026 at 2:51 AM CEST, 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 commit 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. #define NVMET_TCP_MAXH2CDATA 0x400000 /* 16M arbitrary limit */ So the comment should be fixed, it says NVMET_TCP_MAXH2CDATA is 16M. Also, it's not entirely clear to me how the target enforces this. Suppose an host sends a 2 MiB command, violating the MDTS setting, what happens? Maurizio ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-01 14:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20260828222356.1264-1-gerencia@idkmanager.com>
2026-08-29 1:39 ` [PATCH v2] nvmet-tcp: report a bounded MDTS instead of "no limit" Alfonso Kuen
2026-08-30 21:00 ` Sagi Grimberg
2026-08-30 21:02 ` Sagi Grimberg
2026-08-31 0:51 ` [PATCH v3] " Alfonso Kuen
2026-09-01 14:06 ` Maurizio Lombardi
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®