|WARNING| ipc: increase hotplug devargs limit

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Tue Nov 3 18:43:48 CET 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172603

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-03

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

### 1. Missing release notes update
**File:** Not present in patch
**Issue:** This patch changes the maximum devargs length from 127 to 247 bytes, which is a user-visible change to API behavior. Such a change should be documented in release notes.
**Suggested fix:** Add an entry to `doc/guides/rel_notes/release_XX_XX.rst` under an appropriate section (e.g., "API Changes" or "New Features") documenting the increased devargs limit.

### 2. Format specifier type mismatch in log message
**File:** lib/eal/common/eal_common_dev.c
**Line:** `EAL_LOG(ERR, "devargs truncated (len %zu, max %zu)",`

**Issue:** While changing the format specifier from `%d` to `%zu` is appropriate for the new `size_t` type of `EAL_DEV_MP_DEV_ARGS_MAX_LEN`, the actual value being passed is still the result of a preprocessor calculation that produces an integer. However, this is acceptable since `size_t` can hold integer values, and the change improves consistency. The only concern is that `EAL_DEV_MP_DEV_ARGS_MAX_LEN` is now a compile-time expression rather than a simple constant, which is fine.

Actually, on second review, `EAL_DEV_MP_DEV_ARGS_MAX_LEN` is defined as:
```c
(RTE_MP_MAX_PARAM_LEN - sizeof(enum eal_dev_req_type) - sizeof(int))
```

This expression evaluates to `size_t` due to `sizeof` operators, so `%zu` is the correct format specifier. No issue here. (Deleted per guidelines - this is actually correct)

## INFO

### 1. Documentation of new limit value
The commit message states the new maximum is 247 bytes (excluding null terminator), which should be verified:
- `RTE_MP_MAX_PARAM_LEN` is typically 256
- Minus `sizeof(enum eal_dev_req_type)` (typically 4 bytes on most platforms, but could be implementation-defined)
- Minus `sizeof(int)` (typically 4 bytes)
- Result: 256 - 4 - 4 = 248 bytes for the array
- Minus 1 for null terminator = 247 usable bytes

This calculation appears correct, assuming typical sizes. The `static_assert` added provides compile-time verification that the struct fits within IPC limits, which is good defensive programming.

### 2. Include order
**File:** lib/eal/common/hotplug_mp.h
**Line:** `#include "rte_eal.h"`

The patch adds `#include "rte_eal.h"` to access `RTE_MP_MAX_PARAM_LEN`. This include should be verified to be the minimal necessary header. If `RTE_MP_MAX_PARAM_LEN` is defined in a more specific header (e.g., `rte_mp.h` or similar), that would be preferable to reduce compilation dependencies. However, this is a minor consideration and the current approach is acceptable.

---

Review-Result: WARNING


More information about the test-report mailing list