|WARNING| [v7] pcapng: add user-supplied timestamp support

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171354

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-14

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

### 1. Test error path: file descriptor not closed on `rte_pcapng_fdopen` failure
**File:** `app/test/test_pcapng.c`
**Line:** 
```c
if (pcapng == NULL) {
	printf("rte_pcapng_fdopen failed\n");
	close(tmp_fd);
	goto fail;
}
```
**Issue:** If `rte_pcapng_fdopen` succeeds but later operations fail, the code jumps to `fail:` which calls `rte_pcapng_close(pcapng)`. However, `rte_pcapng_fdopen` takes ownership of the file descriptor, so the manual `close(tmp_fd)` on this error path may be unnecessary or could lead to a double-close if the library implementation closes it on failure.

**Recommendation:** Verify the ownership semantics of `rte_pcapng_fdopen`. If it always takes ownership even on failure, remove the `close(tmp_fd)`. If it only takes ownership on success, the current code is correct.

### 2. Release notes: timestamp conversion could clarify nanosecond precision
**File:** `doc/guides/rel_notes/release_26_11.rst`
**Lines:**
```rst
* pcapng: Added a ``timestamp`` parameter to ``rte_pcapng_copy``.
  The application can give the time of the packet,
  for example a hardware Rx timestamp, in nanoseconds since the Unix epoch.
  The value 0 keeps the previous behavior: the TSC at copy time is used.
```
**Issue:** The description does not explicitly state that the timestamp must have bit 63 clear, which is a constraint enforced by the code and documented in the header comment.

**Recommendation:** Add a note: "The timestamp must have bit 63 clear (values exceeding 2^63-1 are reserved)."

## INFO

### 1. Pre-existing: test_long_timestamp does not check pcapng_timestamp return
**File:** `app/test/test_pcapng.c` (context around line 866)
The `test_long_timestamp` function does not validate that `rte_pcapng_copy` succeeds before proceeding. This is a pre-existing pattern in the test file that could mask failures, but is not introduced by this patch.

### 2. Documentation enhancement opportunity
**File:** `doc/guides/prog_guide/pcapng_lib.rst`
The new text states:
```rst
The timestamp of each packet is the TSC at the time of ``rte_pcapng_copy``,
or the time in nanoseconds that the caller gives to ``rte_pcapng_copy``.
```
This could be enhanced to mention the bit 63 constraint and the 2262 year limit, matching the commit message detail. Not a requirement, but would improve clarity.

---

**Summary:**
- The core logic correctly implements timestamp handling with the TSC flag mechanism
- The test coverage is comprehensive, including validation of the bit 63 rejection and timestamp round-trip
- The API change is properly documented in release notes
- Two minor concerns: file descriptor ownership semantics in the test error path, and release note completeness regarding bit 63

Review-Result: WARNING