* [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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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
2026-09-19 3:00 ` Ing. Alfonso Kuen Arroyo
0 siblings, 1 reply; 9+ 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] 9+ messages in thread
* Re: [PATCH v3] nvmet-tcp: report a bounded MDTS instead of "no limit"
2026-09-01 14:06 ` Maurizio Lombardi
@ 2026-09-19 3:00 ` Ing. Alfonso Kuen Arroyo
2026-09-19 7:53 ` Maurizio Lombardi
0 siblings, 1 reply; 9+ messages in thread
From: Ing. Alfonso Kuen Arroyo @ 2026-09-19 3:00 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: hch, sagi, kch, linux-nvme, linux-kernel
On Tue Sep 01, 2026, Maurizio Lombardi wrote:
> #define NVMET_TCP_MAXH2CDATA 0x400000 /* 16M arbitrary limit */
>
> So the comment should be fixed, it says NVMET_TCP_MAXH2CDATA is 16M.
Agreed - the value is 4 MiB and the comment is stale; it predates this
patch. I'll send a separate one-line patch for the comment rather than
fold an unrelated change into this one.
> 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?
It doesn't enforce it, and this patch doesn't add enforcement. MDTS is a
limit the host is required to honour (the base spec says the host shall
not submit a command exceeding it), and nvmet has no per-command check
against it, neither in the core nor in the transports.
What the target does have is the hard limit in nvmet_tcp_map_data():
since 4a3f00262a04 a data length above NVMET_TCP_MAXH2CDATA fails with
NVME_SC_SGL_INVALID_DATA | DNR before any buffer is allocated. So today
there are two numbers: the one the target will actually accept (4 MiB)
and the one it advertises (none). The patch makes them the same number,
so a compliant host never builds the command that the second check would
reject. With MDTS = 4 MiB, a 2 MiB command is within the limit and is
served normally; a command above the limit from a host that ignores MDTS
is rejected by the existing map_data check exactly as it is now - only
with a reason the host could have known in advance.
If you'd rather see the advertised limit derived from
NVMET_TCP_MAXH2CDATA directly (so the two cannot drift) instead of the
constant I used, I'm happy to respin as v4 that way.
Alfonso
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] nvmet-tcp: report a bounded MDTS instead of "no limit"
2026-09-19 3:00 ` Ing. Alfonso Kuen Arroyo
@ 2026-09-19 7:53 ` Maurizio Lombardi
2026-09-19 7:57 ` Maurizio Lombardi
0 siblings, 1 reply; 9+ messages in thread
From: Maurizio Lombardi @ 2026-09-19 7:53 UTC (permalink / raw)
To: Ing. Alfonso Kuen Arroyo, Maurizio Lombardi
Cc: hch, sagi, kch, linux-nvme, linux-kernel
On Sat Sep 19, 2026 at 5:00 AM CEST, Ing. Alfonso Kuen Arroyo wrote:
> On Tue Sep 01, 2026, Maurizio Lombardi wrote:
>> #define NVMET_TCP_MAXH2CDATA 0x400000 /* 16M arbitrary limit */
>>
>> So the comment should be fixed, it says NVMET_TCP_MAXH2CDATA is 16M.
>
> Agreed - the value is 4 MiB and the comment is stale; it predates this
> patch. I'll send a separate one-line patch for the comment rather than
> fold an unrelated change into this one.
>
>> 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?
>
> It doesn't enforce it, and this patch doesn't add enforcement. MDTS is a
> limit the host is required to honour (the base spec says the host shall
> not submit a command exceeding it), and nvmet has no per-command check
> against it, neither in the core nor in the transports.
And indeed this is what's missing, because the specification explicitely
says that "if a command is submitted that exceeds [MDTS], then the
command is aborted with a status code of Invalid Field in Command."
(Identify – Identify Controller Data Structure, I/O Command Set
Independent)
Maybe it can be added separatedly with a dedicated patch.
>
> What the target does have is the hard limit in nvmet_tcp_map_data():
> since 4a3f00262a04 a data length above NVMET_TCP_MAXH2CDATA fails with
> NVME_SC_SGL_INVALID_DATA | DNR before any buffer is allocated. So today
> there are two numbers: the one the target will actually accept (4 MiB)
> and the one it advertises (none). The patch makes them the same number,
> so a compliant host never builds the command that the second check would
> reject. With MDTS = 4 MiB, a 2 MiB command is within the limit and is
> served normally;
Yes, but your patch sets NVMET_RDMA_MAX_MDTS to 8, which means 1MiB, not
4MiB, so a 2MiB command should be rejected, even if it's under the MAXH2CDATA
limit of 4MiB; or am I missing something?
Maurizio
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] nvmet-tcp: report a bounded MDTS instead of "no limit"
2026-09-19 7:53 ` Maurizio Lombardi
@ 2026-09-19 7:57 ` Maurizio Lombardi
2026-09-19 14:39 ` Ing. Alfonso Kuen Arroyo
0 siblings, 1 reply; 9+ messages in thread
From: Maurizio Lombardi @ 2026-09-19 7:57 UTC (permalink / raw)
To: Maurizio Lombardi, Ing. Alfonso Kuen Arroyo
Cc: hch, sagi, kch, linux-nvme, linux-kernel
On Sat Sep 19, 2026 at 9:53 AM CEST, Maurizio Lombardi wrote:
>
> Yes, but your patch sets NVMET_RDMA_MAX_MDTS to 8, which means 1MiB, not
> 4MiB, so a 2MiB command should be rejected, even if it's under the MAXH2CDATA
> limit of 4MiB; or am I missing something?
>
Typo, I meant NVMET_TCP_MAX_MDTS, not NVMET_RDMA_MAX_MDTS.
Maurizio
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] nvmet-tcp: report a bounded MDTS instead of "no limit"
2026-09-19 7:57 ` Maurizio Lombardi
@ 2026-09-19 14:39 ` Ing. Alfonso Kuen Arroyo
0 siblings, 0 replies; 9+ messages in thread
From: Ing. Alfonso Kuen Arroyo @ 2026-09-19 14:39 UTC (permalink / raw)
To: Maurizio Lombardi
Cc: Maurizio Lombardi, hch, sagi, kch, linux-nvme, linux-kernel
On Sat Sep 19, 2026, Maurizio Lombardi wrote:
> Yes, but your patch sets NVMET_TCP_MAX_MDTS to 8, which means 1MiB, not
> 4MiB, so a 2MiB command should be rejected, even if it's under the
> MAXH2CDATA limit of 4MiB; or am I missing something?
You are not missing anything - I mixed the two numbers up in my previous
mail. The patch advertises MDTS = 8, i.e. 2^8 * 4 KiB = 1 MiB, the same
value nvmet-rdma reports, not the 4 MiB of NVMET_TCP_MAXH2CDATA. So with
the patch a 2 MiB command does exceed the advertised MDTS, and today the
target would still serve it: nothing in nvmet compares a command's
transfer length against MDTS, the only rejection is the 4 MiB bound in
nvmet_tcp_map_data(). Sorry for the confusion.
> And indeed this is what's missing, because the specification
> explicitely says that "if a command is submitted that exceeds [MDTS],
> then the command is aborted with a status code of Invalid Field in
> Command."
>
> Maybe it can be added separatedly with a dedicated patch.
Agreed on both counts. The enforcement belongs in the core rather than in
a transport - a transfer-length check against nvmet_ctrl_mdts() when the
request is initialised, returning NVME_SC_INVALID_FIELD | NVME_STATUS_DNR -
so that every transport with a get_mdts gets it for free. It is a separate
change with its own blast radius (it turns a currently-served oversized
command into a failed one for non-compliant hosts), so I would rather keep
this patch as the "advertise what we can serve" step and send the
enforcement as a follow-up on top of it. I can prepare that follow-up if
you and the maintainers think it is the right direction; otherwise I am
happy to leave it to whoever prefers to shape it.
Alfonso
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-19 14:39 UTC | newest]
Thread overview: 9+ 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
2026-09-19 3:00 ` Ing. Alfonso Kuen Arroyo
2026-09-19 7:53 ` Maurizio Lombardi
2026-09-19 7:57 ` Maurizio Lombardi
2026-09-19 14:39 ` Ing. Alfonso Kuen Arroyo
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®