[PATCH net-next v3 0/9] net: devmem: size TX net_iovs from the dmabuf layout

[PATCH net-next v3 0/9] net: devmem: size TX net_iovs from the dmabuf layout

[Stanislav Fomichev]

TX bindings use PAGE_SIZE net_iovs, so a frag covers at most a page.
A 64KB skb needs 16 frags, and MAX_SKB_FRAGS caps devmem skbs at about
68KB, which makes BIG TCP ineffective for devmem TX.

This series picks the TX net_iov size from the dmabuf sg table instead:
the largest power of 2, up to 2MB, which aligns every DMA segment. With
a 2MB hugepage-backed udmabuf a BIG TCP skb takes a few frags. There's
no UAPI change since TX addresses are dmabuf offsets.

TCP still bounds skbs by size_goal, so frags get larger, skbs don't.
Devmem TX frags are capped by a new per-device limit, PAGE_SIZE by
default, so drivers opt in to larger frags:
- mlx5: no limit, 32-bit length per data segment
- bnxt, fbnic: 32KB, 16-bit TX descriptor length
- gve keeps PAGE_SIZE frags

Tested on two mlx5 400Gb/s hosts with kperf (udmabuf, 32KB RX
net_iovs, 2MB hugepage-backed TX buffer), series backported to a 7.1
kernel on the sender only. 1 flow, MTU 4200, receiver tcp_rmem capped
at 16MB so it doesn't drop, median of 5 x 60s runs:

                              before    after
  frags per skb (sender)       16-17      1-2
  throughput (Gb/s)            208.6    211.2
  sender sendmsg CPU (sys)     40.8%    19.5%   (-52%)

The receiver's CPU is the bottleneck, so the gain is CPU, not
throughput. More of the transmit runs from the ACK path, so the
sender's softirq grows (52% -> 59%), but sys + sirq still drops by 16%.

kperf output (sender side) of runs near the median, before:

  client: == Source <sender>
  client:   Tx213.826 Gbps (1603901849600 bytes in 60007723 usec)
  client:   Rx  0.000 Gbps (0 bytes in 60007723 usec)
  client:   TCP retrans reord rtt rttvar d_ce snd_wnd cwnd
  client:             0     0 633      2    0 15556608 7657
  client:   net CPU 49: usr: 0.01% sys: 0.10% idle:44.13% iow: 0.00% irq: 3.21% sirq:52.53%
  client:   app CPU 50: usr: 0.05% sys:41.55% idle:53.88% iow: 0.00% irq: 1.40% sirq: 3.10%

after:

  client: == Source <sender>
  client:   Tx211.242 Gbps (1584440279040 bytes in 60004741 usec)
  client:   Rx  0.000 Gbps (0 bytes in 60004741 usec)
  client:   TCP retrans reord rtt rttvar d_ce snd_wnd cwnd
  client:             0     0 556      3    0 15499264 7650
  client:   net CPU 13: usr: 0.01% sys: 0.08% idle:37.75% iow: 0.00% irq: 3.87% sirq:58.26%
  client:   app CPU 14: usr: 0.06% sys:19.48% idle:77.23% iow: 0.00% irq: 1.19% sirq: 2.02%

The bnxt patches (1 and 5) were tested on two 50Gb/s bnxt hosts with
CONFIG_MAX_SKB_FRAGS=45 (with 17 the check in patch 1 compiles out).
kperf devmem TX from a 2MB hugepage-backed udmabuf runs at line rate
(46.4Gb/s, payload validated) with 2-3 frags per skb, none above 32KB.
ncdevmem sending 1KB chunks builds skbs with up to 45 frags, the stack
segments them and the data arrives intact. The fbnic patch is
build-tested only.

Patches:
- 1: bnxt: segment GSO skbs with too many frags in the stack
- 2-3: devmem TX prep (truesize, niov_shift in the TX lookup)
- 4-7: per-device netmem TX frag size limit, PAGE_SIZE by default,
  opt-ins for bnxt, fbnic and mlx5
- 8: pick the TX net_iov size from the sg table
- 9: selftest for TX from a hugepage-backed dmabuf

v3:
- default the netmem TX frag size limit to PAGE_SIZE, opt in bnxt,
  fbnic and mlx5 (Mina)

v2: https://lore.kernel.org/netdev/20261103235152.1904727-1-sdf@fomichev.me/
- reword the TX truesize commit message (Mina)
- explain why the TX net_iov size ignores the device limit (Mina)
- use 1 flow results in the cover letter (Mina)

v1: https://lore.kernel.org/netdev/20261102191438.1059771-1-sdf@fomichev.me/

Stanislav Fomichev (9):
  bnxt_en: let the stack segment GSO skbs with too many frags
  net: devmem: charge the frag length to TX skb truesize
  net: devmem: use niov_shift in net_devmem_get_niov_at()
  net: devmem: add a per-device netmem TX frag size limit
  bnxt_en: set the netmem TX frag size limit to 32KB
  eth: fbnic: set the netmem TX frag size limit to 32KB
  net/mlx5e: lift the netmem TX frag size limit
  net: devmem: size TX net_iovs from the dmabuf sg table
  selftests: drv-net: devmem: TX from a hugepage-backed dmabuf

 .../networking/net_cachelines/net_device.rst  |  1 +
 Documentation/networking/netmem.rst           |  7 +++
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  6 +++
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  1 +
 .../net/ethernet/meta/fbnic/fbnic_netdev.c    |  1 +
 include/linux/netdevice.h                     |  2 +
 net/core/datagram.c                           |  3 +-
 net/core/dev.c                                |  1 +
 net/core/devmem.c                             | 37 +++++++++++++---
 net/core/devmem.h                             |  1 +
 net/core/netdev-genl.c                        |  2 +-
 .../selftests/drivers/net/hw/devmem.py        | 10 ++++-
 .../selftests/drivers/net/hw/devmem_lib.py    | 43 +++++++++++++++++--
 .../selftests/drivers/net/hw/ncdevmem.c       |  8 +++-
 .../selftests/drivers/net/hw/nk_devmem.py     | 11 ++++-
 15 files changed, 118 insertions(+), 16 deletions(-)

-- 
2.53.0-Meta

[PATCH net-next v3 1/9] bnxt_en: let the stack segment GSO skbs with too many frags

[Stanislav Fomichev]

With MAX_SKB_FRAGS > TX_MAX_FRAGS, the xmit path linearizes skbs with
more frags than a packet can have BDs. For GSO skbs, linearizing needs a
large allocation, can produce a head which doesn't fit in a BD with BIG
TCP, and always fails for unreadable skbs. Clear the GSO features of
such skbs in ndo_features_check instead, so that the stack segments
them. Non-GSO skbs are still linearized.

Suggested-by: Eric Dumazet <edumazet@google.com>
Link: https://lore.kernel.org/all/CANn89i+ps_CozfOyRnKav0_LUmSekJ9ExB5JkDbTAVf_ss_98g@mail.gmail.com/
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 757c7266ce15..e7a4190a8c78 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -14300,6 +14300,11 @@ static netdev_features_t bnxt_features_check(struct sk_buff *skb,
 				skb_shinfo(skb)->gso_size))
 		features &= ~NETIF_F_GSO_MASK;
 
