|WARNING| [v4,23/23] doc: add release note about rte_ethdev changes

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

I'll review this DPDK patch series against the guidelines in AGENTS.md. This is a large series (23 patches) focused on removing deprecated Flow Director API and cleaning up header dependencies in `rte_ethdev.h`.

## Review Summary

The patch series makes significant cleanup improvements to DPDK's header structure and removes deprecated Flow Director functionality. I found several style issues but no correctness bugs.

---

## Patch 01/23: drivers/net: remove unused include in dpaa and dpaa2

**No issues found.**

Simple removal of deprecated `rte_eth_ctrl.h` include from two files that don't use it.

---

## Patch 02/23: ethdev: use DPDK byte order conversion

**No issues found.**

Good change replacing `ntohl()` with `rte_be_to_cpu_32()` for consistency.

---

## Patch 03/23: net/ixgbe: remove experimental FDIR API

**Info:**

Patch removes 188 lines of experimental API (`rte_pmd_ixgbe_get_fdir_info`, `rte_pmd_ixgbe_get_fdir_stats`). Since these are marked experimental, removal is allowed per DPDK policy. However:

- Consider whether users depend on these functions. The functions were added in 20.08 (August 2020), so they've existed for 6 years.
- No deprecation notice was added in advance. Per DPDK policy, experimental API can be removed without notice, but advance warning is courteous for long-lived APIs.

This is procedurally correct but worth considering user impact.

---

## Patch 04/23: net/i40e: remove experimental FDIR API

**Info:**

Same considerations as patch 03/23 - experimental APIs added in 20.08, now removed after 6 years.

---

## Patch 05/23: app/testpmd: remove support for flow director

**Info:**

Removes testpmd CLI commands `show port fdir` and `clear port fdir`. The commit message states "testpmd CLI is not a stable API" which is correct - testpmd commands are not covered by ABI/API guarantees. However, scripts and test automation may depend on these commands. Consider:

- Impact on existing test suites
- Whether deprecation notice should have been added to release notes in advance

Procedurally correct but disruptive.

---

## Patch 06/23: app/testpmd: move str_to_flowtype to i40e

**No issues found.**

Good refactoring - moves helper function to its only remaining user.

---

## Patch 07/23: app/test: include headers directly

**No issues found.**

Fixes include dependencies. Good hygiene.

---

## Patch 08/23: gro: include headers directly

**No issues found.**

Good cleanup using `iwyu` to identify required headers.

---

## Patch 09/23: crypto/dpaa_sec: include UDP header

**No issues found.**

Simple fix for missing include.

---

## Patch 10/23: net/gve: include UDP, SCTP and TCP headers

**No issues found.**

Good use of `iwyu` to ensure complete includes.

---

## Patch 11/23: net/nfp: break implicit dependency on rte_eth_ctrl.h

**Warning:**

```c
#define NFP_UINT64_BIT (CHAR_BIT * sizeof(uint64_t))
```

This definition duplicates a removed symbol. The name `NFP_UINT64_BIT` follows DPDK naming (driver prefix) which is good. However, the definition should use `RTE_` utilities if available. DPDK provides `RTE_SIZEOF_FIELD()` and related macros. Consider whether a DPDK standard macro exists for this.

Actually, this is simply the number of bits in `uint64_t`, which is always 64. The expression `CHAR_BIT * sizeof(uint64_t)` is overly complex. Better:

```c
#define NFP_UINT64_BIT 64
```

Or use the standard definition if available in DPDK headers.

---

## Patch 12/23: net/mana: include used network headers

**No issues found.**

Good use of `iwyu`. The added includes are comprehensive.

---

## Patch 13/23: node: get UDP header

**No issues found.**

Good cleanup using `iwyu`.

---

## Patch 14/23: net/rnp: include network headers

**No issues found.**

Simple include additions.

---

## Patch 15/23: net/r8169: get network headers

**No issues found.**

Good replacement of `ntohs()` with `rte_be_to_cpu_16()` for consistency.

---

## Patch 16/23: net/ngbe: include network protocol headers

**No issues found.**

Simple include additions.

---

## Patch 17/23: examples: include network headers

**Warning:**

In `examples/l3fwd/l3fwd.h`:

```c
#include <rte_ip.h>

```

Extra blank line after the include. Not critical, but inconsistent with DPDK style (one blank line between groups).

---

## Patch 18/23: net/mlx5: include rte_flow as needed

**No issues found.**

Good cleanup ensuring explicit includes.

---

## Patch 19/23: net/sfc: include rte_flow

**No issues found.**

Simple fix.

---

## Patch 20/23: net/intel/common: include network headers

**No issues found.**

Simple include additions.

---

## Patch 21/23: net/enetfec: add missing sys/types.h include

**Warning:**

The commit message states:
> The enet_regs.h header uses BSD types 'uint' and 'ushort'

The use of `uint` and `ushort` (BSD-style type names) in DPDK code is questionable. DPDK coding standards prefer explicit `uint32_t`, `uint16_t`, etc. The header `enet_regs.h` should be updated to use standard integer types rather than relying on BSD typedefs from `sys/types.h`.

Current fix is acceptable as a minimal change, but the driver should be updated to avoid BSD types in future cleanup.

---

## Patch 22/23: ethdev, drivers: isolate flow director

**Error:**

