Skip to content

draft: try running live tests - v2 - #16076

Draft
jasonish wants to merge 2 commits into
OISF:mainfrom
jasonish:sv-live/v2
Draft

draft: try running live tests - v2 #16076
jasonish wants to merge 2 commits into
OISF:mainfrom
jasonish:sv-live/v2

Conversation

@jasonish

@jasonish jasonish commented Aug 21, 2026

Copy link
Copy Markdown
Member

SV_BRANCH=OISF/suricata-verify#3298

Exists for testing the SV PR above.

@jasonish
jasonish marked this pull request as draft August 21, 2026 23:04
@suricata-qa

Copy link
Copy Markdown

Information: QA skipped due to no C or rust code changed detected. Set to force a run.

Pipeline = code

@jasonish

Copy link
Copy Markdown
Member Author

AI-generated review posted automatically by Suricata ai-review.

Verdict: request changes

  • Medium — .github/workflows/builds.yml:1897 (also :2148, :2282, :2977) — Passing run selects tests whose names contain that pattern; none do, so all four invocations execute zero tests and the namespace job cannot produce its expected profile. Remove the positional argument and make empty selections fail.
  • Medium — suricata-verify/live/run.py:1514stop_suricata() returns Suricata’s exit status, but it is discarded. Post-startup crashes and ASAN aborts can therefore pass when client/output checks succeed. Treat unexpected nonzero exits as failures and test a forced crash.
  • Medium — suricata-verify/.github/workflows/live.yaml:72 — Ubuntu 24.04 installs Rust 1.75, while current Suricata requires 1.85 or newer. The linked PR’s live job currently fails during configure before running any tests. Install Suricata’s declared Rust toolchain.
  • Medium — suricata-verify/live/tests/demo-afp-xdp-bypass-stats-shutdown/test.yaml:35 — The Redmine Pr 8341 and payload len fix/v5 #8440 test asserts counters already present on unfixed main, so it misses the loss of final bypass statistics during shutdown. Pin the bypass timeout beyond the test duration and assert positive flow_bypassed.pkts and flow_bypassed.bytes.
  • Medium — suricata-verify/live/tests/demo-replay-xdp-bypass-vlan/test.yaml:27 — The Redmine Suricata library v2 #8699 test only proves that packets reached capture, which also succeeds when VLAN-aware XDP bypass is broken. Assert positive bypass counters or equivalent evidence that the VLAN-tagged flow entered capture bypass.

Adds podman where needed.
@suricata-qa

Copy link
Copy Markdown

Information: QA skipped due to no C or rust code changed detected. Set to force a run.

Pipeline = code

@jasonish

Copy link
Copy Markdown
Member Author

AI-generated review posted automatically by Suricata ai-review.

Verdict: request changes

  • Medium — .github/workflows/builds.yml:1897 (also :2148, :2282, :2977) — All four invocations omit eBPF/XDP support, tcpdump, and tcpreplay; unmet requirements are successful skips. Consequently, every XDP regression and both bond ports remain untested. Provision the required features/tools in appropriate jobs and fail on unexpected skips or zero executions.
  • Medium — suricata-verify/live/run.py:1514stop_suricata() returns Suricata’s status, including crashes and forced SIGKILL, but the result is discarded. Post-startup failures and ASAN aborts can therefore pass when output checks succeed. Fail on unexpected nonzero exits and cover a forced crash or shutdown timeout.
  • Medium — suricata-verify/live/run.py:716 — Cleanup claims every existing namespace matching clientN, serverN, or dut, kills its processes, and deletes it. Running the documented root command can destroy unrelated network labs and workloads. Use runner-owned names or ownership markers and test that pre-existing namespaces remain untouched.
  • Medium — suricata-verify/.github/workflows/live.yaml:72 — Ubuntu 24.04 installs Rust 1.75, while Suricata requires 1.85 or newer. The linked PR’s live check fails during configure before any test executes. Install Suricata’s required Rust toolchain.
  • Medium — suricata-verify/live/tests/demo-afp-xdp-bypass-stats-shutdown/test.yaml:35 — Redmine 8440 requires final flow_bypassed.pkts and flow_bypassed.bytes during shutdown, but this test asserts only counters already present without the fix. Keep the bypass timeout beyond the test duration and assert both final bypass counters are positive.
  • Medium — suricata-verify/live/tests/demo-replay-xdp-bypass-vlan/test.yaml:26 — Redmine 8699 distinguishes the fix through a positive bypass count; capture.kernel_packets >= 100 also passes when VLAN-aware XDP bypass is broken. Assert positive bypass counters or equivalent evidence that the VLAN flow entered capture bypass.

