Skip to content

Dataset string memcap 3910/v14 #11296

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

Closed
wants to merge 5 commits into from

Conversation

inashivb
Copy link
Member

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

Previous PR: #11289

Changes since v13:

  • fixed up doc language as per review

SV_BRANCH=OISF/suricata-verify#1913

victorjulien and others added 5 commits June 10, 2024 17:27
Add a callback and helper function to handle data expiration.

Update datasets to explicitly not use expiration.
In order to have access to the length of datatypes with variable lengths
to correctly update memuse to calculate memcaps.

Bug 3910
So far, when the data size was passed to the THash API, it was sent as
a sizeof(Struct) which works fine for the other data types as they have
a fixed length but not for the StringType.
However, because of the sizeof construct, the length of a string type
dataset was always taken to be 16 Bytes which is only the size of the struct
itself. It did not accomodate the actual size of the string that the
StringType holds. Fix this so that the memuse that is used to determine
whether memcap was reached also takes into consideration the size of the
actual string.

Bug 3910
Copy link

codecov bot commented Jun 12, 2024

Codecov Report

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

Project coverage is 82.44%. Comparing base (f0dbfe8) to head (d1f2276).
Report is 65 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11296      +/-   ##
==========================================
- Coverage   82.45%   82.44%   -0.02%     
==========================================
  Files         961      961              
  Lines      251710   251784      +74     
==========================================
+ Hits       207552   207577      +25     
- Misses      44158    44207      +49     
Flag Coverage Δ
fuzzcorpus 60.28% <39.79%> (-0.02%) ⬇️
livemode 18.68% <25.51%> (-0.01%) ⬇️
pcap 43.79% <22.44%> (+<0.01%) ⬆️
suricata-verify 61.17% <47.95%> (-0.02%) ⬇️
unittests 59.94% <0.00%> (-0.03%) ⬇️

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

@inashivb inashivb marked this pull request as ready for review June 12, 2024 06:45
@suricata-qa
Copy link

ERROR:

ERROR: QA failed on SURI_TLPR1_alerts_cmp.

Pipeline 21063

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 21070

@victorjulien victorjulien added this to the 8.0 milestone Jun 15, 2024
@inashivb inashivb added the needs rebase Needs rebase to master label Jun 24, 2024
@inashivb inashivb closed this Jun 28, 2024
@inashivb inashivb deleted the dataset-string-memcap-3910/v14 branch June 28, 2024 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Needs rebase to master
Development

Successfully merging this pull request may close these issues.

4 participants