|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