pcap-file: move packet counter to PCAP packet structure v5.2 #14657

lukashino · 2026-01-20T15:45:18Z

Follow-up of #14656

Code refactoring to gather all PCAP-related structure members under one structure.

Link to ticket: https://redmine.openinfosecfoundation.org/issues/7835

Some considerations:

Unit and Fuzz tests use pcap_cnt - the setter/getter functions are used
Plugins - only NDPI used the pcap_cnt, so I converted the code to use the packet counter. If it would like to use pcap_cnt in non-PCAP file runmodes, then it should create its own plugin variables structure and map it to the plugin_v Packet structure member.
PCAP getter/setter functions were considered slow and therefore disregarded in the hot per-packet data paths.

The PR has been previously OKAY-ed by Philippe (#13972 ).

Describe changes:
v5.2:

PcapNum -> PcapCnt (Victor's comment)
reduced number of Get calls per Copilot review

v5.1 (address copilot review comments):

steam condensed to stream (typo update)
removed comment in the packet structure about accessing it only through setter/getter functions as the previous version violated that
SCRunmode variable to cache before the triple comparison

v5:

rebased
In PCAP-native files, the structures are accessed directly
The packet init function does not initialize the PCAP counter value; instead, it relies on the initialization in the PCAP RX loop itself (it assigns the correct packet number directly).
removed use of function getter/setters in the hot paths, where possible. In most places, the functions are used only in Debug mode. Currently, I have identified these non-debug paths, still reaching the pcap count variable:
- alert-syslog.c
- alert-fastlog.c
- output-json.c
- output-json-alert.c
- util-lua-packetlib.c
- StreamTcpValidateChecksum() - though this is heavily mitigated through p->livedev check.

v4.2

rebased
moved pcap_cnt access functions to decode.c/h files to avoid extra files as per Victor's suggestion

v4.1

compilation fixes

v4

rebased
split into two commits, the first commit represents the "change-everywhere" access patterns. This is done through helper macros. The second commit actually moves the structure member.

v2+3

added function guards to only access the pcap_cnt if we are in the right mode

v1

moved pcap_cnt under pcap_v unioned structured

Use of t_pcapcnt is only relevant when compiled in debug mode only. This patch adds additional debug guard to also shield the declaration and assignment.

For an easier review process, this is a two-step change process, in which pcap_cnt is first accessed by functions-to-be, implemented as simple macros. In the follow-up commit, the actual refactor is implemented with the new function. The old macros are deleted. Ticket: 7835

Code refactor to gather all PCAP-related structure members under one structure. New pcap_v structure guards protect the union variables from other capture modes trying to access the packet number incorrectly. Ticket: 7835

Copilot

Pull request overview

This PR refactors the packet counter handling by moving the pcap_cnt field from the main Packet structure into the PcapPacketVars structure (pcap_v), which is part of the existing union for packet source-specific variables. This consolidates all PCAP-related structure members under one structure.

Changes:

Introduced getter/setter functions (PcapPacketCntGet, PcapPacketCntSet) with runmode validation to control access to pcap_cnt
Updated all code references to use the getter/setter functions instead of direct field access
Moved the pcap_cnt field from the Packet structure to the PcapPacketVars structure

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/decode.h	Removed `pcap_cnt` from Packet structure; added function declarations for getter/setter
src/decode.c	Implemented getter/setter functions with runmode validation logic
src/source-pcap.h	Added `pcap_cnt` field to `PcapPacketVars` structure
src/source-pcap-file-helper.c	Direct access to `p->pcap_v.pcap_cnt` in PCAP-native code
src/packet.c	Removed `pcap_cnt` initialization from `PacketReinit`
src/stream-tcp.c	Moved `t_pcapcnt` extern declaration inside DEBUG block; converted all access to use getter
src/stream-tcp-reassemble.c	Moved `t_pcapcnt` declaration inside DEBUG block
src/stream-tcp-private.h	Updated macro to use getter function
src/stream-tcp-sack.c	Converted to use getter function in debug logging
src/util-profiling.c	Converted to use getter function
src/util-lua-packetlib.c	Converted to use getter function
src/util-exception-policy.c	Converted to use getter function in debug logging
src/output-tx.c	Converted to use getter function in debug logging
src/output-json.c	Converted to use getter with local variable caching
src/output-json-alert.c	Converted to use getter with local variable caching
src/log-tlsstore.c	Converted to use getter with local variable caching
src/flow.c	Converted to use getter function in debug logging
src/flow-worker.c	Converted to use getter function in debug logging
src/flow-hash.c	Converted to use getter function in DEBUG code
src/detect.c	Converted to use getter function in debug logging
src/detect-*.c	Converted to use getter function in debug logging across multiple detect modules
src/defrag.c	Converted to use getter function in DEBUG code
src/defrag-hash.c	Converted to use getter function in DEBUG code
src/app-layer.c	Converted to use getter function in debug logging
src/alert-*.c	Converted to use getter with local variable caching for output formatting
src/tests/fuzz/*.c	Converted to use setter function
src/tests/detect-*.c	Converted to use setter function in test setup
plugins/ndpi/ndpi.c	Converted to use getter function in debug logging

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-01-20T16:05:56Z

Codecov Report

❌ Patch coverage is 91.30435% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.11%. Comparing base (c333b28) to head (8f4a39f).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14657      +/-   ##
==========================================
- Coverage   82.11%   82.11%   -0.01%     
==========================================
  Files        1011     1011              
  Lines      262812   262863      +51     
==========================================
+ Hits       215812   215847      +35     
- Misses      47000    47016      +16

Flag	Coverage Δ
fuzzcorpus	`60.26% <80.26%> (+0.08%)`	⬆️
livemode	`18.71% <8.07%> (+<0.01%)`	⬆️
pcap	`44.57% <56.50%> (-0.02%)`	⬇️
suricata-verify	`65.29% <79.37%> (+<0.01%)`	⬆️
unittests	`59.26% <28.85%> (-0.01%)`	⬇️

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.

victorjulien · 2026-01-22T09:10:20Z

src/output-json-alert.c

    SCJbSetString(js, "proto", addr.proto);

    SCJbSetUint(js, "depth", p->recursion_level);
+    uint64_t pcap_cnt = PcapPacketCntGet(p->root);


This adds an additional check, once for PcapPacketCntRunmodeCanAccess then for the result of PcapPacketCntGet. Could be a nice micro optimization to do something like

if (PcapPacketCntRunmodeCanAccess()) { pcap_cnt = PcapPacketCntGet ... if (pcap_cnt != 0) { ...

victorjulien · 2026-01-22T09:10:50Z

src/output-json.c

    /* pcap_cnt */
-    if (p->pcap_cnt != 0) {
-        SCJbSetUint(js, "pcap_cnt", p->pcap_cnt);
+    uint64_t pcap_cnt = PcapPacketCntGet(p);


same micro optimization opportunity here

victorjulien

Looks good. Minor comments inline, that can be addressed in a follow up post-merge.

suricata-qa · 2026-01-22T12:00:03Z

Information: QA ran without warnings.

Pipeline = 29241

victorjulien · 2026-01-22T20:47:52Z

Merged in #14673, thanks!

Lukas Sismis added 4 commits January 19, 2026 16:04

stream: guard auxiliary packet counter to debug

2239add

Use of t_pcapcnt is only relevant when compiled in debug mode only. This patch adds additional debug guard to also shield the declaration and assignment.

stream: remove dead commented out code

afb7461

pcap-file: move packet counter to PCAP packet structure

8f4a39f

Code refactor to gather all PCAP-related structure members under one structure. New pcap_v structure guards protect the union variables from other capture modes trying to access the packet number incorrectly. Ticket: 7835

Copilot AI review requested due to automatic review settings January 20, 2026 15:45

lukashino requested review from a team and victorjulien as code owners January 20, 2026 15:45

lukashino mentioned this pull request Jan 20, 2026

pcap-file: move packet counter to PCAP packet structure v5.1 #14656

Closed

Copilot started reviewing on behalf of lukashino January 20, 2026 15:45 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

lukashino added the waiting for qa label Jan 20, 2026

suricata-qa removed the waiting for qa label Jan 20, 2026

victorjulien added the waiting for qa label Jan 22, 2026

OISF deleted a comment from suricata-qa Jan 22, 2026

victorjulien reviewed Jan 22, 2026

View reviewed changes

victorjulien approved these changes Jan 22, 2026

View reviewed changes

victorjulien added this to the 9.0 milestone Jan 22, 2026

victorjulien mentioned this pull request Jan 22, 2026

next/1187/20260122/v1 #14673

Merged

suricata-qa removed the waiting for qa label Jan 22, 2026

victorjulien closed this Jan 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pcap-file: move packet counter to PCAP packet structure v5.2 #14657

pcap-file: move packet counter to PCAP packet structure v5.2 #14657

Uh oh!

lukashino commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

codecov bot commented Jan 20, 2026 •

edited

Loading

Uh oh!

victorjulien Jan 22, 2026

Uh oh!

victorjulien Jan 22, 2026

Uh oh!

victorjulien left a comment

Uh oh!

suricata-qa commented Jan 22, 2026

Uh oh!

victorjulien commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

pcap-file: move packet counter to PCAP packet structure v5.2 #14657

pcap-file: move packet counter to PCAP packet structure v5.2 #14657

Uh oh!

Conversation

lukashino commented Jan 20, 2026

Some considerations:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

victorjulien Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

victorjulien Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

victorjulien left a comment

Choose a reason for hiding this comment

Uh oh!

suricata-qa commented Jan 22, 2026

Uh oh!

victorjulien commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 20, 2026 •

edited

Loading