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: adds flow integer keywords #9416

Closed
wants to merge 3 commits into from

Conversation

catenacyber
Copy link
Contributor

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

Describe changes:

  • detect: adds flow.pkts_toclient keyword

OISF/suricata-verify#1267

SV_BRANCH=pr/1267

#9095 rebased to erase the status as changes requested and continue the debate

Commit message needs a bit more explanation.

What do you want as an explanation ?
Doing git log --oneline | grep keyw , I see the latest added keyword is commit 9cb06d4 which does not have any more explanation than adding a keyword + reference to the redmine ticket...

Also, can we support prefilter for the byte keywords?

Not obviously.
I think this would require to grow the size of the union PrefilterPacketHeaderValue (which may have perf impacts)
Any better ideas ?

Ticket: OISF#6164

flow.pkts_toclient
flow.pkts_toserver
flow.bytes_toclient
flow.bytes_toserver
@catenacyber catenacyber requested a review from jasonish as a code owner August 30, 2023 09:24
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 15775

@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Merging #9416 (36bfd35) into master (becb8ce) will decrease coverage by 0.05%.
The diff coverage is 71.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9416      +/-   ##
==========================================
- Coverage   82.17%   82.13%   -0.05%     
==========================================
  Files         968      969       +1     
  Lines      274198   274353     +155     
==========================================
+ Hits       225331   225341      +10     
- Misses      48867    49012     +145     
Flag Coverage Δ
fuzzcorpus 63.99% <27.67%> (-0.05%) ⬇️
suricata-verify 60.88% <71.06%> (-0.01%) ⬇️
unittests 62.85% <29.01%> (-0.03%) ⬇️

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 15777

@jasonish jasonish mentioned this pull request Aug 30, 2023
@victorjulien victorjulien added this to the 8.0 milestone Sep 6, 2023
@jufajardini
Copy link
Contributor

The documentation change looks good to me.
The non-zero bit flag... Looks ok, too, I think.

@catenacyber
Copy link
Contributor Author

The non-zero bit flag... Looks ok, too, I think.

Oops, from another PR, they were merged

Now #9463

@catenacyber catenacyber closed this Sep 7, 2023
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.

4 participants