@jasonish

Copy link
Copy Markdown
Member Author

AI-generated review posted automatically by Suricata ai-review.

Verdict: request changes

  • High — suricata-verify/live/run.py:194 — The root-only runner follows a fixed /tmp lock-file symlink and later truncates its target. On hosts without protected-symlink enforcement, an unprivileged user can overwrite any root-writable file. Use a private root-owned runtime directory or O_NOFOLLOW with ownership/type validation, and test symlink preplacement.
  • Medium — suricata-verify/live/run.py:716 — Cleanup claims every namespace named dut, clientN, or serverN, kills its processes, and deletes it; matching host interfaces are similarly removed. Running the documented root command can destroy unrelated network labs. Use runner-owned unique names or ownership markers and verify pre-existing resources survive.
  • Medium — suricata-verify/live/run.py:909 — Blocking readline() prevents the 60-second startup deadline from firing when Suricata remains alive without producing a complete line, potentially stalling CI until its outer timeout. Use nonblocking I/O or a reader thread and test a silent hung process.
  • Medium — suricata-verify/live/run.py:1514stop_suricata() returns crash, ASAN-abort, and forced-SIGKILL statuses, but the result is discarded. Tests can therefore pass after Suricata fails post-startup. Fail on unexpected nonzero exits and cover a forced crash or shutdown timeout.
  • Medium — suricata-verify/live/run.py:1684 — A selection matching no tests returns success with zero passes, failures, and skips, allowing misspelled targeted CI patterns to report green. Require at least one selected test and cover an empty selection.
  • Medium — .github/workflows/builds.yml:1900 (also :2153, :2290, and :2988) — All seven resulting executions use builds configured without eBPF/XDP, so all three XDP regressions skip successfully. Invoke the live runner from an eBPF-enabled job or enable the required feature in an appropriate invocation.
  • Medium — suricata-verify/live/tests/demo-afp-xdp-bypass-stats-shutdown/test.yaml:35 — Redmine Pr 8341 and payload len fix/v5 #8440’s unfixed case already reports the asserted local_capture_* counters, so this test cannot detect loss of final bypass statistics during shutdown. Keep the bypass timeout beyond the test duration and assert positive flow_bypassed.pkts and flow_bypassed.bytes.
  • Medium — suricata-verify/live/tests/demo-replay-xdp-bypass-vlan/test.yaml:27 — Redmine Suricata library v2 #8699 distinguishes the fix through a positive bypass count, but capture.kernel_packets >= 100 also passes when VLAN-aware XDP bypass is broken. Assert positive bypass counters or equivalent evidence that the VLAN flow entered capture bypass.

@suricata-qa

Copy link
Copy Markdown

Information: QA skipped due to no C or rust code changed detected. Set to force a run.

Pipeline = code

@suricata-review-bot

Copy link
Copy Markdown

Test comment from the Suricata Review Bot account.

This verifies that ai-review can post using its fine-grained PAT.

@jasonish

Copy link
Copy Markdown
Member Author

@suricata-review-bot please elaborate on:

Medium — suricata-verify/live/tests/demo-replay-xdp-bypass-vlan/test.yaml:27 — Redmine #8699 distinguishes the fix through a positive bypass count, but capture.kernel_packets >= 100 also passes when VLAN-aware XDP bypass is broken. Assert positive bypass counters or equivalent evidence that the VLAN flow entered capture bypass.

