|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