+#if (MAX_SKB_FRAGS > TX_MAX_FRAGS)
+	if (skb_shinfo(skb)->nr_frags > TX_MAX_FRAGS)
+		features &= ~NETIF_F_GSO_MASK;
+#endif
+
 	switch (vlan_get_protocol(skb)) {
 	case htons(ETH_P_IP):
 		if (!skb->encapsulation)
-- 
2.53.0-Meta

Re: [PATCH net-next v3 1/9] bnxt_en: let the stack segment GSO skbs with too many frags

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch fixes real functional bugs in existing bnxt code, but
  it has no Fixes: tag and is not routed to stable.
- [Low] Clearing NETIF_F_GSO_MASK in bnxt_features_check() makes the
  stack segment a GSO skb with nr_frags > TX_MAX_FRAGS.

--

bnxt_en: let the stack segment GSO skbs with too many frags

When MAX_SKB_FRAGS > TX_MAX_FRAGS, this patch clears the GSO features in
bnxt_features_check() for skbs that have more than TX_MAX_FRAGS frags.
The stack then segments those skbs, and bnxt_start_xmit() no longer
linearizes them. Non-GSO skbs with too many frags are still linearized in
the xmit path.

Link: https://lore.kernel.org/all/CANn89i+ps_CozfOyRnKav0_LUmSekJ9ExB5JkDbTAVf_ss_98g@mail.gmail.com/

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 33c4cacc8ffe3..f20b61180ab2d 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -14320,6 +14320,11 @@ static netdev_features_t bnxt_features_check(struct sk_buff *skb,
>  				skb_shinfo(skb)->gso_size))
>  		features &= ~NETIF_F_GSO_MASK;
>  
> +#if (MAX_SKB_FRAGS > TX_MAX_FRAGS)
> +	if (skb_shinfo(skb)->nr_frags > TX_MAX_FRAGS)
> +		features &= ~NETIF_F_GSO_MASK;
> +#endif

[Severity: Low]
Should this patch have a Fixes: tag?

