|WARNING| eal: introduce division round up macro
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Aug 13 01:11:05 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
# Review of DPDK Patch: "eal: introduce division round up macro"
## Summary
This patch introduces `RTE_DIV_ROUND_UP()` to `rte_common.h` and replaces existing driver-local `DIV_ROUND_UP` definitions throughout the codebase. The patch is primarily a mechanical refactoring for code consolidation.
---
## Errors
None found.
---
## Warnings
### 1. Missing release notes update
**File:** N/A
**Issue:** This patch introduces a new public macro (`RTE_DIV_ROUND_UP`) in an installed header (`lib/eal/include/rte_common.h`) but does not update the release notes.
**Rationale:** New API additions require release notes documentation. While this is a simple utility macro, it is now part of the public EAL API and applications may use it.
**Suggested fix:**
Add a release notes entry in `doc/guides/rel_notes/release_XX_YY.rst`:
```rst
New Features
------------
* **Added RTE_DIV_ROUND_UP macro.**
Added ``RTE_DIV_ROUND_UP(n, d)`` macro to ``rte_common.h`` for integer division
with rounding up. This consolidates driver-local definitions into a common utility.
```
### 2. Macro definition placement
**File:** `lib/eal/include/rte_common.h`
**Line:** 723
**Issue:** The new macro is placed after `RTE_ROUND_UP_WIDTH()` but before `RTE_PTR_ALIGN()`. Consider grouping with related rounding/alignment macros for better organization.
**Rationale:** `rte_common.h` already has several division and rounding utilities (`RTE_ALIGN`, `RTE_ALIGN_FLOOR`, `RTE_ALIGN_CEIL`). Placing `RTE_DIV_ROUND_UP` near these related macros would improve readability.
**Suggested fix:**
Consider moving the definition closer to line 549 (near `RTE_ALIGN_MUL_CEIL` and related macros) where other arithmetic rounding utilities are grouped.
---
## Info
### 1. Doxygen formatting style
**File:** `lib/eal/include/rte_common.h`
**Lines:** 720-722
**Observation:** The new macro uses a single-line comment style, while other macros in the file use multi-line Doxygen comment blocks.
**Context:** Most macros in `rte_common.h` use the full Doxygen format:
```c
/**
* Macro description.
*
* @param n
* Parameter description.
*/
```
**Suggestion:**
Consider using the full Doxygen format for consistency:
```c
/**
* Divide n by d, rounding up to the next integer if d does not evenly divide n.
*
* @param n
* Dividend (must be non-negative for correct rounding).
* @param d
* Divisor (must be positive).
* @return
* The result of the division, rounded up.
*/
#define RTE_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
```
This is a minor style preference; the current documentation is acceptable.
### 2. Macro safety note
**File:** `lib/eal/include/rte_common.h`
**Line:** 723
**Observation:** The macro evaluates `d` twice. This is acceptable for a utility macro, but worth noting.
**Context:** If `d` is an expression with side effects (e.g., `RTE_DIV_ROUND_UP(x, i++)`), it would increment `i` twice. This is a known limitation of many existing DPDK macros.
**Note:** This is not a defect for this patch--it is consistent with existing practice in `rte_common.h` (e.g., `RTE_ALIGN`, `RTE_MIN`, `RTE_MAX` all evaluate arguments multiple times). The macro is simple enough that this is acceptable.
---
## Positive Observations
1. **Correct consolidation**: The patch correctly identifies all instances of `DIV_ROUND_UP` across drivers and replaces them uniformly.
2. **No functional changes**: The replacement is mechanical and does not alter behavior.
3. **Formatting preserved**: Indentation and formatting changes are minimal and appropriate (only adjusting for the longer macro name where line length required it).
4. **App usage included**: The patch correctly updates `app/proc-info/main.c`, showing awareness that the macro was used outside drivers.
---
## Overall Assessment
This is a clean consolidation patch. The only substantive issue is the missing release notes entry, which should be added since this introduces a new public API macro. The placement and documentation style suggestions are minor quality-of-life improvements.
More information about the test-report
mailing list