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.

Sign up for GitHub

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

Jump to bottom

Batch backports to main-7.0.x #12007

Closed
wants to merge 13 commits into from
Closed

Batch backports to main-7.0.x #12007

wants to merge 13 commits into from

Conversation

jlucovsky
Copy link
Contributor

Continuation of #12006

Backports to main-7.0..x of 6509 and 7275

Link to tickets:

Describe changes:

  • Cherry-picks of commits for these issues

Updates:

  • Fixup of cherry-pick that introduced dup symbols.

Provide values to any of the below to override the defaults.

  • To use an LibHTP, Suricata-Verify or Suricata-Update pull request,
    link to the pull request in the respective _BRANCH variable.
  • Leave unused overrides blank or remove.

SV_REPO=
SV_BRANCH=
SU_REPO=
SU_BRANCH=
LIBHTP_REPO=
LIBHTP_BRANCH=

AntsKnows and others added 13 commits October 22, 2024 08:09
@AntsKnows @jlucovsky
stream/reassembly: optimize GetBlock
bdbe712 
Current GetBlock degrees the sbb search from rb tree to
line, which costs much cpu time, and could be replaced by
SBB_RB_FIND_INCLUSIVE. It reduces time complexity from
O(nlogn) to O(logn).

Ticket: 7208.
(cherry picked from commit 951bcff)
@jufajardini @jlucovsky
exceptions: make types and ToStr fns more accessible
cf6df33 
Decode file needed ExceptionPolicy types and exception-policy file
needed Decode types, rendering some works quite difficult to work
around.

ExceptionPolicyToStr is useful for registering exception policy
counters, so make that public.

Part of
Task OISF#5816

(cherry picked from commit c2c8cdb)
@jufajardini @jlucovsky
decode/flow: add exception policy stats counters
ba1764c 
We will register stats counters for all policies, even though for now
Suri only uses one possible configuration policy at a time. The idea is
that this could change in the near future, so we want to have this
ready.

Task OISF#5816

(cherry picked from commit 657419b)
@jufajardini @jlucovsky
defrag: add exception policy memcap stats counters
ea880cb 
Add defrag memcap stats counter.

Task OISF#5816

(cherry picked from commit 485c0e1)
@jufajardini @jlucovsky
applayer: add stats counters for exception errors
1a2e6bb 
Add stats counters for exception policy are applied for app-layer errors

Part of
Task OISF#5816

(cherry picked from commit a71ace8)
@jufajardini @jlucovsky
stream/tcp: add ssnmemcap exception policy counter
e90b9f9 
Add stats counters for exception policies applied in case a stream
session memcap is hit.

Task OISF#5816

(cherry picked from commit 2dee377)
@jufajardini @jlucovsky
stream/reassemble: add exception policy counters
771f681 
Add stats counters for exception policies applied in case of memcap hit
during stream reassembly.

Task OISF#5816

(cherry picked from commit fd9a20f)
@jufajardini @jlucovsky
stream/midstream: add counter for exception policy
ef442bf 
Add stats counters for when there is an exception policy applied in case
of a session picked up midstream.

Task OISF#5816

(cherry picked from commit caf590d)
@jufajardini @jlucovsky
userguide: highlight exception policy effects
4e27d7d 
Some exception policies can only be applied to entire flows or
individual packets, for some exception scenarios. Make this easier to
read, in the documentation.

Related to
Task OISF#5816

(cherry picked from commit 94b1112)
@jufajardini @jlucovsky
userguide: document exception policy stats
4224340 
Configuration options and defaults, existing counters etc.

Related to
Task OISF#5816

(cherry picked from commit 514e8b8)
@jufajardini @jlucovsky
yaml: explicitly mention exception policy in conf
daa9f2c 
While our documentation indicated what were the possible configuration
settings for exception policies, our yaml only explicitly mentioned
exception policy for the master switch. Clearly indicate which config
settings are about exception policies.

Related to
Task OISF#5816

(cherry picked from commit 8defee9)
@jufajardini @jlucovsky
yaml: minor wording fix on Suricata package version
8bf3f83 
(cherry picked from commit 0f6dbf6)
@jufajardini @jlucovsky
log/stats: allow longer counter names
466964b 
With the addition of exception policy stats counters, the human readable
version of the sats log was mis-aligned, when counters for per-app-proto
were enabled.

Width change made large enough to accomodate a counter as long as
"app_layer.error.bittorrent-dht.exception_policy.pass_packet" which
could be valid.

Task OISF#5816

(cherry picked from commit 172b55c)
@jlucovsky jlucovsky mentioned this pull request Oct 22, 2024
Closed
victorjulien
victorjulien reviewed Oct 22, 2024
View reviewed changes
src/log-stats.c
@@ -89,17 +89,16 @@ static int LogStatsLogger(ThreadVars *tv, void *thread_data, const StatsTable *s
int days = in_hours / 24;

MemBufferWriteString(aft->buffer, "----------------------------------------------"
"--------------------------------------\n");
"-----------------------------------------------------\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changes the format, which may break tools that might parse them. Should we backport this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to move the PR to draft mode as there is consideration for this and other related items.

@codecov Codecov
Copy link

codecov bot commented Oct 22, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.43860% with 42 lines in your changes missing coverage. Please review.

Project coverage is 83.32%. Comparing base (8810d7f) to head (466964b).
Report is 33 commits behind head on main-7.0.x.

Additional details and impacted files 
@@              Coverage Diff               @@
##           main-7.0.x   #12007      +/-   ##
==============================================
+ Coverage       82.93%   83.32%   +0.38%     
==============================================
  Files             922      923       +1     
  Lines          251298   260942    +9644     
==============================================
+ Hits           208418   217434    +9016     
- Misses          42880    43508     +628
Flag Coverage Δ
fuzzcorpus 64.61% <36.07%> (+0.51%) ⬆️
suricata-verify 63.29% <74.05%> (+0.44%) ⬆️
unittests 62.37% <30.40%> (+0.15%) ⬆️

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

@jufajardini
Copy link
Contributor

If I recall correctly, we are still pondering on whether the format that we have in master for the exception policy stats - especially for app-layer error - is what we want -- that's why the backport has been sitting for so long. We should resurface this discussion...

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 23160

@jlucovsky jlucovsky marked this pull request as draft October 24, 2024 12:48
@jlucovsky jlucovsky changed the title Batch backports to mai-7.0.x Batch backports to main-7.0.x Oct 25, 2024
@catenacyber
Copy link
Contributor

Should this be rebased ? There was the merge of https://redmine.openinfosecfoundation.org/issues/7275 right ?

@victorjulien
Copy link
Member

Needs to be reconsidered later.

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

@victorjulien victorjulien victorjulien left review comments

@jufajardini jufajardini Awaiting requested review from jufajardini jufajardini is a code owner

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jlucovsky @jufajardini @suricata-qa @catenacyber @victorjulien @AntsKnows