diff mbox series

[ovs-dev,1/2] ovn-northd: Avoid creation of identical datapath groups.

Message ID 20210827191742.2307529-2-i.maximets@ovn.org
State Accepted
Headers show
Series northd: Rework and optimize reconciliation of datapath groups. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Ilya Maximets Aug. 27, 2021, 7:17 p.m. UTC 
Currently, in a case where datapath groups doesn't need to be changed,
but a new flow should be added, northd will create a new identical
datapath group for a new flow.  It happens because we're not looking
for suitable group in Southbound database, but only tracking ones
created on current iteration.

With this change, northd will collect existing Sb DB datapath groups
while checking logical flows by linking them to unique datapath
groups generated on this run.  This way we can reduce number of
datapath groups.  It wasn't really a problem, almost all the flows
in typical use case are using the same datapath group with only
a few of them using separate copies.  More importantly, this change
allows to avoid full comparison of datapath groups if we already
checked this group from the database once and linked it to a new group
generated on this run.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 northd/ovn-northd.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Dumitru Ceara Aug. 31, 2021, 1:41 p.m. UTC | #1 
On 8/27/21 9:17 PM, Ilya Maximets wrote:
> Currently, in a case where datapath groups doesn't need to be changed,
> but a new flow should be added, northd will create a new identical
> datapath group for a new flow.  It happens because we're not looking
> for suitable group in Southbound database, but only tracking ones
> created on current iteration.
> 
> With this change, northd will collect existing Sb DB datapath groups
> while checking logical flows by linking them to unique datapath
> groups generated on this run.  This way we can reduce number of
> datapath groups.  It wasn't really a problem, almost all the flows
> in typical use case are using the same datapath group with only
> a few of them using separate copies.  More importantly, this change
> allows to avoid full comparison of datapath groups if we already
> checked this group from the database once and linked it to a new group
> generated on this run.
> 
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks!
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index cdc351c26..ae5874518 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4308,6 +4308,7 @@  struct ovn_lflow {
     char *io_port;
     char *stage_hint;
     char *ctrl_meter;
+    struct ovn_dp_group *dpg;    /* Link to unique Sb datapath group. */
     const char *where;
 };
 
@@ -4356,6 +4357,7 @@  ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
     lflow->io_port = io_port;
     lflow->stage_hint = stage_hint;
     lflow->ctrl_meter = ctrl_meter;
+    lflow->dpg = NULL;
     lflow->where = where;
 }
 
@@ -13208,6 +13210,7 @@  build_lflows(struct northd_context *ctx, struct hmap *datapaths,
             hmapx_clone(&dpg->map, &lflow->od_group);
             hmap_insert(&dp_groups, &dpg->node, hash);
         }
+        lflow->dpg = dpg;
     }
 
     /* Adding datapath to the flow hash for logical flows that have only one,
@@ -13305,8 +13308,17 @@  build_lflows(struct northd_context *ctx, struct hmap *datapaths,
             bool update_dp_group = false;
 
             if (n_datapaths != hmapx_count(&lflow->od_group)) {
+                /* Groups are different. */
                 update_dp_group = true;
-            } else {
+            } else if (lflow->dpg && lflow->dpg->dp_group) {
+                /* We know the datapath group in Sb that should be used. */
+                if (lflow->dpg->dp_group != dp_group) {
+                    /* Flow has different datapath group in the database.  */
+                    update_dp_group = true;
+                }
+                /* Datapath group is already up to date. */
+            } else if (n_datapaths) {
+                /* Have to compare datapath groups in full. */
                 for (i = 0; i < n_datapaths; i++) {
                     if (od[i] && !hmapx_contains(&lflow->od_group, od[i])) {
                         update_dp_group = true;
@@ -13318,7 +13330,12 @@  build_lflows(struct northd_context *ctx, struct hmap *datapaths,
             if (update_dp_group) {
                 ovn_sb_set_lflow_logical_dp_group(ctx, &dp_groups,
                                                   sbflow, &lflow->od_group);
+            } else if (lflow->dpg && !lflow->dpg->dp_group) {
+                /* Setting relation between unique datapath group and
+                 * Sb DB datapath goup. */
+                lflow->dpg->dp_group = dp_group;
             }
+
             /* This lflow updated.  Not needed anymore. */
             ovn_lflow_destroy(&lflows, lflow);
         } else {