This adds a new engine node "bfd_chassis" which handles changes
on the Sb_ha_chassis_group table.

This improves heavily performance at scale.

Without that patch we are permanently reconciling bfd_run and
the ovn-controller is capped at 100% cpu usage. Especially wqon
gateway chassis.

3000 HA_Chassis_Group (many ref_chassis)
1500 Chassis

performance trace:
- 98.86% main
  - 89.55% bfd_run
    - 89.23% bfd_calculate_chassis
      - 28.07% sset_add__
      - 27.86% sset_add
      - 18.20% smap_get_bool
      - 13.58% sset_destroy

Signed-off-by: Max Lamprecht <[email protected]>
Co-authored-by: Ihtisham ul Haq <[email protected]>
Signed-off-by: Ihtisham ul Haq <[email protected]>
Co-authored-by: Maxim Korezkij <[email protected]>
Signed-off-by: Maxim Korezkij <[email protected]>
Co-authored-by: Felix Huettner <[email protected]>
Signed-off-by: Felix Huettner <[email protected]>
---
 controller/bfd.c            | 14 +++----
 controller/bfd.h            |  7 +++-
 controller/ovn-controller.c | 79 +++++++++++++++++++++++++++++++++----
 3 files changed, 84 insertions(+), 16 deletions(-)

diff --git a/controller/bfd.c b/controller/bfd.c
index 22a8c6695..9889b7e7a 100644
--- a/controller/bfd.c
+++ b/controller/bfd.c
@@ -117,7 +117,7 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge 
*br_int,
  *
  * If 'our_chassis' is C5 then this function returns empty bfd set.
  */
