|SUCCESS| [v2, 14/14] app/procinfo: use common division round up macro
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Aug 13 19:54:20 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/167722
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-13
# DPDK Patch Review
## Patch Series: Introduction and usage of RTE_DIV_ROUND_UP macro
This is a mechanical refactor series that introduces a common `RTE_DIV_ROUND_UP` macro in `rte_common.h` and replaces driver-local `DIV_ROUND_UP` definitions throughout the codebase.
---
## Summary
**No critical issues found.**
The series is a straightforward consolidation of identical `DIV_ROUND_UP` macros scattered across drivers and subsystems into a single definition in the EAL common header. All substitutions are mechanical and correct.
---
## Detailed Review
### Patch 01/14: eal: introduce division round up macro
**File:** `lib/eal/include/rte_common.h`
**Changes:**
- Adds `RTE_DIV_ROUND_UP(n, d)` macro definition
**Findings:** None.
The macro definition `(((n) + (d) - 1) / (d))` is the standard idiom for ceiling division on unsigned integers. The documentation is clear and concise.
---
### Patches 02-14: Replace driver-local DIV_ROUND_UP with RTE_DIV_ROUND_UP
**Files:**
- `drivers/net/intel/{i40e,idpf,ice}/base/*_osdep.h`
- `drivers/net/qede/base/bcm_osal.h` and multiple `.c` files
- `drivers/net/{rnp,hns3,hinic,bnx2x,cxgbe,ena}/` various files
- `drivers/common/nfp/nfp_platform.h` and related
- `drivers/raw/ifpga/base/opae_osdep.h`
- `app/proc-info/main.c`
**Changes:**
All patches mechanically replace:
- `#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))` - removed
- `DIV_ROUND_UP(...)` - `RTE_DIV_ROUND_UP(...)`
**Findings:** None.
Each substitution is a direct 1:1 replacement. The macro definition being removed is textually identical to the new `RTE_DIV_ROUND_UP`, so semantics are preserved. All call sites are updated consistently.
### Special Cases Reviewed
**Patch 06/14 (net/rnp):**
- Changes conditional guard from `#ifndef DIV_ROUND_UP` to `#ifndef BITS_TO_LONGS`
- This is correct: `BITS_TO_LONGS` uses `RTE_DIV_ROUND_UP`, so the guard ensures the entire block is skipped if `BITS_TO_LONGS` is already defined (e.g., from system headers)
**Patch 05/14 (net/qede):**
- Removes both `#define RTE_DIV_ROUND_UP ...` and `#define DIV_ROUND_UP ...` from `bcm_osal.h`
- The `RTE_DIV_ROUND_UP` definition in this file was a local override; replacing it with the central definition is correct
---
## Style and Process
- No forbidden tokens introduced
- No changes to API surface (internal macro consolidation only)
- No ABI impact
- No new experimental symbols
- Patch series is bisectable (each patch compiles independently)
- Header include order preserved
- No trailing whitespace or formatting issues visible in diffs
### Release Notes
**Info:**
This is an internal consolidation of existing functionality (macro deduplication). Release notes are not required per the guidelines: no user-visible API changes, no new features, no deprecated API removal.
### Documentation
**Info:**
The `RTE_DIV_ROUND_UP` macro is documented inline in `rte_common.h`. No additional documentation is needed for this utility macro.
---
## Correctness Review
### Integer Overflow Considerations
**Info:**
The `RTE_DIV_ROUND_UP(n, d)` idiom can overflow when `n + (d - 1)` exceeds the type width. However:
- This is identical behavior to all the `DIV_ROUND_UP` definitions being replaced
- The pattern is used throughout DPDK and the kernel for bounded values (ring sizes, descriptor counts, page calculations)
- Callsites typically use this on configuration constants or hardware-limited values
Flagging this would be a false positive: the patch does not introduce new overflow risk, it consolidates existing use.
---
## Conclusion
This is a clean refactor with no functional changes. The series improves code maintainability by eliminating duplicated macro definitions. No issues found.
More information about the test-report
mailing list