On Sun, Feb 28, 2016 at 10:33:24PM -0800, Gurucharan Shetty wrote:
> Signed-off-by: Gurucharan Shetty <g...@ovn.org>
GCC complains about strtok_r() in parse_ct_lb_action():
../ovn/lib/actions.c: In function 'actions_parse':
../ovn/lib/actions.c:251:5: error: 'save' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
ds_put_format(ds, ",bucket=bucket_id=%u,weight:100,actions="
^
../ovn/lib/actions.c:227:22: note: 'save' was declared here
char *ips, *ip, *save;
^
This is a common GCC complaint; we usually end up with an initializer on
the save pointer, e.g.:
char *ips, *ip, *save = NULL;
even though I'm pretty sure it's not really necessary.
I'm uncomfortable with the form of the argument to ct_lb. It seems odd
that it would be a string, since it is naturally a list of IP addresses
and the OVN match/action language is well suited for lists of IP
addresses.
I don't see any documentation for the new actions in ovn-sb.xml.
s/paranthesis/parenthesis/ here:
static void
parse_ct_lb_action(struct action_context *ctx)
{
if (!lexer_match(ctx->lexer, LEX_T_LPAREN)) {
action_error(ctx, "\"ct_lb\" needs paranthesis.");
return;
}
OFPG_MAX is 0xffffff00. This allocates a bitmap with that many bits, or
about 512 MB of RAM. I think that's overkill.
It's probably better to use bitmap_scan() than to open-code a loop.
I think that parse_ct_lb_action() leaks the string it constructs in the
case where it succeeds but doesn't create a new group.
parse_ct_lb_action() ends with a plain "return;", which can be removed.
There is a typo in the parameter name for ovn_group_lookup():
s/exisiting/existing/.
run_S_CLEAR_FLOWS() calls ovn_flow_table_clear(). I think it should
also clear existing_groups, so that the desired_groups get reinstalled.
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
