|WARNING| [v2,1/1] net/nbl: fix incorrect MTU stored in rxmode

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Tue Sep 29 09:17:40 CEST 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/170128

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-29

# DPDK Patch Review

## Patch: net/nbl: fix incorrect MTU stored in rxmode

### Summary
This patch fixes two issues in the nbl PMD's MTU set callback:
1. Removes write to `dev_conf.rxmode.mtu` (which becomes stale after configure)
2. Removes confusion between MTU and frame_size (frame_size = MTU + overhead)

### Review Findings

**ERRORS: None**

**WARNINGS: None**

**INFO:**

1. **Comment typo fix is good** (Info)
   Changed "port is start" to "port is started" - improves clarity.

2. **Missing Rx function re-selection** (Info - consider)
   When MTU changes, the PMD may need to switch between scalar and scattered Rx functions
   depending on whether the new frame size exceeds mbuf data room size.
   The patch removes the stale `rxmode.mtu` write and the MTU/frame_size confusion,
   but does not add Rx function re-selection logic.
   
   If the nbl driver has separate Rx burst functions for single-segment vs multi-segment receive,
   `nbl_mtu_set()` should call the Rx function selection logic after successfully calling
   `disp_ops->set_mtu()` to ensure the appropriate burst function is installed.
   
   Pattern to consider:
   ```c
   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 based on new frame size vs mbuf size */
   nbl_select_rx_function(eth_dev);  /* if such function exists */
   return 0;
   ```
   
   Note: This is only needed if the driver has multiple Rx implementations.
   If nbl always uses a single scattered-capable Rx function, this is not required.

### Correctness Assessment

The patch correctly addresses the two stated issues:
-  Removes write to stale `dev_conf.rxmode.mtu` (correct per AGENTS.md MTU section)
-  Removes confusion between MTU and frame_size (correct per AGENTS.md MTU section)
-  Error paths are correct (returns error code without side effects)
-  No resource leaks introduced
-  No use-after-free issues

### Style and Process Compliance

-  C coding style compliant
-  Appropriate Fixes tag with Cc: stable
-  Commit message explains both issues addressed
-  No forbidden tokens introduced
-  Code matches DPDK patterns for error handling

### Conclusion

**The patch is correct and ready to merge** with the caveat that the driver maintainer
should verify whether Rx function re-selection is needed for this PMD.
The two core fixes (stale rxmode.mtu write and MTU/frame_size confusion) are both correct
and follow DPDK guidelines.


More information about the test-report mailing list