|WARNING| [v2, 14/14] app/procinfo: use common division round up macro
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Aug 13 20:01:50 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/167722
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-13
# Review of DPDK Patch Series: Division Round Up Macro Consolidation
## Summary
This patch series introduces `RTE_DIV_ROUND_UP()` to `rte_common.h` and replaces driver-local `DIV_ROUND_UP` definitions with the common macro. Overall, the series is well-structured and the changes are mechanical and correct. However, there are a few issues to address.
---
## PATCH v2 01/14: eal: introduce division round up macro
**File:** `lib/eal/include/rte_common.h`
### Errors
None.
### Warnings
1. **Doxygen documentation incomplete**
The macro comment lacks detail. It should document the parameters and include a note about the overflow risk when `(n) + (d) - 1` exceeds the type width.
```c
/* Current - minimal */
/**
* Macro to divide dividend n by divisor d, rounding up if d does not divide n.
*/
#define RTE_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
/* Suggested - complete */
/**
* Divide and round up.
*
* Divides dividend @p n by divisor @p d, rounding up if @p d does not evenly divide @p n.
* Evaluates arguments multiple times.
*
* @warning
* The intermediate calculation (n) + (d) - 1 may overflow if the sum exceeds
* the width of the operand types. Callers should ensure the result fits in the target type.
*
* @param n
* Dividend (must be nonnegative for correct rounding).
* @param d
* Divisor (must be positive and nonzero).
* @return
* The quotient (n / d) rounded up to the nearest integer.
*/
#define RTE_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
```
**Why it matters:** Public API macros in `rte_common.h` require complete Doxygen. The overflow note is important because the addition can silently wrap on narrow types.
2. **Missing release notes entry**
This is a new public API macro. It should be documented in `doc/guides/rel_notes/release_XX_YY.rst` under "New Features" or "API Changes."
**Suggested entry:**
```rst
* **Added RTE_DIV_ROUND_UP macro.**
A new macro ``RTE_DIV_ROUND_UP(n, d)`` has been added to ``rte_common.h``
to perform division with rounding up. This replaces driver-local definitions
and provides a common implementation across DPDK.
```
---
## PATCH v2 02/14 through 14/14: Driver and application changes
**Files:** Multiple drivers and `app/proc-info`
### Errors
None. All replacements are correct.
### Warnings
None.
### Info
All patches correctly replace `DIV_ROUND_UP` with `RTE_DIV_ROUND_UP` and remove the local definitions. The substitutions are mechanical and safe.
**Verified patterns:**
- `drivers/net/intel/i40e/base/i40e_osdep.h`: Removes `DIV_ROUND_UP`, uses `RTE_DIV_ROUND_UP` in `usleep_range`
- `drivers/net/intel/idpf/base/idpf_osdep.h`: Same pattern
- `drivers/net/intel/ice/base/ice_osdep.h`: Same pattern
- `drivers/net/qede/base/bcm_osal.h`: Removes both `RTE_DIV_ROUND_UP` and `DIV_ROUND_UP` aliases
- `drivers/net/rnp/base/rnp_osdep.h`: Changes `BITS_TO_LONGS` guard from `DIV_ROUND_UP` to `BITS_TO_LONGS` (correct approach)
- `drivers/net/hns3/hns3_ethdev.h`: Removes definition, usage updated in `.c` files
- `drivers/net/hinic/base/hinic_compat.h`: Updates `ROUND_UP` to use `RTE_DIV_ROUND_UP`
- `drivers/net/bnx2x/bnx2x.h`: Removes definition, updates multiple macro uses
- `drivers/net/cxgbe/cxgbe_compat.h`: Removes definition, updates `usleep_range`
- `drivers/net/cxgbe/base/`: Multiple call sites updated
- `drivers/net/ena/base/ena_plat_dpdk.h`: Removes definition
- `drivers/raw/ifpga/base/opae_osdep.h`: Removes definition, updates `usleep_range`
- `drivers/common/nfp/nfp_platform.h`: Removes definition
- `drivers/net/nfp/`: Multiple call sites updated with proper line wrapping
- `app/proc-info/main.c`: Removes local definition, updates call site
---
## General Observations
1. **No correctness bugs identified.**
All changes are safe replacements of identical macro definitions.
2. **Consistent formatting.**
Where line length exceeded 100 characters after replacement, the patch correctly wraps to multiple lines with proper indentation (e.g., `drivers/net/nfp/nfdk/nfp_nfdk.h`, `drivers/net/qede/base/ecore_cxt.c`).
3. **Header guard handling.**
The `rnp` driver's approach (changing from `DIV_ROUND_UP` guard to `BITS_TO_LONGS` guard) is correct - it avoids redefining a macro now available in `rte_common.h` while preserving the conditional compilation logic.
4. **No intermediate compilation failures.**
Patch 01 adds the macro, patches 02-14 consume it. Each patch compiles independently (the macro is available before any driver tries to use it).
---
## Summary of Findings
### Errors
None.
### Warnings
1. **Patch 01:** Doxygen documentation for `RTE_DIV_ROUND_UP` should include parameter descriptions, return value, and overflow warning.
2. **Patch 01:** Missing release notes entry for new public macro.
### Info
All driver and application changes are correct and follow DPDK style guidelines.
---
## Recommended Actions
1. **Enhance Doxygen comment** in patch 01 per the example above.
2. **Add release notes entry** in patch 01 documenting the new macro.
3. After these changes, the series is ready for merge.
---
**Overall assessment:** This is a clean consolidation patch series. The two warnings are documentation-related and easy to address. No code correctness issues were found.
More information about the test-report
mailing list