-static void
+void
 bfd_calculate_chassis(
     const struct sbrec_chassis *our_chassis,
     const struct sbrec_ha_chassis_group_table *ha_chassis_grp_table,
@@ -154,7 +154,9 @@ bfd_calculate_chassis(
                 if (smap_get_bool(&ref_ch->other_config, "is-remote", false)) {
                     continue;
                 }
-                sset_add(&grp_chassis, ref_ch->name);
+                /* we have bfd_setup_required == true anyway, so we skip adding
+                 * it to an sset that we later move to another sset again. */
+                sset_add(bfd_chassis, ref_ch->name);
             }
         } else {
             /* This is not an HA chassis. Check if this chassis is present
@@ -181,16 +183,13 @@ bfd_calculate_chassis(
 void
 bfd_run(const struct ovsrec_interface_table *interface_table,
         const struct ovsrec_bridge *br_int,
+        const struct sset *bfd_chassis,
         const struct sbrec_chassis *chassis_rec,
-        const struct sbrec_ha_chassis_group_table *ha_chassis_grp_table,
         const struct sbrec_sb_global_table *sb_global_table)
 {
     if (!chassis_rec) {
         return;
     }
-    struct sset bfd_chassis = SSET_INITIALIZER(&bfd_chassis);
-    bfd_calculate_chassis(chassis_rec, ha_chassis_grp_table,
-                          &bfd_chassis);
 
     /* Identify tunnels ports(connected to remote chassis id) to enable bfd */
     struct sset tunnels = SSET_INITIALIZER(&tunnels);
@@ -205,7 +204,7 @@ bfd_run(const struct ovsrec_interface_table 
*interface_table,
             sset_add(&tunnels, port_name);
 
             if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL, NULL)) {
-                if (sset_contains(&bfd_chassis, chassis_name)) {
+                if (sset_contains(bfd_chassis, chassis_name)) {
                     sset_add(&bfd_ifaces, port_name);
                 }
                 free(chassis_name);
@@ -270,5 +269,4 @@ bfd_run(const struct ovsrec_interface_table 
*interface_table,
     smap_destroy(&bfd);
     sset_destroy(&tunnels);
     sset_destroy(&bfd_ifaces);
-    sset_destroy(&bfd_chassis);
 }
diff --git a/controller/bfd.h b/controller/bfd.h
index 17fab5323..be0d599b7 100644
--- a/controller/bfd.h
+++ b/controller/bfd.h
@@ -31,10 +31,15 @@ void bfd_register_ovs_idl(struct ovsdb_idl *);
 
 void bfd_run(const struct ovsrec_interface_table *,
              const struct ovsrec_bridge *,
+             const struct sset *,
              const struct sbrec_chassis *,
-             const struct sbrec_ha_chassis_group_table *,
              const struct sbrec_sb_global_table *);
 
+void bfd_calculate_chassis(
+    const struct sbrec_chassis *,
+    const struct sbrec_ha_chassis_group_table *,
+    struct sset *);
+
 void  bfd_calculate_active_tunnels(const struct ovsrec_bridge *br_int,
                                    struct sset *active_tunnels);
 
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 854d80bdf..bff73a6e7 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -841,6 +841,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 #define SB_NODES \
     SB_NODE(sb_global, "sb_global") \
     SB_NODE(chassis, "chassis") \
+    SB_NODE(ha_chassis_group, "ha_chassis_group") \
     SB_NODE(encap, "encap") \
     SB_NODE(address_set, "address_set") \
     SB_NODE(port_group, "port_group") \
@@ -3290,6 +3291,47 @@ en_mac_cache_cleanup(void *data)
     hmap_destroy(&cache_data->fdbs);
 }
 
+struct ed_type_bfd_chassis {
+    struct sset bfd_chassis;
+};
+
+static void *
+en_bfd_chassis_init(struct engine_node *node OVS_UNUSED,
+                   struct engine_arg *arg OVS_UNUSED)
+{
+    struct ed_type_bfd_chassis *data = xzalloc(sizeof *data);
+    sset_init(&data->bfd_chassis);
+    return data;
+}
+
+static void
+en_bfd_chassis_run(struct engine_node *node, void *data OVS_UNUSED)
+{
+    struct ed_type_bfd_chassis *bfd_chassis = data;
+    const struct ovsrec_open_vswitch_table *ovs_table =
+        EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
+    const char *chassis_id = get_ovs_chassis_id(ovs_table);
+    const struct sbrec_ha_chassis_group_table *ha_chassis_grp_table =
+        EN_OVSDB_GET(engine_get_input("SB_ha_chassis_group", node));
+    struct ovsdb_idl_index *sbrec_chassis_by_name =
+        engine_ovsdb_node_get_index(
+            engine_get_input("SB_chassis", node),
+            "name");
+    const struct sbrec_chassis *chassis
+        = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
+
+    sset_clear(&bfd_chassis->bfd_chassis);
+    bfd_calculate_chassis(chassis, ha_chassis_grp_table,
+                          &bfd_chassis->bfd_chassis);
+    engine_set_node_state(node, EN_UPDATED);
+}
+
+static void
+en_bfd_chassis_cleanup(void *data OVS_UNUSED){
+    struct ed_type_bfd_chassis *bfd_chassis = data;
+    sset_destroy(&bfd_chassis->bfd_chassis);
+}
+
 /* Engine node which is used to handle the Non VIF data like
  *   - OVS patch ports
  *   - Tunnel ports and the related chassis information.
@@ -4716,6 +4758,14 @@ controller_output_mac_cache_handler(struct engine_node 
*node,
     return true;
 }
 
+static bool
+controller_output_bfd_chassis_handler(struct engine_node *node,
+                                    void *data OVS_UNUSED)
+{
+    engine_set_node_state(node, EN_UPDATED);
+    return true;
+}
+
 /* Handles sbrec_chassis changes.
  * If a new chassis is added or removed return false, so that
  * flows are recomputed.  For any updates, there is no need for
@@ -5025,6 +5075,7 @@ main(int argc, char *argv[])
     ENGINE_NODE(if_status_mgr, "if_status_mgr");
     ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data");
     ENGINE_NODE(mac_cache, "mac_cache");
+    ENGINE_NODE(bfd_chassis, "bfd_chassis");
 
 #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR);
     SB_NODES
@@ -5064,6 +5115,10 @@ main(int argc, char *argv[])
 
     engine_add_input(&en_if_status_mgr, &en_ovs_interface,
                      if_status_mgr_ovs_interface_handler);
+    engine_add_input(&en_bfd_chassis, &en_ovs_open_vswitch, NULL);
+    engine_add_input(&en_bfd_chassis, &en_sb_sb_global, NULL);
+    engine_add_input(&en_bfd_chassis, &en_sb_chassis, NULL);
+    engine_add_input(&en_bfd_chassis, &en_sb_ha_chassis_group, NULL);
 
     /* Note: The order of inputs is important, all OVS interface changes must
      * be handled before any ct_zone changes.
@@ -5221,6 +5276,8 @@ main(int argc, char *argv[])
                      controller_output_pflow_output_handler);
     engine_add_input(&en_controller_output, &en_mac_cache,
                      controller_output_mac_cache_handler);
+    engine_add_input(&en_controller_output, &en_bfd_chassis,
+                     controller_output_bfd_chassis_handler);
 
     struct engine_arg engine_arg = {
         .sb_idl = ovnsb_idl_loop.idl,
@@ -5263,6 +5320,8 @@ main(int argc, char *argv[])
         engine_get_internal_data(&en_pflow_output);
     struct ed_type_ct_zones *ct_zones_data =
         engine_get_internal_data(&en_ct_zones);
+    struct ed_type_bfd_chassis *bfd_chassis_data =
+        engine_get_internal_data(&en_bfd_chassis);
     struct ed_type_runtime_data *runtime_data =
         engine_get_internal_data(&en_runtime_data);
     struct ed_type_template_vars *template_vars_data =
@@ -5571,6 +5630,7 @@ main(int argc, char *argv[])
                     }
 
                     ct_zones_data = engine_get_data(&en_ct_zones);
+                    bfd_chassis_data = engine_get_data(&en_bfd_chassis);
                     if (ovs_idl_txn) {
                         if (ct_zones_data) {
                             stopwatch_start(CT_ZONE_COMMIT_STOPWATCH_NAME,
@@ -5580,13 +5640,18 @@ main(int argc, char *argv[])
                             stopwatch_stop(CT_ZONE_COMMIT_STOPWATCH_NAME,
                                            time_msec());
                         }
-                        stopwatch_start(BFD_RUN_STOPWATCH_NAME, time_msec());
-                        bfd_run(ovsrec_interface_table_get(ovs_idl_loop.idl),
-                                br_int, chassis,
-                                sbrec_ha_chassis_group_table_get(
-                                    ovnsb_idl_loop.idl),
-                                sbrec_sb_global_table_get(ovnsb_idl_loop.idl));
-                        stopwatch_stop(BFD_RUN_STOPWATCH_NAME, time_msec());
+                        if (bfd_chassis_data) {
+                            stopwatch_start(
+                                BFD_RUN_STOPWATCH_NAME, time_msec());
+                            bfd_run(
+                                ovsrec_interface_table_get(ovs_idl_loop.idl),
+                                br_int, &bfd_chassis_data->bfd_chassis,
+                                chassis, sbrec_sb_global_table_get(
+                                    ovnsb_idl_loop.idl)
+                            );
+                            stopwatch_stop(
+                                BFD_RUN_STOPWATCH_NAME, time_msec());
+                        }
                     }
 
                     runtime_data = engine_get_data(&en_runtime_data);
-- 
2.34.1



Diese E Mail enthält möglicherweise vertrauliche Inhalte und ist nur für die 
Verwertung durch den vorgesehenen Empfänger bestimmt.
Sollten Sie nicht der vorgesehene Empfänger sein, setzen Sie den Absender bitte 
unverzüglich in Kenntnis und löschen diese E Mail.
​
Hinweise zum Datenschutz finden Sie 
hier
.

​This e-mail may contain confidential content and is intended only for the 
specified recipient/s.
If you are not the intended recipient, please inform the sender immediately and 
delete this e-mail.
​
Information on data protection can be found here.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to