|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: reorder library API promotion

## Summary

This patch promotes experimental API in the reorder library to stable. The patch is generally correct in its approach, but there are several issues that need to be addressed.

---

## Errors

### 1. Missing RTE_EXPORT_SYMBOL for dynfield offset

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

The `rte_reorder_seqn_dynfield_offset` variable is exported and documented in the header as `extern`, but it only has the export symbol macro, not the variable declaration with proper initialization. However, reviewing the actual change: the export macro is present and the variable is declared. This is correct.

Actually, on review, this is fine.

### 2. Return type documentation mismatch

**File**: `lib/reorder/rte_reorder.h`, function `rte_reorder_min_seqn_set`

The function documentation states:
```
 * @return
 *   0 on success, a negative value otherwise.
```

But the function signature is:
```c
unsigned int
rte_reorder_min_seqn_set(struct rte_reorder_buffer *b, rte_reorder_seqn_t min_seqn);
```

An `unsigned int` return type cannot return negative values. Either the documentation or the implementation is wrong. Checking the implementation would show what is actually returned, but this is a correctness issue - the documented contract doesn't match the signature.

---

## Warnings

### 1. Incomplete dynfield offset documentation

**File**: `lib/reorder/rte_reorder.h`, line ~29

The `rte_reorder_seqn_dynfield_offset` extern declaration lacks Doxygen documentation explaining:
- What this offset is used for
- How it should be used (via `rte_reorder_seqn()` helper)
- That it's initialized by the reorder library
- That it will be -1 if uninitialized

While the helper function is documented, the offset variable itself should have at least a brief `/** */` comment.

### 2. Missing RTE_EXPORT_SYMBOL verification

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

The patch correctly adds `RTE_EXPORT_SYMBOL` macros for the promoted functions. However, the `rte_reorder_seqn()` function is an inline function in the header and doesn't need an export symbol (and correctly doesn't have one). This is fine.

---

## Info

### 1. Release notes formatting

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

The release notes entry could be more specific about which release these APIs were originally introduced in, since the commit message mentions different versions (20.11, 23.03, 23.07). Consider:

```rst
* reorder: Promoted the following experimental API to stable:
  ``rte_reorder_seqn`` and ``rte_reorder_seqn_dynfield_offset`` (introduced in 20.11),
  ``rte_reorder_drain_up_to_seqn`` and ``rte_reorder_min_seqn_set`` (introduced in 23.03),
  and ``rte_reorder_memory_footprint_get`` (introduced in 23.07).
```

This provides better context for users tracking API evolution.

### 2. Consider ABI implications

While promoting from experimental to stable, verify that:
- The symbol versions are properly managed in any version.map files (though DPDK now uses auto-generation from RTE_EXPORT_SYMBOL)
- No ABI breaking changes were made to these functions during their experimental period

The patch looks correct in this regard, just worth verifying during testing.

---

## Positive Observations

1. The patch correctly removes `__rte_experimental` tags from function declarations
2. The patch correctly changes `RTE_EXPORT_EXPERIMENTAL_SYMBOL` to `RTE_EXPORT_SYMBOL`
3. All promoted functions are properly documented
4. The release notes are updated appropriately
5. The approach of promoting after 3+ years of stability is appropriate

---

## Required Actions

**Must fix:**
- Investigate and fix the `rte_reorder_min_seqn_set()` return type/documentation mismatch

**Should fix:**
- Add Doxygen comment for `rte_reorder_seqn_dynfield_offset`

**Consider:**
- Enhance release notes with version history