[PATCH 0/2] pcapng: bug fixes

[PATCH 0/2] pcapng: bug fixes

[Stephen Hemminger]

Fix two pcapng-related issues:

The test code does not build on Windows because it uses
POSIX-only APIs (clock_gettime, mkstemps, unlink). Add
Windows wrappers and use portable alternatives where possible.

The library timestamp conversion can produce bogus values
if rte_pcapng_copy() is called before rte_pcapng_fdopen(),
since the TSC delta wraps unsigned. Handle the negative
delta case explicitly.

Stephen Hemminger (2):
  test: fix pcapng test to work on Windows
  pcapng: handle packets copied before file open

 app/test/test_pcapng.c  | 65 ++++++++++++++++++++++++++++++++++++++---
 lib/pcapng/rte_pcapng.c | 17 ++++++-----
 2 files changed, 71 insertions(+), 11 deletions(-)

-- 
2.51.0

[PATCH 1/2] test: fix pcapng test to work on Windows

[Stephen Hemminger]

The pcapng test needs additional wrappers to be able to
build and run on Windows.

This is a pre-existing problem, only exposed when the
test was enabled on Windows builds, and when libpcap
is setup on Windows.

Fixes: 0edc1f408a8b ("test: enable subset of tests on Windows")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 65 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index ad9ad51f4c..f0faead728 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -4,7 +4,16 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
+
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <winsock2.h>
+#include <io.h>
+#include <fcntl.h>
+#include <windows.h>
+#else
 #include <unistd.h>
+#endif
 
 #include <rte_bus_vdev.h>
 #include <rte_ethdev.h>
@@ -23,6 +32,52 @@
 
 #include "test.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+static uint64_t
+current_timestamp(void)
+{
+	FILETIME ft;
+	ULARGE_INTEGER ul;
+
+	GetSystemTimeAsFileTime(&ft);
+	ul.LowPart = ft.dwLowDateTime;
+	ul.HighPart = ft.dwHighDateTime;
+	/* FILETIME is 100ns intervals since 1601-01-01, convert to ns since Unix epoch */
+	return (ul.QuadPart - 116444736000000000ULL) * 100;
+}
+
+/*
+ * Create temporary file with suffix for Windows.
+ * Returns file descriptor or -1 on failure.
+ */
+static int
+mkstemps(char *tmpl, int suffixlen)
+{
+	char temp_dir[MAX_PATH];
+	char temp_file[MAX_PATH];
+	DWORD ret;
+
+	ret = GetTempPathA(sizeof(temp_dir), temp_dir);
+	if (ret == 0 || ret > sizeof(temp_dir))
+		return -1;
+
+	if (GetTempFileNameA(temp_dir, "pcap", 0, temp_file) == 0)
+		return -1;
+
+	/*
+	 * GetTempFileNameA with uUnique=0 creates the file to reserve the name.
+	 * Remove it since we open a different name with the original suffix appended.
+	 */
+	DeleteFileA(temp_file);
+
+	/* Append the original suffix (e.g. ".pcapng") to the temp file */
+	strlcat(temp_file, tmpl + strlen(tmpl) - suffixlen, sizeof(temp_file));
+	strlcpy(tmpl, temp_file, PATH_MAX);
+
+	return _open(tmpl, _O_RDWR | _O_BINARY | _O_CREAT | _O_EXCL, 0666);
+}
+#endif /* RTE_EXEC_ENV_WINDOWS */
+
 #define PCAPNG_TEST_DEBUG 0
 
 #define TOTAL_PACKETS	10000
