@@ -377,6 +377,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);
@@ -402,6 +402,23 @@ 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. */
+static struct simap localvif_to_ofport =
+ SIMAP_INITIALIZER(&localvif_to_ofport);
+
+/* Contains the list of known tunnels. */
+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[])
{
@@ -528,42 +545,52 @@ main(int argc, char *argv[])
&all_lports);
}
+ bool physical_change = true;
if (br_int && chassis) {
+ enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
+ &pending_ct_zones);
+ 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,
- &pending_ct_zones);
pinctrl_run(&ctx, &lports, br_int, chassis_id, &local_datapaths);
update_ct_zones(&all_lports, &patched_datapaths, &ct_zones,
ct_zone_bitmap, &pending_ct_zones);
commit_ct_zones(&ctx, br_int, &pending_ct_zones);
- 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, &pending_ct_zones,
- 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 (ctx.ovs_idl_txn && (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, &pending_ct_zones,
+ 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);
}
@@ -91,5 +91,6 @@ enum chassis_tunnel_type {
uint32_t get_tunnel_type(const char *name);
+void force_full_process(void);
#endif /* ovn/ovn-controller.h */
@@ -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;
}
}
@@ -269,6 +271,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
@@ -294,6 +297,7 @@ add_logical_patch_ports_postprocess(struct hmap *patched_datapaths)
if (pd_cur_node->stale == true) {
hmap_remove(patched_datapaths, &pd_cur_node->hmap_node);
free(pd_cur_node);
+ force_full_process();
}
}
}
@@ -366,6 +370,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();
}
}
}
@@ -55,10 +55,6 @@ physical_register_ovs_idl(struct ovsdb_idl *ovs_idl)
ovsdb_idl_add_column(ovs_idl, &ovsrec_interface_col_external_ids);
}
-static struct simap localvif_to_ofport =
- SIMAP_INITIALIZER(&localvif_to_ofport);
-static struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
-
/* Maps from a chassis to the OpenFlow port number of the tunnel that can be
* used to reach that chassis. */
struct chassis_tunnel {
@@ -69,11 +65,11 @@ struct chassis_tunnel {
};
static struct chassis_tunnel *
-chassis_tunnel_find(const char *chassis_id)
+chassis_tunnel_find(struct hmap *tunnels_p, const char *chassis_id)
{
struct chassis_tunnel *tun;
HMAP_FOR_EACH_WITH_HASH (tun, hmap_node, hash_string(chassis_id, 0),
- &tunnels) {
+ tunnels_p) {
if (!strcmp(tun->chassis_id, chassis_id)) {
return tun;
}
@@ -160,7 +156,9 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
struct hmap *patched_datapaths,
const struct sbrec_port_binding *binding,
struct ofpbuf *ofpacts_p,
- struct hmap *flow_table)
+ struct hmap *flow_table,
+ struct simap *localvif_to_ofport_p,
+ struct hmap *tunnels_p)
{
/* 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
@@ -217,13 +215,13 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
if (!binding->tag) {
return;
}
- ofport = u16_to_ofp(simap_get(&localvif_to_ofport,
+ ofport = u16_to_ofp(simap_get(localvif_to_ofport_p,
binding->parent_port));
if (ofport) {
tag = *binding->tag;
}
} else {
- ofport = u16_to_ofp(simap_get(&localvif_to_ofport,
+ ofport = u16_to_ofp(simap_get(localvif_to_ofport_p,
binding->logical_port));
if ((!strcmp(binding->type, "localnet")
|| !strcmp(binding->type, "l2gateway"))
@@ -242,13 +240,13 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
return;
}
if (localnet_port) {
- ofport = u16_to_ofp(simap_get(&localvif_to_ofport,
+ ofport = u16_to_ofp(simap_get(localvif_to_ofport_p,
localnet_port->logical_port));
if (!ofport) {
return;
}
} else {
- tun = chassis_tunnel_find(binding->chassis->name);
+ tun = chassis_tunnel_find(tunnels_p, binding->chassis->name);
if (!tun) {
return;
}
@@ -511,7 +509,9 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
const struct sbrec_multicast_group *mc,
struct ofpbuf *ofpacts_p,
struct ofpbuf *remote_ofpacts_p,
- struct hmap *flow_table)
+ struct hmap *flow_table,
+ struct simap *localvif_to_ofport_p,
+ struct hmap *tunnels_p)
{
struct sset remote_chassis = SSET_INITIALIZER(&remote_chassis);
struct match match;
@@ -554,7 +554,7 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32,
remote_ofpacts_p);
put_resubmit(OFTABLE_CHECK_LOOPBACK, remote_ofpacts_p);
- } else if (simap_contains(&localvif_to_ofport,
+ } else if (simap_contains(localvif_to_ofport_p,
(port->parent_port && *port->parent_port)
? port->parent_port : port->logical_port)) {
put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, ofpacts_p);
@@ -600,7 +600,7 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
const struct chassis_tunnel *prev = NULL;
SSET_FOR_EACH (chassis, &remote_chassis) {
const struct chassis_tunnel *tun
- = chassis_tunnel_find(chassis);
+ = chassis_tunnel_find(tunnels_p, chassis);
if (!tun) {
continue;
}
@@ -624,13 +624,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;
@@ -638,6 +641,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)) {
@@ -706,7 +710,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
}
simap_put(&new_tunnel_to_ofport, chassis_id, ofport);
- struct chassis_tunnel *tun = chassis_tunnel_find(chassis_id);
+ struct chassis_tunnel *tun = chassis_tunnel_find(tunnels_p,
+ chassis_id);
if (tun) {
/* If the tunnel's ofport has changed, update. */
if (tun->ofport != u16_to_ofp(ofport) ||
@@ -717,7 +722,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);
@@ -737,9 +742,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);
}
@@ -747,29 +752,43 @@ 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) {
- /* Reprocess logical flow table immediately. */
- poll_immediate_wake();
- }
+
+ 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)
+{
+ struct simap localvif_to_ofport = SIMAP_INITIALIZER(&localvif_to_ofport);
+ struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
+
+ detect_and_save_physical_changes(&localvif_to_ofport, &tunnels,
+ mff_ovn_geneve, br_int, this_chassis_id);
struct ofpbuf ofpacts;
ofpbuf_init(&ofpacts, 0);
@@ -783,7 +802,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
* should clear the old flows to avoid collisions. */
consider_port_binding(mff_ovn_geneve, ct_zones, local_datapaths,
patched_datapaths, binding, &ofpacts,
- flow_table);
+ flow_table, &localvif_to_ofport, &tunnels);
}
/* Handle output to multicast groups, in tables 32 and 33. */
@@ -796,7 +815,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
* so that we avoid warning messages on collisions. */
consider_mc_group(mff_ovn_geneve, ct_zones,
local_datapaths, mc, &ofpacts, &remote_ofpacts,
- flow_table);
+ flow_table, &localvif_to_ofport, &tunnels);
}
ofpbuf_uninit(&remote_ofpacts);
@@ -812,6 +831,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);
@@ -909,7 +929,9 @@ 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);
+ simap_destroy(&localvif_to_ofport);
+ HMAP_FOR_EACH_POP (tun, hmap_node, &tunnels) {
+ free(tun);
+ }
+ hmap_destroy(&tunnels);
}
@@ -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,
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. It does this by persisting a copy of the localvif_to_ofport and tunnel information in the controller module, rather than in the physical.c module as was the case with previous commits. [1] http://openvswitch.org/pipermail/dev/2016-August/078272.html Signed-off-by: Ryan Moats <rmoats@us.ibm.com> --- Note: it may make sense to consider a follow on patch to clean up all the tests for the existence of ovs_idl_txn into the main ovn-controller loop... ovn/controller/ofctrl.c | 2 + ovn/controller/ovn-controller.c | 71 +++++++++++++++++++--------- ovn/controller/ovn-controller.h | 1 + ovn/controller/patch.c | 5 ++ ovn/controller/physical.c | 100 ++++++++++++++++++++++++---------------- ovn/controller/physical.h | 5 ++ 6 files changed, 123 insertions(+), 61 deletions(-)