|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