-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add a configurable limit to the number of current flows #58
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! Very nice!
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @robertodauria)
main.go
line 39 at r1 (raw file):
sigtermWaitTime = flag.Duration("sigtermwait", 1*time.Second, "How long should the daemon hang around before exiting after receiving a SIGTERM.") streamToDisk = flag.Bool("stream", false, "Stream results to disk instead of buffering them in RAM.") maxFlows = flag.Int("maxflows", 0, "The maximum number of concurrent flows allowed. When this threshold is reached, new flows will be ignored.")
A valid flow is one that will receive a uuid. An invalid one is a flow that will never receive a uuid (e.g. from a syn flood).
Do I understand correctly that when -flowtimeout
is also lowered to 5s, that any invalid flows below -maxflows
(say 100) will be ignored after 5s, making room for additional valid flows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @stephen-soltesz)
main.go
line 39 at r1 (raw file):
Previously, stephen-soltesz (Stephen Soltesz) wrote…
A valid flow is one that will receive a uuid. An invalid one is a flow that will never receive a uuid (e.g. from a syn flood).
Do I understand correctly that when
-flowtimeout
is also lowered to 5s, that any invalid flows below-maxflows
(say 100) will be ignored after 5s, making room for additional valid flows?
Every time the GC is invoked (i.e. every -flowtimeout
) "current" flows become "old" flows, and saver goroutines associated to "old" flows are stopped by closing their packet/uuid channels. A new packet for a given flow puts it back into the "current" flows map.
-maxFlows
only considers "current" flows, so yes, every 5s there would be more room for additional flows. SYN flood flows are invalid, never come back from the "old" state and are eventually removed after 2 * flowtimeout
(on the second GC pass). However, if the vast majority of packets we receive are SYNs, it's likely that this additional room will be filled by more invalid flows. A machine under attack would probably not save a single PCAP, but at least the machine would still be responsive and the experiments would keep working.
Noting that I'm pondering if the new mechanism introduced in this PR could be used to add an expiring IP -> connection count
map with a short TTL to keep track of abusive source IPs and route their packets to the drain directly, without spawning any saver goroutine -- at least until they stop sending SYNs for a while. This would allow us to keep the ability to save valid PCAPs while being flooded.
Excellent work - excellent ideas - thank you |
This PR adds a new
-maxflows
flag that sets the maximum amount of concurrent flows packet-headers will handle.When the limit is reached, whenever a new flow would be created for an incoming packet, the packet is routed to a "drain" saver that does nothing except for draining the packet and the uuid channels. Since this saver is a singleton and shared across multiple flows, we avoid creating new goroutines and allocating more memory.
Also, the new drain saver is now used when the memory usage goes over the configured threshold.
getSaver
would previously returnnil
and cause an increment of thepcap_missed_packets_total
metric, which (according to its help text) should always be zero.This change is