diff mbox series

[ovs-dev,v5,11/20] ovn-controller: incremental processing for multicast group changes

Message ID 1534182499-75914-12-git-send-email-hzhou8@ebay.com
State Deferred
Headers show
Series ovn-controller incremental processing | expand

Commit Message

Han Zhou Aug. 13, 2018, 5:48 p.m. UTC
This patch handles SB Multicast_Group table changes incrementally.
The Multicast_Group table changes can be triggered by creating/deleting
a lport of a lswitch. It can be also triggered indirectly by an
updating of a port-binding which is referenced by the multicast
group.

This patch together with previous incremental processing engine
related changes supports incremental processing for lflow changes
and port-binding changes of lports on other HVs, which are the most
common scenarios in a cloud where workloads come up and down.

In ovn-scale-test env [1], the total execution time of creating and
binding 10k ports on 1k HVs with 40 lswitches and 8 lrouters
(5 lswitches/lrouter), decreased from 3h40m to 1h50m because of the
less CPU on HVs. The CPU time of ovn-controller for additional 500
lports creating and binding (on top of already existed 10k lports)
decreased 90% comparing with master.

Latency for end-to-end operations of one extra port on top of the
10k lports, start from port-creation until all flows installation
on all related HVs is also improved significantly:

before: 20.6s in total
- lsp-add: 0.4s
- wait-until port up=true: 4.8s
- --wait=hv sync: 15.4s

after: 7.3s in total
- lsp-add: 0.4s
- wait-until port up=true: 4.0s
- --wait=hv sync: 2.9s

[1] https://github.com/openvswitch/ovn-scale-test

Signed-off-by: Han Zhou <hzhou8@ebay.com>
---
 lib/ovsdb-idl.c                 |  4 +++-
 ovn/controller/ovn-controller.c | 52 ++++++++++++++++++++++++++++++++++++++++-
 ovn/controller/physical.c       | 23 ++++++++++++++++++
 ovn/controller/physical.h       |  7 ++++++
 4 files changed, 84 insertions(+), 2 deletions(-)

Comments

