|WARNING| [v2] net/ice: add explicit control of link auto-negotiation
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Nov 2 21:45:30 CET 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172350
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-02
# DPDK Patch Review
## ERRORS
None identified.
## WARNINGS
**1. Missing `parse_bool` error handling (driver control path)**
```c
ret = rte_kvargs_process(kvlist, ICE_AUTO_NEG_DISABLE_ARG,
&parse_bool, &ad->devargs.auto_neg_disable);
```
In `ice_parse_devargs()`, the `rte_kvargs_process()` call for `ICE_AUTO_NEG_DISABLE_ARG` does not check `ret` before falling through to `bail`. Other calls in the same function check `ret` and goto `bail` on error. This inconsistency means a malformed `auto_neg_disable` argument could silently fail to parse.
**Suggested fix:** Add error check after the new `rte_kvargs_process()` call:
```c
ret = rte_kvargs_process(kvlist, ICE_AUTO_NEG_DISABLE_ARG,
&parse_bool, &ad->devargs.auto_neg_disable);
if (ret)
goto bail;
```
**2. Release notes entry does not mention the `Fixes:` commit**
The release notes describe the new devarg but do not mention that this restores default behavior changed by commit `3dc2933866d9`. The notes say "By default, fixed-speed links now preserve the firmware's negotiation settings," which reverses the prior default. Documenting the behavior change more explicitly (e.g., "Restored default firmware negotiation for fixed-speed links; use `auto_neg_disable=1` to match the 26.07 behavior") would help users upgrading from the previous release.
## INFO
**Documentation structure (RST definition list pattern):**
In `ice.rst`, the `auto_neg_disable` documentation uses a bullet-like heading (`(default ``0``)`) followed by prose. For consistency with other devarg entries in DPDK docs, consider a definition list format:
```rst
auto_neg_disable (default ``0``)
Control hardware link negotiation when a fixed speed is requested with ``RTE_ETH_LINK_SPEED_FIXED``.
By default, the firmware's negotiation settings are preserved,
allowing a fixed-speed link to use negotiation such as Clause 37 when required by its peer.
Set ``auto_neg_disable=1`` to disable Clause 28, Clause 37 and Clause 73 negotiation.
This option has no effect when automatic speed selection is requested.
```
(Note: The current format is acceptable; this is a minor consistency suggestion.)
---
Review-Result: WARNING
More information about the test-report
mailing list