From: RYAN D. MOATS <rmo...@us.ibm.com>
This is a prerequisite for incremental processing.
Signed-off-by: RYAN D. MOATS <rmo...@us.ibm.com>
---
lib/ofp-actions.c | 12 ++
lib/ofp-actions.h | 2 +
ovn/controller/binding.c | 1 +
ovn/controller/lflow.c | 65 ++++++++--
ovn/controller/lflow.h | 5 +-
ovn/controller/lport.c | 3 +
ovn/controller/ofctrl.c | 264 +++++++++++++++++++++++++++-----------
ovn/controller/ofctrl.h | 16 ++-
ovn/controller/ovn-controller.c | 12 +-
ovn/controller/physical.c | 108 ++++++++++++----
ovn/controller/physical.h | 4 +-
11 files changed, 362 insertions(+), 130 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index aac4ff0..b7c1839 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -7354,6 +7354,18 @@ ofpacts_equal(const struct ofpact *a, size_t a_len,
return a_len == b_len && !memcmp(a, b, a_len);
}
+uint32_t
+ofpacts_hash(const struct ofpact *a, size_t a_len, uint32_t basis)
+{
+ size_t i;
+ uint32_t interim = basis;
+ for (i = 0; i < a_len; i += 4) {
+ uint32_t *term = (uint32_t *) ((uint8_t *)a+i);
+ interim = hash_add(*term, interim);
+ }
+ return hash_finish(interim, a_len);
+}
+
/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
* 'ofpacts'. If found, returns its meter ID; if not, returns 0.
*
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 4bd8854..5c45b56 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -887,6 +887,8 @@ bool ofpacts_output_to_group(const struct ofpact[], size_t
ofpacts_len,
uint32_t group_id);
bool ofpacts_equal(const struct ofpact a[], size_t a_len,
const struct ofpact b[], size_t b_len);
+uint32_t ofpacts_hash(const struct ofpact a[], size_t a_len, uint32_t basis);
+
const struct mf_field *ofpact_get_mf_dst(const struct ofpact *ofpact);
uint32_t ofpacts_get_meter(const struct ofpact[], size_t ofpacts_len);
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index d3ca9c9..b001d40 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -133,6 +133,7 @@ add_local_datapath(struct hmap *local_datapaths,
struct local_datapath *ld = xzalloc(sizeof *ld);
hmap_insert(local_datapaths, &ld->hmap_node,
binding_rec->datapath->tunnel_key);
+ reset_flow_processing();
}
static void
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index bcad318..041fa66 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -35,6 +35,14 @@ VLOG_DEFINE_THIS_MODULE(lflow);
/* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
static struct shash symtab;
+static bool restart_flow_processing = false;
+
+void
+reset_flow_processing(void)
+{
+ restart_flow_processing = true;
+}
+
static void
add_logical_register(struct shash *symtab, enum mf_field_id id)
{
@@ -199,12 +207,26 @@ add_logical_flows(struct controller_ctx *ctx, const
struct lport_index *lports,
const struct mcgroup_index *mcgroups,
const struct hmap *local_datapaths,
const struct hmap *patched_datapaths,
- const struct simap *ct_zones, struct hmap *flow_table)
+ const struct simap *ct_zones)
{
uint32_t conj_id_ofs = 1;
const struct sbrec_logical_flow *lflow;
- SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
+ SBREC_LOGICAL_FLOW_FOR_EACH_TRACKED (lflow, ctx->ovnsb_idl) {
+ unsigned int del_seqno = sbrec_logical_flow_row_get_seqno(lflow,
+ OVSDB_IDL_CHANGE_DELETE);
+ unsigned int mod_seqno = sbrec_logical_flow_row_get_seqno(lflow,
+ OVSDB_IDL_CHANGE_MODIFY);
+
+ /* if the row has a del_seqno > 0, then trying to process the
+ * row isn't going to work (as it has already been freed).
+ * What we can do is to pass a pointer to the ovs_idl_row to
+ * ofctrl_remove_flows() to remove flows from this record */
+ if (del_seqno > 0) {
+ ofctrl_remove_flows(&lflow->header_.uuid);
+ continue;
+ }
+
/* Determine translation of logical table IDs to physical table IDs. */
bool ingress = !strcmp(lflow->pipeline, "ingress");
@@ -336,8 +358,8 @@ add_logical_flows(struct controller_ctx *ctx, const struct
lport_index *lports,
m->match.flow.conj_id += conj_id_ofs;
}
if (!m->n) {
- ofctrl_add_flow(flow_table, ptable, lflow->priority,
- &m->match, &ofpacts);
+ ofctrl_add_flow(ptable, lflow->priority, &m->match, &ofpacts,
+ &lflow->header_.uuid, mod_seqno);
} else {
uint64_t conj_stubs[64 / 8];
struct ofpbuf conj;
@@ -352,8 +374,8 @@ add_logical_flows(struct controller_ctx *ctx, const struct
lport_index *lports,
dst->clause = src->clause;
dst->n_clauses = src->n_clauses;
}
- ofctrl_add_flow(flow_table, ptable, lflow->priority,
- &m->match, &conj);
+ ofctrl_add_flow(ptable, lflow->priority, &m->match, &conj,
+ &lflow->header_.uuid, mod_seqno);
ofpbuf_uninit(&conj);
}
}
@@ -383,7 +405,7 @@ put_load(const uint8_t *data, size_t len,
* numbers. */
static void
add_neighbor_flows(struct controller_ctx *ctx,
- const struct lport_index *lports, struct hmap *flow_table)
+ const struct lport_index *lports)
{
struct ofpbuf ofpacts;
struct match match;
@@ -391,7 +413,21 @@ add_neighbor_flows(struct controller_ctx *ctx,
ofpbuf_init(&ofpacts, 0);
const struct sbrec_mac_binding *b;
- SBREC_MAC_BINDING_FOR_EACH (b, ctx->ovnsb_idl) {
+ SBREC_MAC_BINDING_FOR_EACH_TRACKED (b, ctx->ovnsb_idl) {
+ unsigned int del_seqno = sbrec_mac_binding_row_get_seqno(b,
+ OVSDB_IDL_CHANGE_DELETE);
+ unsigned int mod_seqno = sbrec_mac_binding_row_get_seqno(b,
+ OVSDB_IDL_CHANGE_MODIFY);
+
+ /* if the row has a del_seqno > 0, then trying to process the
+ * row isn't going to work (as it has already been freed).
+ * What we can do pass a pointer to the ovs_idl_row to
+ * ofctrl_remove_flows() to remove the flow */
+ if (del_seqno > 0) {
+ ofctrl_remove_flows(&b->header_.uuid);
+ continue;
+ }
+
const struct sbrec_port_binding *pb
= lport_lookup_by_name(lports, b->logical_port);
if (!pb) {
@@ -419,8 +455,8 @@ add_neighbor_flows(struct controller_ctx *ctx,
ofpbuf_clear(&ofpacts);
put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100,
- &match, &ofpacts);
+ ofctrl_add_flow(OFTABLE_MAC_BINDING, 100, &match, &ofpacts,
+ &b->header_.uuid, mod_seqno);
}
ofpbuf_uninit(&ofpacts);
}
@@ -432,11 +468,14 @@ lflow_run(struct controller_ctx *ctx, const struct
lport_index *lports,
const struct mcgroup_index *mcgroups,
const struct hmap *local_datapaths,
const struct hmap *patched_datapaths,
- const struct simap *ct_zones, struct hmap *flow_table)
+ const struct simap *ct_zones)
{
+ if (restart_flow_processing) {
+ ovn_flow_table_clear();
+ }
add_logical_flows(ctx, lports, mcgroups, local_datapaths,
- patched_datapaths, ct_zones, flow_table);
- add_neighbor_flows(ctx, lports, flow_table);
+ patched_datapaths, ct_zones);
+ add_neighbor_flows(ctx, lports);
}
void
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index a3fc50c..4f3ea28 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -63,8 +63,9 @@ void lflow_run(struct controller_ctx *, const struct
lport_index *,
const struct mcgroup_index *,
const struct hmap *local_datapaths,
const struct hmap *patched_datapaths,
- const struct simap *ct_zones,
- struct hmap *flow_table);
+ const struct simap *ct_zones);
void lflow_destroy(void);
+void reset_flow_processing(void);
+
#endif /* ovn/lflow.h */
diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
index e1ecf21..e09930a 100644
--- a/ovn/controller/lport.c
+++ b/ovn/controller/lport.c
@@ -17,6 +17,7 @@
#include "lport.h"
#include "hash.h"
+#include "lflow.h"
#include "openvswitch/vlog.h"
#include "ovn/lib/ovn-sb-idl.h"
@@ -50,6 +51,7 @@ lport_index_init(struct lport_index *lports, struct ovsdb_idl
*ovnsb_idl)
hmap_insert(&lports->by_key, &p->key_node,
hash_int(pb->tunnel_key, pb->datapath->tunnel_key));
p->pb = pb;
+ reset_flow_processing();
}
}
@@ -123,6 +125,7 @@ mcgroup_index_init(struct mcgroup_index *mcgroups, struct
ovsdb_idl *ovnsb_idl)
hmap_insert(&mcgroups->by_dp_name, &m->dp_name_node,
hash_string(mg->name, uuid_hash(dp_uuid)));
m->mg = mg;
+ reset_flow_processing();
}
}
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 55ca98d..a46a10e 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -37,19 +37,23 @@ VLOG_DEFINE_THIS_MODULE(ofctrl);
/* An OpenFlow flow. */
struct ovn_flow {
/* Key. */
- struct hmap_node hmap_node;
+ struct hmap_node match_hmap_node; /* for match based hashing */
+ struct hmap_node uuid_hmap_node; /* for uuid based hashing */
uint8_t table_id;
uint16_t priority;
- struct match match;
+ const struct uuid *uuid;
/* Data. */
+ struct match match;
struct ofpact *ofpacts;
size_t ofpacts_len;
};
-static uint32_t ovn_flow_hash(const struct ovn_flow *);
-static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table,
- const struct ovn_flow *target);
+static uint32_t ovn_flow_match_hash(const struct ovn_flow *);
+static struct ovn_flow *ovn_flow_lookup_by_uuid(struct hmap *,
+ const struct ovn_flow *target);
+static struct ovn_flow *ovn_flow_lookup_by_match(struct hmap *,
+ const struct ovn_flow *target);
static char *ovn_flow_to_string(const struct ovn_flow *);
static void ovn_flow_log(const struct ovn_flow *, const char *action);
static void ovn_flow_destroy(struct ovn_flow *);
@@ -97,11 +101,14 @@ static struct hmap installed_flows;
* S_CLEAR_FLOWS or S_UPDATE_FLOWS, this is really the option we have. */
static enum mf_field_id mff_ovn_geneve;
-static void ovn_flow_table_clear(struct hmap *flow_table);
-static void ovn_flow_table_destroy(struct hmap *flow_table);
+void ovn_flow_table_clear(void);
+static void ovn_flow_table_destroy(void);
static void ofctrl_recv(const struct ofp_header *, enum ofptype);
+struct hmap match_flow_table = HMAP_INITIALIZER(&match_flow_table);
+struct hmap uuid_flow_table = HMAP_INITIALIZER(&uuid_flow_table);
+
void
ofctrl_init(void)
{
@@ -310,7 +317,7 @@ run_S_CLEAR_FLOWS(void)
VLOG_DBG("clearing all flows");
/* Clear installed_flows, to match the state of the switch. */
- ovn_flow_table_clear(&installed_flows);
+ ovn_flow_table_clear();
state = S_UPDATE_FLOWS;
}
@@ -428,7 +435,7 @@ void
ofctrl_destroy(void)
{
rconn_destroy(swconn);
- ovn_flow_table_destroy(&installed_flows);
+ ovn_flow_table_destroy();
rconn_packet_counter_destroy(tx_counter);
}
@@ -461,63 +468,134 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype
type)
}
}
-/* Flow table interface to the rest of ovn-controller. */
+/* Flow table interfaces to the rest of ovn-controller. */
-/* Adds a flow to 'desired_flows' with the specified 'match' and 'actions' to
+/* Adds a flow to flow tables with the specified 'match' and 'actions' to
* the OpenFlow table numbered 'table_id' with the given 'priority'. The
* caller retains ownership of 'match' and 'actions'.
*
- * This just assembles the desired flow table in memory. Nothing is actually
+ * Because it is possible for both actions and matches to change on a rule,
+ * and because the hmap struct only supports a single hash, this method
+ * uses two hash maps - one that uses table_id+priority+matches for its hash
+ * and the other that uses table_id+priority+actions.
+ *
+ * This just assembles the desired flow tables in memory. Nothing is actually
* sent to the switch until a later call to ofctrl_run().
*
- * The caller should initialize its own hmap to hold the flows. */
+ * The caller should initialize its own hmaps to hold the flows. */
void
-ofctrl_add_flow(struct hmap *desired_flows,
- uint8_t table_id, uint16_t priority,
- const struct match *match, const struct ofpbuf *actions)
+ofctrl_add_flow(uint8_t table_id, uint16_t priority,
+ const struct match *match, const struct ofpbuf *actions,
+ const struct uuid *uuid, unsigned int mod_seqno)
{
+ // structure that uses table_id+priority+various things as hashes
struct ovn_flow *f = xmalloc(sizeof *f);
f->table_id = table_id;
f->priority = priority;
f->match = *match;
f->ofpacts = xmemdup(actions->data, actions->size);
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);
+ f->uuid = uuid;
+ f->match_hmap_node.hash = ovn_flow_match_hash(f);
+ f->uuid_hmap_node.hash = uuid_hash(f->uuid);
+
+ /* if mod_seqno > 0 then this is a modify operation, so look up
+ * the old flow via the match hash. If you can't find it,
+ * then look up via the action hash. */
+
+ if (mod_seqno > 0) {
+ struct ovn_flow *d = ovn_flow_lookup_by_match(&match_flow_table, f);
+ if (!d) {
+ d = ovn_flow_lookup_by_uuid(&uuid_flow_table, f);
}
- ovn_flow_destroy(f);
- return;
+ if (d) {
+ hmap_remove(&match_flow_table, &d->match_hmap_node);
+ hmap_remove(&uuid_flow_table, &d->uuid_hmap_node);
+ ovn_flow_destroy(d);
+ }
+ } else {
+ /* this is an insert operation, so check to see if this
+ * is a duplicate via the match hash. If so, then
+ * check if the actions have changed. If it is a complete
+ * duplicate (i.e. the actions are the same) drop the new
+ * flow. If not, then drop the old flow as superseded.
+ * If the new rule is not a duplicate, check the action
+ * hash to see if this flow is superseding a previous
+ * flow and if so, drop the old flow and insert the
+ * new one */
+
+ struct ovn_flow *d = ovn_flow_lookup_by_match(&match_flow_table, f);
+
+ if (d) {
+ if (ofpacts_equal(f->ofpacts, f->ofpacts_len,
+ d->ofpacts, d->ofpacts_len)) {
+ ovn_flow_destroy(f);
+ return;
+ }
+ hmap_remove(&match_flow_table, &d->match_hmap_node);
+ hmap_remove(&uuid_flow_table, &d->uuid_hmap_node);
+ ovn_flow_destroy(d);
+ }
}
+ hmap_insert(&match_flow_table, &f->match_hmap_node,
+ f->match_hmap_node.hash);
+ hmap_insert(&uuid_flow_table, &f->uuid_hmap_node,
+ f->uuid_hmap_node.hash);
+}
- hmap_insert(desired_flows, &f->hmap_node, f->hmap_node.hash);
+/* removes a bundles of flows from the flow table */
+
+void
+ofctrl_remove_flows(const struct uuid *uuid)
+{
+ // structure that uses table_id+priority+various things as hashes
+ struct ovn_flow *f, *next;
+ HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &match_flow_table) {
+ if (uuid_equals(f->uuid, uuid)) {
+ hmap_remove(&match_flow_table, &f->match_hmap_node);
+ hmap_remove(&uuid_flow_table, &f->uuid_hmap_node);
+ ovn_flow_destroy(f);
+ }
+ }
}
+
/* ovn_flow. */
-/* Returns a hash of the key in 'f'. */
+/* 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->uuid = source->uuid;
+ answer->match_hmap_node.hash = ovn_flow_match_hash(answer);
+ answer->uuid_hmap_node.hash = uuid_hash(source->uuid);
+ return answer;
+}
+
+/* Returns a hash of the match key in 'f'. */
static uint32_t
-ovn_flow_hash(const struct ovn_flow *f)
+ovn_flow_match_hash(const struct ovn_flow *f)
{
return hash_2words((f->table_id << 16) | f->priority,
match_hash(&f->match, 0));
-
}
/* Finds and returns an ovn_flow in 'flow_table' whose key is identical to
- * 'target''s key, or NULL if there is none. */
+ * 'target''s key, or NULL if there is none, using the match hashmap. */
static struct ovn_flow *
-ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target)
+ovn_flow_lookup_by_match(struct hmap* flow_table,
+ const struct ovn_flow *target)
{
struct ovn_flow *f;
- HMAP_FOR_EACH_WITH_HASH (f, hmap_node, target->hmap_node.hash,
+ HMAP_FOR_EACH_WITH_HASH (f, match_hmap_node, target->match_hmap_node.hash,
flow_table) {
if (f->table_id == target->table_id
&& f->priority == target->priority
@@ -528,6 +606,39 @@ ovn_flow_lookup(struct hmap *flow_table, const struct
ovn_flow *target)
return NULL;
}
+/* Finds and returns an ovn_flow in 'flow_table' whose key is identical to
+ * 'target''s key, or NULL if there is none, using the uuid hashmap. */
+static struct ovn_flow *
+ovn_flow_lookup_by_uuid(struct hmap* flow_table,
+ const struct ovn_flow *target)
+{
+ struct ovn_flow *f;
+
+ HMAP_FOR_EACH_WITH_HASH (f, uuid_hmap_node,
+ target->uuid_hmap_node.hash, flow_table) {
+ if (f->table_id == target->table_id
+ && f->priority == target->priority
+ && (match_equal(&f->match, &target->match)
+ || ((flow_wildcards_has_extra(&f->match.wc,
+ &target->match.wc)
+ && flow_equal_except(&f->match.flow,
+ &target->match.flow,
+ &f->match.wc)
+ && !flow_wildcards_has_extra(&target->match.wc,
+ &f->match.wc))
+ || (flow_wildcards_has_extra(&target->match.wc,
+ &f->match.wc)
+ && flow_equal_except(&target->match.flow,
+ &f->match.flow,
+ &target->match.wc)
+ && !flow_wildcards_has_extra(&f->match.wc,
+ &target->match.wc))))) {
+ return f;
+ }
+ }
+ return NULL;
+}
+
static char *
ovn_flow_to_string(const struct ovn_flow *f)
{
@@ -554,28 +665,32 @@ static void
ovn_flow_destroy(struct ovn_flow *f)
{
if (f) {
- free(f->ofpacts);
+ if (f->ofpacts) {
+ free(f->ofpacts);
+ }
free(f);
}
}
/* Flow tables of struct ovn_flow. */
-static void
-ovn_flow_table_clear(struct hmap *flow_table)
+void
+ovn_flow_table_clear(void)
{
struct ovn_flow *f, *next;
- HMAP_FOR_EACH_SAFE (f, next, hmap_node, flow_table) {
- hmap_remove(flow_table, &f->hmap_node);
+ HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &match_flow_table) {
+ hmap_remove(&match_flow_table, &f->match_hmap_node);
+ hmap_remove(&uuid_flow_table, &f->uuid_hmap_node);
ovn_flow_destroy(f);
}
}
static void
-ovn_flow_table_destroy(struct hmap *flow_table)
+ovn_flow_table_destroy(void)
{
- ovn_flow_table_clear(flow_table);
- hmap_destroy(flow_table);
+ ovn_flow_table_clear();
+ hmap_destroy(&match_flow_table);
+ hmap_destroy(&uuid_flow_table);
}
/* Flow table update. */
@@ -595,19 +710,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)
+ofctrl_put(void)
{
/* 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;
}
@@ -615,8 +727,8 @@ ofctrl_put(struct hmap *flow_table)
* longer desired, delete them; if any of them should have different
* actions, update them. */
struct ovn_flow *i, *next;
- HMAP_FOR_EACH_SAFE (i, next, hmap_node, &installed_flows) {
- struct ovn_flow *d = ovn_flow_lookup(flow_table, i);
+ HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) {
+ struct ovn_flow *d = ovn_flow_lookup_by_match(&match_flow_table, i);
if (!d) {
/* Installed flow is no longer desirable. Delete it from the
* switch and from installed_flows. */
@@ -627,9 +739,9 @@ ofctrl_put(struct hmap *flow_table)
.command = OFPFC_DELETE_STRICT,
};
queue_flow_mod(&fm);
- ovn_flow_log(i, "removing");
+ ovn_flow_log(i, "removing installed");
- hmap_remove(&installed_flows, &i->hmap_node);
+ hmap_remove(&installed_flows, &i->match_hmap_node);
ovn_flow_destroy(i);
} else {
if (!ofpacts_equal(i->ofpacts, i->ofpacts_len,
@@ -644,40 +756,38 @@ ofctrl_put(struct hmap *flow_table)
.command = OFPFC_MODIFY_STRICT,
};
queue_flow_mod(&fm);
- ovn_flow_log(i, "updating");
+ ovn_flow_log(i, "updating installed");
/* Replace 'i''s actions by 'd''s. */
free(i->ofpacts);
- i->ofpacts = d->ofpacts;
+ i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len);
i->ofpacts_len = d->ofpacts_len;
- 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);
+ HMAP_FOR_EACH_SAFE (d, next, match_hmap_node, &match_flow_table) {
+ struct ovn_flow *i = ovn_flow_lookup_by_match(&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 installed");
+
+ /* Copy 'd' from 'flow_table' to installed_flows. */
+ struct ovn_flow *new_node = ofctrl_dup_flow(d);
+ hmap_insert(&installed_flows, &new_node->match_hmap_node,
+ new_node->match_hmap_node.hash);
+ }
}
}
diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
index 93ef8ea..1e70ad6 100644
--- a/ovn/controller/ofctrl.h
+++ b/ovn/controller/ofctrl.h
@@ -20,6 +20,7 @@
#include <stdint.h>
#include "meta-flow.h"
+#include "ovsdb-idl.h"
struct controller_ctx;
struct hmap;
@@ -30,12 +31,19 @@ struct ovsrec_bridge;
/* Interface for OVN main loop. */
void ofctrl_init(void);
enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int);
-void ofctrl_put(struct hmap *flows);
+void ofctrl_put(void);
void ofctrl_wait(void);
void ofctrl_destroy(void);
-/* 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);
+struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source);
+
+/* Flow table interfaces to the rest of ovn-controller. */
+void ofctrl_add_flow(uint8_t table_id, uint16_t priority,
+ const struct match *, const struct ofpbuf *ofpacts,
+ const struct uuid *uuid, unsigned int mod_seqno);
+
+void ofctrl_remove_flows(const struct uuid *uuid);
+
+void ofctrl_flow_table_clear(void);
#endif /* ovn/ofctrl.h */
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 6027011..d9ba2b2 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -285,6 +285,10 @@ main(int argc, char *argv[])
char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
+
+ /* track the southbound idl */
+ ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
+
ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
int probe_interval = 0;
@@ -337,16 +341,14 @@ main(int argc, char *argv[])
pinctrl_run(&ctx, &lports, br_int);
- struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
- &patched_datapaths, &ct_zones, &flow_table);
+ &patched_datapaths, &ct_zones);
if (chassis_id) {
physical_run(&ctx, mff_ovn_geneve,
- br_int, chassis_id, &ct_zones, &flow_table,
+ br_int, chassis_id, &ct_zones,
&local_datapaths, &patched_datapaths);
}
- ofctrl_put(&flow_table);
- hmap_destroy(&flow_table);
+ ofctrl_put();
mcgroup_index_destroy(&mcgroups);
lport_index_destroy(&lports);
}
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 795db31..9169d0b 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -144,14 +144,21 @@ get_localnet_port(struct hmap *local_datapaths, int64_t
tunnel_key)
return ld ? ld->localnet_port : NULL;
}
+struct uuid *hc_uuid = NULL; // uuid to identify OF flows not associated with
+ // ovsdb rows.
+
void
physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
const struct ovsrec_bridge *br_int, const char *this_chassis_id,
- const struct simap *ct_zones, struct hmap *flow_table,
+ const struct simap *ct_zones,
struct hmap *local_datapaths, struct hmap *patched_datapaths)
{
struct simap localvif_to_ofport = SIMAP_INITIALIZER(&localvif_to_ofport);
struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
+ if (!hc_uuid) {
+ hc_uuid = xmalloc(sizeof(struct uuid));
+ uuid_generate(hc_uuid);
+ }
for (int i = 0; i < br_int->n_ports; i++) {
const struct ovsrec_port *port_rec = br_int->ports[i];
@@ -231,7 +238,21 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Set up flows in table 0 for physical-to-logical translation and in table
* 64 for logical-to-physical translation. */
const struct sbrec_port_binding *binding;
- SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
+ SBREC_PORT_BINDING_FOR_EACH_TRACKED (binding, ctx->ovnsb_idl) {
+ unsigned int del_seqno = sbrec_port_binding_row_get_seqno(binding,
+ OVSDB_IDL_CHANGE_DELETE);
+ unsigned int mod_seqno = sbrec_port_binding_row_get_seqno(binding,
+ OVSDB_IDL_CHANGE_MODIFY);
+
+ /* if the row has a del_seqno > 0, then trying to process the
+ * row isn't going to work (as it has already been freed).
+ * What we can do pass a pointer to the ovs_idl_row to
+ * ofctrl_remove_flows() to remove the flow */
+ if (del_seqno > 0) {
+ ofctrl_remove_flows(&binding->header_.uuid);
+ continue;
+ }
+
/* Skip the port binding if the port is on a datapath that is neither
* local nor with any logical patch port connected, because local ports
* would never need to talk to those ports.
@@ -379,8 +400,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Resubmit to first logical ingress pipeline table. */
put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG,
- tag ? 150 : 100, &match, &ofpacts);
+ ofctrl_add_flow(OFTABLE_PHY_TO_LOG,
+ tag ? 150 : 100, &match, &ofpacts,
+ &binding->header_.uuid, mod_seqno);
if (!tag && !strcmp(binding->type, "localnet")) {
/* Add a second flow for frames that lack any 802.1Q
@@ -388,7 +410,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
* action. */
ofpbuf_pull(&ofpacts, ofpacts_orig_size);
match_set_dl_tci_masked(&match, 0, htons(VLAN_CFI));
- ofctrl_add_flow(flow_table, 0, 100, &match, &ofpacts);
+ ofctrl_add_flow(0, 100, &match, &ofpacts,
+ &binding->header_.uuid, mod_seqno);
}
/* Table 33, priority 100.
@@ -413,8 +436,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Resubmit to table 34. */
put_resubmit(OFTABLE_DROP_LOOPBACK, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, &match,
- &ofpacts);
+ ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT,
+ 100, &match, &ofpacts,
+ &binding->header_.uuid, mod_seqno);
/* Table 34, Priority 100.
* =======================
@@ -423,10 +447,12 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
match_init_catchall(&match);
ofpbuf_clear(&ofpacts);
match_set_metadata(&match, htonll(binding->datapath->tunnel_key));
- match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0,
binding->tunnel_key);
- match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0,
binding->tunnel_key);
- ofctrl_add_flow(flow_table, OFTABLE_DROP_LOOPBACK, 100,
- &match, &ofpacts);
+ match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0,
+ binding->tunnel_key);
+ match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0,
+ binding->tunnel_key);
+ ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 100, &match, &ofpacts,
+ &binding->header_.uuid, mod_seqno);
/* Table 64, Priority 100.
* =======================
@@ -461,8 +487,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
ofpact_put_STRIP_VLAN(&ofpacts);
put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(&ofpacts));
}
- ofctrl_add_flow(flow_table, OFTABLE_LOG_TO_PHY, 100,
- &match, &ofpacts);
+ ofctrl_add_flow(OFTABLE_LOG_TO_PHY, 100, &match, &ofpacts,
+ &binding->header_.uuid, mod_seqno);
} else if (!tun) {
/* Remote port connected by localnet port */
/* Table 33, priority 100.
@@ -485,8 +511,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Resubmit to table 33. */
put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, &match,
- &ofpacts);
+ ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT, 100, &match,
+ &ofpacts, &binding->header_.uuid, mod_seqno);
} else {
/* Remote port connected by tunnel */
/* Table 32, priority 100.
@@ -510,8 +536,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Output to tunnel. */
ofpact_put_OUTPUT(&ofpacts)->port = ofport;
- ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
- &match, &ofpacts);
+ ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 100, &match, &ofpacts,
+ &binding->header_.uuid, mod_seqno);
}
}
@@ -519,7 +545,21 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
const struct sbrec_multicast_group *mc;
struct ofpbuf remote_ofpacts;
ofpbuf_init(&remote_ofpacts, 0);
- SBREC_MULTICAST_GROUP_FOR_EACH (mc, ctx->ovnsb_idl) {
+ SBREC_MULTICAST_GROUP_FOR_EACH_TRACKED (mc, ctx->ovnsb_idl) {
+ unsigned int del_seqno = sbrec_multicast_group_row_get_seqno(mc,
+ OVSDB_IDL_CHANGE_DELETE);
+ unsigned int mod_seqno = sbrec_multicast_group_row_get_seqno(mc,
+ OVSDB_IDL_CHANGE_MODIFY);
+
+ /* if the row has a del_seqno > 0, then trying to process the
+ * row isn't going to work (as it has already been freed).
+ * What we can do is to pass a pointer to the ovs_idl_row to
+ * ofctrl_remove_flows() to remove the flow */
+ if (del_seqno > 0) {
+ ofctrl_remove_flows(&mc->header_.uuid);
+ continue;
+ }
+
struct sset remote_chassis = SSET_INITIALIZER(&remote_chassis);
struct match match;
@@ -586,8 +626,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
* group as the logical output port. */
put_load(mc->tunnel_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100,
- &match, &ofpacts);
+ ofctrl_add_flow(OFTABLE_LOCAL_OUTPUT,
+ 100, &match, &ofpacts,
+ &mc->header_.uuid, mod_seqno);
}
/* Table 32, priority 100.
@@ -624,8 +665,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
if (local_ports) {
put_resubmit(OFTABLE_LOCAL_OUTPUT, &remote_ofpacts);
}
- ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
- &match, &remote_ofpacts);
+ ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 100,
+ &match, &remote_ofpacts,
+ &mc->header_.uuid, mod_seqno);
}
}
sset_destroy(&remote_chassis);
@@ -668,7 +710,10 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts);
+ /* note: we hardcode the insert sequence number to 1 to
+ * avoid collisions */
+ ofctrl_add_flow(OFTABLE_PHY_TO_LOG, 100, &match, &ofpacts,
+ hc_uuid, 0);
}
/* Add flows for VXLAN encapsulations. Due to the limited amount of
@@ -701,8 +746,11 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
put_load(binding->tunnel_key, MFF_LOG_INPORT, 0, 15, &ofpacts);
put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 100, &match,
- &ofpacts);
+ /* note: we hardcode the insert sequence number to 2 to
+ * avoid collisions */
+ ofctrl_add_flow(OFTABLE_PHY_TO_LOG, 100,
+ &match, &ofpacts,
+ hc_uuid, 0);
}
}
@@ -715,7 +763,10 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
match_init_catchall(&match);
ofpbuf_clear(&ofpacts);
put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts);
+ /* note: we hardcode the insert sequence number to 3 to
+ * avoid collisions */
+ ofctrl_add_flow(OFTABLE_REMOTE_OUTPUT, 0, &match, &ofpacts,
+ hc_uuid, 0);
/* Table 34, Priority 0.
* =======================
@@ -729,7 +780,10 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
MFF_LOG_REGS;
#undef MFF_LOG_REGS
put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, &ofpacts);
- ofctrl_add_flow(flow_table, OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts);
+ /* note: we hardcode the insert sequence number to 4 to
+ * avoid collisions */
+ ofctrl_add_flow(OFTABLE_DROP_LOOPBACK, 0, &match, &ofpacts,
+ hc_uuid, 0);
ofpbuf_uninit(&ofpacts);
simap_destroy(&localvif_to_ofport);
diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
index 9f40574..ed8052e 100644
--- a/ovn/controller/physical.h
+++ b/ovn/controller/physical.h
@@ -43,7 +43,7 @@ struct simap;
void physical_register_ovs_idl(struct ovsdb_idl *);
void physical_run(struct controller_ctx *, enum mf_field_id mff_ovn_geneve,
const struct ovsrec_bridge *br_int, const char *chassis_id,
- const struct simap *ct_zones, struct hmap *flow_table,
- struct hmap *local_datapaths, struct hmap
*patched_datapaths);
+ const struct simap *ct_zones, struct hmap *local_datapaths,
+ struct hmap *patched_datapaths);
#endif /* ovn/physical.h */
--
1.7.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
