|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 13:55:49 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

None.

## WARNINGS

**1. Missing verification of MTU compatibility with current Rx configuration**

The patch removes the `dev_started` check from `nbl_mtu_set()`, allowing runtime MTU changes. However, the code does not verify that the new MTU is compatible with the current receive buffer configuration. When the port is running, the Rx queues have already been set up with specific mbuf sizes. If the new MTU produces a frame length exceeding the mbuf data room and scatter Rx is not enabled, packets will be silently dropped or truncated.

**Expected behavior:** The `mtu_set` callback should:
1. Calculate the maximum frame length from the new MTU plus Ethernet overhead
2. Compare against the configured mbuf data room size
3. If frame length exceeds mbuf size and scatter Rx is not enabled, either:
   - Return `-EINVAL` with an error message, or
   - Auto-enable scatter Rx and re-select the Rx burst function

```c
/* Suggested addition after validating mtu with hardware */
uint32_t frame_size = mtu + eth_overhead;
uint32_t buf_size = dev_data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM;

if (frame_size > buf_size && !dev_data->scattered_rx) {
    if (!(dev_info.rx_offload_capa & RTE_ETH_RX_OFFLOAD_SCATTER)) {
        NBL_LOG(ERR, "MTU %u requires scatter Rx which is not supported",
                mtu);
        return -EINVAL;
    }
    /* Auto-enable scatter and re-select Rx function */
    dev_data->dev_conf.rxmode.offloads |= RTE_ETH_RX_OFFLOAD_SCATTER;
    dev_data->scattered_rx = 1;
    /* Re-select Rx burst function for all queues */
    nbl_rx_queue_function_select(eth_dev);
}
```

Without this check, the PMD accepts an MTU that it cannot receive, violating the contract that `mtu_set` returning success means packets of that size will be handled.

---

**2. Missing re-selection of Rx burst function**

Even if the mbuf size check is added, when MTU changes at runtime and scatter Rx needs to be toggled (or was already enabled but is no longer needed), the Rx burst function must be re-selected. The existing code calls `disp_ops->set_mtu()` but does not call any function to update the Rx/Tx function pointers. If the PMD has separate scalar and scattered Rx implementations, they will continue using the old function, potentially dropping oversized packets or using a slower scattered path unnecessarily.

The fix must include a call to re-select the Rx (and possibly Tx) burst function after changing the MTU, similar to what would be done in `rx_queue_setup()`.

---

## INFO

None.

---

Review-Result: WARNING


More information about the test-report mailing list