@@ -344,6 +399,7 @@ parse_pcap_packet(u_char *user, const struct pcap_pkthdr *h,
 	pcap_breakloop(ctx->pcap);
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static uint64_t
 current_timestamp(void)
 {
@@ -352,6 +408,7 @@ current_timestamp(void)
 	clock_gettime(CLOCK_REALTIME, &ts);
 	return rte_timespec_to_ns(&ts);
 }
+#endif
 
 /*
  * Open the resulting pcapng file with libpcap
@@ -395,7 +452,7 @@ valid_pcapng_file(const char *file_name, uint64_t started, unsigned int expected
 static int
 test_add_interface(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	static rte_pcapng_t *pcapng;
 	int ret, tmp_fd;
 	uint64_t now = current_timestamp();
@@ -444,7 +501,7 @@ test_add_interface(void)
 	ret = valid_pcapng_file(file_name, now, 0);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
@@ -456,7 +513,7 @@ test_add_interface(void)
 static int
 test_write_packets(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	rte_pcapng_t *pcapng = NULL;
 	int ret, tmp_fd, count;
 	uint64_t now = current_timestamp();
@@ -508,7 +565,7 @@ test_write_packets(void)
 	ret = valid_pcapng_file(file_name, now, count);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
-- 
2.51.0

[PATCH 2/2] pcapng: handle packets copied before file open

[Stephen Hemminger]

Either through misuse of the API, or races it might happen
that rte_pcapng_copy() is called before rte_pcapng_fdopen().

Simplest solution to handle this is to modify the logic for
TSC to nanosecond epoch conversion to handle the case where
the difference is negative.

Bugzilla ID: 1291
Fixes: 166591931b72 ("pcapng: modify timestamp calculation")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pcapng/rte_pcapng.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/lib/pcapng/rte_pcapng.c b/lib/pcapng/rte_pcapng.c
index b93af418aa..1a1935e876 100644
--- a/lib/pcapng/rte_pcapng.c
+++ b/lib/pcapng/rte_pcapng.c
@@ -155,13 +155,16 @@ tsc_clock_init(struct tsc_clock *clk)
 static inline uint64_t
 tsc_to_ns_epoch(const struct tsc_clock *clk, uint64_t tsc)
 {
-	uint64_t delta, ns;
-
-	delta = tsc - clk->tsc_base;
-	ns = (delta >> clk->shift) * NSEC_PER_SEC;
-	ns = rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
-
-	return clk->ns_base + ns;
+	int64_t delta = tsc - clk->tsc_base;
+	uint64_t ns;
+
+	if (unlikely(delta < 0)) {
+		ns = (-delta >> clk->shift) * NSEC_PER_SEC;
+		return clk->ns_base - rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
+	} else {
+		ns = (delta >> clk->shift) * NSEC_PER_SEC;
+		return clk->ns_base + rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
+	}
 }
 
 /* length of option including padding */
-- 
2.51.0

[PATCH v2 0/4] pcapng: fix test and copy before open

[Stephen Hemminger]

The test code does not build on Windows because it uses
POSIX-only APIs (clock_gettime, mkstemps, unlink). Add
Windows wrappers and use portable alternatives where possible.

The test code can timeout on slow emulated CPU's.

The library timestamp conversion can produce bogus values
if rte_pcapng_copy() is called before rte_pcapng_fdopen(),
since the TSC delta wraps unsigned. Handle the negative
delta case explicitly.

Stephen Hemminger (4):
  test: fix pcapng test to work on Windows
  pcapng: handle packets copied before file open
  test: add pcapng test for copy before open
  test: use fixed time length for write packet test

 app/test/test_pcapng.c  | 161 +++++++++++++++++++++++++++++++++++++---
 lib/pcapng/rte_pcapng.c |  10 ++-
 2 files changed, 157 insertions(+), 14 deletions(-)

-- 
2.51.0

[PATCH v2 1/4] test: fix pcapng test to work on Windows

[Stephen Hemminger]

The pcapng test needs additional wrappers to be able to
build and run on Windows.

This is a pre-existing problem, only exposed when the
test was enabled on Windows builds, and when libpcap
is setup on Windows.

Fixes: 0edc1f408a8b ("test: enable subset of tests on Windows")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 65 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index ad9ad51f4c..f0faead728 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -4,7 +4,16 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
+
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <winsock2.h>
+#include <io.h>
+#include <fcntl.h>
+#include <windows.h>
+#else
 #include <unistd.h>
+#endif
 
 #include <rte_bus_vdev.h>
 #include <rte_ethdev.h>
@@ -23,6 +32,52 @@
 
 #include "test.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+static uint64_t
+current_timestamp(void)
+{
+	FILETIME ft;
+	ULARGE_INTEGER ul;
+
+	GetSystemTimeAsFileTime(&ft);
+	ul.LowPart = ft.dwLowDateTime;
+	ul.HighPart = ft.dwHighDateTime;
+	/* FILETIME is 100ns intervals since 1601-01-01, convert to ns since Unix epoch */
+	return (ul.QuadPart - 116444736000000000ULL) * 100;
+}
+
+/*
+ * Create temporary file with suffix for Windows.
+ * Returns file descriptor or -1 on failure.
+ */
+static int
+mkstemps(char *tmpl, int suffixlen)
+{
+	char temp_dir[MAX_PATH];
+	char temp_file[MAX_PATH];
+	DWORD ret;
+
+	ret = GetTempPathA(sizeof(temp_dir), temp_dir);
+	if (ret == 0 || ret > sizeof(temp_dir))
+		return -1;
+
+	if (GetTempFileNameA(temp_dir, "pcap", 0, temp_file) == 0)
+		return -1;
+
+	/*
+	 * GetTempFileNameA with uUnique=0 creates the file to reserve the name.
+	 * Remove it since we open a different name with the original suffix appended.
+	 */
+	DeleteFileA(temp_file);
+
+	/* Append the original suffix (e.g. ".pcapng") to the temp file */
+	strlcat(temp_file, tmpl + strlen(tmpl) - suffixlen, sizeof(temp_file));
+	strlcpy(tmpl, temp_file, PATH_MAX);
+
+	return _open(tmpl, _O_RDWR | _O_BINARY | _O_CREAT | _O_EXCL, 0666);
+}
+#endif /* RTE_EXEC_ENV_WINDOWS */
+
 #define PCAPNG_TEST_DEBUG 0
 
 #define TOTAL_PACKETS	10000
@@ -344,6 +399,7 @@ parse_pcap_packet(u_char *user, const struct pcap_pkthdr *h,
 	pcap_breakloop(ctx->pcap);
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static uint64_t
 current_timestamp(void)
 {
@@ -352,6 +408,7 @@ current_timestamp(void)
 	clock_gettime(CLOCK_REALTIME, &ts);
 	return rte_timespec_to_ns(&ts);
 }
+#endif
 
 /*
  * Open the resulting pcapng file with libpcap
@@ -395,7 +452,7 @@ valid_pcapng_file(const char *file_name, uint64_t started, unsigned int expected
 static int
 test_add_interface(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	static rte_pcapng_t *pcapng;
 	int ret, tmp_fd;
 	uint64_t now = current_timestamp();
@@ -444,7 +501,7 @@ test_add_interface(void)
 	ret = valid_pcapng_file(file_name, now, 0);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
@@ -456,7 +513,7 @@ test_add_interface(void)
 static int
 test_write_packets(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	rte_pcapng_t *pcapng = NULL;
 	int ret, tmp_fd, count;
 	uint64_t now = current_timestamp();
@@ -508,7 +565,7 @@ test_write_packets(void)
 	ret = valid_pcapng_file(file_name, now, count);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
-- 
2.51.0

[PATCH v2 2/4] pcapng: handle packets copied before file open

[Stephen Hemminger]

Either through misuse of the API, or races it might happen
that rte_pcapng_copy() is called before rte_pcapng_fdopen().

Simplest solution to handle this is to modify the logic for
TSC to nanosecond epoch conversion to handle the case where
the difference is negative.

Bugzilla ID: 1291
Fixes: 166591931b72 ("pcapng: modify timestamp calculation")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pcapng/rte_pcapng.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/pcapng/rte_pcapng.c b/lib/pcapng/rte_pcapng.c
index b93af418aa..b5d1026891 100644
--- a/lib/pcapng/rte_pcapng.c
+++ b/lib/pcapng/rte_pcapng.c
@@ -157,11 +157,15 @@ tsc_to_ns_epoch(const struct tsc_clock *clk, uint64_t tsc)
 {
 	uint64_t delta, ns;
 
+	if (unlikely(tsc < clk->tsc_base)) {
+		delta = clk->tsc_base - tsc;
+		ns = (delta >> clk->shift) * NSEC_PER_SEC;
+		return clk->ns_base - rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
+	}
+
 	delta = tsc - clk->tsc_base;
 	ns = (delta >> clk->shift) * NSEC_PER_SEC;
-	ns = rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
-
-	return clk->ns_base + ns;
+	return clk->ns_base + rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
 }
 
 /* length of option including padding */
-- 
2.51.0

[PATCH v2 3/4] test: add pcapng test for copy before open

[Stephen Hemminger]

If packets are copied before open is called, the timestamps
will be before the start of epoch. Add a test for this which
makes sure pcapng file is still valid.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 82 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index f0faead728..deba08cc6f 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -574,6 +574,87 @@ test_write_packets(void)
 	return -1;
 }
 
+static int
+test_write_before_open(void)
+{
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	struct dummy_mbuf mbfs;
+	struct rte_mbuf *clones[MAX_BURST];
+	rte_pcapng_t *pcapng = NULL;
+	int ret, tmp_fd, i;
+	unsigned int count = 8;
+	uint64_t now;
+	ssize_t len;
+
+	mbuf1_prepare(&mbfs);
+	mbuf1_resize(&mbfs, rte_rand_max(MAX_DATA_SIZE));
+
+	/* Copy packets BEFORE opening the pcapng file.
+	 * This exercises the negative TSC delta path in tsc_to_ns_epoch().
+	 */
+	for (i = 0; i < (int)count; i++) {
+		clones[i] = rte_pcapng_copy(port_id, 0, &mbfs.mb[0], mp,
+					    rte_pktmbuf_pkt_len(&mbfs.mb[0]),
+					    RTE_PCAPNG_DIRECTION_IN, NULL);
+		if (clones[i] == NULL) {
+			fprintf(stderr, "Cannot copy packet before open\n");
+			rte_pktmbuf_free_bulk(clones, i);
+			return -1;
+		}
+	}
+
+	/* Small delay so fdopen's tsc_base is measurably after the copies */
+	rte_delay_us_block(100);
+
+	now = current_timestamp();
+
+	tmp_fd = mkstemps(file_name, strlen(".pcapng"));
+	if (tmp_fd == -1) {
+		perror("mkstemps() failure");
+		rte_pktmbuf_free_bulk(clones, count);
+		return -1;
+	}
+
+	pcapng = rte_pcapng_fdopen(tmp_fd, NULL, NULL, "pcapng_preopen", NULL);
+	if (pcapng == NULL) {
+		fprintf(stderr, "rte_pcapng_fdopen failed\n");
+		close(tmp_fd);
+		rte_pktmbuf_free_bulk(clones, count);
+		return -1;
+	}
+
+	ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB,
+				       NULL, NULL, NULL);
+	if (ret < 0) {
+		fprintf(stderr, "can not add port %u\n", port_id);
+		goto fail;
+	}
+
+	/* Write the pre-captured packets — timestamps precede tsc_base */
+	len = rte_pcapng_write_packets(pcapng, clones, count);
+	rte_pktmbuf_free_bulk(clones, count);
+	if (len <= 0) {
+		fprintf(stderr, "Write of pre-open packets failed: %s\n",
+			rte_strerror(rte_errno));
+		goto fail;
+	}
+
+	rte_pcapng_close(pcapng);
+
+	/* Validate the file is parseable — timestamps should be
+	 * slightly before 'now' but still reasonable.
+	 */
+	ret = valid_pcapng_file(file_name, now - NS_PER_S, count);
+	if (ret == 0)
+		remove(file_name);
+
+	return ret;
+
+fail:
+	rte_pcapng_close(pcapng);
+	return -1;
+}
+
 static void
 test_cleanup(void)
 {
@@ -589,6 +670,7 @@ unit_test_suite test_pcapng_suite  = {
 	.unit_test_cases = {
 		TEST_CASE(test_add_interface),
 		TEST_CASE(test_write_packets),
+		TEST_CASE(test_write_before_open),
 		TEST_CASES_END()
 	}
 };
-- 
2.51.0

[PATCH v2 4/4] test: use fixed time length for write packet test

[Stephen Hemminger]

This test would take excessively long if the TSC frequency
was very low. Instead of scaling test by frequency, just
use a fixed length for the test.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index deba08cc6f..ea62c949c0 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -262,13 +262,13 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 		 "generations of example code. The magic number 32 is not documented because "
 		 "nobody remembers why. Trust the process."),
 	};
