Skip to content

pcap-file: synchronize PCAP filename output v1#13643

Closed
lukashino wants to merge 2 commits into
OISF:masterfrom
lukashino:bug/5255-pcap-filename-race-v1
Closed

pcap-file: synchronize PCAP filename output v1#13643
lukashino wants to merge 2 commits into
OISF:masterfrom
lukashino:bug/5255-pcap-filename-race-v1

Conversation

@lukashino

Copy link
Copy Markdown
Contributor

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

Describe changes:

  • added file structure tracked by packets/flows to report true filename

When RX thread was reading packets from multiple PCAPs, it held
internally pcap_filename variable of the currently processed PCAP.
This variable, accessed through PcapFileGetFilename(), was used to add
pcap_filename property to all eve.json events by all threads.

However, as RX thread is connected to Worker/Detect threads with queues,
packets of previous PCAPs were still in queues, while RX thread was
reading different PCAPs. This is a synchronizaiton issue.

PcapFileInfo structure is used to reference the PCAP file name. RX
thread associates the structure with all packets, Workers in turn
associate the flow with the filename. PcapFileGetFilename() is still
used when reporting stats events.

Lukas Sismis added 2 commits July 24, 2025 10:20
PcapFileInfo is a structure which will be referenced by packets/flows.

Ticket: 5255
When RX thread was reading packets from multiple PCAPs, it held
internally pcap_filename variable of the currently processed PCAP.
This variable, accessed through PcapFileGetFilename(), was used to add
pcap_filename property to all eve.json events by all threads.

However, as RX thread is connected to Worker/Detect threads with queues,
packets of previous PCAPs were still in queues, while RX thread was
reading different PCAPs. This is a synchronizaiton issue.

PcapFileInfo structure is used to reference the PCAP file name. RX
thread associates the structure with all packets, Workers in turn
associate the flow with the filename. PcapFileGetFilename() is still
used when reporting stats events.

Ticket: 5255
@lukashino lukashino requested a review from victorjulien as a code owner July 24, 2025 11:42
@codecov

codecov Bot commented Jul 24, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.42857% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.20%. Comparing base (2e69e0d) to head (348db12).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13643      +/-   ##
==========================================
- Coverage   83.69%   83.20%   -0.49%     
==========================================
  Files        1011     1012       +1     
  Lines      275071   275118      +47     
==========================================
- Hits       230210   228910    -1300     
- Misses      44861    46208    +1347     
Flag Coverage Δ
fuzzcorpus 62.84% <70.00%> (+0.08%) ⬆️
livemode 18.07% <22.85%> (-1.06%) ⬇️
pcap 44.74% <71.42%> (+<0.01%) ⬆️
suricata-verify 65.10% <81.42%> (+<0.01%) ⬆️
unittests 59.18% <22.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.

@suricata-qa

Copy link
Copy Markdown

ERROR:

ERROR: QA failed on IPS_AFP_trex.

Pipeline = 27052

@suricata-qa

Copy link
Copy Markdown

ERROR:

ERROR: QA failed on IPS_AFP_trex.

Pipeline = 27054

@suricata-qa

Copy link
Copy Markdown

ERROR:

ERROR: QA failed on IPS_AFP_trex.

Pipeline = 27056

@ct0br0

ct0br0 commented Jul 25, 2025

Copy link
Copy Markdown

no idea how this is causing a failure at the same spot. the weird part is that isn't even the suricata side.

@lukashino

Copy link
Copy Markdown
Contributor Author

continue in #13652

@lukashino lukashino closed this Jul 27, 2025
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