Skip to content

Conversation

@gogapp
Copy link
Owner

@gogapp gogapp commented Jun 18, 2018

This patch implements the space saving algorithm in Gatekeeper

@gogapp gogapp force-pushed the space_saving branch 2 times, most recently from a0734ef to 5b985ee Compare June 25, 2018 16:10
Copy link
Collaborator

@mengxiang0811 mengxiang0811 left a comment

Choose a reason for hiding this comment

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

lib: Implementing Space saving algorithm in Gatekeeper

#include "gatekeeper_net.h"
#include "gatekeeper_flow.h"

/* Data structure for Counter Table */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a period at the end of each comment.

/* Data structure for Counter bucket */
struct counter_bucket {

int proto;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it enough to use only 16-bit number for this field?


/* Data present in a Counter bucket */
struct bucket_data {

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would like to remove this blank line.


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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please keep one tab space for the alignment.


static int
create_bucket(struct gatekeeper_if *iface, struct gk_config *gk_conf,
int bkt_id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please keep one tab space for the alignment.

return ret;

out:
ret = -1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can return ret directly.

if(ret < 0)
goto out;

//avl_update(bkt_id, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this line if it's unused.


/*
static int
iterate_counter_table(struct counter_bkt *ct_bkt, int proto, int threshold,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the code is not used, please remove it.

*/

/* Bucket id of element with minimum frequency */
int min_bkt_id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move it to the beginning of the function. Same applies to the variables below.

@gogapp gogapp changed the title lib: Implementing Space saving algorithm in Gatekeeper GSOC 18: Implementing blackholing in Gatekeeper Aug 12, 2018
@gogapp
Copy link
Owner Author

gogapp commented Aug 14, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants