-
Notifications
You must be signed in to change notification settings - Fork 9
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
agent: add tcp connection disruptor #357
Conversation
b275bba
to
6e24dc1
Compare
71f4272
to
cf7e29d
Compare
32b4110
to
f7c9e26
Compare
522f2a4
to
8ee5904
Compare
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.
@roobre I'm liking the PR. I have some comments in the structure of the code but I think overall we have a very solid base. I haven't had time to run the tests, I'll do it later but I wanted to give you some feedback as soon as possible.
Also, I remember you commented you were concerned regarding the performance. I think there are two relatively easy changes we could try, if not in this PR, in a follow-up PR but I want to take note here so we don't forget:
- Copy only the metadata of the packages, instead of the whole package. I haven't checked how this would actually work, but I think it is worth researching
- Change the way we calculate the hash for each connection. This is in the critical path for processing packages We are using CRC32. After some research, I found xxHash which is optimized for small inputs, as in our case. Moreover, given our input is of a fixed size, I was wondering if we could use a block cipher instead.
38c1e34
to
7e9cf08
Compare
3339b17
to
621666e
Compare
iptables: remove unused methods, add unit tests
621666e
to
d0700f8
Compare
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.
Only to minor non blocking comments
365245b
to
ddf89d1
Compare
ddf89d1
to
43c3371
Compare
Description
This PR implements the TCP connection disruptor as proposed in #154.
In its current state, includes a working disruptor with its associated command, which has been tested manually.
The following things remain to be addressed:
The current NFQueue library sometimes hangs when closing, for which a workaround has been addedReplaced.Checklist:
make lint
) and all checks pass.make test
) and all tests pass.make integration-xxx
for affected packages)make e2e-xxx
fordisruptors
, orcluster
related changes)