|WARNING| [v2] fib, rib: fix name prefixes
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Aug 13 10:03:58 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/167634
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-13
# DPDK Patch Review
## Overview
This patch fixes naming prefixes for RIB and FIB memory objects to avoid double-prefixing (MP_MP_<name>) and adds length validation to prevent silent failures.
---
## ERRORS
### 1. sizeof() on string literal yields pointer size, not string length
**File:** `lib/rib/rte_rib.c:434`
```c
if (unlikely(ret < 0 || ret >= (int)RTE_MEMPOOL_NAMESIZE)) {
RIB_LOG(ERR, "RIB name %s is too long, limit is %zu characters",
name, RTE_MEMPOOL_NAMESIZE - sizeof(RIB_MEM_PREFIX));
```
`sizeof(RIB_MEM_PREFIX)` on a string literal `"RIB_"` yields 5 (includes null terminator), which is correct. However, this should use `strlen()` to make intent clearer and avoid confusion, or better, use the constant directly: `RTE_MEMPOOL_NAMESIZE - (sizeof(RIB_MEM_PREFIX) - 1)` where the `-1` accounts for the null terminator being replaced.
**Same issue in:**
- `lib/rib/rte_rib6.c:489` with `RIB6_MEM_PREFIX`
**Suggested fix:**
```c
RIB_LOG(ERR, "RIB name %s is too long, limit is %zu characters",
name, RTE_MEMPOOL_NAMESIZE - (sizeof(RIB_MEM_PREFIX) - 1));
```
This makes it clear that we're accounting for the prefix length minus the null terminator that gets overwritten during concatenation.
---
## WARNINGS
### 1. Release notes formatting inconsistency
**File:** `doc/guides/rel_notes/release_26_11.rst:104`
The release notes have a spacing inconsistency:
```rst
As the prefixes above are added on top of the name,the new maximum length of
```
Missing space after comma: "name,the" should be "name, the".
---
### 2. Inconsistent TAILQ naming
**File:** `lib/rib/rte_rib6.c:26-28`
```c
TAILQ_HEAD(rte_rib6_list, rte_tailq_entry);
static struct rte_tailq_elem rte_rib6_tailq = {
.name = "RTE_RIB6",
```
This is existing code, not introduced by the patch, but the TAILQ is named "RTE_RIB6" while RIB (IPv4) uses a registered TAILQ. This inconsistency is pre-existing but worth noting. **Do not flag** as it's outside the scope of this patch.
---
## INFO
### 1. Name length documentation could be clearer
**File:** `doc/guides/rel_notes/release_26_11.rst:93-108`
The documentation states "the new maximum length of a name are the following" but could more clearly explain that these limits exist because the prefix is prepended and the total must fit in `RTE_MEMPOOL_NAMESIZE` (32 bytes).
**Suggested improvement:**
```rst
* rib, fib: The name of a RIB, RIB6, FIB or FIB6 is used to derive the name of
its node mempool, which must fit in ``RTE_MEMPOOL_NAMESIZE`` (32 bytes).
Since prefixes are prepended (``RIB_``, ``RIB6_``, ``FIB_``, ``FIB6_``),
the maximum lengths are:
* RIB: 21 characters (32 - 4 - 1 - 6 for "RIB_" + name + null + "FIB_")
* RIB6: 20 characters (32 - 5 - 1 - 6)
* FIB: 17 characters (after FIB adds its prefix to RIB name)
* FIB6: 15 characters (after FIB6 adds its prefix to RIB6 name)
```
However, the current documentation is acceptable, so this is just a suggestion for clarity.
---
### 2. Error message calculation
The error messages in the RIB/RIB6 create functions compute the limit as:
```c
RTE_MEMPOOL_NAMESIZE - sizeof(RIB_MEM_PREFIX)
```
This reports the limit as 27 characters for RIB (32 - 5), but the actual usable length is 26 because we also need space for the null terminator. The check itself is correct (it validates against `RTE_MEMPOOL_NAMESIZE`), but the error message might confuse users who count characters.
**Current behavior:** Correct validation, slightly misleading error message.
**Impact:** Minor - users will figure it out when they shorten the name by one more character.
---
## SUMMARY
**Critical Issues:** 0
**Errors:** 1 (sizeof() usage in error message could be clearer)
**Warnings:** 1 (release notes typo)
**Info:** 2 (documentation clarity suggestions)
The patch correctly fixes the double-prefix bug and adds proper length validation. The main issue is a minor clarity problem in the error message calculation using `sizeof()` on the prefix constant, and a typo in the release notes.
More information about the test-report
mailing list