|WARNING| [v2,2/2] net/iavf: rename LLDP Tx devarg

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-14

# DPDK Patch Review

## PATCH 1/2: net/iavf: remove Tx LLDP dynfield method

### Errors

None.

### Warnings

**drivers/net/intel/iavf/iavf_ethdev.c:**

Release notes entry missing the version number of the release where `enable_ptype_lldp` was introduced.
The note says "This is considered safe because the old devarg name only featured in one release"
but the release notes don't identify which release introduced it,
making it harder for users to determine if they're affected.

### Info

**Overall architecture:**

The removal appears sound.
The patch correctly removes all vestiges of the dynfield method,
updates documentation, cleans up the tri-state to boolean conversion,
and provides a clear deprecation path.

**Code changes are clean:**
- `lldp_mode` tri-state properly reduced to `lldp_enabled` boolean
- All references to the dynfield offset removed
- AVX2/AVX512 vector paths correctly updated to use the boolean
- The warning when dynfield is detected is appropriate

**Documentation:**
The documentation updates clearly explain the change and migration path.

---

## PATCH 2/2: net/iavf: rename LLDP Tx devarg

### Errors

**drivers/net/intel/iavf/iavf_ethdev.c:**

The old devarg name `IAVF_ENABLE_PTYPE_LLDP_ARG` is still defined
and present in the `iavf_valid_args[]` array (lines 61-62 in patch context).
This means `rte_kvargs_process()` will accept it as valid before the explicit check rejects it.

If the intent is to hard-fail on the old name (which the code does),
then remove `IAVF_ENABLE_PTYPE_LLDP_ARG` from `iavf_valid_args[]` entirely.
Otherwise, `rte_devargs_parse()` will accept the argument,
and only the later explicit check will catch it --
this is inconsistent and may confuse users with two different error paths.

**Suggested fix:**
```c
static const char * const iavf_valid_args[] = {
	IAVF_PROTO_XTR_ARG,
	IAVF_QUANTA_SIZE_ARG,
	IAVF_MAX_LATENCY_ARG,
	IAVF_CMN_CPU_BUDGET_ARG,
	IAVF_ENABLE_AUTO_RECONFIG_ARG,
	IAVF_NO_POLL_ON_LINK_DOWN_ARG,
	IAVF_MBUF_CHECK_ARG,
	IAVF_ENABLE_LLDP_ARG,
	/* Remove IAVF_ENABLE_PTYPE_LLDP_ARG */
	NULL
};
```

And keep the explicit check to provide the helpful error message directing users to the new name.

### Warnings

None.

### Info

**Devarg rename rationale:**

The reasoning is sound -- "ptype" was an implementation detail
that became irrelevant after the dynfield removal.
Simplifying to `enable_lldp` is clearer.

**Error message:**

The error message on line 2528-2529 correctly directs users from the old name to the new one,
which is helpful for migration.

---

## Summary

**Patch 1/2:** Clean removal with appropriate warnings. Release notes could be more specific about version history.

**Patch 2/2:** One error to fix (remove old devarg from valid args array).
The explicit error check is good and should be kept for the helpful message,
but the array should not list the old name.