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

lukashino · 2026-01-20T10:33:39Z

Follow-up of #14653

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.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 packet counter (pcap_cnt) handling by moving it from the main Packet structure into the PCAP-specific PcapPacketVars structure. The change introduces getter/setter functions (PcapPacketCntGet() and PcapPacketCntSet()) that validate runmode before accessing the counter, ensuring it's only used in appropriate contexts (PCAP file, unittest, or unix socket runmodes).

Changes:

Moved pcap_cnt from Packet struct to PcapPacketVars struct within the packet
Introduced runmode-aware getter/setter accessor functions in decode.c/h
Updated all references throughout the codebase to use the new accessor functions

Reviewed changes

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

Show a summary per file

File	Description
src/decode.h	Removed pcap_cnt member from Packet struct; added getter/setter declarations
src/decode.c	Implemented getter/setter functions with runmode validation
src/source-pcap.h	Added pcap_cnt to PcapPacketVars structure
src/source-pcap-file-helper.c	Direct access to pcap_cnt in PCAP-native code path
src/packet.c	Removed pcap_cnt initialization from PacketReinit
src/stream-tcp.c	Replaced direct access with getter; fixed "steam" to "stream" typos; moved t_pcapcnt declaration under DEBUG
src/stream-tcp-reassemble.c	Replaced direct access with getter; moved t_pcapcnt definition under DEBUG
src/stream-tcp-sack.c	Replaced direct access with getter in debug logs
src/stream-tcp-private.h	Updated StreamTcpSetEvent macro to use getter
src/output-json.c	Replaced direct access with getter, properly cached result
src/output-json-alert.c	Replaced direct access with getter, properly cached result
src/output-tx.c	Replaced direct access with getter in debug logs
src/alert-syslog.c	Replaced direct access with getter
src/alert-fastlog.c	Replaced direct access with getter
src/alert-debuglog.c	Replaced direct access with getter
src/log-tlsstore.c	Replaced direct access with getter
src/util-profiling.c	Replaced direct access with getter
src/util-lua-packetlib.c	Replaced direct access with getter
src/util-exception-policy.c	Replaced direct access with getter in debug log
src/flow.c, src/flow-worker.c, src/flow-hash.c	Replaced direct access with getter in debug logs
src/detect*.c files	Replaced direct access with getter in debug logs
src/defrag.c, src/defrag-hash.c	Replaced direct access with getter
src/app-layer.c	Replaced direct access with getter in debug log
tests/fuzz/*.c	Replaced direct assignment with setter
tests/detect-tls-*.c	Replaced direct assignment with setter in test setup
tests/detect-http-header.c	Replaced direct assignment with setter in test setup
plugins/ndpi/ndpi.c	Replaced direct access with getter in debug logs

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

Copilot · 2026-01-20T10:38:32Z

src/decode.c

+static inline bool PcapPacketNumRunmodeCanAccess(void)
+{
+    SCRunMode m = SCRunmodeGet();
+    return m == RUNMODE_PCAP_FILE || m == RUNMODE_UNITTEST || m == RUNMODE_UNIX_SOCKET;
+}
+
+inline uint64_t PcapPacketCntGet(const Packet *p)
+{
+    if (PcapPacketNumRunmodeCanAccess() && p != NULL) {
+        return p->pcap_v.pcap_cnt;
+    }
+    return 0;
+}
+
+inline void PcapPacketCntSet(Packet *p, uint64_t pcap_cnt)
+{
+    if (PcapPacketNumRunmodeCanAccess() && p != NULL) {
+        p->pcap_v.pcap_cnt = pcap_cnt;
+    }
+}


The getter and setter functions call SCRunmodeGet() on every invocation, which can be expensive in hot paths. Since the runmode does not change during execution, consider caching the result of PcapPacketNumRunmodeCanAccess() in a static variable initialized at startup. This would avoid the function call overhead on every packet.

For example, you could use a static boolean variable set during initialization that remains constant for the lifetime of the process. This would improve performance in hot paths where these functions are called frequently.

Copilot · 2026-01-20T10:38:32Z

src/alert-syslog.c

+        if (PcapPacketCntGet(p) != 0) {
+            snprintf(temp_buf_tail, sizeof(temp_buf_tail), "] [pcap file packet: %" PRIu64 "]",
+                    PcapPacketCntGet(p));


PcapPacketCntGet(p) is called twice (on lines 346 and 348) in the same code block. Since this function has overhead (calls SCRunmodeGet()), the result should be cached in a local variable to avoid redundant calls. For example:

uint64_t pcap_cnt = PcapPacketCntGet(p); if (pcap_cnt != 0) { snprintf(temp_buf_tail, sizeof(temp_buf_tail), "] [pcap file packet: %" PRIu64 "]", pcap_cnt);

Suggested change

if (PcapPacketCntGet(p) != 0) {

snprintf(temp_buf_tail, sizeof(temp_buf_tail), "] [pcap file packet: %" PRIu64 "]",

PcapPacketCntGet(p));

uint64_t pcap_cnt = PcapPacketCntGet(p);

if (pcap_cnt != 0) {

snprintf(temp_buf_tail, sizeof(temp_buf_tail), "] [pcap file packet: %" PRIu64 "]",

pcap_cnt);

Copilot · 2026-01-20T10:38:33Z

src/alert-fastlog.c

+            if (PcapPacketCntGet(p) != 0) {
+                PrintBufferData(alert_buffer, &size, MAX_FASTLOG_ALERT_SIZE,
+                        "] [pcap file packet: %" PRIu64 "]\n", PcapPacketCntGet(p));


PcapPacketCntGet(p) is called twice (on lines 182 and 184) in the same code block. Since this function has overhead (calls SCRunmodeGet()), the result should be cached in a local variable to avoid redundant calls. For example:

uint64_t pcap_cnt = PcapPacketCntGet(p); if (pcap_cnt != 0) { PrintBufferData(alert_buffer, &size, MAX_FASTLOG_ALERT_SIZE, "] [pcap file packet: %" PRIu64 "]\n", pcap_cnt);

Suggested change

if (PcapPacketCntGet(p) != 0) {

PrintBufferData(alert_buffer, &size, MAX_FASTLOG_ALERT_SIZE,

"] [pcap file packet: %" PRIu64 "]\n", PcapPacketCntGet(p));

uint64_t pcap_cnt = PcapPacketCntGet(p);

if (pcap_cnt != 0) {

PrintBufferData(alert_buffer, &size, MAX_FASTLOG_ALERT_SIZE,

"] [pcap file packet: %" PRIu64 "]\n", pcap_cnt);

Copilot · 2026-01-20T10:38:33Z

src/log-tlsstore.c

+        if (PcapPacketCntGet(p) > 0) {
+            if (fprintf(fpmeta, "PCAP PKT NUM:      %" PRIu64 "\n", PcapPacketCntGet(p)) < 0)


PcapPacketCntGet(p) is called twice (on lines 220 and 221) in the same code block. Since this function has overhead (calls SCRunmodeGet()), the result should be cached in a local variable to avoid redundant calls. For example:

uint64_t pcap_cnt = PcapPacketCntGet(p); if (pcap_cnt > 0) { if (fprintf(fpmeta, "PCAP PKT NUM: %" PRIu64 "\n", pcap_cnt) < 0)

Suggested change

if (PcapPacketCntGet(p) > 0) {

if (fprintf(fpmeta, "PCAP PKT NUM: %" PRIu64 "\n", PcapPacketCntGet(p)) < 0)

uint64_t pcap_cnt = PcapPacketCntGet(p);

if (pcap_cnt > 0) {

if (fprintf(fpmeta, "PCAP PKT NUM: %" PRIu64 "\n", pcap_cnt) < 0)

Copilot · 2026-01-20T10:38:33Z

src/alert-debuglog.c

+    if (PcapPacketCntGet(p) > 0) {
+        MemBufferWriteString(aft->buffer, "PCAP PKT NUM:      %" PRIu64 "\n", PcapPacketCntGet(p));


PcapPacketCntGet(p) is called twice (on lines 170 and 171) in the same code block. Since this function has overhead (calls SCRunmodeGet()), the result should be cached in a local variable to avoid redundant calls. For example:

uint64_t pcap_cnt = PcapPacketCntGet(p); if (pcap_cnt > 0) { MemBufferWriteString(aft->buffer, "PCAP PKT NUM: %" PRIu64 "\n", pcap_cnt);

Suggested change

if (PcapPacketCntGet(p) > 0) {

MemBufferWriteString(aft->buffer, "PCAP PKT NUM: %" PRIu64 "\n", PcapPacketCntGet(p));

uint64_t pcap_cnt = PcapPacketCntGet(p);

if (pcap_cnt > 0) {

MemBufferWriteString(aft->buffer, "PCAP PKT NUM: %" PRIu64 "\n", pcap_cnt);

Copilot · 2026-01-20T10:38:34Z

src/alert-debuglog.c

+    if (PcapPacketCntGet(p) > 0) {
+        MemBufferWriteString(aft->buffer, "PCAP PKT NUM:      %" PRIu64 "\n", PcapPacketCntGet(p));


PcapPacketCntGet(p) is called twice (on lines 330 and 331) in the same code block. Since this function has overhead (calls SCRunmodeGet()), the result should be cached in a local variable to avoid redundant calls. For example:

uint64_t pcap_cnt = PcapPacketCntGet(p); if (pcap_cnt > 0) { MemBufferWriteString(aft->buffer, "PCAP PKT NUM: %" PRIu64 "\n", pcap_cnt);

Suggested change

if (PcapPacketCntGet(p) > 0) {

MemBufferWriteString(aft->buffer, "PCAP PKT NUM: %" PRIu64 "\n", PcapPacketCntGet(p));

uint64_t pcap_cnt = PcapPacketCntGet(p);

if (pcap_cnt > 0) {

MemBufferWriteString(aft->buffer, "PCAP PKT NUM: %" PRIu64 "\n", pcap_cnt);

codecov · 2026-01-20T11:06:36Z

Codecov Report

❌ Patch coverage is 91.93548% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.10%. Comparing base (c333b28) to head (748516a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14656      +/-   ##
==========================================
- Coverage   82.11%   82.10%   -0.01%     
==========================================
  Files        1011     1011              
  Lines      262812   262858      +46     
==========================================
+ Hits       215812   215830      +18     
- Misses      47000    47028      +28

Flag	Coverage Δ
fuzzcorpus	`60.26% <81.19%> (+0.08%)`	⬆️
livemode	`18.71% <8.25%> (+<0.01%)`	⬆️
pcap	`44.58% <57.79%> (-0.01%)`	⬇️
suricata-verify	`65.28% <79.81%> (-0.01%)`	⬇️
unittests	`59.26% <29.43%> (-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.

suricata-qa · 2026-01-20T13:00:02Z

Information: QA ran without warnings.

Pipeline = 29189

victorjulien · 2026-01-20T14:42:31Z

src/decode.c

    SCLogDebug("detect->packet_alert_max set to %d", packet_alert_max);
 }

+static inline bool PcapPacketNumRunmodeCanAccess(void)


nit: PcapPacketCntRunmodeCanAccess, so s/Num/Cnt/g

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

Continues in #14657

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

748516a

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

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

Copilot AI review requested due to automatic review settings January 20, 2026 10:33

lukashino mentioned this pull request Jan 20, 2026

pcap-file: move packet counter to PCAP packet structure v5 #14653

Closed

Copilot started reviewing on behalf of lukashino January 20, 2026 10:34 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

victorjulien approved these changes Jan 20, 2026

View reviewed changes

victorjulien added this to the 9.0 milestone Jan 20, 2026

lukashino mentioned this pull request Jan 20, 2026

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

Closed

lukashino closed this Jan 20, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Uh oh!

lukashino commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

codecov bot commented Jan 20, 2026 •

edited

Loading

Uh oh!

suricata-qa commented Jan 20, 2026

Uh oh!

victorjulien Jan 20, 2026

Uh oh!

lukashino commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		if (PcapPacketCntGet(p) > 0) {
		if (fprintf(fpmeta, "PCAP PKT NUM: %" PRIu64 "\n", PcapPacketCntGet(p)) < 0)

-        if (PcapPacketCntGet(p) > 0) {
-            if (fprintf(fpmeta, "PCAP PKT NUM:      %" PRIu64 "\n", PcapPacketCntGet(p)) < 0)
+        uint64_t pcap_cnt = PcapPacketCntGet(p);
+        if (pcap_cnt > 0) {
+            if (fprintf(fpmeta, "PCAP PKT NUM:      %" PRIu64 "\n", pcap_cnt) < 0)

		if (PcapPacketCntGet(p) > 0) {
		MemBufferWriteString(aft->buffer, "PCAP PKT NUM: %" PRIu64 "\n", PcapPacketCntGet(p));

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

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

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!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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!

suricata-qa commented Jan 20, 2026

Uh oh!

victorjulien Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

lukashino commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 20, 2026 •

edited

Loading