|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