Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect cleanups/v3 #9949

Closed
wants to merge 19 commits into from
Closed

Conversation

victorjulien
Copy link
Member

Replaces #9948

Includes #9932

Lots of cleanups done as part of bigger work.

Most interesting commits:

Since the asn1 keyword is processing payload data, move the handling of
the keyword into the PMATCH with content inspection.

Use u32 as buffer length in the Rust FFI
Hitting the recursion limit should be rare.
Integrate with rest of content inspect code.
Use stack local var instead of DetectEngineThreadCtx member.

Make sure the limit is a const so we can avoid rereading it.
Flatten else branches after terminating ifs.
Since recursive content matching goes through the buffer from left to
right, it is possible to bail early when isdataat is part of the
recursive checking. If `isdataat:50,relative` fails for offset 10, it
will surely also fail for offset 20. So break inspection in such cases.

The exception is for dynamic isdataat, where the value is determined
by a prior byte_extract that may be updated during the recursion.
DetectEngineThreadCtx::replist is managed elsewhere.
Adjust includes to enable this.
Move reference count to top of DetectEngineThreadCtx, to move it to the
same cache line as the other members that are checked first in Detect().
Move members used by DetectEngineContentInspection() to the same cache line.
Core logic of the tests moved to content inspection code in separate commit.
Copy link

codecov bot commented Dec 2, 2023

Codecov Report

Merging #9949 (3583b42) into master (13cc493) will increase coverage by 0.00%.
The diff coverage is 96.10%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #9949    +/-   ##
========================================
  Coverage   82.39%   82.40%            
========================================
  Files         970      970            
  Lines      271359   271244   -115     
========================================
- Hits       223586   223518    -68     
+ Misses      47773    47726    -47     
Flag Coverage Δ
fuzzcorpus 64.44% <73.77%> (-0.01%) ⬇️
suricata-verify 61.33% <78.68%> (+0.02%) ⬆️
unittests 62.87% <83.11%> (-0.02%) ⬇️

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

@victorjulien victorjulien mentioned this pull request Dec 2, 2023
@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW2_autofp_stats_chk
.uptime 181 192 106.08%

Pipeline 16839

@victorjulien victorjulien mentioned this pull request Dec 5, 2023
@victorjulien
Copy link
Member Author

replaced by #9974

@victorjulien victorjulien deleted the detect-cleanups/v3 branch February 20, 2024 12:23
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.

2 participants