|WARNING| eal: introduce division round up macro

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Aug 13 01:08:24 CEST 2026


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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-12

# DPDK Patch Review

## Patch Summary
This patch consolidates the `DIV_ROUND_UP` macro (used across multiple drivers and applications) into a single `RTE_DIV_ROUND_UP` macro in `rte_common.h`, replacing all existing definitions.

---

## Errors

None identified.

---

## Warnings

### 1. Missing experimental API marking
**File:** `lib/eal/include/rte_common.h`
**Location:** New macro `RTE_DIV_ROUND_UP`

New public API macro added without `__rte_experimental` marking. While this is a utility macro (not a function), it is being added to a stable public header and becomes part of the API surface. According to DPDK policy, new API should be marked experimental in the first release.

**Suggested fix:**
Either mark as experimental with a comment indicating planned stabilization, or document in the release notes that this is a mechanical consolidation of existing internal macros (making it effectively not "new" API).

### 2. Incomplete documentation
**File:** `lib/eal/include/rte_common.h`
**Location:** `RTE_DIV_ROUND_UP` documentation

The Doxygen comment lacks parameter documentation and return value description.

**Suggested fix:**
```c
/**
 * Divide dividend by divisor, rounding up.
 *
 * Computes the quotient (n / d), rounding up if d does not divide n evenly.
 * Both n and d must be nonnegative for correct rounding behavior.
 *
 * @param n
 *   Dividend (numerator).
 * @param d
 *   Divisor (denominator). Must not be zero.
 * @return
 *   The quotient rounded up: (n + d - 1) / d
 */
#define RTE_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
```

### 3. Missing release notes update
**File:** Not present in patch

This patch adds new public API (`RTE_DIV_ROUND_UP` in `rte_common.h`) but does not update the release notes. While the change is primarily a consolidation, the new macro is now part of the public DPDK API and should be documented in `doc/guides/rel_notes/release_X_Y.rst` (where X_Y is the current release version).

**Suggested addition to release notes:**
```rst
* **Added RTE_DIV_ROUND_UP utility macro.**

  Added ``RTE_DIV_ROUND_UP(n, d)`` macro to ``rte_common.h`` for dividing
  with rounding up. This consolidates multiple driver-local definitions
  of ``DIV_ROUND_UP`` into a single public macro.
```

---

## Info

### 1. Macro safety consideration (informational only)
**File:** `lib/eal/include/rte_common.h`
**Location:** `RTE_DIV_ROUND_UP` definition

The macro evaluates its arguments multiple times (`n` appears twice, `d` appears twice). This is acceptable for a simple arithmetic macro, but callers should be aware that expressions with side effects (e.g., `RTE_DIV_ROUND_UP(i++, 8)`) will produce incorrect results.

This is a common pattern in DPDK and does not need to change, but consider documenting this caveat if it's not already covered in general DPDK macro guidelines.

---

## Positive Observations

1. **Consistent replacement**: All instances of `DIV_ROUND_UP` across the codebase are correctly replaced with `RTE_DIV_ROUND_UP`.

2. **Proper alignment formatting**: Multi-line macro invocations maintain proper indentation (continuation lines double-indented or aligned as appropriate).

3. **Removes code duplication**: Eliminates ~15 duplicate definitions of the same macro across drivers and applications.

4. **No behavioral change**: The new macro has identical semantics to all replaced definitions.

5. **Clean removal**: Old definitions are properly removed without leaving stale code.

---

## Summary

This is a good cleanup patch that reduces code duplication. The main issues are documentation completeness and following the new-API process (experimental marking or release notes justification). The code changes themselves are mechanically correct.

**Recommendation**: Address the documentation and release notes warnings before merging.


More information about the test-report mailing list