@jasonish

Copy link
Copy Markdown
Member Author

@suricata-review-bot Tell me more about: Medium — suricata-verify/live/tests/demo-afp-xdp-bypass-stats-shutdown/test.yaml:35 — Redmine https://github.com/OISF/suricata/pull/8440’s unfixed case already reports the asserted local_capture_* counters, so this test cannot detect loss of final bypass statistics during shutdown. Keep the bypass timeout beyond the test duration and assert positive flow_bypassed.pkts and flow_bypassed.bytes.

@suricata-review-bot

Copy link
Copy Markdown

@jasonish

⚠️ AI-generated response. This answer was produced by the Suricata ai-review bot for this question against commit 218c9d6fde1b.

The test checks the wrong counters. local_capture_pkts/bytes count packets still seen by Suricata while capture bypass takes effect; they can be positive even when XDP’s final accounting is lost (src/flow-worker.c:218, ebpf-xdp.rst:605).

Ticket #8440 concerns flow_bypassed.pkts/bytes, populated when the capture backend is queried (src/flow-manager.c:235). On the unfixed path, an unexpired flow skips that query, and shutdown merely syncs existing counters (src/flow-manager.c:359, src/flow-manager.c:974).

Make the regression test explicit:

# include.yaml
flow-timeouts:
  tcp:
    bypassed: 100

# test.yaml
flow_bypassed.pkts.__gt: 0
flow_bypassed.bytes.__gt: 0

The client runs for roughly one second, so the 100-second timeout ensures these values must come from shutdown collection rather than natural expiry. The existing local_capture_* checks may remain as supplemental bypass evidence.

@suricata-review-bot

Copy link
Copy Markdown

@jasonish

⚠️ AI-generated response. This answer was produced by the Suricata ai-review bot for this question against commit 218c9d6fde1b.

capture.kernel_packets measures packets delivered to AF_PACKET, not packets bypassed by XDP. If VLAN lookup fails, the XDP map misses and returns XDP_PASS; all 100 replayed packets reach Suricata, so the current assertion passes (test.yaml:21-28). A successful lookup instead increments the eBPF map counters and returns XDP_DROP (ebpf/xdp_filter.c:282-318).

This directly exercises the suspected mismatch: userspace inserts decoded VLAN IDs (src/source-af-packet.c:2248-2253), while XDP currently masks the network-order TCI without converting it (ebpf/xdp_filter.c:517-540).

Replace the capture assertion with direct evidence of map hits:

checks:
  - stats:
      flow_bypassed.pkts.__gt: 0
      flow_bypassed.bytes.__gt: 0

Those counters are populated from the bypass-map accounting (src/flow-bypass.c:104-115), proving the VLAN flow actually entered XDP bypass.

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.12%. Comparing base (acd9d3a) to head (218c9d6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16076      +/-   ##
==========================================
+ Coverage   83.05%   83.12%   +0.06%     
==========================================
  Files        1004     1004              
  Lines      277359   277359              
==========================================
+ Hits       230370   230557     +187     
+ Misses      46989    46802     -187     
Flag Coverage Δ
fuzzcorpus 61.51% <ø> (-0.03%) ⬇️
livemode 26.46% <ø> (+7.87%) ⬆️
netns 27.10% <ø> (+4.19%) ⬆️
pcap 45.36% <ø> (-0.05%) ⬇️
suricata-verify 67.12% <ø> (-0.03%) ⬇️
unittests 58.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jasonish

Copy link
Copy Markdown
Member Author

@suricata-review-bot Tell me about any private work being done on SMTP.

@suricata-review-bot

Copy link
Copy Markdown

@jasonish

⚠️ AI-generated response. This answer was produced by the Suricata ai-review bot for this question against commit 218c9d6fde1b.

Question from @jasonish:

@suricata-review-bot Tell me about any private work being done on SMTP.

I can’t provide information about private work. This PR contains no SMTP-related changes; its diff only updates live-test CI wiring in .github/workflows/builds.yml:1900.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants