Skip to content

Conversation

@victorjulien
Copy link
Member

#11786 with all hashmaps turned to lru

Don't tag the session as gap'd when the GAP is in a precise location:

1. in "skip" data, where the GAP just fits the skip data

2. in file data, where we pass the GAP on to the file

This reduces load of GAP post-processing that is unnecessary in these
case.
Use `lru` crate.

Add `app-layer.protocols.smb.max-guid-cache-size` to control the max
size of the LRU cache.
@victorjulien victorjulien mentioned this pull request Oct 24, 2024
5 tasks
@codecov
Copy link

codecov bot commented Oct 24, 2024

Codecov Report

Attention: Patch coverage is 86.19529% with 41 lines in your changes missing coverage. Please review.

Project coverage is 83.25%. Comparing base (1860aa8) to head (b510f43).
Report is 17 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #12028   +/-   ##
=======================================
  Coverage   83.24%   83.25%           
=======================================
  Files         910      910           
  Lines      258136   258165   +29     
=======================================
+ Hits       214895   214928   +33     
+ Misses      43241    43237    -4     
Flag Coverage Δ
fuzzcorpus 61.54% <85.85%> (+0.04%) ⬆️
livemode 19.39% <3.36%> (+<0.01%) ⬆️
pcap 44.37% <71.38%> (-0.06%) ⬇️
suricata-verify 62.70% <73.73%> (-0.04%) ⬇️
unittests 59.28% <15.82%> (-0.01%) ⬇️

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

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 23175

SCLogError!("Invalid max-dcerpc-frag-cache-size value");
}
}
let retval = conf_get("app-layer.protocols.smb.max-ssn2vec-cache-size");
Copy link
Member Author

Choose a reason for hiding this comment

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

this is a bit of a multi purpose store for mapping ssn id to various things, hard to come up with a good name

Copy link
Member

Choose a reason for hiding this comment

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

Also seems a bit of a low level tuneable. I wonder if such items should be left out of our default configuration file and well documented in an SMB chapter of the user guide.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah probably, but still need a reasonable name :)

@victorjulien victorjulien mentioned this pull request Oct 25, 2024
@victorjulien
Copy link
Member Author

replaced by #12036

@victorjulien victorjulien deleted the smb-hashmap/v6 branch January 15, 2025 08:01
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