|WARNING| lib: fix mempool name prefixes
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Aug 12 13:27:22 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
# DPDK Patch Review
## Summary
This patch fixes mempool naming issues in the RIB and FIB libraries and adds length validation to prevent oversized names. The patch is generally well-structured, but there are a few issues to address.
---
## Errors
### 1. Off-by-one in name length calculation (rte_rib.c)
**File:** `lib/rib/rte_rib.c:433`
The sizeof operator on a string literal returns the size including the null terminator, making the limit calculation incorrect. The actual available space is `RTE_MEMPOOL_NAMESIZE - strlen(RIB_MEM_PREFIX)`.
```c
/* Current code - WRONG */
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));
...
}
/* Suggested fix */
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 - strlen(RIB_MEM_PREFIX) - 1);
...
}
```
### 2. Same off-by-one in rte_rib6.c
**File:** `lib/rib/rte_rib6.c:495`
Same issue as above - `sizeof()` includes the null terminator when it should use `strlen()`.
```c
/* Current code - WRONG */
if (unlikely(ret < 0 || ret >= (int)RTE_MEMPOOL_NAMESIZE)) {
RIB_LOG(ERR, "RIB6 name %s is too long, limit is %zu characters",
name, RTE_MEMPOOL_NAMESIZE - sizeof(RIB6_MEM_PREFIX));
...
}
/* Suggested fix */
if (unlikely(ret < 0 || ret >= (int)RTE_MEMPOOL_NAMESIZE)) {
RIB_LOG(ERR, "RIB6 name %s is too long, limit is %zu characters",
name, RTE_MEMPOOL_NAMESIZE - strlen(RIB6_MEM_PREFIX) - 1);
...
}
```
---
## Warnings
### 1. Missing NULL check after rte_rib_create() in rte_fib.c
**File:** `lib/fib/rte_fib.c:182`
While the code does check if `rib == NULL`, if the failure is due to `ENAMETOOLONG` from the new validation, the error message "Can not allocate RIB" is misleading. The error should be propagated more clearly or the check should happen before calling `rte_rib_create()`.
Suggested improvement: Add a pre-check for name length before creating the RIB, or update the error message to be more generic.
```c
/* Suggested approach */
if (strlen(name) > (RTE_MEMPOOL_NAMESIZE - strlen(FIB_MEM_PREFIX) - 1)) {
rte_errno = ENAMETOOLONG;
FIB_LOG(ERR, "FIB name %s is too long", name);
return NULL;
}
```
### 2. Same issue in rte_fib6.c
**File:** `lib/fib/rte_fib6.c:182`
Same as above - the error message doesn't reflect the possibility of `ENAMETOOLONG`.
---
## Info
### 1. Documentation could be more specific about new limits
**File:** `doc/guides/rel_notes/release_26_11.rst`
The release notes mention the name changes but don't document the new length restrictions. Consider adding a note about the maximum name lengths for user-facing RIB and FIB objects (17 chars for FIB, 15 for FIB6, 21 for RIB, 20 for RIB6 as mentioned in the commit message).
### 2. Inconsistent error message format
The error messages in `rte_rib.c` and `rte_rib6.c` report the limit in a slightly confusing way - they show the total MEMPOOL_NAMESIZE minus the prefix size, but users provide names for RIB/FIB objects, not mempools. It would be clearer to state "RIB name must not exceed X characters" where X is the actual user-facing limit.
---
## Correctness Review - No Issues Found
No resource leaks detected
All error paths properly handle allocated resources
No use-after-free conditions
Error codes are properly propagated via `rte_errno`
The new length validation prevents buffer overflows from `snprintf`
---
## Style Review - No Issues Found
Naming conventions followed correctly
No forbidden tokens used
Comment style is appropriate
Code formatting is consistent
---
## API/ABI Review
Changes are documented in release notes
No ABI breakage (naming changes are internal)
The new `ENAMETOOLONG` error is a reasonable API extension
---
**Overall Assessment:** The patch addresses a real bug and adds proper validation. The main issues are the off-by-one errors in the error messages and the potential for misleading error reporting when name length validation fails.
More information about the test-report
mailing list