|WARNING| [v4,4/4] ethdev: fix promoted flow metadata symbols

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/4: eal: fix macro for versioned experimental symbol

### Errors

**Missing semicolon in macro (correctness)**
The patch correctly identifies and fixes a missing semicolon after the `__asm__` block in `RTE_VERSION_EXPERIMENTAL_SYMBOL`. This is a genuine compilation error with clang. The fix is correct.

No issues found with the fix itself.

---

## Patch 2/4: build: support function versioning for drivers

### Errors

**Missing RTE_EXPORT_* macros in shared library build**
When `use_function_versioning` is true, the patch rebuilds sources with `-DRTE_BUILD_SHARED_LIB` for the shared library. However, this only works if the driver sources use the new `RTE_EXPORT_*` macros. If driver functions still rely on the version map auto-generation from `RTE_EXPORT_SYMBOL()` annotations in static builds, rebuilding with `-DRTE_BUILD_SHARED_LIB` will produce a shared library with no exported symbols (the export macros are conditional on this flag). The patch should document that drivers enabling `use_function_versioning` must use the new `RTE_EXPORT_*` macros, or verify that mlx5 already does.

---

## Patch 3/4: net/mlx5: fix stabilized function versions

### Errors

**Recursive function calls in versioning wrappers**
The experimental symbol wrappers call themselves recursively instead of the DPDK_26 version:

```c
RTE_VERSION_EXPERIMENTAL_SYMBOL(int, rte_pmd_mlx5_driver_event_cb_register,
                               (rte_pmd_mlx5_driver_event_callback_t cb, void *opaque))
{
    return rte_pmd_mlx5_driver_event_cb_register(cb, opaque);  // calls itself
}
```

This will cause infinite recursion. The wrapper should call the `_v26` symbol:

```c
RTE_VERSION_EXPERIMENTAL_SYMBOL(int, rte_pmd_mlx5_driver_event_cb_register,
                               (rte_pmd_mlx5_driver_event_callback_t cb, void *opaque))
{
    return rte_pmd_mlx5_driver_event_cb_register_v26(cb, opaque);
}
```

The `RTE_DEFAULT_SYMBOL` macro generates `funcname_v26`, so the experimental wrapper must reference that.

This applies to all four functions in the patch:
- `rte_pmd_mlx5_driver_event_cb_register`
- `rte_pmd_mlx5_driver_event_cb_unregister`
- `rte_pmd_mlx5_disable_steering`
- `rte_pmd_mlx5_enable_steering`

**Missing RTE_EXPORT_* macros**
The default versions use `RTE_DEFAULT_SYMBOL` but there are no `RTE_EXPORT_SYMBOL` annotations. With the new export system, the shared library needs `RTE_EXPORT_SYMBOL(rte_pmd_mlx5_driver_event_cb_register_v26)` etc. The versioning macros define the symbols but do not export them.

---

## Patch 4/4: ethdev: fix promoted flow metadata symbols

### Errors

**Recursive function call in versioning wrapper**
Same issue as patch 3:

```c
RTE_VERSION_EXPERIMENTAL_SYMBOL(int, rte_flow_dynf_metadata_register, (void))
{
    return rte_flow_dynf_metadata_register();  // calls itself, infinite recursion
}
```

Should be:
```c
RTE_VERSION_EXPERIMENTAL_SYMBOL(int, rte_flow_dynf_metadata_register, (void))
{
    return rte_flow_dynf_metadata_register_v26();
}
```

**Missing RTE_EXPORT_SYMBOL for versioned default**
The default version uses `RTE_DEFAULT_SYMBOL(26, ...)` but there's no `RTE_EXPORT_SYMBOL` to actually export `rte_flow_dynf_metadata_register_v26` from the shared library.

**Global variable versioning approach may not work as intended**
The patch uses `RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_flow_dynf_metadata_offs, 19.11)` on global variables. The `RTE_EXPORT_EXPERIMENTAL_SYMBOL` macro is designed for functions (it uses `.symver` directives intended for function symbols). Applying it to global variables may produce incorrect version map entries or linker errors. The typical pattern for versioned global variables is to create two separate variables (`var_exp` and `var_v26`) and alias them with `.symver`, not reuse the same variable name for both versions. Verify that this approach produces correct symbol versioning for data symbols.

---

## Warnings

**Patch 2: Windows compatibility message**
The message `'@0@: Function versioning is not supported by Windows.'` is informational but could be clearer. Since `use_function_versioning` is explicitly set by the driver, the message should indicate this is expected and not an error. Consider: `'@0@: Function versioning requested but not supported on Windows'`.

**Patch 3: No release notes**
Adding function versioning to stabilized mlx5 API is an ABI fix. This should be documented in the release notes (though it may be covered by patch 4's Bugzilla entry).

**Patch 4: Variable symbol versioning in 26.11**
The commit message states: "EXPERIMENTAL version will be removed from these global variables in 26.11 release cycle on next ABI version bump." This implies DPDK_26 versions of the variables will be created at that time. Document the plan for transitioning these variables (will they use `RTE_VERSION_SYMBOL`? Will the experimental alias be dropped?).

---

## Summary

**Critical issues (must fix):**
1. Recursive function calls in experimental symbol wrappers (patches 3 & 4) - will cause stack overflow
2. Missing `RTE_EXPORT_SYMBOL` annotations for versioned functions

**Review needed:**
- Global variable versioning approach in patch 4 may not work correctly with the function-oriented versioning macros
- Patch 2 assumes drivers use new export macros when rebuilding for shared libs