As discussed in [1], what the incremental processing code
actually accomplished was that the ovn-controller would
be "quiet" and not burn CPU when things weren't changing.
This patch set recreates this state by calculating whether
changes have occured that would require a full calculation
to be performed.
This commit depends on f5d916cb ("ovn-controller:
Back out incremental processing")
[1] http://openvswitch.org/pipermail/dev/2016-August/078272.html
Signed-off-by: Ryan Moats <rmo...@us.ibm.com>
---
ovn/controller/ofctrl.c | 2 ++
ovn/controller/ovn-controller.c | 77 +++++++++++++++++++++++++++++------------
ovn/controller/ovn-controller.h | 1 +
ovn/controller/patch.c | 11 ++++++
ovn/controller/physical.c | 55 +++++++++++++++++++----------
ovn/controller/physical.h | 5 +++
6 files changed, 110 insertions(+), 41 deletions(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index d8e111d..5b1434e 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -366,6 +366,8 @@ run_S_CLEAR_FLOWS(void)
queue_msg(encode_group_mod(&gm));
ofputil_uninit_group_mod(&gm);
+ force_full_process();
+
/* Clear existing groups, to match the state of the switch. */
if (groups) {
ovn_group_table_clear(groups, true);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 41e0eb0..ef424e1 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -304,6 +304,27 @@ get_nb_cfg(struct ovsdb_idl *idl)
return sb ? sb->nb_cfg : 0;
}
+/* Contains mapping of localvifs to OF ports for detecting if the physical
+ * configuration of the switch has changed. This is kept independently of
+ * the same variable name in the physical module as the two are used for
+ * different purposes. */
+static struct simap localvif_to_ofport =
+ SIMAP_INITIALIZER(&localvif_to_ofport);
+
+/* Contains the list of known tunnels. This is kept independently of
+ * the same variable name in the physical module as the two are used for
+ * different purposes. */
+static struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
+
+/* The last sequence number seen from the southbound IDL. */
+static unsigned int seqno = 0;
+
+void
+force_full_process(void)
+{
+ seqno = 0;
+}
+
int
main(int argc, char *argv[])
{
@@ -427,39 +448,51 @@ main(int argc, char *argv[])
&all_lports);
}
+ enum mf_field_id mff_ovn_geneve;
+ bool physical_change = true;
if (br_int && chassis_id) {
+ mff_ovn_geneve = ofctrl_run(br_int);
+ physical_change = detect_and_save_physical_changes(
+ &localvif_to_ofport, &tunnels, mff_ovn_geneve, br_int,
+ chassis_id);
+ unsigned int cur_seqno = ovsdb_idl_get_seqno(ovnsb_idl_loop.idl);
+
patch_run(&ctx, br_int, chassis_id, &local_datapaths,
&patched_datapaths);
static struct lport_index lports;
- static struct mcgroup_index mcgroups;
lport_index_init(&lports, ctx.ovnsb_idl);
- mcgroup_index_init(&mcgroups, ctx.ovnsb_idl);
-
- enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
-
- pinctrl_run(&ctx, &lports, br_int, chassis_id, &local_datapaths);
+ pinctrl_run(&ctx, &lports, br_int, chassis_id,
+ &local_datapaths);
update_ct_zones(&all_lports, &patched_datapaths, &ct_zones,
ct_zone_bitmap);
- struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
- lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
- &patched_datapaths, &group_table, &ct_zones,
- &flow_table);
-
- physical_run(&ctx, mff_ovn_geneve,
- br_int, chassis_id, &ct_zones, &flow_table,
- &local_datapaths, &patched_datapaths);
-
- ofctrl_put(&flow_table, get_nb_cfg(ctx.ovnsb_idl));
- hmap_destroy(&flow_table);
- if (ctx.ovnsb_idl_txn) {
- int64_t cur_cfg = ofctrl_get_cur_cfg();
- if (cur_cfg && cur_cfg != chassis->nb_cfg) {
- sbrec_chassis_set_nb_cfg(chassis, cur_cfg);
+
+ if (physical_change || seqno < cur_seqno) {
+ seqno = cur_seqno;
+
+ static struct mcgroup_index mcgroups;
+ mcgroup_index_init(&mcgroups, ctx.ovnsb_idl);
+
+ struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
+ lflow_run(&ctx, &lports, &mcgroups, &local_datapaths,
+ &patched_datapaths, &group_table, &ct_zones,
+ &flow_table);
+
+ physical_run(&ctx, mff_ovn_geneve,
+ br_int, chassis_id, &ct_zones, &flow_table,
+ &local_datapaths, &patched_datapaths);
+
+ ofctrl_put(&flow_table, get_nb_cfg(ctx.ovnsb_idl));
+ hmap_destroy(&flow_table);
+ if (ctx.ovnsb_idl_txn) {
+ int64_t cur_cfg = ofctrl_get_cur_cfg();
+ if (cur_cfg && cur_cfg != chassis->nb_cfg) {
+ sbrec_chassis_set_nb_cfg(chassis, cur_cfg);
+ }
}
+ mcgroup_index_destroy(&mcgroups);
}
- mcgroup_index_destroy(&mcgroups);
lport_index_destroy(&lports);
}
diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
index 470b1f5..14cf861 100644
--- a/ovn/controller/ovn-controller.h
+++ b/ovn/controller/ovn-controller.h
@@ -76,5 +76,6 @@ enum chassis_tunnel_type {
uint32_t get_tunnel_type(const char *name);
+void force_full_process(void);
#endif /* ovn/ovn-controller.h */
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index c8e47b4..d2bb30f 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -96,6 +96,7 @@ create_patch_port(struct controller_ctx *ctx,
ovsrec_bridge_set_ports(src, ports, src->n_ports + 1);
free(ports);
+ force_full_process();
}
static void
@@ -124,6 +125,7 @@ remove_port(struct controller_ctx *ctx,
ovsrec_bridge_set_ports(bridge, new_ports, bridge->n_ports - 1);
free(new_ports);
ovsrec_port_delete(port);
+ force_full_process();
return;
}
}
@@ -243,6 +245,12 @@ add_bridge_mappings(struct controller_ctx *ctx,
br_int, name1, br_ln, name2, existing_ports);
create_patch_port(ctx, patch_port_id, binding->logical_port,
br_ln, name2, br_int, name1, existing_ports);
+ /* TODO (regXboi): this next line is needed for the 3 HVs, 3 LS,
+ * 3 lports/LS, 1 LR test case, but has the potential side effect
+ * of defeating quiet mode once a logical router leads to creating
+ * patch ports. Need to understand the failure mode better and
+ * what is needed to remove this. */
+ force_full_process();
free(name1);
free(name2);
}
@@ -270,6 +278,7 @@ add_patched_datapath(struct hmap *patched_datapaths,
/* stale is set to false. */
hmap_insert(patched_datapaths, &pd->hmap_node,
binding_rec->datapath->tunnel_key);
+ force_full_process();
}
static void
@@ -296,6 +305,7 @@ add_logical_patch_ports_postprocess(struct hmap
*patched_datapaths)
hmap_remove(patched_datapaths, &pd_cur_node->hmap_node);
free(pd_cur_node->key);
free(pd_cur_node);
+ force_full_process();
}
}
}
@@ -368,6 +378,7 @@ add_logical_patch_ports(struct controller_ctx *ctx,
if (local_port) {
if (binding->chassis != chassis_rec && ctx->ovnsb_idl_txn) {
sbrec_port_binding_set_chassis(binding, chassis_rec);
+ force_full_process();
}
}
}
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index bf63300..3865902 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -636,13 +636,16 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
sset_destroy(&remote_chassis);
}
-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,
- struct hmap *local_datapaths, struct hmap *patched_datapaths)
+/* Scans the bridge 'br_int' and compares it to the supplied expected state.
+ * Returns true and updates the expected state if changes are detected.
+ * Returns false if no changes are found. */
+bool
+detect_and_save_physical_changes(struct simap *localvif_to_ofport_p,
+ struct hmap *tunnels_p,
+ enum mf_field_id mff_ovn_geneve,
+ const struct ovsrec_bridge *br_int,
+ const char *this_chassis_id)
{
-
/* This bool tracks physical mapping changes. */
bool physical_map_changed = false;
@@ -650,6 +653,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
SIMAP_INITIALIZER(&new_localvif_to_ofport);
struct simap new_tunnel_to_ofport =
SIMAP_INITIALIZER(&new_tunnel_to_ofport);
+
for (int i = 0; i < br_int->n_ports; i++) {
const struct ovsrec_port *port_rec = br_int->ports[i];
if (!strcmp(port_rec->name, br_int->name)) {
@@ -729,7 +733,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
}
} else {
tun = xmalloc(sizeof *tun);
- hmap_insert(&tunnels, &tun->hmap_node,
+ hmap_insert(tunnels_p, &tun->hmap_node,
hash_string(chassis_id, 0));
tun->chassis_id = chassis_id;
tun->ofport = u16_to_ofp(ofport);
@@ -749,9 +753,9 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Remove tunnels that are no longer here. */
struct chassis_tunnel *tun, *tun_next;
- HMAP_FOR_EACH_SAFE (tun, tun_next, hmap_node, &tunnels) {
+ HMAP_FOR_EACH_SAFE (tun, tun_next, hmap_node, tunnels_p) {
if (!simap_find(&new_tunnel_to_ofport, tun->chassis_id)) {
- hmap_remove(&tunnels, &tun->hmap_node);
+ hmap_remove(tunnels_p, &tun->hmap_node);
physical_map_changed = true;
free(tun);
}
@@ -759,26 +763,41 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
/* Capture changed or removed openflow ports. */
struct simap_node *vif_name, *vif_name_next;
- SIMAP_FOR_EACH_SAFE (vif_name, vif_name_next, &localvif_to_ofport) {
+ SIMAP_FOR_EACH_SAFE (vif_name, vif_name_next, localvif_to_ofport_p) {
int newport;
if ((newport = simap_get(&new_localvif_to_ofport, vif_name->name))) {
- if (newport != simap_get(&localvif_to_ofport, vif_name->name)) {
- simap_put(&localvif_to_ofport, vif_name->name, newport);
+ if (newport != simap_get(localvif_to_ofport_p, vif_name->name)) {
+ simap_put(localvif_to_ofport_p, vif_name->name, newport);
physical_map_changed = true;
}
} else {
- simap_find_and_delete(&localvif_to_ofport, vif_name->name);
+ simap_find_and_delete(localvif_to_ofport_p, vif_name->name);
physical_map_changed = true;
}
}
SIMAP_FOR_EACH (vif_name, &new_localvif_to_ofport) {
- if (!simap_get(&localvif_to_ofport, vif_name->name)) {
- simap_put(&localvif_to_ofport, vif_name->name,
+ if (!simap_get(localvif_to_ofport_p, vif_name->name)) {
+ simap_put(localvif_to_ofport_p, vif_name->name,
simap_get(&new_localvif_to_ofport, vif_name->name));
physical_map_changed = true;
}
}
- if (physical_map_changed) {
+
+ simap_destroy(&new_localvif_to_ofport);
+ simap_destroy(&new_tunnel_to_ofport);
+
+ return physical_map_changed;
+}
+
+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,
+ struct hmap *local_datapaths, struct hmap *patched_datapaths)
+{
+ if (detect_and_save_physical_changes(&localvif_to_ofport, &tunnels,
+ mff_ovn_geneve, br_int,
+ this_chassis_id)) {
/* Reprocess logical flow table immediately. */
poll_immediate_wake();
}
@@ -824,6 +843,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
* ports. We set MFF_LOG_DATAPATH, MFF_LOG_INPORT, and
* MFF_LOG_OUTPORT from the tunnel key data, then resubmit to table
* 33 to handle packets to the local hypervisor. */
+ struct chassis_tunnel *tun;
HMAP_FOR_EACH (tun, hmap_node, &tunnels) {
struct match match = MATCH_CATCHALL_INITIALIZER;
match_set_in_port(&match, tun->ofport);
@@ -921,7 +941,4 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id
mff_ovn_geneve,
ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 0, &match, &ofpacts);
ofpbuf_uninit(&ofpacts);
-
- simap_destroy(&new_localvif_to_ofport);
- simap_destroy(&new_tunnel_to_ofport);
}
diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
index 86ce93c..d11615e 100644
--- a/ovn/controller/physical.h
+++ b/ovn/controller/physical.h
@@ -41,6 +41,11 @@ struct simap;
#define OVN_GENEVE_LEN 4
void physical_register_ovs_idl(struct ovsdb_idl *);
+bool detect_and_save_physical_changes(struct simap *localvif_to_ofport_p,
+ struct hmap *tunnels_p,
+ enum mf_field_id mff_ovn_geneve,
+ const struct ovsrec_bridge *br_int,
+ const char *this_chassis_id);
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,
--
2.7.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