-	/* How many microseconds does it take TSC to wrap around 32 bits */
-	const unsigned wrap_us
-		= (US_PER_S * (uint64_t)UINT32_MAX) / rte_get_tsc_hz();
 
-	/* Want overall test to take to wraparound at least twice. */
-	const unsigned int avg_gap = (2 * wrap_us)
-		/ (TOTAL_PACKETS / (MAX_BURST / 2));
+	/*
+	 * Need this test to take long enough to try and exercise wrap around.
+	 * On fast CPU's this is 4 seconds.
+	 */
+	const unsigned int num_bursts = TOTAL_PACKETS / (MAX_BURST / 2);
+	const unsigned int max_gap = (5 * US_PER_S) / num_bursts;
 
 	mbuf1_prepare(&mbfs);
 	orig  = &mbfs.mb[0];
@@ -309,7 +309,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 			return -1;
 		}
 
-		rte_delay_us_block(rte_rand_max(2 * avg_gap));
+		rte_delay_us_block(rte_rand_max(2 * max_gap));
 	}
 
 	return count;
-- 
2.51.0

[PATCH v3 0/4] pcapng: enhancements and test fix

[Stephen Hemminger]

Modify the test code for pcapng so it can run on Windows.
Use wrappers where needed.

The test code can timeout on slow emulated CPU's.
Revise to cap test time and scale independent of TSC.

The library timestamp conversion can produce bogus values
if rte_pcapng_copy() is called before rte_pcapng_fdopen(),
since the TSC delta wraps unsigned. Handle the negative
delta case explicitly.

v3 - better handling of test timeout


Stephen Hemminger (4):
  test: fix pcapng test to work on Windows
  test: use fixed time length for write packet test
  pcapng: handle packets copied before file open
  test: add pcapng test for copy before open

 app/test/test_pcapng.c  | 176 +++++++++++++++++++++++++++++++++++++---
 lib/pcapng/rte_pcapng.c |  10 ++-
 2 files changed, 170 insertions(+), 16 deletions(-)

-- 
2.51.0

[PATCH v3 1/4] test: fix pcapng test to work on Windows

[Stephen Hemminger]

The pcapng test needs additional wrappers to be able to
build and run on Windows.

This is a pre-existing problem, only exposed when the
test was enabled on Windows builds, and when libpcap
is setup on Windows.

Fixes: 0edc1f408a8b ("test: enable subset of tests on Windows")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 65 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index ad9ad51f4c..f0faead728 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -4,7 +4,16 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
+
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <winsock2.h>
+#include <io.h>
+#include <fcntl.h>
+#include <windows.h>
+#else
 #include <unistd.h>
+#endif
 
 #include <rte_bus_vdev.h>
 #include <rte_ethdev.h>
@@ -23,6 +32,52 @@
 
 #include "test.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+static uint64_t
+current_timestamp(void)
+{
+	FILETIME ft;
+	ULARGE_INTEGER ul;
+
+	GetSystemTimeAsFileTime(&ft);
+	ul.LowPart = ft.dwLowDateTime;
+	ul.HighPart = ft.dwHighDateTime;
+	/* FILETIME is 100ns intervals since 1601-01-01, convert to ns since Unix epoch */
+	return (ul.QuadPart - 116444736000000000ULL) * 100;
+}
+
+/*
+ * Create temporary file with suffix for Windows.
+ * Returns file descriptor or -1 on failure.
+ */
+static int
+mkstemps(char *tmpl, int suffixlen)
+{
+	char temp_dir[MAX_PATH];
+	char temp_file[MAX_PATH];
+	DWORD ret;
+
+	ret = GetTempPathA(sizeof(temp_dir), temp_dir);
+	if (ret == 0 || ret > sizeof(temp_dir))
+		return -1;
+
+	if (GetTempFileNameA(temp_dir, "pcap", 0, temp_file) == 0)
+		return -1;
+
+	/*
+	 * GetTempFileNameA with uUnique=0 creates the file to reserve the name.
+	 * Remove it since we open a different name with the original suffix appended.
+	 */
+	DeleteFileA(temp_file);
+
+	/* Append the original suffix (e.g. ".pcapng") to the temp file */
+	strlcat(temp_file, tmpl + strlen(tmpl) - suffixlen, sizeof(temp_file));
+	strlcpy(tmpl, temp_file, PATH_MAX);
+
+	return _open(tmpl, _O_RDWR | _O_BINARY | _O_CREAT | _O_EXCL, 0666);
+}
+#endif /* RTE_EXEC_ENV_WINDOWS */
+
 #define PCAPNG_TEST_DEBUG 0
 
 #define TOTAL_PACKETS	10000
