Conversation
a0734ef to
5b985ee
Compare
mengxiang0811
left a comment
There was a problem hiding this comment.
lib: Implementing Space saving algorithm in Gatekeeper
include/space_saving.h
Outdated
| #include "gatekeeper_net.h" | ||
| #include "gatekeeper_flow.h" | ||
|
|
||
| /* Data structure for Counter Table */ |
There was a problem hiding this comment.
Please add a period at the end of each comment.
include/space_saving.h
Outdated
| /* Data structure for Counter bucket */ | ||
| struct counter_bucket { | ||
|
|
||
| int proto; |
There was a problem hiding this comment.
Isn't it enough to use only 16-bit number for this field?
include/space_saving.h
Outdated
|
|
||
| /* Data present in a Counter bucket */ | ||
| struct bucket_data { | ||
|
|
There was a problem hiding this comment.
I would like to remove this blank line.
include/space_saving.h
Outdated
|
|
||
| static int | ||
| setup_bucket_params(unsigned int socket_id, int bkt_id, int bkt_size, | ||
| int ip_ver, struct rte_hash_parameters *bkt_hash_params); |
There was a problem hiding this comment.
Please keep one tab space for the alignment.
include/space_saving.h
Outdated
|
|
||
| static int | ||
| create_bucket(struct gatekeeper_if *iface, struct gk_config *gk_conf, | ||
| int bkt_id); |
There was a problem hiding this comment.
Please keep one tab space for the alignment.
lib/space_saving.c
Outdated
| return ret; | ||
|
|
||
| out: | ||
| ret = -1; |
There was a problem hiding this comment.
I think you can return ret directly.
lib/space_saving.c
Outdated
| if(ret < 0) | ||
| goto out; | ||
|
|
||
| //avl_update(bkt_id, 1); |
There was a problem hiding this comment.
Please remove this line if it's unused.
lib/space_saving.c
Outdated
|
|
||
| /* | ||
| static int | ||
| iterate_counter_table(struct counter_bkt *ct_bkt, int proto, int threshold, |
There was a problem hiding this comment.
If the code is not used, please remove it.
lib/space_saving.c
Outdated
| */ | ||
|
|
||
| /* Bucket id of element with minimum frequency */ | ||
| int min_bkt_id; |
There was a problem hiding this comment.
Must it be a global variable? Also, please move it to the beginning of the file if it's necessary.
| struct ip_data *data = (struct ip_data *) | ||
| malloc(sizeof(struct ip_data)); | ||
|
|
||
| uint32_t next = 0; |
There was a problem hiding this comment.
Please move it to the beginning of the function. Same applies to the variables below.
|
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 |
49d6975 to
511f606
Compare
511f606 to
fa6f231
Compare
This patch implements the space saving algorithm in Gatekeeper