|WARNING| [v1,1/1] net/nbl: allow MTU change when port is started
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Tue Oct 20 16:49:46 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171744
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-20
# DPDK Patch Review
## ERRORS
### 1. Missing MTU validation and scatter Rx handling
**File:** `drivers/net/nbl/nbl_dev/nbl_dev.c`
**Function:** `nbl_mtu_set()`
The patch removes the dev_started check but does not add proper MTU validation or Rx queue function reselection. When MTU is changed at runtime, the PMD must:
1. Validate the new MTU against mbuf size and scatter capability
2. Re-select the Rx burst function if the frame size now requires scatter Rx
**Missing validation:**
```c
int nbl_mtu_set(struct rte_eth_dev *eth_dev, uint16_t mtu)
{
struct nbl_adapter *adapter = ETH_DEV_TO_NBL_DEV_PF_PRIV(eth_dev);
struct nbl_dev_mgt *dev_mgt = NBL_ADAPTER_TO_DEV_MGT(adapter);
struct nbl_dispatch_ops *disp_ops = NBL_DEV_MGT_TO_DISP_OPS(dev_mgt);
int ret;
ret = disp_ops->set_mtu(NBL_DEV_MGT_TO_DISP_PRIV(dev_mgt), dev_mgt->net_dev->vsi_id, mtu);
if (ret)
return ret;
/* MISSING: frame size vs mbuf size check */
/* MISSING: scatter Rx validation */
/* MISSING: Rx function reselection if dev_started */
```
**Required additions:**
```c
int nbl_mtu_set(struct rte_eth_dev *eth_dev, uint16_t mtu)
{
struct rte_eth_dev_data *dev_data = eth_dev->data;
struct nbl_adapter *adapter = ETH_DEV_TO_NBL_DEV_PF_PRIV(eth_dev);
struct nbl_dev_mgt *dev_mgt = NBL_ADAPTER_TO_DEV_MGT(adapter);
struct nbl_dispatch_ops *disp_ops = NBL_DEV_MGT_TO_DISP_OPS(dev_mgt);
uint32_t frame_size, overhead;
uint16_t mbuf_data_size;
int ret;
/* Calculate frame size (MTU + Ethernet overhead) */
overhead = /* device-specific overhead from dev_info or internal field */;
frame_size = mtu + overhead;
/* Check if scatter Rx is needed */
if (dev_data->nb_rx_queues > 0) {
struct rte_eth_rxq_info qinfo;
ret = rte_eth_rx_queue_info_get(eth_dev, 0, &qinfo);
if (ret == 0) {
mbuf_data_size = rte_pktmbuf_data_room_size(qinfo.mp) -
RTE_PKTMBUF_HEADROOM;
if (frame_size > mbuf_data_size &&
!dev_data->scattered_rx) {
if (!(dev_data->dev_conf.rxmode.offloads &
RTE_ETH_RX_OFFLOAD_SCATTER))
return -EINVAL;
}
}
}
ret = disp_ops->set_mtu(NBL_DEV_MGT_TO_DISP_PRIV(dev_mgt),
dev_mgt->net_dev->vsi_id, mtu);
if (ret)
return ret;
/* Re-select Rx function if port is running */
if (dev_data->dev_started) {
/* Call internal function to update Rx burst function
* based on new frame size vs mbuf size */
nbl_select_rx_function(eth_dev);
}
return 0;
}
```
This is a correctness bug because:
- The PMD will silently use the wrong Rx function (non-scattered) for oversized packets
- Packets larger than the mbuf data room will be truncated or dropped
- The Rx function selection becomes stale after MTU change
---
## WARNINGS
None.
---
## INFO
### Removed dev_started check may require updated release notes
If this patch enables runtime MTU changes where they were previously forbidden, this is a feature enhancement and should be documented in the release notes under the driver's section. However, based on the evidence-only rule, I cannot determine whether this warrants a release note without seeing the rest of the driver implementation and whether this is a new capability.
---
Review-Result: ERROR
More information about the test-report
mailing list