@@ -344,6 +399,7 @@ parse_pcap_packet(u_char *user, const struct pcap_pkthdr *h,
 	pcap_breakloop(ctx->pcap);
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static uint64_t
 current_timestamp(void)
 {
@@ -352,6 +408,7 @@ current_timestamp(void)
 	clock_gettime(CLOCK_REALTIME, &ts);
 	return rte_timespec_to_ns(&ts);
 }
+#endif
 
 /*
  * Open the resulting pcapng file with libpcap
@@ -395,7 +452,7 @@ valid_pcapng_file(const char *file_name, uint64_t started, unsigned int expected
 static int
 test_add_interface(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	static rte_pcapng_t *pcapng;
 	int ret, tmp_fd;
 	uint64_t now = current_timestamp();
@@ -444,7 +501,7 @@ test_add_interface(void)
 	ret = valid_pcapng_file(file_name, now, 0);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
@@ -456,7 +513,7 @@ test_add_interface(void)
 static int
 test_write_packets(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	rte_pcapng_t *pcapng = NULL;
 	int ret, tmp_fd, count;
 	uint64_t now = current_timestamp();
@@ -508,7 +565,7 @@ test_write_packets(void)
 	ret = valid_pcapng_file(file_name, now, count);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
-- 
2.51.0

[PATCH v3 2/4] test: use fixed time length for write packet test

[Stephen Hemminger]

This test would take excessively long if the TSC frequency
was very low. Instead of scaling test by frequency, just
use a fixed time length for the test.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index f0faead728..af31e7d996 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -5,6 +5,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <time.h>
 
 #ifdef RTE_EXEC_ENV_WINDOWS
 #include <winsock2.h>
@@ -80,8 +81,16 @@ mkstemps(char *tmpl, int suffixlen)
 
 #define PCAPNG_TEST_DEBUG 0
 
+/*
+ * Want to write enough packets to exercise timestamp logic.
+ * On fast CPU's TSC wraps around 32 bits in 4 seconds.
+ */
 #define TOTAL_PACKETS	10000
-#define MAX_BURST	64
+#define MAX_BURST	32
+#define NUM_BURSTS	(TOTAL_PACKETS / MAX_BURST)
+#define TEST_TIME_SEC	4
+#define GAP_US		((TEST_TIME_SEC * US_PER_S) / NUM_BURSTS)
+
 #define DUMMY_MBUF_NUM	2
 
 static struct rte_mempool *mp;
@@ -242,6 +251,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 	struct rte_mbuf *orig;
 	unsigned int burst_size;
 	unsigned int count;
+	struct timespec start_time;
 	ssize_t len;
 	/*
 	 * These are some silly comments to test various lengths and alignments sprinkle
@@ -262,21 +272,22 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 		 "generations of example code. The magic number 32 is not documented because "
 		 "nobody remembers why. Trust the process."),
 	};
-	/* How many microseconds does it take TSC to wrap around 32 bits */
-	const unsigned wrap_us
-		= (US_PER_S * (uint64_t)UINT32_MAX) / rte_get_tsc_hz();
-
-	/* Want overall test to take to wraparound at least twice. */
-	const unsigned int avg_gap = (2 * wrap_us)
-		/ (TOTAL_PACKETS / (MAX_BURST / 2));
 
 	mbuf1_prepare(&mbfs);
 	orig  = &mbfs.mb[0];
 
+	clock_gettime(CLOCK_MONOTONIC, &start_time);
+
 	for (count = 0; count < TOTAL_PACKETS; count += burst_size) {
 		struct rte_mbuf *clones[MAX_BURST];
+		struct timespec now;
 		unsigned int i;
 
+		/* break off writing if test is taking too long */
+		clock_gettime(CLOCK_MONOTONIC, &now);
+		if (now.tv_sec >= start_time.tv_sec + TEST_TIME_SEC)
+			break;
+
 		/* put 1 .. MAX_BURST packets in one write call */
 		burst_size = rte_rand_max(MAX_BURST) + 1;
 		for (i = 0; i < burst_size; i++) {
@@ -309,7 +320,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 			return -1;
 		}
 
-		rte_delay_us_block(rte_rand_max(2 * avg_gap));
+		rte_delay_us_block(rte_rand_max(2 * GAP_US));
 	}
 
 	return count;
-- 
2.51.0

[PATCH v3 3/4] pcapng: handle packets copied before file open

[Stephen Hemminger]

Either through misuse of the API, or races it might happen
that rte_pcapng_copy() is called before rte_pcapng_fdopen().

Simplest solution to handle this is to modify the logic for
TSC to nanosecond epoch conversion to handle the case where
the difference is negative.

Bugzilla ID: 1291
Fixes: 166591931b72 ("pcapng: modify timestamp calculation")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pcapng/rte_pcapng.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/pcapng/rte_pcapng.c b/lib/pcapng/rte_pcapng.c
index b93af418aa..b5d1026891 100644
--- a/lib/pcapng/rte_pcapng.c
+++ b/lib/pcapng/rte_pcapng.c
@@ -157,11 +157,15 @@ tsc_to_ns_epoch(const struct tsc_clock *clk, uint64_t tsc)
 {
 	uint64_t delta, ns;
 
+	if (unlikely(tsc < clk->tsc_base)) {
+		delta = clk->tsc_base - tsc;
+		ns = (delta >> clk->shift) * NSEC_PER_SEC;
+		return clk->ns_base - rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
+	}
+
 	delta = tsc - clk->tsc_base;
 	ns = (delta >> clk->shift) * NSEC_PER_SEC;
-	ns = rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
-
-	return clk->ns_base + ns;
+	return clk->ns_base + rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
 }
 
 /* length of option including padding */
-- 
2.51.0

[PATCH v3 4/4] test: add pcapng test for copy before open

[Stephen Hemminger]

If packets are copied before open is called, the timestamps
will be before the start of epoch. Add a test for this which
makes sure pcapng file is still valid.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 82 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index af31e7d996..69a8f44f1a 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -585,6 +585,87 @@ test_write_packets(void)
 	return -1;
 }
 
