This is a prerequisite for incremental processing. Signed-off-by: Ryan Moats <rmo...@us.ibm.com> --- ovn/controller/ofctrl.c | 118 +++++++++++++++++++++++++++------------ ovn/controller/ofctrl.h | 2 + ovn/controller/ovn-controller.c | 4 +- 3 files changed, 87 insertions(+), 37 deletions(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index 3297fb3..7280c8b 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -484,16 +484,53 @@ ofctrl_add_flow(struct hmap *desired_flows, f->ofpacts_len = actions->size; f->hmap_node.hash = ovn_flow_hash(f); - if (ovn_flow_lookup(desired_flows, f)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); - if (!VLOG_DROP_INFO(&rl)) { - char *s = ovn_flow_to_string(f); - VLOG_INFO("dropping duplicate flow: %s", s); - free(s); - } + /* loop through all the flows to see if there is an old flow to be + * removed - do so if the old flow has the same priority, table, and match + * but a different action or if the old flow has the same priority, table + * and action, but the new match is either a superset or subset of the + * old match */ + + struct ovn_flow *d, *next; + HMAP_FOR_EACH_SAFE (d, next, hmap_node, desired_flows) { + if (f->table_id == d->table_id && f->priority == d->priority) { + if ((match_equal(&f->match, &d->match) + && !ofpacts_equal(f->ofpacts, f->ofpacts_len, + d->ofpacts, d->ofpacts_len)) + || (ofpacts_equal(f->ofpacts, f->ofpacts_len, + d->ofpacts, d->ofpacts_len) + && ((flow_wildcards_has_extra(&f->match.wc,&d->match.wc) + && flow_equal_except(&f->match.flow, &d->match.flow, + &f->match.wc)) + || (flow_wildcards_has_extra(&d->match.wc,&f->match.wc) + && flow_equal_except(&d->match.flow, + &f->match.flow, + &d->match.wc))))) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); + if (!VLOG_DROP_INFO(&rl)) { + char *s = ovn_flow_to_string(d); + VLOG_INFO("removing superceded flow: %s", s); + free(s); + } - ovn_flow_destroy(f); - return; + hmap_remove(desired_flows, &d->hmap_node); + ovn_flow_destroy(d); + } + + /* if this is a complete duplicate, remove the new flow */ + if (match_equal(&f->match, &d->match) + && ofpacts_equal(f->ofpacts, f->ofpacts_len, + d->ofpacts, d->ofpacts_len)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); + if (!VLOG_DROP_INFO(&rl)) { + char *s = ovn_flow_to_string(f); + VLOG_INFO("dropping duplicate flow: %s", s); + free(s); + } + + ovn_flow_destroy(f); + return; + } + } } hmap_insert(desired_flows, &f->hmap_node, f->hmap_node.hash); @@ -501,6 +538,20 @@ ofctrl_add_flow(struct hmap *desired_flows, /* ovn_flow. */ +/* duplicate an ovn_flow structure */ +struct ovn_flow * +ofctrl_dup_flow(struct ovn_flow *source) +{ + struct ovn_flow *answer = xmalloc(sizeof *answer); + answer->table_id = source->table_id; + answer->priority = source->priority; + answer->match = source->match; + answer->ofpacts = xmemdup(source->ofpacts, source->ofpacts_len); + answer->ofpacts_len = source->ofpacts_len; + answer->hmap_node.hash = ovn_flow_hash(answer); + return answer; +} + /* Returns a hash of the key in 'f'. */ static uint32_t ovn_flow_hash(const struct ovn_flow *f) @@ -595,19 +646,16 @@ queue_flow_mod(struct ofputil_flow_mod *fm) * flows from 'flow_table' and frees them. (The hmap itself isn't * destroyed.) * - * This called be called be ofctrl_run() within the main loop. */ + * This can be called by ofctrl_run() within the main loop. */ void ofctrl_put(struct hmap *flow_table) { /* The flow table can be updated if the connection to the switch is up and * in the correct state and not backlogged with existing flow_mods. (Our * criteria for being backlogged appear very conservative, but the socket - * between ovn-controller and OVS provides some buffering.) Otherwise, - * discard the flows. A solution to either of those problems will cause us - * to wake up and retry. */ + * between ovn-controller and OVS provides some buffering.) */ if (state != S_UPDATE_FLOWS || rconn_packet_counter_n_packets(tx_counter)) { - ovn_flow_table_clear(flow_table); return; } @@ -653,31 +701,31 @@ ofctrl_put(struct hmap *flow_table) d->ofpacts = NULL; d->ofpacts_len = 0; } - - hmap_remove(flow_table, &d->hmap_node); - ovn_flow_destroy(d); } } - /* The previous loop removed from 'flow_table' all of the flows that are - * already installed. Thus, any flows remaining in 'flow_table' need to - * be added to the flow table. */ + /* Iterate through the new flows and add those that aren't found + * in the installed flow table */ struct ovn_flow *d; HMAP_FOR_EACH_SAFE (d, next, hmap_node, flow_table) { - /* Send flow_mod to add flow. */ - struct ofputil_flow_mod fm = { - .match = d->match, - .priority = d->priority, - .table_id = d->table_id, - .ofpacts = d->ofpacts, - .ofpacts_len = d->ofpacts_len, - .command = OFPFC_ADD, - }; - queue_flow_mod(&fm); - ovn_flow_log(d, "adding"); - - /* Move 'd' from 'flow_table' to installed_flows. */ - hmap_remove(flow_table, &d->hmap_node); - hmap_insert(&installed_flows, &d->hmap_node, d->hmap_node.hash); + struct ovn_flow *i = ovn_flow_lookup(&installed_flows, d); + if (!i) { + /* Send flow_mod to add flow. */ + struct ofputil_flow_mod fm = { + .match = d->match, + .priority = d->priority, + .table_id = d->table_id, + .ofpacts = d->ofpacts, + .ofpacts_len = d->ofpacts_len, + .command = OFPFC_ADD, + }; + queue_flow_mod(&fm); + ovn_flow_log(d, "adding"); + + /* Copy 'd' from 'flow_table' to installed_flows. */ + struct ovn_flow *new_node = ofctrl_dup_flow(d); + hmap_insert(&installed_flows, &new_node->hmap_node, + new_node->hmap_node.hash); + } } } diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h index 93ef8ea..d3fabc0 100644 --- a/ovn/controller/ofctrl.h +++ b/ovn/controller/ofctrl.h @@ -34,6 +34,8 @@ void ofctrl_put(struct hmap *flows); void ofctrl_wait(void); void ofctrl_destroy(void); +struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source); + /* Flow table interface to the rest of ovn-controller. */ void ofctrl_add_flow(struct hmap *flows, uint8_t table_id, uint16_t priority, const struct match *, const struct ofpbuf *ofpacts); diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 3638342..5a4174e 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -205,6 +205,8 @@ main(int argc, char *argv[]) bool exiting; int retval; + struct hmap flow_table = HMAP_INITIALIZER(&flow_table); + ovs_cmdl_proctitle_init(argc, argv); set_program_name(argv[0]); service_start(&argc, &argv); @@ -299,14 +301,12 @@ main(int argc, char *argv[]) pinctrl_run(&ctx, br_int); - struct hmap flow_table = HMAP_INITIALIZER(&flow_table); lflow_run(&ctx, &flow_table, &ct_zones, &local_datapaths); if (chassis_id) { physical_run(&ctx, mff_ovn_geneve, br_int, chassis_id, &ct_zones, &flow_table); } ofctrl_put(&flow_table); - hmap_destroy(&flow_table); } /* local_datapaths contains bare hmap_node instances. -- 1.7.1 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev