On Tue, Mar 10, 2026 at 09:10:02AM -0700, Stephen Hemminger wrote:
> Add unit tests for the pcap PMD covering file and interface modes.
>
> Tests include:
> - basic TX to file and RX from file
> - varied packet sizes and jumbo frames
> - infinite RX mode
> - TX drop mode
> - statistics
> - interface (iface=) pass-through mode
> - link status reporting for file and iface modes
> - link status change (LSC) with interface toggle
> - EOF notification via LSC
> - RX timestamps and timestamp with infinite RX
> - multiple TX/RX queues
> - VLAN strip, insert, and runtime offload configuration
> - snapshot length (snaplen) and truncation
>
> Cross-platform helpers handle temp file creation, interface
> discovery, and VLAN packet generation.
>
> The LSC link toggle test requires a pre-created dummy interface
> (Linux: dummy0, FreeBSD: disc0) and is skipped if unavailable.
>
> Signed-off-by: Stephen Hemminger <[email protected]>
> ---
> app/test/meson.build | 2 +
> app/test/test_pmd_pcap.c | 3755 ++++++++++++++++++++++++
> doc/guides/rel_notes/release_26_03.rst | 1 +
> 3 files changed, 3758 insertions(+)
> create mode 100644 app/test/test_pmd_pcap.c
>
Ran this patch through AI for a code review. Below is the output. In
general seems a good addition.
/Bruce
Code Review: test: add comprehensive test suite for pcap PMD
The patch adds a 3755-line test file. The breadth of coverage is commendable —
it exercises most of the pcap PMD's devargs. However, there are several
concrete problems:
Bugs / Incorrect Logic
1. Duplicate values in test_tx_varied_sizes test array
(test_pmd_pcap.c:1203-1205)
PKT_SIZE_SMALL (128) == PACKET_BURST_GEN_PKT_LEN_128 (128), so only two
distinct sizes are tested. Furthermore, PKT_SIZE_MIN(60) ==
PACKET_BURST_GEN_PKT_LEN (60) — the whole point of mixing these constants
obscures the fact. The array should use distinct, intentional sizes like
PKT_SIZE_MIN, PKT_SIZE_MEDIUM, PKT_SIZE_LARGE.
2. Timestamp exact-match check is non-asserting (test_pmd_pcap.c:2322-2325)
A timestamp accuracy mismatch only prints a warning; the test still passes.
This should be a TEST_ASSERT_EQUAL(ts, expected, ...). This is the entire point
of having controllable timestamps via create_timestamped_pcap().
Test Quality / Weak Assertions
3. test_tx_varied_sizes doesn't verify file content (test_pmd_pcap.c:1240-1244)
It only checks nb_tx > 0 — it never reads the pcap file back to confirm the
sizes were written correctly. Compare with test_jumbo_tx which does read back
via get_pcap_packet_sizes(). Both tests should do the same.
4. Statistics test doesn't verify ibytes/obytes values
(test_pmd_pcap.c:1462-1474)
test_stats validates ipackets/opackets but despite checking ibytes == 0 &&
obytes == 0 at init, it never asserts the actual byte totals after traffic. The
driver tracks bytes; the test should verify ibytes == received *
sizeof(test_packet).
5. test_lsc_iface skips with TEST_SUCCESS instead of TEST_SKIPPED
(test_pmd_pcap.c:1981-2000)
Three early-exit paths when no interface is available return TEST_SUCCESS:
Compare with test_iface() at test_pmd_pcap.c:1644 which correctly returns
TEST_SKIPPED. This makes CI results misleading — a skipped test looks like a
passing one.
6. Similarly in test_rx_timestamp (test_pmd_pcap.c:2308)
When the timestamp dynamic field isn't available, return TEST_SUCCESS is used
instead of TEST_SKIPPED.
Racy / Fragile Tests
7. LSC callback detection relies entirely on usleep(1500ms)
(test_pmd_pcap.c:2050, test_pmd_pcap.c:2074)
The test sleeps 1.5 seconds twice waiting for the pcap LSC polling thread to
detect the interface state change. With no retry/poll loop, this is fragile
under load and adds 3 seconds to every run. A small polling loop (for (i = 0; i
< 20; i++) { if (lsc_callback_count >= 1) break; usleep(100000); }) would be
more robust.
8. EOF callback timing (test_pmd_pcap.c:2229)
usleep(100 * 1000) (100 ms) is used to wait for the EOF alarm to fire, then
immediately asserts eof_callback_count == 1. The driver uses
rte_eal_alarm_set(1, ...) (1 µs delay), but alarm delivery is not instantaneous
under load. The same polling loop pattern would be safer.
Missing Coverage
9. No tests for rx_iface=/tx_iface=/rx_iface_in= devargs
The test covers iface= (symmetric single-interface mode) but completely misses
rx_iface=, tx_iface=, and rx_iface_in= (asymmetric / separate RX and TX
interface modes). These are distinct code paths in the driver including
PCAP_D_IN direction filtering for rx_iface_in.
10. No test for per-queue start/stop
The driver implements rx_queue_start, tx_queue_start, rx_queue_stop,
tx_queue_stop. None of these are exercised.
11. imissed stat not tested
The driver has queue_missed_stat_update() using pcap_stats() to track
kernel-level drops. This code path (iface mode under load, or mocked via the
pcap stat interface) is never exercised.
Minor Issues
12. Shared state between tests (test_pmd_pcap.c:60-61)
tx_pcap_path and vlan_rx_pcap_path are global state shared between test pairs
(test_tx_to_file→test_rx_from_file, test_vlan_strip_rx→test_vlan_no_strip_rx).
Both have fallback access() checks, but this creates implicit ordering
dependencies. The comment documents this, but it's an unusual pattern for a
test suite that should have independent cases.
13. Missing rte_cycles.h include for NS_PER_S (test_pmd_pcap.c:2319)
NS_PER_S is used in test_rx_timestamp() but <rte_cycles.h> (where it's defined)
is not explicitly included. It compiles today because another included header
transitively pulls it in, but the dependency is fragile. A direct #include
<rte_cycles.h> should be added.