Commit b91e82129400 ("bnxt_en: Linearize TX SKB if the fragments exceed
the max") added the unconditional linearize in bnxt_start_xmit():

bnxt_start_xmit() {
    ...
#if (MAX_SKB_FRAGS > TX_MAX_FRAGS)
	if (skb_shinfo(skb)->nr_frags > TX_MAX_FRAGS) {
		...
		if (skb_linearize(skb))
			goto tx_free;
	}
#endif
    ...
}

bnxt calls netif_set_tso_max_size(dev, GSO_MAX_SIZE), so a BIG TCP skb
can be linearized into a head larger than 64KB. That length is then
encoded here:

	flags = (len << TX_BD_LEN_SHIFT) | TX_BD_TYPE_LONG_TX_BD |
		TX_BD_CNT(last_frag + 2);

TX_BD_LEN_SHIFT is 16 and flags is a u32. The upper bits of the length
are lost, so the BD length no longer matches the DMA mapping.

bnxt also sets dev->netmem_tx = NETMEM_TX_DMA. For devmem GSO skbs,
skb_linearize() always fails, because __pskb_pull_tail() returns NULL
when !skb_frags_readable(). Those skbs are dropped at tx_free.

The commit message describes both problems. Without a Fixes: tag, stable
kernels that carry b91e82129400 won't pick up this fix.

[Severity: Low]
Can the segments that the stack produces here still have more than
TX_MAX_FRAGS unreadable frags?

Only the GSO features are cleared, and nothing limits the frag count of
each resulting segment. zerocopy_fill_skb_from_devmem() adds one frag per
iovec entry and does not merge adjacent ranges:

net/core/datagram.c:zerocopy_fill_skb_from_devmem() {
    ...
		size = min_t(size_t, size, iter_iov_len(from));
		netmem_inc_ref(net_iov_to_netmem(niov));
		skb_add_rx_frag_netmem(skb, i, net_iov_to_netmem(niov), off,
				       size, PAGE_SIZE);
    ...
}

Suppose a devmem sender uses small iovecs: frags averaging under about
47 bytes at a 1500 MTU, or under about 290 bytes at a 9K MTU. Then a
single MSS-sized segment from skb_segment() can span more than 30 frags.
__copy_skb_header() also carries over skb->unreadable to that segment.

Such a non-GSO segment then reaches skb_linearize() in bnxt_start_xmit(),
which fails for net_iov frags, and the segment is dropped at tx_free.
TCP retransmissions rebuild the same layout, so the flow may stall.

This is not worse than before the patch, when the whole GSO skb was
dropped in the same way. However, the commit message gives "always fails
for unreadable skbs" as a reason for the change, and for these flows that
failure can still happen after segmentation.

Could the commit message mention this limitation? Fully closing the gap
would need a fallback that handles unreadable frags, or a frag limit per
MSS window at sendmsg time, rather than skb_linearize().

>  	switch (vlan_get_protocol(skb)) {
>  	case htons(ETH_P_IP):
>  		if (!skb->encapsulation)

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105222058.944293-1-sdf%40fomichev.me

[PATCH net-next v3 2/9] net: devmem: charge the frag length to TX skb truesize

[Stanislav Fomichev]

Devmem TX charges each frag the net_iov size, PAGE_SIZE today:
zerocopy_fill_skb_from_devmem() adds PAGE_SIZE to skb->truesize per
frag and __zerocopy_sg_from_iter() charges it to sk_wmem_queued. TCP
uncharges in bytes as acked data is trimmed from the skb
(tcp_trim_head()) and uncharges the rest when the skb is freed. A frag
shorter than a page leaves the difference charged until then, which is
small with PAGE_SIZE net_iovs.

TX net_iovs are about to grow up to 2MB, while frags stay bounded by
the skb and many frags share a net_iov. Charging the net_iov size would
charge up to 2MB per frag: a 64KB skb with two frags would charge 4MB
and fill the default 4MB send buffer on its own.

Charge the frag length instead, which also matches what TCP uncharges.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 net/core/datagram.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index ecc30a6f8302..489d92fe6dae 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -737,7 +737,7 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
 
 		netmem_inc_ref(net_iov_to_netmem(niov));
 		skb_add_rx_frag_netmem(skb, i, net_iov_to_netmem(niov), off,
-				       size, PAGE_SIZE);
+				       size, size);
 		iov_iter_advance(from, size);
 		length -= size;
 		i++;
-- 
2.53.0-Meta

Re: [PATCH net-next v3 2/9] net: devmem: charge the frag length to TX skb truesize

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit author and the Signed-off-by use different email
  addresses.

--

net: devmem: charge the frag length to TX skb truesize

This patch changes zerocopy_fill_skb_from_devmem() so each devmem TX frag
charges its length to skb->truesize, not PAGE_SIZE. TX net_iovs are about
to grow up to 2MB. Charging the net_iov size per frag could then fill the
send buffer with a single skb, and charging by length also matches what
TCP uncharges.

> Author: Stanislav Fomichev <sdf.kernel@gmail.com>

[ ... ]

>     Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

[Severity: Low]
This isn't a bug, but the author address and the Signed-off-by address are
different.

The author is sdf.kernel@gmail.com, and the only Signed-off-by is
sdf@fomichev.me. The body has no in-body From: line to override the
authorship.

The .mailmap entries for sdf@fomichev.me only map sdf@google.com and
stfomichev@gmail.com, so tools that read .mailmap won't match the two
either. checkpatch.pl reports this as "From:/Signed-off-by: email address
mismatch".

The other patches in the series appear to have the same mismatch. Could the
git author identity be updated, or a matching From: line added, before
resending?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105222058.944293-1-sdf%40fomichev.me

[PATCH net-next v3 3/9] net: devmem: use niov_shift in net_devmem_get_niov_at()

[Stanislav Fomichev]

TX bindings use PAGE_SIZE net_iovs, and the TX lookup hard-codes it.
Derive the index and offset from binding->niov_shift instead, so TX
bindings can use larger net_iovs. No functional change, TX bindings are
still created with PAGE_SHIFT.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 net/core/devmem.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 32fafbfa1359..6339f221bc82 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -455,13 +455,15 @@ struct net_iov *
 net_devmem_get_niov_at(struct net_devmem_dmabuf_binding *binding,
 		       size_t virt_addr, size_t *off, size_t *size)
 {
+	size_t niov_size = 1UL << binding->niov_shift;
+
 	if (virt_addr >= binding->dmabuf->size)
 		return NULL;
 
-	*off = virt_addr % PAGE_SIZE;
-	*size = PAGE_SIZE - *off;
+	*off = virt_addr & (niov_size - 1);
+	*size = niov_size - *off;
 
-	return binding->tx_vec[virt_addr / PAGE_SIZE];
+	return binding->tx_vec[virt_addr >> binding->niov_shift];
 }
 
 /*** "Dmabuf devmem memory provider" ***/
-- 
2.53.0-Meta

[PATCH net-next v3 4/9] net: devmem: add a per-device netmem TX frag size limit

[Stanislav Fomichev]

Devmem TX frags are bounded only by the net_iov size, which is about
to grow up to 2MB, more than some devices can describe in a single TX
descriptor.

Add netmem_tx_max_frag_size and cap devmem TX frags to the limit of the
device the binding is attached to. Default it to PAGE_SIZE, the largest
TX frag today, so drivers opt in to larger frags.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 Documentation/networking/net_cachelines/net_device.rst | 1 +
 Documentation/networking/netmem.rst                    | 7 +++++++
 include/linux/netdevice.h                              | 2 ++
 net/core/datagram.c                                    | 1 +
 net/core/dev.c                                         | 1 +
 net/core/devmem.c                                      | 1 +
 net/core/devmem.h                                      | 1 +
 7 files changed, 14 insertions(+)

diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
index 8eceaa80b686..4f5fd5b99154 100644
--- a/Documentation/networking/net_cachelines/net_device.rst
+++ b/Documentation/networking/net_cachelines/net_device.rst
@@ -76,6 +76,7 @@ unsigned_short                      dev_id
 unsigned_short                      dev_port
 spinlock_t                          addr_list_lock
 int                                 irq
+u32                                 netmem_tx_max_frag_size
 struct netdev_hw_addr_list          uc
 struct netdev_hw_addr_list          mc
 struct netdev_hw_addr_list          dev_addrs
diff --git a/Documentation/networking/netmem.rst b/Documentation/networking/netmem.rst
index 6871dcdd700c..a7b5bbe1cc65 100644
--- a/Documentation/networking/netmem.rst
+++ b/Documentation/networking/netmem.rst
@@ -155,3 +155,10 @@ Driver TX Requirements
 
    - `NETMEM_TX_NO_DMA`: for virtual or passthrough devices that do
      not DMA, but still support handling of netmem-backed skbs.
+
+3. Devmem TX frags are capped at `netdev->netmem_tx_max_frag_size`,
+   PAGE_SIZE by default. Drivers which can describe larger TX buffers
+   may raise it, up to the largest buffer a single TX descriptor can
+   describe. A frag can then span a whole net_iov, which may be much
+   larger than a page. The value is read when the dmabuf binding is
+   created.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c0ae442c4344..4453b80bf7ea 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1845,6 +1845,7 @@ enum netdev_reg_state {
  *			drivers. Mainly used by logical interfaces, such as
  *			bonding and tunnels
  *	@netmem_tx:	device netmem TX mode
+ *	@netmem_tx_max_frag_size: max size of a netmem TX frag
  *	@pacing_offload: enable EDT pacing offload.
  *
  *	@name:	This is the first field of the "visible" part of this structure
@@ -2342,6 +2343,7 @@ struct net_device {
 	unsigned short          dev_port;
 	int			irq;
 	u32			priv_len;
+	u32			netmem_tx_max_frag_size;
 
 	spinlock_t		addr_list_lock;
 
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 489d92fe6dae..a8d3f87d38c5 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -734,6 +734,7 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
 
 		size = min_t(size_t, size, length);
 		size = min_t(size_t, size, iter_iov_len(from));
+		size = min_t(size_t, size, binding->tx_max_frag_size);
 
 		netmem_inc_ref(net_iov_to_netmem(niov));
 		skb_add_rx_frag_netmem(skb, i, net_iov_to_netmem(niov), off,
diff --git a/net/core/dev.c b/net/core/dev.c
index 3ac700d37e59..0af4e4debcc8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -12350,6 +12350,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	dev->gro_ipv4_max_size = GRO_LEGACY_MAX_SIZE;
 	dev->tso_max_size = TSO_LEGACY_MAX_SIZE;
 	dev->tso_max_segs = TSO_MAX_SEGS;
+	dev->netmem_tx_max_frag_size = PAGE_SIZE;
 	dev->upper_level = 1;
 	dev->lower_level = 1;
 #ifdef CONFIG_LOCKDEP
diff --git a/net/core/devmem.c b/net/core/devmem.c
index 6339f221bc82..19c039ed2cb2 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -218,6 +218,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 
 	binding->dev = dev;
 	binding->vdev = vdev;
+	binding->tx_max_frag_size = dev->netmem_tx_max_frag_size;
 	write_pnet(&binding->net, sock_net(nl_sk));
 	binding->uid = sk_uid(nl_sk);
 	binding->niov_shift = niov_shift;
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 8fa089776610..97ff1f13fb84 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -79,6 +79,7 @@ struct net_devmem_dmabuf_binding {
 	struct net_iov **tx_vec;
 
 	unsigned int niov_shift;
+	u32 tx_max_frag_size;
 
 	struct work_struct unbind_w;
 };
-- 
2.53.0-Meta

Re: [PATCH net-next v3 4/9] net: devmem: add a per-device netmem TX frag size limit

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new per-device cap is enforced in only one place, when
  zerocopy_fill_skb_from_devmem() (net/core/datagram.c:737) first…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but
  tcp_clone_payload() (net/ipv4/tcp_output.c:2699-2703) can merge frags…

--

net: devmem: add a per-device netmem TX frag size limit

This commit adds a netmem_tx_max_frag_size field to struct net_device,
defaulting to PAGE_SIZE, and copies it into the devmem dmabuf binding
when the binding is created. zerocopy_fill_skb_from_devmem() then caps
each devmem TX frag to that limit.

> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index 489d92fe6dae8..a8d3f87d38c58 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -734,6 +734,7 @@ zerocopy_fill_skb_from_devmem(struct sk_buff *skb, struct iov_iter *from,
>  
>  		size = min_t(size_t, size, length);
>  		size = min_t(size_t, size, iter_iov_len(from));
> +		size = min_t(size_t, size, binding->tx_max_frag_size);

[Severity: Medium]
Is this the only place the new cap is enforced? The netmem.rst text added
by this patch says "Devmem TX frags are capped at
netdev->netmem_tx_max_frag_size". TCP MTU probing looks like it can merge
the capped frags back together later.

tcp_mtu_probe()->tcp_can_coalesce_send_queue_head() uses
tcp_skb_can_collapse(). That check only compares skb_pure_zcopy_same() and
skb_frags_readable(), so two devmem skbs pass it. tcp_mtu_probe() then
calls tcp_clone_payload():

net/ipv4/tcp_output.c:tcp_clone_payload() {
    ...
			if (lastfrag &&
			    skb_frag_page(fragfrom) == skb_frag_page(lastfrag) &&
			    skb_frag_off(fragfrom) == skb_frag_off(lastfrag) +
						      skb_frag_size(lastfrag)) {
				skb_frag_size_add(lastfrag, todo);
				continue;
			}
    ...
}

skb_frag_page() returns NULL for every net_iov, so for devmem frags only
offset contiguity is checked.

Suppose this line splits one large net_iov into two frags, for example
(niov0, 0, 4K) and (niov0, 4K, 4K). Those two frags would be merged into
one frag of up to probe_size bytes. tcp_transmit_skb() then hands it to
the driver, and nothing checks it against binding->tx_max_frag_size.

At this commit TX net_iovs are still PAGE_SIZE, because
netdev_nl_bind_tx_doit() passes PAGE_SHIFT. So the merged frag can't go
past the default cap yet.

The later patch in the series, "net: devmem: size TX net_iovs from the
dmabuf sg table", lets TX net_iovs grow to 2MB. After that, take a device
left at the PAGE_SIZE default that runs a jumbo MTU with MTU probing
enabled. It would get probe frags of roughly MTU size, which is above its
advertised cap.

In-tree drivers seem fine with this today. bnxt and fbnic advertise 32K,
mlx5 advertises U32_MAX, and gve already splits frags into its own
descriptors. Even so, the documented limit would not hold.

Could tcp_clone_payload() compare skb_frag_netmem() and stop merging once
the binding cap is reached? Another option is to skip MTU probe
coalescing for unreadable skbs.

[Severity: High]
This isn't a bug introduced by this patch, but the same merge check in
tcp_clone_payload() looks like it can also combine frags from different
net_iovs. skb_frag_page() returns NULL for both frags, so the page
comparison always passes and only the offsets are compared.

For example, take a single sendmsg on a devmem TX socket with
iov[0] = {dmabuf offset 0, len 2048} and
iov[1] = {dmabuf offset 6144, len 2048}. zerocopy_fill_skb_from_devmem()
builds:

    frag0 = (niov0, off 0, 2048)
    frag1 = (niov1, off 2048, 2048)

Suppose tcp_mtu_probe() then builds a probe that covers both frags.
tcp_clone_payload() sees that frag1's offset equals frag0's end, so it
extends frag0 to (niov0, 0, 4096) and never copies frag1's netmem.

Wouldn't the probe then carry dmabuf bytes [2048, 4096) instead of
[6144, 8192)?

The NIC computes the checksum over the wrong bytes, so the receiver
accepts them. If the probe is ACKed, tcp_eat_one_skb() has already
consumed the original skbs, so the stream stays corrupted.

The merged range stays inside niov0. So the result is wrong data from the
sender's own dmabuf, not an out-of-bounds DMA.

Should the merge test compare skb_frag_netmem(), the way
skb_can_coalesce_netmem() does? Nothing later in the series changes
tcp_output.c.

>  
>  		netmem_inc_ref(net_iov_to_netmem(niov));
>  		skb_add_rx_frag_netmem(skb, i, net_iov_to_netmem(niov), off,

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105222058.944293-1-sdf%40fomichev.me

[PATCH net-next v3 5/9] bnxt_en: set the netmem TX frag size limit to 32KB

[Stanislav Fomichev]

The TX BD length field is 16 bits wide. Set the netmem TX frag size
limit to the largest power of 2 which fits, which keeps 4K aligned
frags 4K aligned.

With 4K pages this raises the PAGE_SIZE default. With 64K pages it
lowers it, 64KB frags would otherwise wrap the BD length to 0 with BIG
TCP.

Tested on a 50Gb/s NIC: devmem TX from a 2MB hugepage-backed udmabuf
runs at line rate with 2-3 frags per 64KB skb, none above 32KB.

kperf output (sender side, 1 flow):

  client: == Source <sender>
  client:   Tx 46.358 Gbps (174483046400 bytes in 30110590 usec)
  client:   Rx  0.000 Gbps (0 bytes in 30110590 usec)
  client:   TCP retrans reord rtt rttvar d_ce snd_wnd cwnd
  client:           729   153 599     48    0 13893632 3147
  client:   net CPU 71: usr: 0.03% sys: 0.06% idle:67.62% iow: 0.00% irq: 1.12% sirq:31.14%
  client:   app CPU 72: usr: 0.16% sys: 6.01% idle:90.66% iow: 0.00% irq: 0.83% sirq: 2.32%

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index e7a4190a8c78..c8a887facc20 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -17439,6 +17439,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (BNXT_SUPPORTS_QUEUE_API(bp))
 		dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops;
 	dev->netmem_tx = NETMEM_TX_DMA;
+	dev->netmem_tx_max_frag_size = SZ_32K;
 
 	rc = register_netdev(dev);
 	if (rc)
-- 
2.53.0-Meta

Re: [PATCH net-next v3 5/9] bnxt_en: set the netmem TX frag size limit to 32KB

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch fixes a real bug in released kernels but has no Fixes:
  tag, and it does not say that it depends on the earlier patch in this…

Pre-existing issues:
- [High] bnxt_start_xmit() still writes non-netmem page frag lengths
  straight into the 16-bit TX BD length field (TX_BD_LEN = 0xffff << 16).

--

bnxt_en: set the netmem TX frag size limit to 32KB

This patch sets dev->netmem_tx_max_frag_size to SZ_32K in bnxt_init_one().
That caps netmem TX frags at the largest power of 2 that fits in the 16-bit
TX BD length field. With 64K pages this lowers the PAGE_SIZE default, so
64KB devmem frags no longer wrap the BD length to 0 with BIG TCP.

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index f20b61180ab2d..4bcf84faf18f1 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -17471,6 +17471,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (BNXT_SUPPORTS_QUEUE_API(bp))
>  		dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops;
>  	dev->netmem_tx = NETMEM_TX_DMA;
> +	dev->netmem_tx_max_frag_size = SZ_32K;

[Severity: Low]
The commit message says:

  With 64K pages it lowers it, 64KB frags would otherwise wrap the BD
  length to 0 with BIG TCP.

Isn't this a bug that already exists on 64K-page kernels, rather than a
side effect of the new limit?

Before this series, net_devmem_get_niov_at() returns size = PAGE_SIZE - off,
and zerocopy_fill_skb_from_devmem() applies no other cap. With BIG TCP, an
aligned devmem send can produce a 65536-byte frag. bnxt_start_xmit() then
does:

    flags = len << TX_BD_LEN_SHIFT;

which leaves 0 in the 16-bit length field.

Should this patch carry a Fixes: tag for commit 4922ca773d9d ("eth: bnxt:
add netmem TX support")?

This change also only works together with "net: devmem: add a per-device
netmem TX frag size limit". That patch adds dev->netmem_tx_max_frag_size and
binding->tx_max_frag_size, and it has no Fixes: tag either.

Without the tags and a note about the dependency, stable kernels on 64K-page
systems (arm64 64K, ppc64) would keep the wrap. Would a separate minimal fix
for the net tree work better?

[Severity: High]
This isn't a bug introduced by this patch, but the same 16-bit wrap still
looks reachable for frags that are not netmem. bnxt_start_xmit() writes
every frag length straight into the TX BD:

    len = skb_frag_size(frag);
    ...
    flags = len << TX_BD_LEN_SHIFT;
    txbd->tx_bd_len_flags_type = cpu_to_le32(flags);

TX_BD_LEN is defined as (0xffff << 16), and the length is not clamped or
split. Meanwhile the DMA mapping and dma_unmap_len cover the full frag.

The new limit is only applied in zerocopy_fill_skb_from_devmem(), through
binding->tx_max_frag_size. bnxt enables BIG TCP via
netif_set_tso_max_size(dev, GSO_MAX_SIZE) in bnxt_init_one(). That means
ordinary page frags can reach 64K or more.

On 64K-PAGE_SIZE kernels, SKB_FRAG_PAGE_ORDER is 0, so
skb_page_frag_refill() hands out 64K pages. tcp_sendmsg_locked() can then
coalesce data into one frag of a full 65536 bytes. Sendfile or splice of a
full 64K page-cache page gives the same result.

On any page size, MSG_ZEROCOPY on a THP or hugetlb buffer goes through
zerocopy_fill_skb_from_iter(). That function merges contiguous subpages of
the same compound head:

    if (head == skb_frag_page(last) &&
        start == skb_frag_off(last) + skb_frag_size(last)) {
        skb_frag_size_add(last, size);

so a single frag can be larger than 64K.

In these cases, can the NIC get a BD whose length (len mod 64K, or 0) does
not match the mapped buffer? That could produce malformed TSO segments or a
TX timeout.

The RX side already handles this limit. bnxt.h has the comment "The RXBD
length is 16-bit so we can only support page sizes < 64K". There doesn't
seem to be an equivalent check on TX, for example in bnxt_features_check()
or bnxt_start_xmit().

BIG TCP has been enabled in bnxt since commit b6488b161ab2.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105222058.944293-1-sdf%40fomichev.me

[PATCH net-next v3 6/9] eth: fbnic: set the netmem TX frag size limit to 32KB

[Stanislav Fomichev]

The TX descriptor length field (FBNIC_TWD_LEN_MASK) is 16 bits wide.
Set the netmem TX frag size limit to the largest power of 2 which fits,
like bnxt. With 64K pages this also lowers the PAGE_SIZE default, whose
64KB frags don't fit the length field.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 drivers/net/ethernet/meta/fbnic/fbnic_netdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index 8a6703afca38..0a2b66202aec 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -760,6 +760,7 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
 	netdev->stat_ops = &fbnic_stat_ops;
 	netdev->queue_mgmt_ops = &fbnic_queue_mgmt_ops;
 	netdev->netmem_tx = NETMEM_TX_DMA;
+	netdev->netmem_tx_max_frag_size = SZ_32K;
 
 	fbnic_set_ethtool_ops(netdev);
 
-- 
2.53.0-Meta

[PATCH net-next v3 7/9] net/mlx5e: lift the netmem TX frag size limit

[Stanislav Fomichev]

TX data segments have a 32-bit byte count, so devmem TX frags don't
need a size limit. Set netmem_tx_max_frag_size to U32_MAX.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index c3c2c5720c6f..28ba04bd5f4a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5967,6 +5967,7 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
 	netdev->priv_flags       |= IFF_UNICAST_FLT;
 
 	netdev->netmem_tx = NETMEM_TX_DMA;
+	netdev->netmem_tx_max_frag_size = U32_MAX;
 
 	netif_set_tso_max_size(netdev, GSO_MAX_SIZE);
 	mlx5e_set_xdp_feature(priv);
-- 
2.53.0-Meta

Re: [PATCH net-next v3 7/9] net/mlx5e: lift the netmem TX frag size limit

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch sets `netdev->netmem_tx_max_frag_size = U32_MAX` in
  `mlx5e_build_nic_netdev()`…

--

net/mlx5e: lift the netmem TX frag size limit

This commit sets netmem_tx_max_frag_size to U32_MAX for mlx5e NIC netdevs.
The commit message says mlx5 TX data segments have a 32-bit byte count,
so devmem TX frags don't need a size limit.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index c3c2c5720c6f3..28ba04bd5f4a4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -5967,6 +5967,7 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
>  	netdev->priv_flags       |= IFF_UNICAST_FLT;
>
>  	netdev->netmem_tx = NETMEM_TX_DMA;
> +	netdev->netmem_tx_max_frag_size = U32_MAX;

[Severity: Low]
Is U32_MAX really the largest length an mlx5 data segment can describe?
The commit message says:

  TX data segments have a 32-bit byte count, so devmem TX frags don't
  need a size limit.

Bit 31 of byte_count looks like it is the inline segment marker rather
than part of the length:

include/linux/mlx5/device.h:
	MLX5_INLINE_SEG = 0x80000000,

The mlx5 IB ODP WQE parser reads it the same way:

drivers/infiniband/hw/mlx5/odp.c:
	inline_segment = !!(byte_count &  MLX5_INLINE_SEG);
	bcnt	       = byte_count & ~MLX5_INLINE_SEG;

The mlx5e XDP, IPsec and PSP TX paths also OR this bit in to mark inline
segments. That leaves 31 bits for the length of a pointer segment.

mlx5e_txwqe_build_dsegs() stores the frag size in a signed int and writes
it to byte_count without masking:

drivers/net/ethernet/mellanox/mlx5/core/en_tx.c:mlx5e_txwqe_build_dsegs() {
    ...
		int fsz = skb_frag_size(frag);
    ...
		dseg->byte_count = cpu_to_be32(fsz);
    ...
}

If a frag of 2^31 bytes or more ever got here, fsz would be negative.
The pointer segment would also have the inline bit set, so the WQE would
be malformed.

Today nothing can trigger this. net_devmem_bind_dmabuf() is called with
niov_shift=0 for TX, so the net_iov size is at most SZ_2M. Each frag in
zerocopy_fill_skb_from_devmem() is limited by that size.

The documented contract in Documentation/networking/netmem.rst still
says:

   Drivers which can describe larger TX buffers
   may raise it, up to the largest buffer a single TX descriptor can
   describe.

zerocopy_fill_skb_from_devmem() also relies on this value as the
per-device bound:

	size = min_t(size_t, size, binding->tx_max_frag_size);

Would a value that matches the hardware, such as (1U << 31) - 1 or SZ_1G,
work better here? The commit message would need updating to match.

>
>  	netif_set_tso_max_size(netdev, GSO_MAX_SIZE);
>  	mlx5e_set_xdp_feature(priv);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105222058.944293-1-sdf%40fomichev.me

[PATCH net-next v3 8/9] net: devmem: size TX net_iovs from the dmabuf sg table

[Stanislav Fomichev]

PAGE_SIZE TX net_iovs cap devmem skbs at MAX_SKB_FRAGS pages (~68KB),
which makes BIG TCP ineffective.

TX net_iovs only need to be DMA contiguous. Use the largest power of 2,
from PAGE_SIZE up to 2MB, which aligns every DMA segment of the dmabuf:
- 2MB hugepage-backed udmabufs get 2MB net_iovs
- no UAPI change, TX addresses are dmabuf offsets
- skbs are still bounded by TCP's size_goal, only frags get larger

The net_iov size doesn't depend on the device: frags are capped to the
device limit when they are built, and many frags share a net_iov.
Capping the net_iov instead would only split frags at net_iov
boundaries and allocate more net_iovs.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 net/core/devmem.c      | 28 ++++++++++++++++++++++++++--
 net/core/netdev-genl.c |  2 +-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 19c039ed2cb2..98c9594dbf99 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -181,6 +181,20 @@ int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 	return err;
 }
 
+static bool net_devmem_sgt_aligned(struct sg_table *sgt, size_t size)
+{
+	struct scatterlist *sg;
+	unsigned int i;
+
+	for_each_sgtable_dma_sg(sgt, sg, i) {
+		if (!IS_ALIGNED(sg_dma_address(sg), size) ||
+		    !IS_ALIGNED(sg_dma_len(sg), size))
+			return false;
+	}
+	return true;
+}
+
+/* niov_shift == 0 picks the net_iov size from the dmabuf, TX only */
 struct net_devmem_dmabuf_binding *
 net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		       struct device *dma_dev,
@@ -191,11 +205,11 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		       struct netlink_ext_ack *extack)
 {
 	struct net_devmem_dmabuf_binding *binding;
-	size_t niov_size = 1UL << niov_shift;
 	static u32 id_alloc_next;
 	struct scatterlist *sg;
 	struct dma_buf *dmabuf;
 	unsigned int sg_idx;
+	size_t niov_size;
 	size_t niov_idx;
 	size_t i;
 	long err;
@@ -221,7 +235,6 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 	binding->tx_max_frag_size = dev->netmem_tx_max_frag_size;
 	write_pnet(&binding->net, sock_net(nl_sk));
 	binding->uid = sk_uid(nl_sk);
-	binding->niov_shift = niov_shift;
 	xa_init_flags(&binding->bound_rxqs, XA_FLAGS_ALLOC);
 
 	err = percpu_ref_init(&binding->ref,
@@ -266,6 +279,17 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		goto err_unmap;
 	}
 
+	if (!niov_shift) {
+		niov_size = SZ_2M;
+		while (niov_size > PAGE_SIZE &&
+		       (!IS_ALIGNED(dmabuf->size, niov_size) ||
+			!net_devmem_sgt_aligned(binding->sgt, niov_size)))
+			niov_size /= 2;
+		niov_shift = ilog2(niov_size);
+	}
+	binding->niov_shift = niov_shift;
+	niov_size = 1UL << niov_shift;
+
 	if (!IS_ALIGNED(dmabuf->size, niov_size)) {
 		err = -EINVAL;
 		NL_SET_ERR_MSG_FMT(extack,
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 384fdc5ae514..bfd229bd31ca 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -1261,7 +1261,7 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
 	binding = net_devmem_bind_dmabuf(bind_dev,
 					 bind_dev != netdev ? netdev : NULL,
 					 dma_dev, DMA_TO_DEVICE, dmabuf_fd,
-					 PAGE_SHIFT, priv, NETLINK_CB(skb).sk,
+					 0, priv, NETLINK_CB(skb).sk,
 					 info->extack);
 	if (IS_ERR(binding)) {
 		err = PTR_ERR(binding);
-- 
2.53.0-Meta

Re: [PATCH net-next v3 8/9] net: devmem: size TX net_iovs from the dmabuf sg table

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] This patch makes TX net_iovs grow from PAGE_SIZE to as much as
  2MB.

--

net: devmem: size TX net_iovs from the dmabuf sg table

This patch changes how TX net_iovs are sized. Instead of PAGE_SIZE, each
TX net_iov is now the largest power of two from PAGE_SIZE up to 2MB that
aligns both the dmabuf size and every DMA segment in the sg table.
netdev_nl_bind_tx_doit() passes niov_shift == 0 to ask for this, so that
BIG TCP can build larger frags.

> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index b607cf495980c..74de200f66be2 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c

[ ... ]

> @@ -388,6 +401,17 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
>  		goto err_unmap;
>  	}
>  
> +	if (!niov_shift) {
> +		niov_size = SZ_2M;
> +		while (niov_size > PAGE_SIZE &&
> +		       (!IS_ALIGNED(dmabuf->size, niov_size) ||
> +			!net_devmem_sgt_aligned(binding->sgt, niov_size)))
> +			niov_size /= 2;
> +		niov_shift = ilog2(niov_size);
> +	}
> +	binding->niov_shift = niov_shift;
> +	niov_size = 1UL << niov_shift;
> +

[Severity: Medium]
netdev_nl_bind_tx_doit() now passes niov_shift == 0, so a hugepage-backed
dmabuf gets 2MB TX net_iovs. Can a devmem TX frag then end up larger than
dev->netmem_tx_max_frag_size?

The cap is only applied when a frag is first built:

net/core/datagram.c:zerocopy_fill_skb_from_devmem() {
    ...
		size = min_t(size_t, size, binding->tx_max_frag_size);
    ...
}

Later, tcp_mtu_probe()->tcp_clone_payload() merges contiguous frags and
never checks the binding's limit:

net/ipv4/tcp_output.c:tcp_clone_payload() {
    ...
			if (lastfrag &&
			    skb_frag_page(fragfrom) == skb_frag_page(lastfrag) &&
			    skb_frag_off(fragfrom) == skb_frag_off(lastfrag) +
						      skb_frag_size(lastfrag)) {
				skb_frag_size_add(lastfrag, todo);
				continue;
			}
    ...
}

Before this patch, contiguous frags inside one net_iov could add up to at
most PAGE_SIZE. That is also the default netmem_tx_max_frag_size, so a
merged frag still stayed within the cap.

With 2MB net_iovs, contiguous TCP segments in the same net_iov can be
merged into one frag of up to probe_size bytes. On a path whose MTU is
above PAGE_SIZE, that frag is larger than the limit the device declared.

Documentation/networking/netmem.rst says:

    3. Devmem TX frags are capped at netdev->netmem_tx_max_frag_size,
       PAGE_SIZE by default.

The commit message says:

    frags are capped to the device limit when they are built

Is a cap applied only at build time enough to keep that contract?

The other merge paths are tcp_grow_skb() and SACK shifting. Both go
through skb_shift(), which bails out on skb_zcopy(), so tcp_clone_payload()
looks like the only path left.

All of these are needed to reach it:

  - an unprivileged bind-tx (NETDEV_CMD_BIND_TX has no GENL_ADMIN_PERM)
  - MTU probing turned on (tcp_mtu_probing=2, or 1 after black hole
    detection)
  - a path MTU above the device's frag limit

I could not find an in-tree driver that breaks today:

  - gve keeps the PAGE_SIZE default but splits buffers at
    GVE_TX_MAX_BUF_SIZE_DQO
  - bnxt and fbnic use 32K
  - mlx5 uses U32_MAX

A driver that relies on the documented cap would still get a frag larger
than it declared.

There is also a related issue in the same code. It is pre-existing and
was not introduced by this patch.

skb_frag_page() returns NULL for every net_iov frag. So for devmem frags
the skb_frag_page() comparison above is always true, even when the frags
come from different net_iovs.

Suppose the next frag's offset inside its net_iov equals the end offset
of lastfrag inside its own net_iov. Then lastfrag is extended over bytes
from the wrong net_iov, and the wrong dmabuf data is sent under real
sequence numbers. The extended range stays inside the first net_iov, so
there is no DMA outside the dmabuf.

Would either of these fix both problems:

  - compare skb_frag_netmem() and apply binding->tx_max_frag_size in
    tcp_clone_payload()
  - don't coalesce unreadable skbs when building MTU probes

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105222058.944293-1-sdf%40fomichev.me

[PATCH net-next v3 9/9] selftests: drv-net: devmem: TX from a hugepage-backed dmabuf

[Stanislav Fomichev]

TX net_iovs are now sized from the dmabuf layout, but the TX tests
use a regular memfd, which usually keeps them small. Add -H to ncdevmem
to back the udmabuf with 2MB hugepages and a TX test which sends a 3MB
buffer from it, so that frags end at and continue past the 2MB net_iov
boundary. The payload has a 7 byte period to catch offset errors.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 .../selftests/drivers/net/hw/devmem.py        | 10 ++++-
 .../selftests/drivers/net/hw/devmem_lib.py    | 43 +++++++++++++++++--
 .../selftests/drivers/net/hw/ncdevmem.c       |  8 +++-
 .../selftests/drivers/net/hw/nk_devmem.py     | 11 ++++-
 4 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/devmem.py b/tools/testing/selftests/drivers/net/hw/devmem.py
index 82c11ffc4add..aa7141fb36f7 100755
--- a/tools/testing/selftests/drivers/net/hw/devmem.py
+++ b/tools/testing/selftests/drivers/net/hw/devmem.py
@@ -3,7 +3,7 @@
 
 from os import path
 from devmem_lib import (setup_test, run_rx, run_tx, run_tx_chunks, run_rx_hds,
-                        run_rx_large_niov)
+                        run_rx_large_niov, run_tx_large_niov)
 from lib.py import ksft_run, ksft_exit, ksft_disruptive
 from lib.py import NetDrvEpEnv
 
@@ -36,12 +36,18 @@ from lib.py import NetDrvEpEnv
     run_rx_large_niov(cfg)
 
 
+@ksft_disruptive
+def check_tx_large_niov(cfg) -> None:
+    """Run the devmem TX test from a hugepage-backed dmabuf."""
+    run_tx_large_niov(cfg)
+
+
 def main() -> None:
     """Run the devmem test cases."""
     with NetDrvEpEnv(__file__) as cfg:
         setup_test(cfg, path.abspath(path.dirname(__file__) + "/ncdevmem"))
         ksft_run([check_rx, check_tx, check_tx_chunks, check_rx_hds,
-                  check_rx_large_niov],
+                  check_rx_large_niov, check_tx_large_niov],
                  args=(cfg,))
     ksft_exit()
 
diff --git a/tools/testing/selftests/drivers/net/hw/devmem_lib.py b/tools/testing/selftests/drivers/net/hw/devmem_lib.py
index 71b916c16196..8058fd1ddd50 100644
--- a/tools/testing/selftests/drivers/net/hw/devmem_lib.py
+++ b/tools/testing/selftests/drivers/net/hw/devmem_lib.py
@@ -2,6 +2,7 @@
 # pylint: disable=invalid-name,too-many-arguments
 """Shared helpers for devmem TCP selftests."""
 
+import hashlib
 import os
 import re
 
@@ -42,15 +43,17 @@ NR_HUGEPAGES_FILE = "/proc/sys/vm/nr_hugepages"
     defer(_restore_nr_hugepages, nr_hugepages)
 
 
-def _probe_devmem(cfg, rx_page_size):
+def _probe_devmem(cfg, rx_page_size, hugepages=False):
     """Return True if ncdevmem can bind cfg.ifname at @rx_page_size."""
     probe_command = f"{cfg.bin_local} -f {cfg.ifname}"
     if rx_page_size != RX_PAGE_SIZE_DEFAULT:
         probe_command += f" -b {rx_page_size}"
+    if hugepages:
+        probe_command += " -H"
     return cmd(probe_command, fail=False, shell=True).ret == 0
 
 
-def require_devmem(cfg, rx_page_size=RX_PAGE_SIZE_DEFAULT):
+def require_devmem(cfg, rx_page_size=RX_PAGE_SIZE_DEFAULT, hugepages=False):
     """Probe ncdevmem on cfg.ifname and SKIP the test if devmem isn't supported."""
     if rx_page_size not in PROBE_RX_PAGE_SIZES:
         raise RuntimeError(
@@ -63,10 +66,15 @@ NR_HUGEPAGES_FILE = "/proc/sys/vm/nr_hugepages"
         # ncdevmem's queue range and cause the probe to fail.
         cfg.devmem_supported = {size: _probe_devmem(cfg, size)
                                 for size in PROBE_RX_PAGE_SIZES}
+        cfg.devmem_hugepages = _probe_devmem(cfg, RX_PAGE_SIZE_DEFAULT,
+                                             hugepages=True)
 
     if not cfg.devmem_supported[RX_PAGE_SIZE_DEFAULT]:
         raise KsftSkipEx("Test requires devmem support")
 
+    if hugepages and not cfg.devmem_hugepages:
+        raise KsftSkipEx("Test requires a 2MB hugepage-backed dmabuf")
+
     if rx_page_size != RX_PAGE_SIZE_DEFAULT:
         page_size = os.sysconf("SC_PAGE_SIZE")
         if not _is_aligned(rx_page_size, page_size):
@@ -169,7 +177,7 @@ def ncdevmem_rx(cfg, port, verify=True, fail_on_linear=False, flow_steer=False,
     return " ".join(parts)
 
 
-def ncdevmem_tx(cfg, port, chunk_size=0):
+def ncdevmem_tx(cfg, port, chunk_size=0, hugepages=False):
     """Build the ncdevmem TX send command."""
     if hasattr(cfg, 'netns'):
         ifname = cfg.nk_guest_ifname
@@ -182,6 +190,8 @@ def ncdevmem_rx(cfg, port, verify=True, fail_on_linear=False, flow_steer=False,
 
     if chunk_size:
         extras.append(f"-z {chunk_size}")
+    if hugepages:
+        extras.append("-H")
 
     parts = [cfg.bin_local, f"-f {ifname}", f"-s {addr}",
              f"-p {port}", *extras]
@@ -269,6 +279,33 @@ def ncdevmem_rx(cfg, port, verify=True, fail_on_linear=False, flow_steer=False,
     ksft_eq(socat.stdout.strip(), "hello\nworld")
 
 
+def run_tx_large_niov(cfg):
+    """Run the devmem TX test from a 2MB hugepage-backed dmabuf.
+
+    The kernel sizes TX niovs from the dmabuf layout. Send a single buffer
+    larger than a hugepage so that frags end at and continue past the niov
+    boundary.
+    """
+    require_devmem(cfg, hugepages=True)
+    _reserve_hugepages()
+    configure_nic(cfg)
+    netns = getattr(cfg, "netns", None)
+    port = rand_port()
+    size = 3 * 1024 * 1024
+    pattern = "abcdefg"
+    expected = (pattern * (size // len(pattern) + 1))[:size]
+    tx_cmd = ncdevmem_tx(cfg, port, hugepages=True)
+    # Hash on the remote, bkg() only drains stdout once the command exits.
+    listen_cmd = f"{socat_listen(cfg, port)} | sha256sum"
+
+    with bkg(listen_cmd, host=cfg.remote, exit_wait=True) as socat:
+        wait_port_listen(port, host=cfg.remote)
+        cmd(f"bash -c 'yes {pattern} | tr -d \"\\n\" | head -c {size} | {tx_cmd}'",
+            ns=netns, shell=True)
+    ksft_eq(socat.stdout.split()[0],
+            hashlib.sha256(expected.encode()).hexdigest())
+
+
 def run_rx_large_niov(cfg):
     """Run the devmem RX test with a large niov (rx-page-size > PAGE_SIZE).
 
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
index 918e3b51f3b8..e29ff27087e7 100644
--- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c
+++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
@@ -104,6 +104,7 @@ static uint32_t tx_dmabuf_id;
 static int waittime_ms = 500;
 static bool fail_on_linear;
 static uint32_t rx_page_size;
+static bool use_hugepages;
 
 /* System state loaded by current_config_load() */
 #define MAX_FLOWS	8
@@ -162,7 +163,7 @@ static struct memory_buffer *udmabuf_alloc(size_t size)
 	}
 
 	memfd_flags = MFD_ALLOW_SEALING;
-	if (rx_page_size > getpagesize())
+	if (use_hugepages || rx_page_size > getpagesize())
 		memfd_flags |= MFD_HUGETLB | MFD_HUGE_2MB;
 
 	ctx->memfd = memfd_create("udmabuf-test", memfd_flags);
@@ -1428,11 +1429,14 @@ int main(int argc, char *argv[])
 	int is_server = 0, opt;
 	int ret, err = 1;
 
-	while ((opt = getopt(argc, argv, "Lls:c:p:v:q:t:f:z:nb:")) != -1) {
+	while ((opt = getopt(argc, argv, "Lls:c:p:v:q:t:f:z:nb:H")) != -1) {
 		switch (opt) {
 		case 'L':
 			fail_on_linear = true;
 			break;
+		case 'H':
+			use_hugepages = true;
+			break;
 		case 'l':
 			is_server = 1;
 			break;
diff --git a/tools/testing/selftests/drivers/net/hw/nk_devmem.py b/tools/testing/selftests/drivers/net/hw/nk_devmem.py
index 61c6f31f01e5..799030db7c12 100755
--- a/tools/testing/selftests/drivers/net/hw/nk_devmem.py
+++ b/tools/testing/selftests/drivers/net/hw/nk_devmem.py
@@ -4,7 +4,7 @@
 
 import os
 from devmem_lib import (setup_test, run_rx, run_tx, run_tx_chunks, run_rx_hds,
-                        run_rx_large_niov)
+                        run_rx_large_niov, run_tx_large_niov)
 from lib.py import ksft_run, ksft_exit, ksft_disruptive
 from lib.py import NetDrvContEnv
 
@@ -37,6 +37,12 @@ from lib.py import NetDrvContEnv
     run_rx_large_niov(cfg)
 
 
+@ksft_disruptive
+def check_nk_tx_large_niov(cfg) -> None:
+    """Run the devmem TX large-niov test through netkit."""
+    run_tx_large_niov(cfg)
+
+
 def main() -> None:
     """Run the netkit devmem test cases."""
     with NetDrvContEnv(__file__, rxqueues=2, primary_rx_redirect=True) as cfg:
@@ -44,7 +50,8 @@ from lib.py import NetDrvContEnv
                    os.path.join(os.path.dirname(os.path.abspath(__file__)),
                                 "ncdevmem"))
         ksft_run([check_nk_rx, check_nk_tx, check_nk_tx_chunks,
-                  check_nk_rx_hds, check_nk_rx_large_niov], args=(cfg,))
+                  check_nk_rx_hds, check_nk_rx_large_niov,
+                  check_nk_tx_large_niov], args=(cfg,))
     ksft_exit()
 
 
-- 
2.53.0-Meta

Re: [PATCH net-next v3 9/9] selftests: drv-net: devmem: TX from a hugepage-backed dmabuf

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit author is "Stanislav Fomichev <sdf.kernel@gmail.com>"
  but the only Signed-off-by is "Stanislav Fomichev <sdf@fomichev.me>".

--

selftests: drv-net: devmem: TX from a hugepage-backed dmabuf

This commit adds a -H option to ncdevmem that backs the udmabuf with 2MB
hugepages. It also adds a TX test that sends a 3MB buffer from that
dmabuf, so frags end at the 2MB net_iov boundary and continue past it.
The payload repeats every 7 bytes so that offset errors are caught.

> Author: Stanislav Fomichev <sdf.kernel@gmail.com>
> ...
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

[Severity: Low]
This isn't a bug, but the author address doesn't match the Signed-off-by
address. The author is sdf.kernel@gmail.com, and the only sign-off is from
sdf@fomichev.me.

checkpatch.pl reports this as FROM_SIGN_OFF_MISMATCH ("From:/Signed-off-by:
email address mismatch"), and the netdev automated checks will flag it too.

Could a Signed-off-by be added for the author address? Alternatively, an
in-body "From: Stanislav Fomichev <sdf@fomichev.me>" line would make the
author match the sign-off.

Every commit in the series has the same mismatch, from "bnxt_en: let the
stack segment GSO skbs with too many frags" through this one. It probably
comes from how the series was sent, so the fix would need to go into every
patch.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105222058.944293-1-sdf%40fomichev.me