[1] indicates that with incremental processing of logical flows desired conntrack groups are not being persisted. This patch adds this capability, with the side effect of adding a ds_copy method that this capability leverages.
[1] http://openvswitch.org/pipermail/dev/2016-July/076320.html Signed-off-by: Ryan Moats <rmo...@us.ibm.com> --- include/openvswitch/dynamic-string.h | 1 + lib/dynamic-string.c | 8 ++++++++ ovn/controller/lflow.c | 4 +++- ovn/controller/ofctrl.c | 33 +++++++++++++++++++++++---------- ovn/controller/ofctrl.h | 3 +++ ovn/lib/actions.c | 11 ++++++++--- ovn/lib/actions.h | 9 +++++++-- tests/test-ovn.c | 5 ++++- 8 files changed, 57 insertions(+), 17 deletions(-) diff --git a/include/openvswitch/dynamic-string.h b/include/openvswitch/dynamic-string.h index dfe2688..398e41a 100644 --- a/include/openvswitch/dynamic-string.h +++ b/include/openvswitch/dynamic-string.h @@ -73,6 +73,7 @@ void ds_swap(struct ds *, struct ds *); int ds_last(const struct ds *); bool ds_chomp(struct ds *, int c); +void ds_copy(struct ds *, struct ds *); /* Inline functions. */ diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c index 1f17a9f..692468f 100644 --- a/lib/dynamic-string.c +++ b/lib/dynamic-string.c @@ -456,3 +456,11 @@ ds_chomp(struct ds *ds, int c) return false; } } + +void +ds_copy(struct ds *dst, struct ds *source) +{ + dst->length = source->length; + dst->allocated = source->allocated; + dst->string = xmemdup(source->string, source->allocated + 1); +} diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index 42c9055..67b702c 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -383,6 +383,7 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports, if (full_flow_processing) { ovn_flow_table_clear(); + ovn_group_table_clear(group_table, false); full_logical_flow_processing = true; full_neighbor_flow_processing = true; full_flow_processing = false; @@ -522,7 +523,8 @@ consider_logical_flow(const struct lport_index *lports, .output_ptable = output_ptable, .arp_ptable = OFTABLE_MAC_BINDING, }; - error = actions_parse_string(lflow->actions, &ap, &ofpacts, &prereqs); + error = actions_parse_string(lflow->actions, &ap, &ofpacts, &prereqs, + &lflow->header_.uuid); if (error) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "error parsing actions \"%s\": %s", diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index f0451b7..40591c7 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -118,9 +118,6 @@ static enum mf_field_id mff_ovn_geneve; static void ovn_flow_table_destroy(void); -static void ovn_group_table_clear(struct group_table *group_table, - bool existing); - static void ofctrl_recv(const struct ofp_header *, enum ofptype); static struct hmap match_flow_table = HMAP_INITIALIZER(&match_flow_table); @@ -630,6 +627,16 @@ ofctrl_remove_flows(const struct uuid *uuid) ovn_flow_destroy(f); } } + + /* Remove any group_info information created by this logical flow. */ + struct group_info *g, *next_g; + HMAP_FOR_EACH_SAFE (g, next_g, hmap_node, &groups->desired_groups) { + if (uuid_equals(&g->lflow_uuid, uuid)) { + hmap_remove(&groups->desired_groups, &g->hmap_node); + ds_destroy(&g->group); + free(g); + } + } } /* Shortcut to remove all flows matching the supplied UUID and add this @@ -777,6 +784,15 @@ queue_flow_mod(struct ofputil_flow_mod *fm) /* group_table. */ +static struct group_info * +group_info_clone(struct group_info *source) { + struct group_info *clone = xmalloc(sizeof *clone); + ds_copy(&clone->group, &source->group); + clone->group_id = source->group_id; + clone->hmap_node.hash = source->hmap_node.hash; + return clone; +} + /* Finds and returns a group_info in 'existing_groups' whose key is identical * to 'target''s key, or NULL if there is none. */ static struct group_info * @@ -795,7 +811,7 @@ ovn_group_lookup(struct hmap *exisiting_groups, } /* Clear either desired_groups or existing_groups in group_table. */ -static void +void ovn_group_table_clear(struct group_table *group_table, bool existing) { struct group_info *g, *next; @@ -1000,13 +1016,10 @@ ofctrl_put(struct group_table *group_table) /* Move the contents of desired_groups to existing_groups. */ HMAP_FOR_EACH_SAFE(desired, next_group, hmap_node, &group_table->desired_groups) { - hmap_remove(&group_table->desired_groups, &desired->hmap_node); if (!ovn_group_lookup(&group_table->existing_groups, desired)) { - hmap_insert(&group_table->existing_groups, &desired->hmap_node, - desired->hmap_node.hash); - } else { - ds_destroy(&desired->group); - free(desired); + struct group_info *clone = group_info_clone(desired); + hmap_insert(&group_table->existing_groups, &clone->hmap_node, + clone->hmap_node.hash); } } } diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h index 49b95b0..a6fffec 100644 --- a/ovn/controller/ofctrl.h +++ b/ovn/controller/ofctrl.h @@ -53,4 +53,7 @@ void ofctrl_flow_table_clear(void); void ovn_flow_table_clear(void); +void ovn_group_table_clear(struct group_table *group_table, + bool existing); + #endif /* ovn/ofctrl.h */ diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 6e2bf93..64a9de8 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -42,6 +42,7 @@ struct action_context { char *error; /* Error, if any, otherwise NULL. */ struct ofpbuf *ofpacts; /* Actions. */ struct expr *prereqs; /* Prerequisites to apply to match. */ + struct uuid lflow_uuid; /* Parent lflow UUID (for group removal). */ }; static bool parse_action(struct action_context *); @@ -761,6 +762,7 @@ parse_ct_lb_action(struct action_context *ctx) group_info = xmalloc(sizeof *group_info); group_info->group = ds; group_info->group_id = group_id; + group_info->lflow_uuid = ctx->lflow_uuid; group_info->hmap_node.hash = hash; hmap_insert(&ctx->ap->group_table->desired_groups, @@ -1120,7 +1122,8 @@ parse_actions(struct action_context *ctx) */ char * OVS_WARN_UNUSED_RESULT actions_parse(struct lexer *lexer, const struct action_params *ap, - struct ofpbuf *ofpacts, struct expr **prereqsp) + struct ofpbuf *ofpacts, struct expr **prereqsp, + const struct uuid *uuid) { size_t ofpacts_start = ofpacts->size; @@ -1130,6 +1133,7 @@ actions_parse(struct lexer *lexer, const struct action_params *ap, .error = NULL, .ofpacts = ofpacts, .prereqs = NULL, + .lflow_uuid = *uuid, }; parse_actions(&ctx); @@ -1147,14 +1151,15 @@ actions_parse(struct lexer *lexer, const struct action_params *ap, /* Like actions_parse(), but the actions are taken from 's'. */ char * OVS_WARN_UNUSED_RESULT actions_parse_string(const char *s, const struct action_params *ap, - struct ofpbuf *ofpacts, struct expr **prereqsp) + struct ofpbuf *ofpacts, struct expr **prereqsp, + const struct uuid *uuid) { struct lexer lexer; char *error; lexer_init(&lexer, s); lexer_get(&lexer); - error = actions_parse(&lexer, ap, ofpacts, prereqsp); + error = actions_parse(&lexer, ap, ofpacts, prereqsp, uuid); lexer_destroy(&lexer); return error; diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h index 114c71e..72efa95 100644 --- a/ovn/lib/actions.h +++ b/ovn/lib/actions.h @@ -22,7 +22,9 @@ #include "compiler.h" #include "openvswitch/hmap.h" #include "openvswitch/dynamic-string.h" +#include "openvswitch/uuid.h" #include "util.h" +#include "uuid.h" struct expr; struct lexer; @@ -43,6 +45,7 @@ struct group_table { struct group_info { struct hmap_node hmap_node; struct ds group; + struct uuid lflow_uuid; uint32_t group_id; }; @@ -132,10 +135,12 @@ struct action_params { }; char *actions_parse(struct lexer *, const struct action_params *, - struct ofpbuf *ofpacts, struct expr **prereqsp) + struct ofpbuf *ofpacts, struct expr **prereqsp, + const struct uuid *uuid) OVS_WARN_UNUSED_RESULT; char *actions_parse_string(const char *s, const struct action_params *, - struct ofpbuf *ofpacts, struct expr **prereqsp) + struct ofpbuf *ofpacts, struct expr **prereqsp, + const struct uuid *uuid) OVS_WARN_UNUSED_RESULT; #endif /* ovn/actions.h */ diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 26055bb..afd7b31 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1319,7 +1319,10 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) .output_ptable = 64, .arp_ptable = 65, }; - error = actions_parse_string(ds_cstr(&input), &ap, &ofpacts, &prereqs); + struct uuid uuid; + uuid_zero(&uuid); + error = actions_parse_string(ds_cstr(&input), &ap, &ofpacts, &prereqs, + &uuid); if (!error) { struct ds output; -- 2.7.4 (Apple Git-66) _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