+static int
+test_write_before_open(void)
+{
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	struct dummy_mbuf mbfs;
+	struct rte_mbuf *clones[MAX_BURST];
+	rte_pcapng_t *pcapng = NULL;
+	int ret, tmp_fd, i;
+	unsigned int count = 8;
+	uint64_t now;
+	ssize_t len;
+
+	mbuf1_prepare(&mbfs);
+	mbuf1_resize(&mbfs, rte_rand_max(MAX_DATA_SIZE));
+
+	/* Copy packets BEFORE opening the pcapng file.
+	 * This exercises the negative TSC delta path in tsc_to_ns_epoch().
+	 */
+	for (i = 0; i < (int)count; i++) {
+		clones[i] = rte_pcapng_copy(port_id, 0, &mbfs.mb[0], mp,
+					    rte_pktmbuf_pkt_len(&mbfs.mb[0]),
+					    RTE_PCAPNG_DIRECTION_IN, NULL);
+		if (clones[i] == NULL) {
+			fprintf(stderr, "Cannot copy packet before open\n");
+			rte_pktmbuf_free_bulk(clones, i);
+			return -1;
+		}
+	}
+
+	/* Small delay so fdopen's tsc_base is measurably after the copies */
+	rte_delay_us_block(100);
+
+	now = current_timestamp();
+
+	tmp_fd = mkstemps(file_name, strlen(".pcapng"));
+	if (tmp_fd == -1) {
+		perror("mkstemps() failure");
+		rte_pktmbuf_free_bulk(clones, count);
+		return -1;
+	}
+
+	pcapng = rte_pcapng_fdopen(tmp_fd, NULL, NULL, "pcapng_preopen", NULL);
+	if (pcapng == NULL) {
+		fprintf(stderr, "rte_pcapng_fdopen failed\n");
+		close(tmp_fd);
+		rte_pktmbuf_free_bulk(clones, count);
+		return -1;
+	}
+
+	ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB,
+				       NULL, NULL, NULL);
+	if (ret < 0) {
+		fprintf(stderr, "can not add port %u\n", port_id);
+		goto fail;
+	}
+
+	/* Write the pre-captured packets — timestamps precede tsc_base */
+	len = rte_pcapng_write_packets(pcapng, clones, count);
+	rte_pktmbuf_free_bulk(clones, count);
+	if (len <= 0) {
+		fprintf(stderr, "Write of pre-open packets failed: %s\n",
+			rte_strerror(rte_errno));
+		goto fail;
+	}
+
+	rte_pcapng_close(pcapng);
+
+	/* Validate the file is parseable — timestamps should be
+	 * slightly before 'now' but still reasonable.
+	 */
+	ret = valid_pcapng_file(file_name, now - NS_PER_S, count);
+	if (ret == 0)
+		remove(file_name);
+
+	return ret;
+
+fail:
+	rte_pcapng_close(pcapng);
+	return -1;
+}
+
 static void
 test_cleanup(void)
 {
@@ -600,6 +681,7 @@ unit_test_suite test_pcapng_suite  = {
 	.unit_test_cases = {
 		TEST_CASE(test_add_interface),
 		TEST_CASE(test_write_packets),
+		TEST_CASE(test_write_before_open),
 		TEST_CASES_END()
 	}
 };
-- 
2.51.0

[PATCH v3 0/5] pcapng: fixes for Windows and timestamps

[Stephen Hemminger]

Fix pcapng test and library issues:

The test did not build on Windows because it used POSIX-only
APIs (clock_gettime, mkstemps, unlink). Add Windows wrappers
and use portable alternatives where possible.

The test timing was tied to TSC frequency which could make
it run excessively long on slow emulated platforms. Use a
fixed time bound instead.

The test was mixing stdout and stderr making CI log analysis
confusing. Consolidate on stdout and switch to UTC timestamps.

The library timestamp conversion produced bogus values if
rte_pcapng_copy() was called before rte_pcapng_fdopen(),
since the unsigned TSC delta wraps. Handle the case where
the packet TSC precedes the clock baseline.

Add a regression test for the pre-open timestamp fix.

v3 - improve test log output

Stephen Hemminger (5):
  test: fix pcapng test to work on Windows
  test: use fixed time length for write packet test
  test: better logging for pcapng test
  pcapng: handle packets copied before file open
  test: add pcapng test for copy before open

 app/test/test_pcapng.c  | 224 ++++++++++++++++++++++++++++++++++------
 lib/pcapng/rte_pcapng.c |  10 +-
 2 files changed, 197 insertions(+), 37 deletions(-)

-- 
2.51.0

[PATCH v3 1/5] test: fix pcapng test to work on Windows

[Stephen Hemminger]

The pcapng test needs additional wrappers to be able to
build and run on Windows.

This is a pre-existing problem, only exposed when the
test was enabled on Windows builds, and when libpcap
is setup on Windows.

Fixes: 0edc1f408a8b ("test: enable subset of tests on Windows")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 65 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index ad9ad51f4c..f0faead728 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -4,7 +4,16 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
+
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include <winsock2.h>
+#include <io.h>
+#include <fcntl.h>
+#include <windows.h>
+#else
 #include <unistd.h>
+#endif
 
 #include <rte_bus_vdev.h>
 #include <rte_ethdev.h>
@@ -23,6 +32,52 @@
 
 #include "test.h"
 
+#ifdef RTE_EXEC_ENV_WINDOWS
+static uint64_t
+current_timestamp(void)
+{
+	FILETIME ft;
+	ULARGE_INTEGER ul;
+
+	GetSystemTimeAsFileTime(&ft);
+	ul.LowPart = ft.dwLowDateTime;
+	ul.HighPart = ft.dwHighDateTime;
+	/* FILETIME is 100ns intervals since 1601-01-01, convert to ns since Unix epoch */
+	return (ul.QuadPart - 116444736000000000ULL) * 100;
+}
+
+/*
+ * Create temporary file with suffix for Windows.
+ * Returns file descriptor or -1 on failure.
+ */
+static int
+mkstemps(char *tmpl, int suffixlen)
+{
+	char temp_dir[MAX_PATH];
+	char temp_file[MAX_PATH];
+	DWORD ret;
+
+	ret = GetTempPathA(sizeof(temp_dir), temp_dir);
+	if (ret == 0 || ret > sizeof(temp_dir))
+		return -1;
+
+	if (GetTempFileNameA(temp_dir, "pcap", 0, temp_file) == 0)
+		return -1;
+
+	/*
+	 * GetTempFileNameA with uUnique=0 creates the file to reserve the name.
+	 * Remove it since we open a different name with the original suffix appended.
+	 */
+	DeleteFileA(temp_file);
+
+	/* Append the original suffix (e.g. ".pcapng") to the temp file */
+	strlcat(temp_file, tmpl + strlen(tmpl) - suffixlen, sizeof(temp_file));
+	strlcpy(tmpl, temp_file, PATH_MAX);
+
+	return _open(tmpl, _O_RDWR | _O_BINARY | _O_CREAT | _O_EXCL, 0666);
+}
+#endif /* RTE_EXEC_ENV_WINDOWS */
+
 #define PCAPNG_TEST_DEBUG 0
 
 #define TOTAL_PACKETS	10000
@@ -344,6 +399,7 @@ parse_pcap_packet(u_char *user, const struct pcap_pkthdr *h,
 	pcap_breakloop(ctx->pcap);
 }
 
+#ifndef RTE_EXEC_ENV_WINDOWS
 static uint64_t
 current_timestamp(void)
 {
@@ -352,6 +408,7 @@ current_timestamp(void)
 	clock_gettime(CLOCK_REALTIME, &ts);
 	return rte_timespec_to_ns(&ts);
 }
+#endif
 
 /*
  * Open the resulting pcapng file with libpcap
@@ -395,7 +452,7 @@ valid_pcapng_file(const char *file_name, uint64_t started, unsigned int expected
 static int
 test_add_interface(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	static rte_pcapng_t *pcapng;
 	int ret, tmp_fd;
 	uint64_t now = current_timestamp();
@@ -444,7 +501,7 @@ test_add_interface(void)
 	ret = valid_pcapng_file(file_name, now, 0);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
@@ -456,7 +513,7 @@ test_add_interface(void)
 static int
 test_write_packets(void)
 {
-	char file_name[] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
 	rte_pcapng_t *pcapng = NULL;
 	int ret, tmp_fd, count;
 	uint64_t now = current_timestamp();
@@ -508,7 +565,7 @@ test_write_packets(void)
 	ret = valid_pcapng_file(file_name, now, count);
 	/* if test fails want to investigate the file */
 	if (ret == 0)
-		unlink(file_name);
+		remove(file_name);
 
 	return ret;
 
-- 
2.51.0

[PATCH v3 2/5] test: use fixed time length for write packet test

[Stephen Hemminger]

This test would take excessively long if the TSC frequency
was very low. Instead of scaling test by frequency, just
use a fixed time length for the test.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index f0faead728..af31e7d996 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -5,6 +5,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <time.h>
 
 #ifdef RTE_EXEC_ENV_WINDOWS
 #include <winsock2.h>
@@ -80,8 +81,16 @@ mkstemps(char *tmpl, int suffixlen)
 
 #define PCAPNG_TEST_DEBUG 0
 
+/*
+ * Want to write enough packets to exercise timestamp logic.
+ * On fast CPU's TSC wraps around 32 bits in 4 seconds.
+ */
 #define TOTAL_PACKETS	10000
-#define MAX_BURST	64
+#define MAX_BURST	32
+#define NUM_BURSTS	(TOTAL_PACKETS / MAX_BURST)
+#define TEST_TIME_SEC	4
+#define GAP_US		((TEST_TIME_SEC * US_PER_S) / NUM_BURSTS)
+
 #define DUMMY_MBUF_NUM	2
 
 static struct rte_mempool *mp;
@@ -242,6 +251,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 	struct rte_mbuf *orig;
 	unsigned int burst_size;
 	unsigned int count;
+	struct timespec start_time;
 	ssize_t len;
 	/*
 	 * These are some silly comments to test various lengths and alignments sprinkle
@@ -262,21 +272,22 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 		 "generations of example code. The magic number 32 is not documented because "
 		 "nobody remembers why. Trust the process."),
 	};
-	/* How many microseconds does it take TSC to wrap around 32 bits */
-	const unsigned wrap_us
-		= (US_PER_S * (uint64_t)UINT32_MAX) / rte_get_tsc_hz();
-
-	/* Want overall test to take to wraparound at least twice. */
-	const unsigned int avg_gap = (2 * wrap_us)
-		/ (TOTAL_PACKETS / (MAX_BURST / 2));
 
 	mbuf1_prepare(&mbfs);
 	orig  = &mbfs.mb[0];
 
+	clock_gettime(CLOCK_MONOTONIC, &start_time);
+
 	for (count = 0; count < TOTAL_PACKETS; count += burst_size) {
 		struct rte_mbuf *clones[MAX_BURST];
+		struct timespec now;
 		unsigned int i;
 
+		/* break off writing if test is taking too long */
+		clock_gettime(CLOCK_MONOTONIC, &now);
+		if (now.tv_sec >= start_time.tv_sec + TEST_TIME_SEC)
+			break;
+
 		/* put 1 .. MAX_BURST packets in one write call */
 		burst_size = rte_rand_max(MAX_BURST) + 1;
 		for (i = 0; i < burst_size; i++) {
@@ -309,7 +320,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 			return -1;
 		}
 
-		rte_delay_us_block(rte_rand_max(2 * avg_gap));
+		rte_delay_us_block(rte_rand_max(2 * GAP_US));
 	}
 
 	return count;
-- 
2.51.0

[PATCH v3 3/5] test: better logging for pcapng test

[Stephen Hemminger]

The pcapng functional test was mixing output on both stdout
and stderr, which made analysis confusing. Use stdout like
the rest of the test framework.

Also, output timestamps in UTC which makes more sense in
CI environment.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 48 ++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index af31e7d996..53b806833b 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -222,7 +222,7 @@ test_setup(void)
 
 	/* Make a dummy null device to snoop on */
 	if (rte_vdev_init(null_dev, NULL) != 0) {
-		fprintf(stderr, "Failed to create vdev '%s'\n", null_dev);
+		printf("Failed to create vdev '%s'\n", null_dev);
 		goto fail;
 	}
 
@@ -232,7 +232,7 @@ test_setup(void)
 					    rte_pcapng_mbuf_size(MAX_DATA_SIZE),
 					    SOCKET_ID_ANY, "ring_mp_sc");
 	if (mp == NULL) {
-		fprintf(stderr, "Cannot create mempool\n");
+		printf("Cannot create mempool\n");
 		goto fail;
 	}
 
@@ -304,7 +304,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 			mc = rte_pcapng_copy(port_id, 0, orig, mp, rte_pktmbuf_pkt_len(orig),
 					     RTE_PCAPNG_DIRECTION_IN, comment);
 			if (mc == NULL) {
-				fprintf(stderr, "Cannot copy packet\n");
+				printf("Cannot copy packet\n");
 				return -1;
 			}
 			clones[i] = mc;
@@ -315,7 +315,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 		rte_pktmbuf_free_bulk(clones, burst_size);
 
 		if (len <= 0) {
-			fprintf(stderr, "Write of packets failed: %s\n",
+			printf("Write of packets failed: %s\n",
 				rte_strerror(rte_errno));
 			return -1;
 		}
@@ -326,6 +326,7 @@ fill_pcapng_file(rte_pcapng_t *pcapng)
 	return count;
 }
 
+/* Convert time in nanoseconds since 1/1/1970 to UTC time string */
 static char *
 fmt_time(char *buf, size_t size, uint64_t ts_ns)
 {
@@ -333,7 +334,8 @@ fmt_time(char *buf, size_t size, uint64_t ts_ns)
 	size_t len;
 
 	sec = ts_ns / NS_PER_S;
-	len = strftime(buf, size, "%X", localtime(&sec));
+
+	len = strftime(buf, size, "%T", gmtime(&sec));
 	snprintf(buf + len, size - len, ".%09lu",
 		 (unsigned long)(ts_ns % NS_PER_S));
 
@@ -358,6 +360,7 @@ print_packet(uint64_t ts_ns, const struct rte_ether_hdr *eh, size_t len)
 	rte_ether_format_addr(src, sizeof(src), &eh->src_addr);
 	printf("%s: %s -> %s type %x length %zu\n",
 	       tbuf, src, dst, rte_be_to_cpu_16(eh->ether_type), len);
+	fflush(stdout);
 }
 
 /* Callback from pcap_loop used to validate packets in the file */
@@ -368,6 +371,7 @@ parse_pcap_packet(u_char *user, const struct pcap_pkthdr *h,
 	struct pkt_print_ctx *ctx = (struct pkt_print_ctx *)user;
 	const struct rte_ether_hdr *eh;
 	const struct rte_ipv4_hdr *ip;
+	static unsigned int total_errors;
 	uint64_t ns;
 
 	eh = (const struct rte_ether_hdr *)bytes;
@@ -386,18 +390,19 @@ parse_pcap_packet(u_char *user, const struct pcap_pkthdr *h,
 
 		fmt_time(tstart, sizeof(tstart), ctx->start_ns);
 		fmt_time(tend, sizeof(tend), ctx->end_ns);
-		fprintf(stderr, "Timestamp out of range [%s .. %s]\n",
-			tstart, tend);
+
+		printf("Timestamp out of range [%s .. %s]\n",
+		       tstart, tend);
 		goto error;
 	}
 
 	if (!rte_is_broadcast_ether_addr(&eh->dst_addr)) {
-		fprintf(stderr, "Destination is not broadcast\n");
+		printf("Destination is not broadcast\n");
 		goto error;
 	}
 
 	if (rte_ipv4_cksum(ip) != 0) {
-		fprintf(stderr, "Bad IPv4 checksum\n");
+		printf("Bad IPv4 checksum\n");
 		goto error;
 	}
 
@@ -406,8 +411,9 @@ parse_pcap_packet(u_char *user, const struct pcap_pkthdr *h,
 error:
 	print_packet(ns, eh, h->len);
 
-	/* Stop parsing at first error */
-	pcap_breakloop(ctx->pcap);
+	/* Stop parsing at tenth error */
+	if (++total_errors >= 10)
+		pcap_breakloop(ctx->pcap);
 }
 
 #ifndef RTE_EXEC_ENV_WINDOWS
@@ -440,14 +446,14 @@ valid_pcapng_file(const char *file_name, uint64_t started, unsigned int expected
 							   PCAP_TSTAMP_PRECISION_NANO,
 							   errbuf);
 	if (ctx.pcap == NULL) {
-		fprintf(stderr, "pcap_open_offline('%s') failed: %s\n",
+		printf("pcap_open_offline('%s') failed: %s\n",
 			file_name, errbuf);
 		return -1;
 	}
 
 	ret = pcap_loop(ctx.pcap, 0, parse_pcap_packet, (u_char *)&ctx);
 	if (ret != 0) {
-		fprintf(stderr, "pcap_dispatch: failed: %s\n",
+		printf("pcap_dispatch: failed: %s\n",
 			pcap_geterr(ctx.pcap));
 	} else if (ctx.count != expected) {
 		printf("Only %u packets, expected %u\n",
@@ -478,7 +484,7 @@ test_add_interface(void)
 	/* open a test capture file */
 	pcapng = rte_pcapng_fdopen(tmp_fd, NULL, NULL, "pcapng_addif", NULL);
 	if (pcapng == NULL) {
-		fprintf(stderr, "rte_pcapng_fdopen failed\n");
+		printf("rte_pcapng_fdopen failed\n");
 		close(tmp_fd);
 		goto fail;
 	}
@@ -487,7 +493,7 @@ test_add_interface(void)
 	ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB,
 				       NULL, NULL, NULL);
 	if (ret < 0) {
-		fprintf(stderr, "can not add port %u\n", port_id);
+		printf("can not add port %u\n", port_id);
 		goto fail;
 	}
 
@@ -495,7 +501,7 @@ test_add_interface(void)
 	ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB,
 				       "myeth", "Some long description", NULL);
 	if (ret < 0) {
-		fprintf(stderr, "can not add port %u with ifname\n", port_id);
+		printf("can not add port %u with ifname\n", port_id);
 		goto fail;
 	}
 
@@ -503,7 +509,7 @@ test_add_interface(void)
 	ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB,
 				       NULL, NULL, "tcp port 8080");
 	if (ret < 0) {
-		fprintf(stderr, "can not add port %u with filter\n", port_id);
+		printf("can not add port %u with filter\n", port_id);
 		goto fail;
 	}
 
@@ -539,7 +545,7 @@ test_write_packets(void)
 	/* open a test capture file */
 	pcapng = rte_pcapng_fdopen(tmp_fd, NULL, NULL, "pcapng_test", NULL);
 	if (pcapng == NULL) {
-		fprintf(stderr, "rte_pcapng_fdopen failed\n");
+		printf("rte_pcapng_fdopen failed\n");
 		close(tmp_fd);
 		goto fail;
 	}
@@ -548,14 +554,14 @@ test_write_packets(void)
 	ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB,
 				       NULL, NULL, NULL);
 	if (ret < 0) {
-		fprintf(stderr, "can not add port %u\n", port_id);
+		printf("can not add port %u\n", port_id);
 		goto fail;
 	}
 
 	/* write a statistics block */
 	ret = rte_pcapng_write_stats(pcapng, port_id, 0, 0, NULL);
 	if (ret <= 0) {
-		fprintf(stderr, "Write of statistics failed\n");
+		printf("Write of statistics failed\n");
 		goto fail;
 	}
 
@@ -567,7 +573,7 @@ test_write_packets(void)
 	ret = rte_pcapng_write_stats(pcapng, port_id,
 				     count, 0, "end of test");
 	if (ret <= 0) {
-		fprintf(stderr, "Write of statistics failed\n");
+		printf("Write of statistics failed\n");
 		goto fail;
 	}
 
-- 
2.51.0

[PATCH v3 4/5] pcapng: handle packets copied before file open

[Stephen Hemminger]

Either through misuse of the API, or races it might happen
that rte_pcapng_copy() is called before rte_pcapng_fdopen().

Simplest solution to handle this is to modify the logic for
TSC to nanosecond epoch conversion to handle the case where
the difference is negative.

Bugzilla ID: 1291
Fixes: 166591931b72 ("pcapng: modify timestamp calculation")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/pcapng/rte_pcapng.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/pcapng/rte_pcapng.c b/lib/pcapng/rte_pcapng.c
index b93af418aa..b5d1026891 100644
--- a/lib/pcapng/rte_pcapng.c
+++ b/lib/pcapng/rte_pcapng.c
@@ -157,11 +157,15 @@ tsc_to_ns_epoch(const struct tsc_clock *clk, uint64_t tsc)
 {
 	uint64_t delta, ns;
 
+	if (unlikely(tsc < clk->tsc_base)) {
+		delta = clk->tsc_base - tsc;
+		ns = (delta >> clk->shift) * NSEC_PER_SEC;
+		return clk->ns_base - rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
+	}
+
 	delta = tsc - clk->tsc_base;
 	ns = (delta >> clk->shift) * NSEC_PER_SEC;
-	ns = rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
-
-	return clk->ns_base + ns;
+	return clk->ns_base + rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
 }
 
 /* length of option including padding */
-- 
2.51.0

[PATCH v3 5/5] test: add pcapng test for copy before open

[Stephen Hemminger]

If packets are copied before open is called, the timestamps
will be before the start of epoch. Add a test for this which
makes sure pcapng file is still valid.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_pcapng.c | 82 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/app/test/test_pcapng.c b/app/test/test_pcapng.c
index 53b806833b..298bcbd31f 100644
--- a/app/test/test_pcapng.c
+++ b/app/test/test_pcapng.c
@@ -591,6 +591,87 @@ test_write_packets(void)
 	return -1;
 }
 
