|WARNING| lib: fix mempool name prefixes
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Aug 12 13:28:20 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/167553
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-12
# Review of DPDK Patch: lib: fix mempool name prefixes
## Summary
This patch fixes naming inconsistencies in RIB/FIB libraries where mempool names were incorrectly prefixed, resulting in names like "MP_MP_<name>". The patch adds proper prefixing ("RIB_", "RIB6_", "FIB_", "FIB6_") and validates name length against mempool constraints.
---
## Errors
### 1. Incorrect length limit calculation in error message
**File:** `lib/rib/rte_rib.c`, `lib/rib/rte_rib6.c`
**Issue:**
```c
RIB_LOG(ERR, "RIB name %s is too long, limit is %zu characters",
name, RTE_MEMPOOL_NAMESIZE - sizeof(RIB_MEM_PREFIX));
```
The `sizeof(RIB_MEM_PREFIX)` returns the size of the string literal including the null terminator (5 bytes for "RIB_"), but the calculation should account for the prefix string length excluding the null (4 bytes), since only one null terminator is needed in the final name.
**Fix:**
```c
RIB_LOG(ERR, "RIB name %s is too long, limit is %zu characters",
name, RTE_MEMPOOL_NAMESIZE - strlen(RIB_MEM_PREFIX) - 1);
```
Same issue exists in `rte_rib6.c` with `RIB6_MEM_PREFIX`.
---
## Warnings
### 1. Missing Cc: stable at dpdk.org
This patch fixes bugs (Bugzilla ID: 1981, 1982) and has explicit `Fixes:` tags. It should include `Cc: stable at dpdk.org` in the commit message to ensure backporting to stable releases.
### 2. Inconsistent mempool creation failure messages
**Files:** `lib/fib/rte_fib.c`, `lib/fib/rte_fib6.c`
The existing error messages after RIB creation failure reference the prefixed name:
```c
FIB_LOG(ERR, "Can not allocate RIB %s", mem_name);
```
However, when `rte_rib_create()` itself fails due to name length validation, it logs the original (unprefixed) name. This could be confusing for debugging. The FIB layer should log the original name the user provided for clarity:
```c
FIB_LOG(ERR, "Can not allocate RIB for FIB %s", name);
```
### 3. Release notes formatting
**File:** `doc/guides/rel_notes/release_26_11.rst`
The release notes correctly document the API changes, but the wording could be more precise. The current text:
```
* rib: The node mempool created by ``rte_rib_create()`` and ``rte_rib6_create()``
is now named ``RIB_<name>`` and ``RIB6_<name>`` instead of ``MP_<name>``.
```
Could clarify that this affects **internal** naming (the mempool is not directly accessible to users):
```
* rib: The internal node mempool created by ``rte_rib_create()`` and ``rte_rib6_create()``
is now named ``RIB_<name>`` and ``RIB6_<name>`` instead of ``MP_<name>``.
This is an internal implementation detail and does not affect API usage.
```
Similarly for the FIB note, clarify that the RIB is an internal component.
---
## Info
### 1. Consider documenting name length limits
The patch introduces effective name length limits (21 chars for RIB, 20 for RIB6, 17 for FIB, 15 for FIB6) but these are not documented in the API headers. Consider adding a note in the Doxygen comments for `rte_rib_create()`, `rte_rib6_create()`, `rte_fib_create()`, and `rte_fib6_create()` about the practical name length constraints.
### 2. Typo in commit message
The commit message contains: "and return ENAMETOOLONG instead. B ecause the prefixes"
There's an errant space: "B ecause" should be "Because".
---
## Positive Observations
1. **Correctness:** Error handling for name length validation is properly implemented with early return and appropriate `rte_errno` setting
2. **Testing:** Unit test names have been correctly shortened to comply with new limits
3. **Consistency:** The prefixing pattern is consistently applied across RIB/RIB6 and FIB/FIB6
4. **Documentation:** Release notes properly document the user-visible API changes
---
## Conclusion
The patch addresses the reported bugs correctly. The primary issue is the error message calculation which would report an incorrect character limit to users. After fixing that and optionally addressing the warnings, the patch should be ready for merge.
More information about the test-report
mailing list