* Re: [PATCH v1 1/1] net/nbl: allow MTU change when port is started
2026-09-28 8:36 ` [PATCH v1 1/1] net/nbl: " Dimon Zhao
@ 2026-09-28 18:30 ` Stephen Hemminger
0 siblings, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2026-09-28 18:30 UTC (permalink / raw)
To: Dimon Zhao; +Cc: dev, stable, Leon Yu, Sam Chen
On Mon, 28 Sep 2026 01:36:16 -0700
Dimon Zhao <dimon.zhao@nebula-matrix.com> wrote:
> Remove the dev_started check in nbl_mtu_set() to allow MTU
> configuration while the port is started.
>
> Fixes: 75cdda36a4c5 ("net/nbl: support MTU update")
> Cc: stable@dpdk.org
> Signed-off-by: Dimon Zhao <dimon.zhao@nebula-matrix.com>
> ---
I don't think this is safe. AI explains in more wordy detail
Review: [PATCH v1 1/1] net/nbl: allow MTU change when port is started
The PMD Rx buffers survive a live MTU change. nbl_res_alloc_rx_bufs()
and nbl_fill_rx_ring() post every descriptor with the full mbuf data
room, not an MTU-derived length, and nbl_res_txrx_recv_pkts() always
chains on num_buffers from the Rx extension header. In-flight
descriptors cannot be overrun by a larger MTU; oversize frames get
chained. What the patch drops without replacement is the scatter
contract with the application, and nothing shows the hardware side
of the MTU change is safe with queues running.
Error
-----
nbl_dev.c: nbl_mtu_set()
With the dev_started check gone, a running port accepts an MTU whose
frame no longer fits in one Rx buffer. scattered_rx is computed only
in nbl_res_alloc_rx_bufs() at dev_start. An application started with
MTU 1500, 2K mbufs and no RTE_ETH_RX_OFFLOAD_SCATTER can call
rte_eth_dev_set_mtu(port, 9000); the call succeeds, scattered_rx
stays 0, and the Rx burst starts returning multi-segment mbufs the
application never agreed to handle.
Replace the removed check with the one ixgbe_dev_mtu_set() uses:
refuse, on a started port, a frame size that needs scatter when
scatter is not already in use. NBL_ETH_OVERHEAD already covers two
VLAN tags, but the Rx extension header occupies the start of the
first buffer and must be counted, as nbl_res_alloc_rx_bufs() does:
if (dev_data->dev_started && !dev_data->scattered_rx &&
frame_size + <rx exthdr len> >
dev_data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)
return -EINVAL;
Warning
-------
Commit message
The PMD does not program MTU itself. nbl_disp_chan_set_mtu_req()
sends NBL_CHAN_MSG_MTU_SET to the kernel driver over the mailbox,
and the Rx queues are not quiesced around it. 75cdda36a4c5 blocked
this on a started port deliberately. The message needs to state what
MTU_SET changes in hardware (ingress length check only, or anything
in queue context), that frames in DMA while the limit changes are
handled, and that it was tested with the MTU raised and lowered
under traffic.
Fixes: / Cc: stable@dpdk.org
Returning -EBUSY on a started port is documented behaviour of
rte_eth_dev_set_mtu(), so the old code was not a bug. This adds a
capability. Drop both tags; changing MTU semantics in the 25.11 LTS
is not a backport.
nbl_dev.c: nbl_mtu_set(), unchanged context
dev_data->dev_conf.rxmode.mtu = frame_size;
rxmode.mtu is an L3 MTU; this stores mtu + NBL_ETH_OVERHEAD. The
value is returned by rte_eth_dev_conf_get(), so a conf_get then
rte_eth_dev_configure() round trip grows the MTU by 26 bytes each
cycle until max_mtu validation fails. It is also written before
set_mtu and not restored on failure. ethdev updates data->mtu on
success; delete the assignment. This one is a real bug: send it as
a separate patch ahead of this one, with Fixes: 75cdda36a4c5 and
Cc: stable@dpdk.org.
Info
----
Configured MTU never reaches hardware (pre-existing)
rte_eth_dev_configure() sets data->mtu from rxmode.mtu without
calling mtu_set, and nbl_dev_port_start()/nbl_dev_txrx_start() never
call disp_ops->set_mtu. An MTU requested at configure time takes
effect only if the application also calls rte_eth_dev_set_mtu().
dev_start should push data->mtu to hardware. Separate patch.
^ permalink raw reply [flat|nested] 3+ messages in thread