[ovs-dev] [PATCH v2 1/5] ovn-northd: Reuse the hmaps - datapaths and ports in ovnsb_db_run()
Han Zhou
zhouhan at gmail.com
Wed Feb 27 20:05:46 UTC 2019
On Wed, Feb 27, 2019 at 2:45 AM <nusiddiq at redhat.com> wrote:
>
> From: Numan Siddique <nusiddiq at redhat.com>
>
> We can reuse the datapaths and ports built during ovnnb_db_run()
> in ovnsb_db_run(). This way we avoid creating the logical ports hash nodes
> during the ovnsb_db_run().
>
> An upcoming patch will make further use of these hashmaps during ovnsb_db_run().
>
> This patch refactors the code accordingly.
>
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> ---
> ovn/northd/ovn-northd.c | 109 ++++++++++++++++++----------------------
> 1 file changed, 48 insertions(+), 61 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 3569ea2be..838251dd1 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -668,7 +668,6 @@ join_datapaths(struct northd_context *ctx, struct hmap *datapaths,
> struct ovs_list *sb_only, struct ovs_list *nb_only,
> struct ovs_list *both)
> {
> - hmap_init(datapaths);
> ovs_list_init(sb_only);
> ovs_list_init(nb_only);
> ovs_list_init(both);
> @@ -1595,7 +1594,6 @@ join_logical_ports(struct northd_context *ctx,
> struct hmap *tag_alloc_table, struct ovs_list *sb_only,
> struct ovs_list *nb_only, struct ovs_list *both)
> {
> - hmap_init(ports);
> ovs_list_init(sb_only);
> ovs_list_init(nb_only);
> ovs_list_init(both);
> @@ -7232,28 +7230,43 @@ sync_dns_entries(struct northd_context *ctx, struct hmap *datapaths)
> }
> hmap_destroy(&dns_map);
> }
> +
> +static void
> +destroy_datapaths_and_ports(struct hmap *datapaths, struct hmap *ports)
> +{
> + struct ovn_datapath *dp, *next_dp;
> + HMAP_FOR_EACH_SAFE (dp, next_dp, key_node, datapaths) {
> + ovn_datapath_destroy(datapaths, dp);
> + }
> + hmap_destroy(datapaths);
>
> + struct ovn_port *port, *next_port;
> + HMAP_FOR_EACH_SAFE (port, next_port, key_node, ports) {
> + ovn_port_destroy(ports, port);
> + }
> + hmap_destroy(ports);
> +}
>
> -
> static void
> ovnnb_db_run(struct northd_context *ctx,
> struct ovsdb_idl_index *sbrec_chassis_by_name,
> - struct ovsdb_idl_loop *sb_loop)
> + struct ovsdb_idl_loop *sb_loop,
> + struct hmap *datapaths, struct hmap *ports)
> {
> if (!ctx->ovnsb_txn || !ctx->ovnnb_txn) {
> return;
> }
> - struct hmap datapaths, ports, port_groups;
> - build_datapaths(ctx, &datapaths);
> - build_ports(ctx, sbrec_chassis_by_name, &datapaths, &ports);
> - build_ipam(&datapaths, &ports);
> - build_port_group_lswitches(ctx, &port_groups, &ports);
> - build_lflows(ctx, &datapaths, &ports, &port_groups);
> + struct hmap port_groups;
> + build_datapaths(ctx, datapaths);
> + build_ports(ctx, sbrec_chassis_by_name, datapaths, ports);
> + build_ipam(datapaths, ports);
> + build_port_group_lswitches(ctx, &port_groups, ports);
> + build_lflows(ctx, datapaths, ports, &port_groups);
>
> sync_address_sets(ctx);
> sync_port_groups(ctx);
> sync_meters(ctx);
> - sync_dns_entries(ctx, &datapaths);
> + sync_dns_entries(ctx, datapaths);
>
> struct ovn_port_group *pg, *next_pg;
> HMAP_FOR_EACH_SAFE (pg, next_pg, key_node, &port_groups) {
> @@ -7261,18 +7274,6 @@ ovnnb_db_run(struct northd_context *ctx,
> }
> hmap_destroy(&port_groups);
>
> - struct ovn_datapath *dp, *next_dp;
> - HMAP_FOR_EACH_SAFE (dp, next_dp, key_node, &datapaths) {
> - ovn_datapath_destroy(&datapaths, dp);
> - }
> - hmap_destroy(&datapaths);
> -
> - struct ovn_port *port, *next_port;
> - HMAP_FOR_EACH_SAFE (port, next_port, key_node, &ports) {
> - ovn_port_destroy(&ports, port);
> - }
> - hmap_destroy(&ports);
> -
> /* Sync ipsec configuration.
> * Copy nb_cfg from northbound to southbound database.
> * Also set up to update sb_cfg once our southbound transaction commits. */
> @@ -7309,53 +7310,25 @@ ovnnb_db_run(struct northd_context *ctx,
> * this column is not empty, it means we need to set the corresponding logical
> * port as 'up' in the northbound DB. */
> static void
> -update_logical_port_status(struct northd_context *ctx)
> +update_logical_port_status(struct northd_context *ctx, struct hmap *ports)
> {
> - struct hmap lports_hmap;
> const struct sbrec_port_binding *sb;
> - const struct nbrec_logical_switch_port *nbsp;
> -
> - struct lport_hash_node {
> - struct hmap_node node;
> - const struct nbrec_logical_switch_port *nbsp;
> - } *hash_node;
> -
> - hmap_init(&lports_hmap);
> -
> - NBREC_LOGICAL_SWITCH_PORT_FOR_EACH(nbsp, ctx->ovnnb_idl) {
> - hash_node = xzalloc(sizeof *hash_node);
> - hash_node->nbsp = nbsp;
> - hmap_insert(&lports_hmap, &hash_node->node, hash_string(nbsp->name, 0));
> - }
>
> SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
> - nbsp = NULL;
> - HMAP_FOR_EACH_WITH_HASH(hash_node, node,
> - hash_string(sb->logical_port, 0),
> - &lports_hmap) {
> - if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
> - nbsp = hash_node->nbsp;
> - break;
> - }
> - }
> + struct ovn_port *op = ovn_port_find(ports, sb->logical_port);
>
> - if (!nbsp) {
> + if (!op || !op->nbsp) {
> /* The logical port doesn't exist for this port binding. This can
> * happen under normal circumstances when ovn-northd hasn't gotten
> * around to pruning the Port_Binding yet. */
> continue;
> }
>
> - bool up = (sb->chassis || !strcmp(nbsp->type, "router"));
> - if (!nbsp->up || *nbsp->up != up) {
> - nbrec_logical_switch_port_set_up(nbsp, &up, 1);
> + bool up = (sb->chassis || !strcmp(op->nbsp->type, "router"));
> + if (!op->nbsp->up || *op->nbsp->up != up) {
> + nbrec_logical_switch_port_set_up(op->nbsp, &up, 1);
> }
> }
> -
> - HMAP_FOR_EACH_POP(hash_node, node, &lports_hmap) {
> - free(hash_node);
> - }
> - hmap_destroy(&lports_hmap);
> }
>
> static struct gen_opts_map supported_dhcp_opts[] = {
> @@ -7675,15 +7648,30 @@ update_northbound_cfg(struct northd_context *ctx,
>
> /* Handle a fairly small set of changes in the southbound database. */
> static void
> -ovnsb_db_run(struct northd_context *ctx, struct ovsdb_idl_loop *sb_loop)
> +ovnsb_db_run(struct northd_context *ctx, struct ovsdb_idl_loop *sb_loop,
> + struct hmap *ports)
> {
> if (!ctx->ovnnb_txn || !ovsdb_idl_has_ever_connected(ctx->ovnsb_idl)) {
> return;
> }
>
> - update_logical_port_status(ctx);
> + update_logical_port_status(ctx, ports);
> update_northbound_cfg(ctx, sb_loop);
> }
> +
> +static void
> +ovn_db_run(struct northd_context *ctx,
> + struct ovsdb_idl_index *sbrec_chassis_by_name,
> + struct ovsdb_idl_loop *ovnsb_idl_loop)
> +{
> + struct hmap datapaths, ports;
> + hmap_init(&datapaths);
> + hmap_init(&ports);
> + ovnnb_db_run(ctx, sbrec_chassis_by_name, ovnsb_idl_loop,
> + &datapaths, &ports);
> + ovnsb_db_run(ctx, ovnsb_idl_loop, &ports);
> + destroy_datapaths_and_ports(&datapaths, &ports);
> +}
>
> static void
> parse_options(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> @@ -7941,8 +7929,7 @@ main(int argc, char *argv[])
> }
>
> if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) {
> - ovnnb_db_run(&ctx, sbrec_chassis_by_name, &ovnsb_idl_loop);
> - ovnsb_db_run(&ctx, &ovnsb_idl_loop);
> + ovn_db_run(&ctx, sbrec_chassis_by_name, &ovnsb_idl_loop);
> if (ctx.ovnsb_txn) {
> check_and_add_supported_dhcp_opts_to_sb_db(&ctx);
> check_and_add_supported_dhcpv6_opts_to_sb_db(&ctx);
> --
> 2.20.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Acked-by: Han Zhou <hzhou8 at ebay.com>
More information about the dev
mailing list