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

Introduce established_connection_remove event #646

Open
0xxon opened this issue Oct 22, 2019 · 0 comments · May be fixed by #677

Comments

@0xxon
Copy link
Member

@0xxon 0xxon commented Oct 22, 2019

Measurements of @JustinAzoff show that the current use of connection_state_remove introduced quite a bit of performance overhead. For more details, see #642

connection_state_remove is raised for all connection state removal - including of connections that were never established. However, in most cases where the event is used, the code in it only is meaningful if a connection was established. It is still called in these cases - which seems to lead to a fair amount of performance overhead.

The solution to this problem which was discussed in #642 is to introduce a new event, called established_connection_remove. This event will have the same semantics as connection_state_remove - but will only be called for connections that got established. Simultaneously we can move nearly all uses of connection_state_remove over to the new event - which should improve performance quite a bit.

@0xxon 0xxon added this to the 3.1.0 milestone Oct 22, 2019
@0xxon 0xxon added this to Unassigned / Todo in Release 3.1.0 via automation Oct 22, 2019
@jsiwek jsiwek self-assigned this Nov 2, 2019
jsiwek added a commit that referenced this issue Nov 2, 2019
And switch Zeek's base scripts over to using it in place of
"connection_state_remove".  The difference between the two is
that "connection_state_remove" is raised for all events while
"credible_connection_remove" excludes TCP connections that were never
established (just SYN packets).  There can be performance benefits
to this change for some use-cases.
@jsiwek jsiwek referenced a pull request that will close this issue Nov 2, 2019
jsiwek added a commit that referenced this issue Nov 5, 2019
And switch Zeek's base scripts over to using it in place of
"connection_state_remove".  The difference between the two is
that "connection_state_remove" is raised for all events while
"established_connection_remove" excludes TCP connections that were never
established (just SYN packets).  There can be performance benefits
to this change for some use-cases.
jsiwek added a commit that referenced this issue Nov 12, 2019
And switch Zeek's base scripts over to using it in place of
"connection_state_remove".  The difference between the two is
that "connection_state_remove" is raised for all events while
"successful_connection_remove" excludes TCP connections that were never
established (just SYN packets).  There can be performance benefits
to this change for some use-cases.

There's also a new event called ``connection_successful`` and a new
``connection`` record field named "successful" to help indicate this new
property of connections.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Release 3.1.0
  
Unassigned / Todo
2 participants
You can’t perform that action at this time.