|WARNING| net/ice: add explicit control of link auto-negotiation

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-30

# DPDK Patch Review

## Errors

None identified.

## Warnings

**Warning 1: Missing functional test**
The patch adds a new devarg `auto_neg_disable` but does not include a functional test for it.
New device configuration options should be tested in `app/test` or have testpmd integration to verify behavior.

**Suggested fix:**
Add a testpmd command to demonstrate/test the new parameter, or add a unit test that exercises the devarg parsing and link configuration with this option enabled/disabled.

**Warning 2: Documentation does not explain link flap scenario**
The commit message and RST documentation mention that link flapping occurs when "two ports are connected back to back and only one is set to a fixed speed," but this is not documented in the user-facing RST.
Users need to understand when to use `auto_neg_disable=1` versus the default.

**Suggested fix:**
In `doc/guides/nics/ice.rst`, add a sentence explaining when each setting is appropriate:
```rst
   When ``0`` (default), fixed-speed links use negotiation (e.g., Clause 37) compatible with peers that require it.
   When ``1``, negotiation is disabled; use this when both link peers have fixed speed without negotiation,
   or when link flapping occurs in back-to-back configurations with mismatched negotiation settings.
```

**Warning 3: Inconsistent formatting in release notes**
The release notes entry uses a bulleted sub-item format that is inconsistent with surrounding entries.
The second line starting with "By default" should be combined with the first or reformatted for consistency.

**Suggested fix:**
```rst
* **Updated Intel ice driver.**

  * Added ``auto_neg_disable`` devarg to disable hardware link negotiation for fixed-speed links;
    by default, fixed-speed links preserve the firmware's negotiation settings.
```

## Info

**Info 1: Code organization suggestion**
The `link_autoneg` variable initialization on line 5258 uses a double-negative logic (`!auto_neg_disable`) that could be clearer if the devarg were named `auto_neg_enable` or if an intermediate variable were used.

Current:
```c
const bool link_autoneg = !ad->devargs.auto_neg_disable ||
		(conf->link_speeds & RTE_ETH_LINK_SPEED_FIXED) == 0;
```

This is correct but slightly harder to read than:
```c
/* Auto-negotiation is enabled unless explicitly disabled for fixed speed */
const bool link_autoneg = (conf->link_speeds & RTE_ETH_LINK_SPEED_FIXED) == 0 ||
			  !ad->devargs.auto_neg_disable;
```

The reordered condition reads more naturally: "autoneg is on if speed is not fixed, or if speed is fixed but disable flag is not set."

**Info 2: Release notes placement**
The release notes entry is correctly placed in the current release file (`release_26_11.rst`) and includes both the new feature and the behavior change (firmware settings now preserved by default).

Review-Result: WARNING