|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.

## Warnings

### 1. Missing Functional Test for New Devarg

**drivers/net/intel/ice/ice_ethdev.c:2548-2549**
```c
ret = rte_kvargs_process(kvlist, ICE_AUTO_NEG_DISABLE_ARG,
			 &parse_bool, &ad->devargs.auto_neg_disable);
```

New driver parameters should have functional tests added to verify correct parsing and behavior. The patch adds `auto_neg_disable` devarg but does not include corresponding test coverage in `app/test/` or document test procedures.

### 2. Incomplete Error Path Handling

**drivers/net/intel/ice/ice_ethdev.c:2543-2549**
```c
ret = rte_kvargs_process(kvlist, ICE_LINK_STATE_ON_CLOSE,
			 &parse_link_state_on_close, &ad->devargs.link_state_on_close);
if (ret)
	goto bail;

ret = rte_kvargs_process(kvlist, ICE_AUTO_NEG_DISABLE_ARG,
			 &parse_bool, &ad->devargs.auto_neg_disable);
```

The second `rte_kvargs_process()` call does not check its return value before reaching the `bail` label. If it fails, `ret` is non-zero but there's no explicit `if (ret) goto bail;` check. The existing code falls through to `bail` regardless, which happens to be correct, but the pattern is inconsistent with the preceding error check and less clear. Add an explicit error check after the second call for consistency:

```c
ret = rte_kvargs_process(kvlist, ICE_AUTO_NEG_DISABLE_ARG,
			 &parse_bool, &ad->devargs.auto_neg_disable);
if (ret)
	goto bail;
```

### 3. Documentation Formatting: Definition List Preferred

**doc/guides/nics/ice.rst:110-117**

The `auto_neg_disable` documentation uses a bullet-list structure with a term followed by a description. RST definition lists produce clearer output for term/description pairs:

```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.
   For example::

      -a 80:00.0,auto_neg_disable=1
```

This should use RST definition list syntax with proper indentation under the term (though the existing format is acceptable, definition lists are preferred per AGENTS.md guidelines for this pattern).

---

Review-Result: WARNING