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

next/668/70x/20241213/v1 #12279

Closed

Conversation

victorjulien
Copy link
Member

inashivb and others added 4 commits December 13, 2024 11:47
(cherry picked from commit f97b4ec)
In multi instance flow manager setups, each flow manager gets a slice
of the hash table to manage. Due to a logic error in the chunked
scanning of the hash slice, instances beyond the first would always
rescan the same (first) subslice of their slice.

The `pos` variable that is used to keep the state of what the starting
position for the next scan was supposed to be, was treated as if it held
a relative value. Relative to the bounds of the slice. It was however,
holding an absolute position. This meant that when doing it's bounds
check it was always considered out of bounds. This would reset the sub-
slice to be scanned to the first part of the instances slice.

This patch addresses the issue by correctly handling the fact that the
value is absolute.

Bug: OISF#7365.

Fixes: e9d2417 ("flow/manager: adaptive hash eviction timing")
(cherry picked from commit ae072d5)
@victorjulien victorjulien requested a review from a team as a code owner December 13, 2024 13:23
Copy link

codecov bot commented Dec 13, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.19%. Comparing base (a578b09) to head (44b68d9).
Report is 13 commits behind head on main-7.0.x.

Additional details and impacted files
@@             Coverage Diff              @@
##           main-7.0.x   #12279    +/-   ##
============================================
  Coverage       83.19%   83.19%            
============================================
  Files             922      922            
  Lines          260900   261173   +273     
============================================
+ Hits           217047   217289   +242     
- Misses          43853    43884    +31     
Flag Coverage Δ
fuzzcorpus 64.20% <25.00%> (+0.01%) ⬆️
suricata-verify 63.42% <75.00%> (+0.02%) ⬆️
unittests 62.38% <0.00%> (-0.01%) ⬇️

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

@victorjulien victorjulien added the needs baseline update QA will need a new base line label Dec 13, 2024
@inashivb
Copy link
Member

I'm not sure if this is correct. I think b96bfea should be the first commit to go in. Rest should be later as they're part of 7.0.9 development

@victorjulien
Copy link
Member Author

I'm not sure if this is correct. I think b96bfea should be the first commit to go in. Rest should be later as they're part of 7.0.9 development

Yes, you are right.

@victorjulien
Copy link
Member Author

Wrong commit order.

@victorjulien victorjulien deleted the next/668/70x/20241213/v1 branch December 13, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs baseline update QA will need a new base line
Development

Successfully merging this pull request may close these issues.

2 participants