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

Bug 6726 livedev ips/v6 #10869

Closed

Conversation

victorjulien
Copy link
Member

Alternative approach to #10863: instead of erroring out, we just disable the offending setting with a warning.
image

I think I like this better, as the livedev.use-for-tracking is enabled by default and will confuse everyone enabling the relevant IPS modes. So this is a more user friendly approach.

https://redmine.openinfosecfoundation.org/issues/5588
https://redmine.openinfosecfoundation.org/issues/6726

Changes since #10864:

  • rebase
  • fix include in odd location

Improve it for af-packet, dpdk, netmap. Check would not consider
an interface IDS if the `default` section contained a copy-mode
field.
In general, improve IPS setup error checking.

Ticket: OISF#5588.
For the capture methods that support livedev and IPS,
livedev.use-for-tracking is not supported.

This setting causes major flow tracking issues, as both sides of
a flow would be tracked in different flows.

This patch disables the livedev.use-for-tracking setting if it
is set to true. A warning will be issued.

Ticket: OISF#6726.
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 11.62791% with 38 lines in your changes are missing coverage. Please review.

Project coverage is 82.99%. Comparing base (240e068) to head (6803bde).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10869      +/-   ##
==========================================
+ Coverage   82.95%   82.99%   +0.03%     
==========================================
  Files         917      917              
  Lines      247367   247353      -14     
==========================================
+ Hits       205198   205284      +86     
+ Misses      42169    42069     -100     
Flag Coverage Δ
fuzzcorpus 64.61% <6.97%> (+0.10%) ⬆️
suricata-verify 62.34% <11.62%> (+0.03%) ⬆️
unittests 62.29% <0.00%> (+<0.01%) ⬆️

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

@suricata-qa
Copy link

Information:

ERROR: QA failed on SURI_TLPW2_autofp_suri_time.

ERROR: QA failed on SURI_TLPR1_suri_time.

field baseline test %
SURI_TLPW2_autofp_stats_chk
.uptime 138 143 103.62%
SURI_TLPR1_stats_chk
.uptime 644 702 109.01%
.memcap.pressure 58 60 103.45%

Pipeline 20109

Copy link
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

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

Thanks for the work Victor

CI : 🟢
Code : looks good... Should we have default livedev: use-for-tracking: auto that is true in IDS and false in IPS ?
Commits segmentation : looks fine
Commit messages : Nice
Git ID set : looks fine for me
CLA : :-p
Doc update : should there be one ?
Redmine ticket : look good
Rustfmt : not needed
Tests : How do we automate the testing of this ?
Dependencies added: none added

@victorjulien victorjulien added this to the 8.0 milestone Apr 18, 2024
@victorjulien
Copy link
Member Author

replaced by #10914

@victorjulien
Copy link
Member Author

Merged in #10921, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants