|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