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.
Performance improved extremely in below test scenario:
- 1 lswitch with 10 lports bound locally
- Each lport has an ingress ACL, referencing the same address-set
- The address-set has 10,000 IPv4 addresses
For each IP address in the address-set, there will be 3
OpenFlow rules generated for each ACL. So the total number
of rules is 300k+.
Without the patch, it takes 50+ minutes to install all the
rules to ovs-vswitchd.
With the patch, it takes 20 seconds to install all the rules
to ovs-vswitchd.
The reason is that the large number of rules are sent to
ovs-vswitchd gradually in many iterations of ovn-controller
main loop. Without the patch, cpu cycles are wasted in
lflow_run to re-processing the large address set in every
main loop iteration. With the patch, lflow_run is not
executed in most iterations because there is no change of
input.
[1] http://openvswitch.org/pipermail/dev/2016-August/078272.html
Signed-off-by: Ryan Moats
Signed-off-by: Han Zhou
---
Notes:
v4->v5: Based on the old patch from Ryan Moats and rebased to current
master. Current version is RFC only because a problem is found in
testing. Sometimes latest SB DB change is not seen by ovn-controller:
cur_seqno in ovn-controller stays behind the real seqno seen by
ovsdb-tool, but ovn-controller does not wakeup until a new change
happens in SB DB, which can be triggered by, e.g.:
ovn-nbctl --wait=hv sync.
ovn/controller/ofctrl.c | 2 +
ovn/controller/ovn-controller.c | 59 ---
ovn/controller/ovn-controller.h | 1 +
ovn/controller/patch.c | 2 +
ovn/controller/physical.c | 102 +---
ovn/controller/physical.h | 5 ++
6 files changed, 116 insertions(+), 55 deletions(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 10c8105..20e4ab6 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -383,6 +383,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 e00f57a..2822eb7 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -484,6 +484,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[])
{
@@ -613,10 +630,8 @@ main(int argc, char *argv[])
struct ldatapath_index ldatapaths;
struct lport_index lports;
-struct mcgroup_index mcgroups;
ldatapath_index_init(&ldatapaths, ctx.ovnsb_idl);
lport_index_init(&lports, ctx.ovnsb_idl);
-mcgroup_index_init(&mcgroups, ctx.ovnsb_idl);
const struct sbrec_chassis *chassis = NULL;
if (chassis_id) {
@@ -626,36 +641,47 @@ main(int argc, char *argv[])
&local_datapaths, &local_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);
+unsigned int cur_seqno = ovsdb_idl_get_seqno(ovnsb_idl_loop.idl);
+
struct shash addr_sets = SHASH_INITIALIZER(&addr_sets);
addr_sets_init(&ctx, &addr_sets);
patch_run(&ctx, br_int, chassis, &local_datapaths);
-enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
- &pending_ct_zones);
-
pinctrl_run(&ctx, &lports, br_int, chassis, &local_datapaths);
update_ct_zones(&local_lports, &local_datapaths, &ct_zones,
