|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