|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