Skip to content

Flow drops/v4#8949

Closed
victorjulien wants to merge 8 commits into
OISF:masterfrom
victorjulien:flow-drops/v4
Closed

Flow drops/v4#8949
victorjulien wants to merge 8 commits into
OISF:masterfrom
victorjulien:flow-drops/v4

Conversation

@victorjulien
Copy link
Copy Markdown
Member

Now that flow drop is applied to packets before other processing,
no drop has to be issued on a packet.
Remove logic to apply flow drop, as this is now handled in the
flow engine.

However, keep the logic that frees/cleans the session state.
When a flow is in the drop flow state, don't use pseudo packets
when it is timing out. There should be no work left to do at this
point.
Test broke after recent changes. Functionality is tested in
suricata-verify, so just remove the test.
@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline 14178

Copy link
Copy Markdown
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

I like that we're handling the drop when updating the flow packet state, and that we're checking for the drop before updating app-layer for UDP protos. Seems like the drop handling is more contained. I saw that we are not calling 'FlowSetNoPacketInspectionFlag' and 'DecodeSetNoPacketInspectionFlag' after checking for the flow drops. Is that because we are handling the dropped flow earlier?

First of the commits seems to have a typo: "apply flow do" -> I imagine that should be "flow drop"?

@victorjulien
Copy link
Copy Markdown
Member Author

I like that we're handling the drop when updating the flow packet state, and that we're checking for the drop before updating app-layer for UDP protos. Seems like the drop handling is more contained. I saw that we are not calling 'FlowSetNoPacketInspectionFlag' and 'DecodeSetNoPacketInspectionFlag' after checking for the flow drops. Is that because we are handling the dropped flow earlier?

Yeah, the detect engine checks for FLOW_ACTION_DROP, so the other flags aren't needed for this case.

@victorjulien victorjulien mentioned this pull request Jun 2, 2023
@victorjulien
Copy link
Copy Markdown
Member Author

Merged in #8951

@victorjulien victorjulien deleted the flow-drops/v4 branch July 17, 2023 11:15
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