|WARNING| [v2,3/3] build: deprecate HPET support
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Nov 6 03:04:36 CET 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172646
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-11-06
# DPDK Patch Review
## Patch 1/3: build: deprecate kernel dir option
**No issues found.**
## Patch 2/3: build: deprecate standalone tests option
### ERRORS
**Error 1: Logic error in conditional build inclusion**
Line in `app/meson.build`:
```python
apps = [
'graph',
'pdump',
'proc-info',
'test',
'test-acl',
```
The patch unconditionally adds `'test'` to the apps list, but then removes the conditional logic that checks `get_option('tests')`. This means the test app will now **always** be built, even when the user sets `tests=false`.
The deprecation notice says the option is "unused" but the code change makes it truly unused by ignoring its value entirely, rather than maintaining backward compatibility during the deprecation period.
**Why it matters:** Users who currently set `-Dtests=false` to skip building tests will find that tests are built anyway, silently breaking their build configuration.
**Suggested fix:**
Maintain backward compatibility during deprecation by keeping the conditional check:
```python
apps = [
'graph',
'pdump',
'proc-info',
'test-acl',
...
]
# Deprecated: use enable_apps/disable_apps instead
if get_option('tests')
apps += 'test'
endif
```
Or if you want to respect both the old and new options during the transition:
```python
if get_option('tests') or 'test' not in get_option('disable_apps')
apps += 'test'
endif
```
This allows the option to keep working while deprecated, which is the standard deprecation practice (mark as deprecated, keep functional, remove in a later release).
---
## Patch 3/3: build: deprecate HPET support
### WARNINGS
**Warning 1: Documentation update incomplete**
The patch removes all HPET usage instructions from `doc/guides/linux_gsg/enable_func.rst` and `doc/guides/linux_gsg/sys_reqs.rst`, which is appropriate for deprecation.
However, the removed section "High Precision Event Timer (HPET) Functionality" provided information about HPET APIs (`rte_get_hpet_cycles()`, `rte_get_hpet_hz()`, `rte_eal_hpet_init()`) and their alternatives (`rte_get_timer_cycles()`, `rte_get_timer_hz()`).
The deprecation notice in `doc/guides/rel_notes/deprecation.rst` mentions the alternative APIs but does not list which HPET-specific APIs will be removed.
**Suggested fix:**
Add to the deprecation notice in `doc/guides/rel_notes/deprecation.rst`:
```rst
* hpet: HPET support is deprecated and will be removed in a future release.
Support has been disabled by default for many years.
The following HPET-specific APIs will be removed:
``rte_eal_hpet_init()``, ``rte_get_hpet_cycles()``, and ``rte_get_hpet_hz()``.
Applications should use the generic timer APIs,
such as ``rte_get_timer_cycles()`` and ``rte_get_timer_hz()`` instead.
```
This provides users with a complete migration path.
---
**Warning 2: Typo in deprecation notice**
Line in `doc/guides/rel_notes/deprecation.rst`:
```rst
- ``use_hpet``: option option will be removed as HPET support itself is deprecated (see below).
```
"option option" is duplicated.
**Suggested fix:**
```rst
- ``use_hpet``: option will be removed as HPET support itself is deprecated (see below).
```
---
## Summary
The series deprecates three build options: `kernel_dir`, `tests`, and `use_hpet`.
Patch 1 (kernel_dir) is clean.
Patch 2 (tests) has a **logic error**: it removes the conditional check that respects the `tests` option value, breaking backward compatibility during the deprecation period. Users setting `tests=false` will unexpectedly build tests anyway.
Patch 3 (HPET) has **minor documentation issues**: the deprecation notice has a typo and could be more explicit about which APIs are being removed.
Review-Result: ERROR
More information about the test-report
mailing list