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.

Sign up for GitHub

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

Jump to bottom

ddosmitigator: handle insertion of already present address #341

Merged
merged 1 commit into from
Oct 6, 2020
Merged

ddosmitigator: handle insertion of already present address #341

merged 1 commit into from
Oct 6, 2020

Conversation

FedeParola
Copy link
Collaborator

@FedeParola FedeParola commented Sep 27, 2020 •
edited
Loading

When an address is already present no operation is performed.
Formerly the address was erroneously removed from the ebpf table.

@FedeParola
ddosmitigator: handle insertion of already present address
731aec0 
When an adress is already present no operation is performed.
Formerly the address was erroneously removed from the ebpf table.
goldenrye
goldenrye approved these changes Sep 28, 2020
View reviewed changes
Copy link
Contributor

@goldenrye goldenrye left a comment

Choose a reason for hiding this comment

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

Even without the check the old code looks harmless, it is weird that the key will be removed from the hash table, it is a bpf_update_elem()

@FedeParola
Copy link
Collaborator Author

Even without the check the old code looks harmless, it is weird that the key will be removed from the hash table, it is a bpf_update_elem()

The problem lies in this emplace:

polycube/src/services/pcn-ddosmitigator/src/Ddosmitigator.cpp

Lines 201 to 202 in 8bd4c5c

blacklistsrc_.emplace(std::piecewise_construct, std::forward_as_tuple(ip),
std::forward_as_tuple(*this, configuration));

It causes the execution of the destructor of the old BlacklistSrc or BlacklistDst entry and the destructor removes the value form the ebpf map.

@acloudiator acloudiator added this to the v.0.11.0 milestone Oct 4, 2020
@acloudiator acloudiator requested a review from a team October 4, 2020 21:32
@FedeParola
Copy link
Collaborator Author

I don't know what the ci/dockercloud test is about? How can I check why it is failing?

acloudiator
acloudiator approved these changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@acloudiator acloudiator left a comment

Choose a reason for hiding this comment

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

LGTM

@acloudiator acloudiator merged commit b9b22a7 into polycube-network:master Oct 6, 2020
@acloudiator acloudiator linked an issue Oct 6, 2020 that may be closed by this pull request
[Feature Request] [PCN DDoS-Mitigiator] ebpf syn flood testing demo #342
Open
@FedeParola FedeParola deleted the ddosmitigator-fixes branch October 30, 2020 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goldenrye goldenrye goldenrye approved these changes

@acloudiator acloudiator acloudiator approved these changes

Assignees

@FedeParola FedeParola

Labels
None yet
Projects
None yet
Milestone
v.0.11.0
Development

Successfully merging this pull request may close these issues.

[Feature Request] [PCN DDoS-Mitigiator] ebpf syn flood testing demo
3 participants
@FedeParola @goldenrye @acloudiator