|WARNING| reorder: promote experimental API to stable

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Summary
This patch promotes several experimental APIs in the reorder library to stable status. The APIs being promoted are: `rte_reorder_seqn()`, `rte_reorder_drain_up_to_seqn()`, `rte_reorder_min_seqn_set()`, and `rte_reorder_memory_footprint_get()`.

---

## Errors

### 1. Incorrect use of `RTE_EXPORT_SYMBOL` for `rte_reorder_seqn_dynfield_offset`

**File:** `lib/reorder/rte_reorder.c:38`

The patch changes:
```c
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_reorder_seqn_dynfield_offset, 20.11)
```
to:
```c
RTE_EXPORT_SYMBOL(rte_reorder_seqn_dynfield_offset)
```

**Problem:** `rte_reorder_seqn_dynfield_offset` is a global variable, not a function. The `RTE_EXPORT_SYMBOL` macro is designed for function symbols. For global variables that need to be exported, the variable should be declared in the header with appropriate visibility attributes, or the symbol should be handled differently in the version map generation.

**Fix:** Verify whether `RTE_EXPORT_SYMBOL` is appropriate for non-function symbols. If not, this may require special handling or the variable should remain in the version map with its current ABI status explicitly managed.

---

## Warnings

### 1. Return type mismatch in function documentation

**File:** `lib/reorder/rte_reorder.h:199`

```c
/**
 * ...
 * @return
 *   0 on success, a negative value otherwise.
 */
unsigned int
rte_reorder_min_seqn_set(struct rte_reorder_buffer *b, rte_reorder_seqn_t min_seqn);
```

**Problem:** The function returns `unsigned int` but the documentation says "0 on success, a negative value otherwise." An unsigned type cannot return negative values.

Looking at the implementation in `rte_reorder.c`, the function appears to actually return counts, not error codes. The documentation should be corrected to match the actual return semantics, or the return type should be changed to `int` if error returns are intended.

**Suggested fix:** Update the documentation to accurately describe what the function returns, or change the return type to `int` if the intention is to return error codes.

---

### 2. Incomplete ABI section in release notes

**File:** `doc/guides/rel_notes/release_26_11.rst`

The patch updates the API Changes section but does not update the ABI Changes section. Promoting experimental APIs to stable is an ABI change that should be documented.

**Suggested fix:** Add an entry to the ABI Changes section describing the promotion of these symbols from experimental to stable ABI.

---

## Info

### 1. Consider updating examples/tests if they exist

If there are examples or test code that conditionally use these APIs based on their experimental status, they should be updated to reflect the stable status. However, this cannot be determined from the patch alone.

---

## Positive Observations

1. **Release notes updated:** The patch correctly documents the API promotion in the release notes.
2. **Consistent changes:** All function declarations properly remove `__rte_experimental` markers.
3. **Export macro updates:** The `RTE_EXPORT_*` macros are consistently updated for the functions (though the variable case needs verification).
4. **Clean removal:** The documentation warnings are completely removed, not just commented out.

---

## Conclusion

The patch requires fixes for:
- The handling of `rte_reorder_seqn_dynfield_offset` export (verify correctness for variable symbols)
- Documentation mismatch for `rte_reorder_min_seqn_set()` return value
- ABI Changes section in release notes

The overall approach of promoting these APIs is sound given that they have been stable since the versions indicated (20.11 to 23.07).