|SUCCESS| [v8] pcapng: add user-supplied timestamp support
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Oct 21 01:06:25 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/171775
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-20
# DPDK Patch Review
## ERRORS
None.
## WARNINGS
None.
## INFO
**Test coverage for timestamp validation:**
The test `test_pcapng_timestamp()` verifies that `rte_pcapng_copy()` rejects a timestamp with bit 63 set (lines 751-745):
```c
mc = rte_pcapng_copy(port_id, 0, &mbfs.mb[0], mp,
rte_pktmbuf_pkt_len(&mbfs.mb[0]),
RTE_PCAPNG_DIRECTION_IN, NULL, UINT64_C(1) << 63);
if (mc != NULL || rte_errno != EINVAL) {
printf("rte_pcapng_copy accepted a timestamp with bit 63 set\n");
rte_pktmbuf_free(mc);
goto fail;
}
```
This correctly validates the `EINVAL` error return documented in the API.
The test then verifies that a valid custom timestamp (one second ahead) is written to the file and correctly read back by libpcap (lines 748-788), confirming end-to-end functionality.
**Pre-existing: potential resource leak in test_pcapng_timestamp():**
In the error path at line 735 (`printf("rte_pcapng_fdopen failed\n")`), if `pcapng` is NULL but `tmp_fd` was successfully created by `mkstemps()`, the file descriptor is closed but the temporary file is not removed. The cleanup at line 749 (`goto fail`) calls `remove(file_name)`, but this only happens if `pcapng` initialization fails. If `rte_pcapng_add_interface()` fails (line 741), the file will be removed. However, the early return at line 736 after `close(tmp_fd)` skips the `remove()` call.
This is a minor test cleanup issue and does not affect the patch's correctness. The temporary file would be cleaned up by the OS, but explicit removal is preferred for test hygiene.
**API design note:**
The choice to use bit 63 as a flag distinguishing TSC values from epoch nanoseconds is a reasonable design for an internal representation. The API contract correctly prohibits callers from setting bit 63, and the implementation validates this (line 538-540). Since bit 63 in epoch nanoseconds corresponds to the year 2262, the restriction does not affect practical use cases.
Review-Result: CLEAN
More information about the test-report
mailing list