|WARNING| [v2,22/22] doc: add release note about rte_ethdev changes

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Aug 3 19:18:41 CEST 2026


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

_AI Code Review_

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

# DPDK Patch Review

## Summary
This patch series removes deprecated Flow Director API elements and breaks implicit header dependencies in `rte_ethdev.h`. The series makes 22 patches total, touching multiple drivers and libraries.

---

## Patch 1: drivers/net: remove unused include in dpaa
**File:** `drivers/net/dpaa/dpaa_ptp.c`, `drivers/net/dpaa2/dpaa2_ptp.c`

**Findings:** None. Clean removal of unused deprecated header.

---

## Patch 2: ethdev: use DPDK byte order conversion
**File:** `lib/ethdev/sff_8472.c`

**Findings:** None. Correct replacement of `ntohl()` with `rte_be_to_cpu_32()` and proper inclusion of `<rte_byteorder.h>`.

---

## Patch 3: net/ixgbe: remove experimental FDIR API
**Files:** `drivers/net/intel/ixgbe/ixgbe_ethdev.h`, `ixgbe_fdir.c`, `rte_pmd_ixgbe.c`, `rte_pmd_ixgbe.h`

**Findings:**

**Warning:**
- Missing release notes update for removal of experimental API (`rte_pmd_ixgbe_get_fdir_info`, `rte_pmd_ixgbe_get_fdir_stats`). The commit message states "These APIs are experimental, therefore they can be removed without additional warnings," but best practice is to document all API removals in release notes.

---

## Patch 4: net/i40e: remove experimental FDIR API
**Files:** `drivers/net/intel/i40e/i40e_ethdev.c`, `i40e_ethdev.h`, `i40e_fdir.c`, `i40e_flow.c`, `rte_pmd_i40e.c`, `rte_pmd_i40e.h`

**Findings:**

**Warning:**
- Same issue as patch 3: missing release notes for removal of `rte_pmd_i40e_get_fdir_info` and `rte_pmd_i40e_get_fdir_stats`.

---

## Patch 5: app/testpmd: remove support for flow director
**Files:** `app/test-pmd/cmdline.c`, `config.c`, `testpmd.h`, `doc/guides/testpmd_app_ug/testpmd_funcs.rst`

**Findings:** None. Testpmd CLI is not a stable API per guidelines, so removal is acceptable. Documentation properly updated.

---

## Patch 6: net/gve: include IP, UDP and TCP headers
**File:** `drivers/net/gve/gve_ethdev.h`

**Findings:** None. Correct fix for implicit header dependencies.

---

## Patch 7: crypto/dpaa_sec: include UDP header
**File:** `drivers/crypto/dpaa_sec/dpaa_sec.h`

**Findings:** None. Correct fix.

---

## Patch 8: net/nfp: break implicit dependency on rte_eth_ctrl.h
**Files:** `drivers/net/nfp/nfp_ethdev.c`, `nfp_net_common.h`

**Findings:**

**Warning:**
- The replacement macro `NFP_UINT64_BIT` is defined locally but uses `CHAR_BIT` from `<limits.h>`. The include of `<limits.h>` is added at the top of `nfp_ethdev.c`, which is correct. However, `NFP_UINT64_BIT` could collide with other code if this pattern is copied. Consider using `sizeof(uint64_t) * CHAR_BIT` inline instead of a macro to avoid namespace pollution.

---

## Patch 9: net/mana: include used network headers
**Files:** `drivers/net/mana/mana.c`, `tx.c`

**Findings:** None. Correct fix. The includes of `<sys/socket.h>` and `<netinet/in.h>` in `mana.c` are for standard networking code, which is appropriate.

---

## Patch 10: gro: include headers directly
**Files:** `lib/gro/gro_tcp4.c`, `gro_vxlan_tcp4.c`, `gro_vxlan_udp4.c`, `rte_gro.c`

**Findings:** None. Correct fixes.

---

## Patch 11: app/test: include headers directly
**Files:** `app/test-eventdev/test_pipeline_common.h`, `app/test/test_bpf.c`, `test_security_inline_macsec.c`, `test_security_inline_proto.c`

**Findings:**

**Warning:**
- In `test_bpf.c`, the replacement of `htons()` with `rte_cpu_to_be_16()` is correct, but the massive include list added by "iwyu" includes headers like `<stdlib.h>`, `<stdbool.h>`, `<stddef.h>`, `<stdalign.h>`, `<errno.h>`, `<limits.h>` that may not all be necessary. The patch should minimize includes to what is actually used. However, this is test code, so it's acceptable as a **Warning** rather than **Error**.

---

## Patch 12: node: get UDP header
**File:** `lib/node/udp4_input.c`

**Findings:** None. Correct fix.

---

## Patch 13: net/rnp: include network headers
**File:** `drivers/net/rnp/rnp_rxtx.c`

**Findings:** None. Correct fix.

---

## Patch 14: net/r8169: get network headers
**File:** `drivers/net/r8169/r8169_rxtx.c`

**Findings:** None. Correct fix.

---

## Patch 15: net/ngbe: include network protocol headers
**File:** `drivers/net/ngbe/ngbe_rxtx.c`

**Findings:** None. Correct fix.

---

## Patch 16: examples: include network headers
**Files:** `examples/l2fwd-macsec/main.c`, `l3fwd-graph/main.c`, `l3fwd/l3fwd.h`

**Findings:** None. Correct fixes.

---

## Patch 17: net/mlx5: include rte_flow as needed
**Files:** `drivers/net/mlx5/mlx5_ethdev.c`, `mlx5_flow.h`, `mlx5_txq.c`, `mlx5_utils.c`, `mlx5_utils.h`

**Findings:** None. Correct fixes.

---

## Patch 18: net/sfc: include rte_flow
**File:** `drivers/net/sfc/sfc_dp.c`

**Findings:** None. Correct fix.

---

## Patch 19: net/intel/common: include network headers
**File:** `drivers/net/intel/common/tx_scalar.h`

**Findings:** None. Correct fix.

---

## Patch 20: net/enetfec: add missing sys/types.h include
**File:** `drivers/net/enetfec/enet_regs.h`

**Findings:** None. Correct fix for BSD types (`uint`, `ushort`).

---

## Patch 21: ethdev, drivers: isolate flow director
**Files:** Multiple (19 files across drivers and lib/ethdev)

**Findings:**

**Error:**
- The new header `ethdev_fdir.h` is created but its relationship to the driver SDK is not clear. The commit message says "it does not have to include as many secondary headers," but the header itself includes `<rte_flow.h>`. This seems inconsistent--if the goal is to reduce dependencies, why does `ethdev_fdir.h` still pull in `rte_flow.h`?

**Warning:**
- This patch touches 19 files and creates a new internal header. While the approach is sound (isolating legacy FDIR), the mechanical correctness of each driver change should be verified by the driver maintainers. The patch looks correct but is too large to verify every driver's specific needs in review.

---

## Patch 22: doc: add release note about rte_ethdev changes
**File:** `doc/guides/rel_notes/release_26_11.rst`

**Findings:**

**Warning:**
- The release notes are added for the `rte_ethdev.h` changes, which is correct. However, the removal of experimental FDIR APIs from patches 3 and 4 is documented here but should have been noted when those APIs were actually removed in those earlier patches. This is a process issue--each API removal should update the release notes atomically.

**Info:**
- The note says "Removed some testpmd commands: The `fdir` option in `show port` and `clear port` were removed." This is fine since testpmd CLI is not stable.

---

## Summary of Findings

### Errors
1. **Patch 21:** `ethdev_fdir.h` still includes `<rte_flow.h>`, which contradicts the stated goal of reducing header dependencies.

### Warnings
1. **Patches 3, 4:** Missing release notes for removal of experimental FDIR APIs (though documented in patch 22, should be in the individual patches).
2. **Patch 8:** Local macro `NFP_UINT64_BIT` could namespace-collide; consider inline calculation.
3. **Patch 11:** Excessive includes added by "iwyu" in `test_bpf.c`; could be trimmed.
4. **Patch 21:** Large multi-driver change; requires per-driver verification by maintainers.

### Info
- Patch 22 documents removal of non-stable testpmd commands, which is acceptable.

---

## Recommendation
**Request Changes:**
- Fix the circular dependency in patch 21 (`ethdev_fdir.h` including `<rte_flow.h>`).
- Document experimental API removals in patches 3 and 4 directly (or confirm that patch 22 is sufficient).


More information about the test-report mailing list