+static int
+test_write_before_open(void)
+{
+	char file_name[PATH_MAX] = "/tmp/pcapng_test_XXXXXX.pcapng";
+	struct dummy_mbuf mbfs;
+	struct rte_mbuf *clones[MAX_BURST];
+	rte_pcapng_t *pcapng = NULL;
+	int ret, tmp_fd, i;
+	unsigned int count = 8;
+	uint64_t now;
+	ssize_t len;
+
+	mbuf1_prepare(&mbfs);
+	mbuf1_resize(&mbfs, rte_rand_max(MAX_DATA_SIZE));
+
+	/* Copy packets BEFORE opening the pcapng file.
+	 * This exercises the negative TSC delta path in tsc_to_ns_epoch().
+	 */
+	for (i = 0; i < (int)count; i++) {
+		clones[i] = rte_pcapng_copy(port_id, 0, &mbfs.mb[0], mp,
+					    rte_pktmbuf_pkt_len(&mbfs.mb[0]),
+					    RTE_PCAPNG_DIRECTION_IN, NULL);
+		if (clones[i] == NULL) {
+			fprintf(stderr, "Cannot copy packet before open\n");
+			rte_pktmbuf_free_bulk(clones, i);
+			return -1;
+		}
+	}
+
+	/* Small delay so fdopen's tsc_base is measurably after the copies */
+	rte_delay_us_block(100);
+
+	now = current_timestamp();
+
+	tmp_fd = mkstemps(file_name, strlen(".pcapng"));
+	if (tmp_fd == -1) {
+		perror("mkstemps() failure");
+		rte_pktmbuf_free_bulk(clones, count);
+		return -1;
+	}
+
+	pcapng = rte_pcapng_fdopen(tmp_fd, NULL, NULL, "pcapng_preopen", NULL);
+	if (pcapng == NULL) {
+		fprintf(stderr, "rte_pcapng_fdopen failed\n");
+		close(tmp_fd);
+		rte_pktmbuf_free_bulk(clones, count);
+		return -1;
+	}
+
+	ret = rte_pcapng_add_interface(pcapng, port_id, DLT_EN10MB,
+				       NULL, NULL, NULL);
+	if (ret < 0) {
+		fprintf(stderr, "can not add port %u\n", port_id);
+		goto fail;
+	}
+
+	/* Write the pre-captured packets — timestamps precede tsc_base */
+	len = rte_pcapng_write_packets(pcapng, clones, count);
+	rte_pktmbuf_free_bulk(clones, count);
+	if (len <= 0) {
+		fprintf(stderr, "Write of pre-open packets failed: %s\n",
+			rte_strerror(rte_errno));
+		goto fail;
+	}
+
+	rte_pcapng_close(pcapng);
+
+	/* Validate the file is parseable — timestamps should be
+	 * slightly before 'now' but still reasonable.
+	 */
+	ret = valid_pcapng_file(file_name, now - NS_PER_S, count);
+	if (ret == 0)
+		remove(file_name);
+
+	return ret;
+
+fail:
+	rte_pcapng_close(pcapng);
+	return -1;
+}
+
 static void
 test_cleanup(void)
 {
@@ -606,6 +687,7 @@ unit_test_suite test_pcapng_suite  = {
 	.unit_test_cases = {
 		TEST_CASE(test_add_interface),
 		TEST_CASE(test_write_packets),
+		TEST_CASE(test_write_before_open),
 		TEST_CASES_END()
 	}
 };
-- 
2.51.0

Re: [PATCH v3 0/5] pcapng: fixes for Windows and timestamps

[Thomas Monjalon]

> Stephen Hemminger (5):
>   test: fix pcapng test to work on Windows
>   test: use fixed time length for write packet test
>   test: better logging for pcapng test
>   pcapng: handle packets copied before file open
>   test: add pcapng test for copy before open

Applied (with test/pcapng prefix), thanks.

Re: [PATCH v3 4/5] pcapng: handle packets copied before file open

[Kevin Traynor]

On 2/28/26 5:35 PM, Stephen Hemminger wrote:
> Either through misuse of the API, or races it might happen
> that rte_pcapng_copy() is called before rte_pcapng_fdopen().
> 
> Simplest solution to handle this is to modify the logic for
> TSC to nanosecond epoch conversion to handle the case where
> the difference is negative.
> 
> Bugzilla ID: 1291
> Fixes: 166591931b72 ("pcapng: modify timestamp calculation")
> Cc: stable@dpdk.org
> 

Hi Stephen,

Fixes tag goes back to 23.11 but the fix modifies a function added in
26.03 commit below, which doesn't apply cleanly either.

So will just drop from LTS. Otherwise we'll need custom backports and
there's quite a lot of churn.

thanks,
Kevin.

commit 4fc65615b274701070c67f8c424d93aef8bf7483
Author: Stephen Hemminger <stephen@networkplumber.org>
Date:   Mon Feb 16 13:38:03 2026 -0800

    pcapng: improve performance of timestamping


> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/pcapng/rte_pcapng.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/pcapng/rte_pcapng.c b/lib/pcapng/rte_pcapng.c
> index b93af418aa..b5d1026891 100644
> --- a/lib/pcapng/rte_pcapng.c
> +++ b/lib/pcapng/rte_pcapng.c
> @@ -157,11 +157,15 @@ tsc_to_ns_epoch(const struct tsc_clock *clk, uint64_t tsc)
>  {
>  	uint64_t delta, ns;
>  
> +	if (unlikely(tsc < clk->tsc_base)) {
> +		delta = clk->tsc_base - tsc;
> +		ns = (delta >> clk->shift) * NSEC_PER_SEC;
> +		return clk->ns_base - rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
> +	}
> +
>  	delta = tsc - clk->tsc_base;
>  	ns = (delta >> clk->shift) * NSEC_PER_SEC;
> -	ns = rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
> -
> -	return clk->ns_base + ns;
> +	return clk->ns_base + rte_reciprocal_divide_u64(ns, &clk->tsc_hz_inv);
>  }
>  
>  /* length of option including padding */

Re: [PATCH v3 4/5] pcapng: handle packets copied before file open

[Stephen Hemminger]

On Thu, 19 Mar 2026 10:34:18 +0000
Kevin Traynor <ktraynor@redhat.com> wrote:

> On 2/28/26 5:35 PM, Stephen Hemminger wrote:
> > Either through misuse of the API, or races it might happen
> > that rte_pcapng_copy() is called before rte_pcapng_fdopen().
> > 
> > Simplest solution to handle this is to modify the logic for
> > TSC to nanosecond epoch conversion to handle the case where
> > the difference is negative.
> > 
> > Bugzilla ID: 1291
> > Fixes: 166591931b72 ("pcapng: modify timestamp calculation")
> > Cc: stable@dpdk.org
> >   


This is a corner case and doesn't need backport