Han Zhou Oct. 5, 2018, 7:28 p.m. UTC | #1
On Mon, Aug 13, 2018 at 10:48 AM Han Zhou <zhouhan@gmail.com> wrote:
>
> This patch handles SB Multicast_Group table changes incrementally.
> The Multicast_Group table changes can be triggered by creating/deleting
> a lport of a lswitch. It can be also triggered indirectly by an
> updating of a port-binding which is referenced by the multicast
> group.
>
> This patch together with previous incremental processing engine
> related changes supports incremental processing for lflow changes
> and port-binding changes of lports on other HVs, which are the most
> common scenarios in a cloud where workloads come up and down.
>
> In ovn-scale-test env [1], the total execution time of creating and
> binding 10k ports on 1k HVs with 40 lswitches and 8 lrouters
> (5 lswitches/lrouter), decreased from 3h40m to 1h50m because of the
> less CPU on HVs. The CPU time of ovn-controller for additional 500
> lports creating and binding (on top of already existed 10k lports)
> decreased 90% comparing with master.
>
> Latency for end-to-end operations of one extra port on top of the
> 10k lports, start from port-creation until all flows installation
> on all related HVs is also improved significantly:
>
> before: 20.6s in total
> - lsp-add: 0.4s
> - wait-until port up=true: 4.8s
> - --wait=hv sync: 15.4s
>
> after: 7.3s in total
> - lsp-add: 0.4s
> - wait-until port up=true: 4.0s
> - --wait=hv sync: 2.9s
>
> [1] https://github.com/openvswitch/ovn-scale-test
>
> Signed-off-by: Han Zhou <hzhou8@ebay.com>
> ---
>  lib/ovsdb-idl.c                 |  4 +++-
>  ovn/controller/ovn-controller.c | 52
++++++++++++++++++++++++++++++++++++++++-
>  ovn/controller/physical.c       | 23 ++++++++++++++++++
>  ovn/controller/physical.h       |  7 ++++++
>  4 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index 468b3bf..784ae4e 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -2308,6 +2308,9 @@ add_tracked_change_for_references(struct
ovsdb_idl_row *row)
>              ovsdb_idl_track_is_set(row->table)) {
>          ovs_list_push_back(&row->table->track_list,
>                             &row->track_node);
> +        row->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
> +            = row->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
> +            = row->table->db->change_seqno + 1;
>

I just noticed that this part of the patch should have been split and
combined with a previous patch in this series.

[PATCH v5 04/20] ovsdb-idl.c: Track changes for table references.

Now that the previous patch has been merged, I sent a patch to add this
change:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/352752.html

Without this new patch, the previous one doesn't break anything, but the
feature is incomplete.

>          const struct ovsdb_idl_arc *arc;
>          LIST_FOR_EACH (arc, dst_node, &row->dst_arcs) {
> @@ -2316,7 +2319,6 @@ add_tracked_change_for_references(struct
ovsdb_idl_row *row)
>      }
>  }
>
> -
>  /* Returns true if a column with mode OVSDB_IDL_MODE_RW changed, false
>   * otherwise.
>   *
> diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> index 9fd2cba..4399d6e 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -1180,6 +1180,56 @@ flow_output_sb_port_binding_handler(struct
engine_node *node)
>      return true;
>  }
>
> +static bool
> +flow_output_sb_multicast_group_handler(struct engine_node *node)
> +{
> +    struct ed_type_runtime_data *data =
> +        (struct ed_type_runtime_data *)engine_get_input(
> +                "runtime_data", node)->data;
> +    struct hmap *local_datapaths = &data->local_datapaths;
> +    struct simap *ct_zones = &data->ct_zones;
> +
> +    struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve =
> +        (struct ed_type_mff_ovn_geneve *)engine_get_input(
> +            "mff_ovn_geneve", node)->data;
> +    enum mf_field_id mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve;
> +
> +    struct ovsrec_open_vswitch_table *ovs_table =
> +        (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
> +            engine_get_input("OVS_open_vswitch", node));
> +    struct ovsrec_bridge_table *bridge_table =
> +        (struct ovsrec_bridge_table *)EN_OVSDB_GET(
> +            engine_get_input("OVS_bridge", node));
> +    const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
ovs_table);
> +    const char *chassis_id = get_chassis_id(ovs_table);
> +
> +    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 = NULL;
> +    if (chassis_id) {
> +        chassis = chassis_lookup_by_name(sbrec_chassis_by_name,
chassis_id);
> +    }
> +    ovs_assert(br_int && chassis);
> +
> +    struct ed_type_flow_output *fo =
> +        (struct ed_type_flow_output *)node->data;
> +    struct ovn_desired_flow_table *flow_table = &fo->flow_table;
> +
> +    struct sbrec_multicast_group_table *multicast_group_table =
> +        (struct sbrec_multicast_group_table *)EN_OVSDB_GET(
> +            engine_get_input("SB_multicast_group", node));
> +
> +    physical_handle_mc_group_changes(multicast_group_table,
> +            mff_ovn_geneve, chassis, ct_zones, local_datapaths,
> +            flow_table);
> +
> +    node->changed = true;
> +    return true;
> +
> +}
> +
>  struct ovn_controller_exit_args {
>      bool *exiting;
>      bool *restart;
> @@ -1295,7 +1345,7 @@ main(int argc, char *argv[])
>
>      engine_add_input(&en_flow_output, &en_sb_chassis, NULL);
>      engine_add_input(&en_flow_output, &en_sb_encap, NULL);
> -    engine_add_input(&en_flow_output, &en_sb_multicast_group, NULL);
> +    engine_add_input(&en_flow_output, &en_sb_multicast_group,
flow_output_sb_multicast_group_handler);
>      engine_add_input(&en_flow_output, &en_sb_datapath_binding, NULL);
>      engine_add_input(&en_flow_output, &en_sb_port_binding,
flow_output_sb_port_binding_handler);
>      engine_add_input(&en_flow_output, &en_sb_mac_binding, NULL);
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 1cb9f7b..7c22aec 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -908,6 +908,29 @@ void physical_handle_port_binding_changes(
>  }
>
>  void
> +physical_handle_mc_group_changes(
> +        const struct sbrec_multicast_group_table *multicast_group_table,
> +        enum mf_field_id mff_ovn_geneve,
> +        const struct sbrec_chassis *chassis,
> +        const struct simap *ct_zones,
> +        const struct hmap *local_datapaths,
> +        struct ovn_desired_flow_table *flow_table)
> +{
> +    const struct sbrec_multicast_group *mc;
> +    SBREC_MULTICAST_GROUP_TABLE_FOR_EACH_TRACKED (mc,
multicast_group_table) {
> +        if (sbrec_multicast_group_is_deleted(mc)) {
> +            ofctrl_remove_flows(flow_table, &mc->header_.uuid);
> +        } else {
> +            if (!sbrec_multicast_group_is_new(mc)) {
> +                ofctrl_remove_flows(flow_table, &mc->header_.uuid);
> +            }
> +            consider_mc_group(mff_ovn_geneve, ct_zones, local_datapaths,
> +                              chassis, mc, flow_table);
> +        }
> +    }
> +}
> +
> +void
>  physical_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
>               struct ovsdb_idl_index *sbrec_port_binding_by_name,
>               const struct sbrec_multicast_group_table
*multicast_group_table,
> diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
> index a9c8445..8f18358 100644
> --- a/ovn/controller/physical.h
> +++ b/ovn/controller/physical.h
> @@ -66,4 +66,11 @@ void physical_handle_port_binding_changes(
>          struct sset *active_tunnels,
>          struct ovn_desired_flow_table *);
>
> +void physical_handle_mc_group_changes(
> +        const struct sbrec_multicast_group_table *,
> +        enum mf_field_id mff_ovn_geneve,
> +        const struct sbrec_chassis *,
> +        const struct simap *ct_zones,
> +        const struct hmap *local_datapaths,
> +        struct ovn_desired_flow_table *);
>  #endif /* ovn/physical.h */
> --
> 2.1.0
>
diff mbox series

