[ovs-dev] [PATCH v3] ovn-controller: add quiet mode

2016-10-05 Thread Ryan Moats 
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 
---
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(-)

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 1d8fbf3..a3f465f 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -377,6 +377,8 @@ run_S_CLEAR_FLOWS(void)
 queue_msg(encode_group_mod());
 ofputil_uninit_group_mod();
 
+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 00392ca..3c6b8b3 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -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(_to_ofport);
+
+/* Contains the list of known tunnels. */
+static struct hmap tunnels = HMAP_INITIALIZER();
+
+/* 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[])
 _lports);
 }
 
+bool physical_change = true;
 if (br_int && chassis) {
+enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
+ _ct_zones);
+physical_change = detect_and_save_physical_changes(
+_to_ofport, , mff_ovn_geneve, br_int,
+chassis_id);
+unsigned int cur_seqno = ovsdb_idl_get_seqno(ovnsb_idl_loop.idl);
+
 patch_run(, br_int, chassis_id, _datapaths,
   _datapaths);
 
 static struct lport_index lports;
-static struct mcgroup_index mcgroups;
 lport_index_init(, ctx.ovnsb_idl);
-mcgroup_index_init(, ctx.ovnsb_idl);
-
-enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
- _ct_zones);
 
 pinctrl_run(, , br_int, chassis_id, _datapaths);
 update_ct_zones(_lports, _datapaths, _zones,
 ct_zone_bitmap, _ct_zones);
 commit_ct_zones(, br_int, _ct_zones);
 
-struct hmap flow_table = HMAP_INITIALIZER(_table);
-lflow_run(, , , _datapaths,
-  _datapaths, _table, _zones,
-  _table);
-
-physical_run(, mff_ovn_geneve,
- br_int, chassis_id, _zones, _table,
- _datapaths, _datapaths);
-
-ofctrl_put(_table, _ct_zones,
-   get_nb_cfg(ctx.ovnsb_idl));
-hmap_destroy(_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(, ctx.ovnsb_idl);
+
+struct hmap flow_table = HMAP_INITIALIZER(_table);
+lflow_run(, , , _datapaths,
+  _datapaths, _table, _zones,
+  _table);
+
+physical_run(, mff_ovn_geneve,
+ br_int, chassis_id, _zones, _table,
+ _datapaths, _datapaths);
+
+ofctrl_put(_table, _ct_zones,
+   get_nb_cfg(ctx.ovnsb_idl));
+hmap_destroy(_table);
+if

Re: [ovs-dev] [PATCH v3] ovn-controller: add quiet mode

2016-08-27 Thread Ryan Moats 
Ben Pfaff  wrote on 08/27/2016 11:45:57 AM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 08/27/2016 11:46 AM
> Subject: Re: [PATCH v3] ovn-controller: add quiet mode
>
> On Fri, Aug 26, 2016 at 07:23:22PM -0500, Ryan Moats wrote:
> >
> >
> > Ryan Moats/Omaha/IBM@IBMUS wrote on 08/26/2016 04:36:20 PM:
> >
> > > From: Ryan Moats/Omaha/IBM@IBMUS
> > > To: dev@openvswitch.org
> > > Cc: b...@ovn.org, Ryan Moats/Omaha/IBM@IBMUS
> > > Date: 08/26/2016 04:36 PM
> > > Subject: [PATCH v3] ovn-controller: add quiet mode
> > >
> > > 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 
> > > ---
> >
> > Not quite sure why this was marked as Not Applicable in patch
> > works, as it sill applies cleanly to v6 of the remove incremental
> > processing patch, so I've put it back as new...
>
> I couldn't get it to apply.  I'll try again.
>

Ok... in the meantime, I realized that I wanted to unpersist some
of the physical.c items in that follow on patch set, so there is a v7
of remove incremental processing (v7 patched a memory leak I made)
and a v4 of the quite mode patch, which should apply cleanly to
v7.  I'm currently working on the "unpersist address sets" follow
on patch and once it clears memory testing, I'll post it...

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3] ovn-controller: add quiet mode

2016-08-27 Thread Ben Pfaff 
On Fri, Aug 26, 2016 at 07:23:22PM -0500, Ryan Moats wrote:
> 
> 
> Ryan Moats/Omaha/IBM@IBMUS wrote on 08/26/2016 04:36:20 PM:
> 
> > From: Ryan Moats/Omaha/IBM@IBMUS
> > To: dev@openvswitch.org
> > Cc: b...@ovn.org, Ryan Moats/Omaha/IBM@IBMUS
> > Date: 08/26/2016 04:36 PM
> > Subject: [PATCH v3] ovn-controller: add quiet mode
> >
> > 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 
> > ---
> 
> Not quite sure why this was marked as Not Applicable in patch
> works, as it sill applies cleanly to v6 of the remove incremental
> processing patch, so I've put it back as new...

I couldn't get it to apply.  I'll try again.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3] ovn-controller: add quiet mode

2016-08-26 Thread Ryan Moats 


Ryan Moats/Omaha/IBM@IBMUS wrote on 08/26/2016 04:36:20 PM:

> From: Ryan Moats/Omaha/IBM@IBMUS
> To: dev@openvswitch.org
> Cc: b...@ovn.org, Ryan Moats/Omaha/IBM@IBMUS
> Date: 08/26/2016 04:36 PM
> Subject: [PATCH v3] ovn-controller: add quiet mode
>
> 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 
> ---

Not quite sure why this was marked as Not Applicable in patch
works, as it sill applies cleanly to v6 of the remove incremental
processing patch, so I've put it back as new...

Ryan
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v3] ovn-controller: add quiet mode

2016-08-26 Thread Ryan Moats 
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 
---
 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());
 ofputil_uninit_group_mod();
 
+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(_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();
+
+/* 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[])
 _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(
+_to_ofport, , mff_ovn_geneve, br_int,
+chassis_id);
+unsigned int cur_seqno = ovsdb_idl_get_seqno(ovnsb_idl_loop.idl);
+
 patch_run(, br_int, chassis_id, _datapaths,
   _datapaths);
 
 static struct lport_index lports;
-static struct mcgroup_index mcgroups;
 lport_index_init(, ctx.ovnsb_idl);
-mcgroup_index_init(, ctx.ovnsb_idl);
-
-enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
-
-pinctrl_run(, , br_int, chassis_id, _datapaths);
+pinctrl_run(, , br_int, chassis_id,
+_datapaths);
 update_ct_zones(_lports, _datapaths, _zones,
 ct_zone_bitmap);
 
-struct hmap flow_table = HMAP_INITIALIZER(_table);
-lflow_run(, , , _datapaths,
-  _datapaths, _table, _zones,
-  _table);
-
-physical_run(, mff_ovn_geneve,
- br_int, chassis_id, _zones, _table,
- _datapaths, _datapaths);
-
-ofctrl_put(_table, get_nb_cfg(ctx.ovnsb_idl));
-hmap_destroy(_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(, ctx.ovnsb_idl);
+
+struct hmap flow_table = HMAP_INITIALIZER(_table);
+lflow_run(, , , _datapaths,
+  _datapaths, _table, _zones,
+  _table);
+
+physical_run(, mff_ovn_geneve,
+ br_int, chassis_id, _zones, _table,
+ _datapaths, _datapaths);
+
+ofctrl_put(_table, get_nb_cfg(ctx.ovnsb_idl));
+hmap_destroy(_table);
+if (ctx.ovnsb_idl_txn) {
+int64_t cur_cfg = ofctrl_get_cur_cfg();
+if (cur_cfg && cur_cfg != chassis->nb_cfg) {
+