|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