Patch

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 468b3bf..784ae4e 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2308,6 +2308,9 @@  add_tracked_change_for_references(struct ovsdb_idl_row *row)
             ovsdb_idl_track_is_set(row->table)) {
         ovs_list_push_back(&row->table->track_list,
                            &row->track_node);
+        row->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
+            = row->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
+            = row->table->db->change_seqno + 1;
 
         const struct ovsdb_idl_arc *arc;
         LIST_FOR_EACH (arc, dst_node, &row->dst_arcs) {
@@ -2316,7 +2319,6 @@  add_tracked_change_for_references(struct ovsdb_idl_row *row)
     }
 }
 
-
 /* Returns true if a column with mode OVSDB_IDL_MODE_RW changed, false
  * otherwise.
  *
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 9fd2cba..4399d6e 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -1180,6 +1180,56 @@  flow_output_sb_port_binding_handler(struct engine_node *node)
     return true;
 }
 
+static bool
+flow_output_sb_multicast_group_handler(struct engine_node *node)
+{
+    struct ed_type_runtime_data *data =
+        (struct ed_type_runtime_data *)engine_get_input(
+                "runtime_data", node)->data;
+    struct hmap *local_datapaths = &data->local_datapaths;
+    struct simap *ct_zones = &data->ct_zones;
+
+    struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve =
+        (struct ed_type_mff_ovn_geneve *)engine_get_input(
+            "mff_ovn_geneve", node)->data;
+    enum mf_field_id mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve;
+
+    struct ovsrec_open_vswitch_table *ovs_table =
+        (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
+            engine_get_input("OVS_open_vswitch", node));
+    struct ovsrec_bridge_table *bridge_table =
+        (struct ovsrec_bridge_table *)EN_OVSDB_GET(
+            engine_get_input("OVS_bridge", node));
+    const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
+    const char *chassis_id = get_chassis_id(ovs_table);
+
+    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 = NULL;
+    if (chassis_id) {
+        chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
+    }
+    ovs_assert(br_int && chassis);
+
+    struct ed_type_flow_output *fo =
+        (struct ed_type_flow_output *)node->data;
+    struct ovn_desired_flow_table *flow_table = &fo->flow_table;
+
+    struct sbrec_multicast_group_table *multicast_group_table =
+        (struct sbrec_multicast_group_table *)EN_OVSDB_GET(
+            engine_get_input("SB_multicast_group", node));
+
+    physical_handle_mc_group_changes(multicast_group_table,
+            mff_ovn_geneve, chassis, ct_zones, local_datapaths,
+            flow_table);
+
+    node->changed = true;
+    return true;
+
+}
+
 struct ovn_controller_exit_args {
     bool *exiting;
     bool *restart;
@@ -1295,7 +1345,7 @@  main(int argc, char *argv[])
 
     engine_add_input(&en_flow_output, &en_sb_chassis, NULL);
     engine_add_input(&en_flow_output, &en_sb_encap, NULL);
-    engine_add_input(&en_flow_output, &en_sb_multicast_group, NULL);
+    engine_add_input(&en_flow_output, &en_sb_multicast_group, flow_output_sb_multicast_group_handler);
     engine_add_input(&en_flow_output, &en_sb_datapath_binding, NULL);
     engine_add_input(&en_flow_output, &en_sb_port_binding, flow_output_sb_port_binding_handler);
     engine_add_input(&en_flow_output, &en_sb_mac_binding, NULL);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 1cb9f7b..7c22aec 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -908,6 +908,29 @@  void physical_handle_port_binding_changes(
 }
 
 void
+physical_handle_mc_group_changes(
+        const struct sbrec_multicast_group_table *multicast_group_table,
+        enum mf_field_id mff_ovn_geneve,
+        const struct sbrec_chassis *chassis,
+        const struct simap *ct_zones,
+        const struct hmap *local_datapaths,
+        struct ovn_desired_flow_table *flow_table)
+{
+    const struct sbrec_multicast_group *mc;
+    SBREC_MULTICAST_GROUP_TABLE_FOR_EACH_TRACKED (mc, multicast_group_table) {
+        if (sbrec_multicast_group_is_deleted(mc)) {
+            ofctrl_remove_flows(flow_table, &mc->header_.uuid);
+        } else {
+            if (!sbrec_multicast_group_is_new(mc)) {
+                ofctrl_remove_flows(flow_table, &mc->header_.uuid);
+            }
+            consider_mc_group(mff_ovn_geneve, ct_zones, local_datapaths,
+                              chassis, mc, flow_table);
+        }
+    }
+}
+
+void
 physical_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
              struct ovsdb_idl_index *sbrec_port_binding_by_name,
              const struct sbrec_multicast_group_table *multicast_group_table,
diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
index a9c8445..8f18358 100644
--- a/ovn/controller/physical.h
+++ b/ovn/controller/physical.h
@@ -66,4 +66,11 @@  void physical_handle_port_binding_changes(
         struct sset *active_tunnels,
         struct ovn_desired_flow_table *);
 
+void physical_handle_mc_group_changes(
+        const struct sbrec_multicast_group_table *,
+        enum mf_field_id mff_ovn_geneve,
+        const struct sbrec_chassis *,
+        const struct simap *ct_zones,
+        const struct hmap *local_datapaths,
+        struct ovn_desired_flow_table *);
 #endif /* ovn/physical.h */