-
Notifications
You must be signed in to change notification settings - Fork 231
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
GSOC 2018: Implementing blackholing in Gatekeeper #94
base: master
Are you sure you want to change the base?
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.
Implement space saving algorithm
Files .directory
, generate_if_map
, and lua/if_map.lua
don't belong to this pull request.
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.
The code in include/space_saving.h
and lib/space_saving.c
should use tabs as indents, not spaces. We also use the DPDK style guidelines for Gatekeeper, which you can find here:
https://doc.dpdk.org/guides/contributing/coding_style.html
Make sure your code abides by these guidelines.
Makefile
Outdated
@@ -30,7 +30,7 @@ include $(RTE_SDK)/mk/rte.vars.mk | |||
|
|||
APP = gatekeeper | |||
|
|||
SRCS-y := main/main.c | |||
SRCS-y := main/main.c |
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.
Remove the extra space at the end of this line.
Hi mentors, I have added the changes that you have requested. I have also merged the commits into a single meaningful commit. Please note that the implementation is still not fully complete and I am working on the TODOs present in the code. |
lib/space_saving.c
Outdated
int ret; | ||
|
||
switch(flow->proto) { | ||
case ETHER_TYPE_IPv4: |
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.
All of the case
statements should be at the same level of indentation as the switch
statement.
lib/space_saving.c
Outdated
{ | ||
int ret; | ||
|
||
switch(flow->proto) { |
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.
Add a space between switch
and the parenthesis.
lib/space_saving.c
Outdated
int ret; | ||
|
||
switch(flow->proto) { | ||
case ETHER_TYPE_IPv4: |
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.
Same thing here: add a space and bring everything inside of this switch
back one level of indentation.
ct_hash_params->hash_func_init_val = 0; | ||
ct_hash_params->reserved = 0; | ||
|
||
char ct_name[64]; |
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.
This should be declared at the top of the function, not in the middle.
lib/space_saving.c
Outdated
* after testing. | ||
*/ | ||
|
||
if(ipv4_if_configured(iface)) { |
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.
Add a space after the if
and before the parenthesis. Look through the rest of the code for similar cases.
I have added the code files for the final review and fixed the styling errors. For testing of the algorithms, I have created a different repository here. https://github.com/gogapp/GSOC-2018-tests |
Can you rebase your code to include one commit to add the space saving algorithm and one commit to add the RHHH algorithm? In other words, we shouldn't have a separate commit for fixing the style issues. Also, the Gatekeeper code does not call the functions you are adding. To merge these changes we will need to have it actually integrated with Gatekeeper. |
49d6975
to
511f606
Compare
511f606
to
fa6f231
Compare
I have rebased the commits as requested. To get the changes integrated with Gatekeeper, I need to think out ways to do this. Currently I am focusing on my College Placements. I will integrate the functions with Gatekeeper as soon as I am free. |
Suggestion for where to integrate: |
436ab22
to
e52bbf2
Compare
This PR implements space saving algorithm in Gatekeeper.