@@ -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);
+ }
}
}
@@ -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);
@@ -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.