In `lib/ethdev/ethdev_fdir.h`:

```c
/**
 * @file
 *
 * Ethernet device definitions for legacy Flow Director support.
 * This API is maintained only for existing driver compatibility.
 * New drivers should use the rte_flow API (rte_flow.h) instead.
 *
 * This header is part of the driver SDK and is installed for drivers
 * that still depend on legacy FDIR structures. Do not use in new drivers.
 */

#include <rte_flow.h>
```

**Missing header guard at start of file.** The original `rte_eth_ctrl.h` had:

```c
#ifndef _RTE_ETH_CTRL_H_
#define _RTE_ETH_CTRL_H_
```

The new file `ethdev_fdir.h` is missing the `#ifndef` / `#define` at the top. The header guard is at the end of the moved content but not at the beginning. This will cause compilation issues if the header is included multiple times.

**Fix:** Move the header guard to the top:

```c
#ifndef _ETHDEV_FDIR_H_
#define _ETHDEV_FDIR_H_

/**
 * @file
 *
 * Ethernet device definitions for legacy Flow Director support.
 * This API is maintained only for existing driver compatibility.
 * New drivers should use the rte_flow API (rte_flow.h) instead.
 *
 * This header is part of the driver SDK and is installed for drivers
 * that still depend on legacy FDIR structures. Do not use in new drivers.
 */

#include <rte_flow.h>
```

---

**Warning:**

The rename from `rte_eth_ctrl.h` to `ethdev_fdir.h` changes the naming convention:
- Old: `RTE_ETH_CTRL_H_` (matches `rte_eth_ctrl.h`)
- New: `_ETHDEV_FDIR_H_` (matches `ethdev_fdir.h`)

This is intentional but creates inconsistency in guard naming style. DPDK doesn't have a strict standard here, but the change is worth noting.

---

**Warning:**

In `lib/ethdev/rte_ethdev.h`:

```c
#include <rte_ether.h>
#include <rte_common.h>
#include <rte_config.h>
#include <rte_mbuf_history.h>
```

The commit adds `rte_mbuf_history.h` to the includes. This is not mentioned in the commit message. The commit message states:

> Change rte_ethdev.h to include the headers required
> which it was previously getting from rte_eth_ctrl.h -> rte_flow.h.

But `rte_mbuf_history.h` is not a network header and was not coming from `rte_flow.h`. This appears to be an unrelated include addition that should be explained or removed.

---

## Patch 23/23: doc: add release note about rte_ethdev changes

**Warning:**

The release note states:

```
* Removed legacy flow director related API and files.

  * Removed experimental APIs in:

    * ixgbe: ``rte_pmd_ixgbe_get_fdir_info``, ``rte_pmd_ixgbe_get_fdir_stats``
    * i40e: ``rte_pmd_i40e_get_fdir_info``, ``rte_pmd_i40e_get_fdir_stats``
```

These were experimental APIs, which is stated in patches 03 and 04. The release note should mention this:

```
  * Removed experimental APIs in:

    * ixgbe: ``rte_pmd_ixgbe_get_fdir_info``, ``rte_pmd_ixgbe_get_fdir_stats`` (experimental since 20.08)
    * i40e: ``rte_pmd_i40e_get_fdir_info``, ``rte_pmd_i40e_get_fdir_stats`` (experimental since 20.08)
```

This clarifies that removal is permitted under DPDK policy but also acknowledges the functions existed for 6 years.

---

**Info:**

The "API Changes" section states:

```
* **Changed ethdev include files.**

  * Dropped inclusion of ``rte_flow.h`` in ``rte_ethdev.h``.
    Applications using flow API should include it directly.
  * Network headers ``rte_ip.h``, ``rte_udp.h``, ``rte_tcp.h`` and ``rte_sctp.h``
    are no longer indirectly included by ``rte_ethdev.h``.
    Applications should include network headers as needed.
  * Legacy flow director definitions are no longer included by ``rte_ethdev.h``.
```

This is a breaking change for applications that transitively depended on these includes. While technically not an ABI break, it's an API break in practice. Applications will fail to compile if they used `rte_flow` types without including `rte_flow.h`.

Consider adding a "Migration Guide" subsection with examples:

```
**Migration Guide:**

  If your application uses rte_flow API:
    #include <rte_flow.h>

  If your application uses IP/TCP/UDP/SCTP headers:
    #include <rte_ip.h>
    #include <rte_tcp.h>
    #include <rte_udp.h>
    #include <rte_sctp.h>
```

This would help application developers.

---

## Summary

**Errors:**
- Patch 22: Missing header guard at start of `ethdev_fdir.h`

**Warnings:**
- Patch 11: `NFP_UINT64_BIT` could be simplified to constant `64`
- Patch 17: Extra blank line in `l3fwd.h`
- Patch 21: Use of BSD types `uint`/`ushort` should be replaced with standard types in future cleanup
- Patch 22: Unexplained addition of `rte_mbuf_history.h` include
- Patch 23: Should note experimental status and version for removed APIs

**Info:**
- Patches 03, 04: Experimental APIs removed after 6 years - disruptive but permitted
- Patch 05: Testpmd command removal may impact scripts
- Patch 23: Consider adding migration guide for API changes

The series is fundamentally sound but has one critical error (header guard) and several quality issues that should be addressed.