Message ID | 1534182499-75914-12-git-send-email-hzhou8@ebay.com |
---|---|
State | Deferred |
Headers | show |
Series | ovn-controller incremental processing | expand |
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 --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 */
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(-)