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 non pf/v1 #12567

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

victorjulien
Copy link
Member

Overhaul handling of non-pf. Prep work for hook.

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 85.79545% with 50 lines in your changes missing coverage. Please review.

Project coverage is 80.74%. Comparing base (10ede91) to head (60e0629).

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #12567    +/-   ##
========================================
  Coverage   80.74%   80.74%            
========================================
  Files         931      931            
  Lines      259144   259316   +172     
========================================
+ Hits       209242   209389   +147     
- Misses      49902    49927    +25     
Flag Coverage Δ
fuzzcorpus 56.98% <85.51%> (+0.01%) ⬆️
livemode 19.45% <38.35%> (+0.07%) ⬆️
pcap 44.16% <44.31%> (+0.01%) ⬆️
suricata-verify 63.43% <85.79%> (+<0.01%) ⬆️
unittests 58.35% <65.90%> (+<0.01%) ⬆️

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

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ASAN_TLPR1_cfg.

Pipeline 24700

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24723

return -1;
}

if (de_ctx->non_pf_engine_names == NULL) {
Copy link
Member Author

Choose a reason for hiding this comment

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

store for names for (at least) profiling

src/detect.c Outdated Show resolved Hide resolved
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24732

@victorjulien victorjulien force-pushed the detect-non-pf/v1 branch 2 times, most recently from d65c560 to fa40d83 Compare February 15, 2025 21:26
@suricata-qa
Copy link

ERROR:

ERROR: QA failed on build_asan.

Pipeline 24738

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on build_asan.

Pipeline 24739

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24740

@victorjulien victorjulien force-pushed the detect-non-pf/v1 branch 2 times, most recently from c1295f8 to 9db5e02 Compare February 16, 2025 11:10
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24742

Several rules matched on both directions even if events are set in a single direction.
Instead of having a per detection engine list of rule that couldn't be
prefiltered, put those into special "prefilter" engines.

For packet and frame rules this doesn't change much, it just removes
some hard coded logic from the detect engine.

For the packet non-prefilter rules in the "non-prefilter" special prefilter
engine, add additional filtering for the packet variant. It can prefilter on
alproto, dsize and dest port.

The frame non-prefilter rules are added to a single engine, that per
rule checks the alproto and the type.

For app-layer, there is an engine per progress value, per app-layer
protocol and per direction. This hooks app-layer non-prefilter rules
into the app inspect logic at the correct "progress" hook.

e.g. a rule like
        dns.query; bsize:1;

Negated MPM rules will also fall into this category:
        dns.query; content:!"abc";

Are part of a special "generic list" app engine for dns, at the
same progress hook as `dns.query`.

This all results in a lot fewer checks:

previous:

  --------------------------------------------------------------------------
  Date: 1/29/2025 -- 10:22:25. Sorted by: number of checks.
  --------------------------------------------------------------------------
   Num      Rule         Gid      Rev      Ticks        %      Checks   Matches  Max Ticks   Avg Ticks   Avg Match   Avg No Match
  -------- ------------ -------- -------- ------------ ------ -------- -------- ----------- ----------- ----------- --------------
  1        20           1        0        181919672    11.85  588808   221      60454       308.96      2691.46     308.07
  2        50           1        0        223455914    14.56  453104   418      61634       493.17      3902.59     490.02
  3        60           1        0        185990683    12.12  453104   418      60950       410.48      1795.40     409.20
  4        51           1        0        192436011    12.54  427028   6084     61223       450.64      2749.12     417.42
  5        61           1        0        180401533    11.75  427028   6084     61093       422.46      2177.04     397.10
  6        70           1        0        153899099    10.03  369836   0        61282       416.13      0.00        416.13
  7        71           1        0        123389405    8.04   369836   12833    44921       333.63      2430.23     258.27
  8        41           1        0        63889876     4.16   155824   12568    39138       410.01      1981.97     272.10
  9        40           1        0        64149724     4.18   155818   210      39792       411.70      4349.57     406.38
  10       10           1        0        70848850     4.62   65558    0        39544       1080.70     0.00        1080.70
  11       11           1        0        94743878     6.17   65558    32214    60547       1445.19     2616.14     313.92

this commit:

  --------------------------------------------------------------------------
  Date: 1/29/2025 -- 10:15:46. Sorted by: number of checks.
  --------------------------------------------------------------------------
   Num      Rule         Gid      Rev      Ticks        %      Checks   Matches  Max Ticks   Avg Ticks   Avg Match   Avg No Match
  -------- ------------ -------- -------- ------------ ------ -------- -------- ----------- ----------- ----------- --------------
  1        50           1        0        138776766    19.23  95920    418      167584      1446.80     3953.11     1435.83
  2        60           1        0        97988084     13.58  95920    418      182817      1021.56     1953.63     1017.48
  3        51           1        0        105318318    14.60  69838    6084     65649       1508.04     2873.38     1377.74
  4        61           1        0        89571260     12.41  69838    6084     164632      1282.56     2208.41     1194.20
  5        11           1        0        91132809     12.63  32779    32214    373569      2780.22     2785.58     2474.45
  6        10           1        0        66095303     9.16   32779    0        56704       2016.39     0.00        2016.39
  7        70           1        0        48107573     6.67   12928    0        42832       3721.19     0.00        3721.19
  8        71           1        0        32308792     4.48   12928    12833    39565       2499.13     2510.05     1025.09
  9        41           1        0        25546837     3.54   12886    12470    41479       1982.53     1980.84     2033.05
  10       40           1        0        26069992     3.61   12886    210      38495       2023.13     4330.05     1984.91
  11       20           1        0        639025       0.09   221      221      14750       2891.52     2891.52     0.00
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