|WARNING| [v1,1/1] net/ice: remove obsolete shared counter support

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Oct 19 17:46:32 CEST 2026


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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

**drivers/net/intel/ice/ice_fdir_filter.c: ice_fdir_counter_free() parameter marked `__rte_unused` removed but function still accepts unused parameter**

Line 369:
```c
ice_fdir_counter_free(struct ice_fdir_counter *counter)
```

The patch removes the `__rte_unused struct ice_pf *pf` parameter from `ice_fdir_counter_free()`.
However, the function still checks `if (!counter)` and returns early,
meaning the `counter` parameter itself could be NULL.
While this is likely an intentional defensive check rather than an unused parameter issue,
consider whether the function should assert non-NULL or document that NULL is acceptable.

This is a minor concern about API design clarity, not a correctness bug.

**Missing release notes**

This patch removes obsolete code paths (shared counter support that was superseded by indirect actions).
While this is technically a cleanup rather than a user-facing change,
the removal of fields from `struct ice_fdir_counter` is an internal ABI change.
Consider adding a brief note to the release notes documenting the removal of shared counter handling,
particularly since the commit message references that the API was removed in 92ef4b8f1688.

## INFO

**Commit message references a past API removal**

The commit message notes that the shared counter attribute was removed in commit 92ef4b8f1688,
and explicitly discusses whether a `Fixes:` tag is appropriate.
The reasoning (that the prior commit did not mandate immediate driver changes) is sound.
This is good documentation in the commit message.

**Code simplification is correct**

The removal of `ice_fdir_counter_shared_search()`, reference counting (`ref_cnt`),
and the shared counter fields (`shared`, `id`, `act_count`) is appropriate given that:
1. The FDIR path always allocates dedicated counters (never shared)
2. The shared counter API was removed from ethdev
3. The removed code paths were unreachable

The cleanup correctly:
- Removes the unused shared-counter lookup logic
- Simplifies `ice_fdir_counter_alloc()` by removing the `shared` and `id` parameters
- Simplifies `ice_fdir_counter_free()` by removing reference counting
- Removes the saved `rte_flow_action_count` structure from filter context
- Updates all call sites consistently

Review-Result: WARNING


More information about the test-report mailing list