|SUCCESS| [v1] net/ice: fix read clock to use PHC hardware time
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Oct 30 16:12:48 CET 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172289
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-30
# DPDK Patch Review
## Summary
This patch fixes `ice_read_clock()` to use the PHC (Precision Time Protocol Hardware Clock) instead of system time, which is the correct behavior for hardware timestamping applications.
## Detailed Review
### ERRORS
None.
### WARNINGS
None.
### INFO
**Unsigned to signed conversion in arithmetic**
```c
*clock = (uint64_t)ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
```
The cast of `tv_sec` to `uint64_t` before multiplication is correct and prevents overflow. However, `tv_sec` is `time_t` (typically `long` or `int64_t`, signed). If `tv_sec` were negative (representing time before epoch), the cast would produce a very large positive value, which could be unexpected. In the context of PHC hardware time this is likely not an issue since hardware clocks typically start from 0 or a recent epoch, but worth noting for completeness.
This is pre-existing in the codebase (same pattern appears in `ice_timesync_read_time()`), and the change maintains consistency with how other functions in the file handle the conversion.
**Correctness verification**
The fix correctly addresses the stated bug:
- Previously: used system clock (`CLOCK_MONOTONIC_RAW` or `CLOCK_MONOTONIC`)
- Now: reads actual PHC register via `ice_timesync_read_time()`
- Error handling: properly checks and propagates the return value from `ice_timesync_read_time()`
- All allocated resources: none to manage here
- No use-after-free, leaks, or race conditions introduced
**Code style compliance**
- Explicit comparison against 0: `if (ret)` is acceptable per guidelines (error return value check)
- Variable naming: follows conventions (`ts` is clear and concise)
- Indentation and formatting: compliant
- No forbidden tokens used
**Release notes requirement**
This is a bug fix that corrects functional behavior. The commit is tagged with `Fixes:` and `Cc: stable at dpdk.org`, which is appropriate. Release notes are not strictly required for this type of correctness fix in existing functionality.
Review-Result: CLEAN
More information about the test-report
mailing list