| Message ID | 20220824172608.3092562-5-i.maximets@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | northd: Optimize preparation of load balancers. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On 8/24/22 19:26, Ilya Maximets wrote: > If the same load balancer group is attached to multiple routers, > IP sets will be constructed for each of them by sequentially calling > sset_add() for each IP for each load balancer for each router. > Instead of doing that, we can create IP sets for load balancer > groups and clone them. That will avoid extra sset_find__() call > making the construction of IP sets about 2x faster. > > Only first load balancer group is cloned, the rest as well as > standalone load balancers are still added one by one, because > there is no more efficient way to merge sets. > > The order of processing changed to make sure that we're actually > optimizing copy of a large group. > > The code can be optimized further by introduction of a reference > counter and not copying at all if the router has no standalone load > balancers and only one group, but that is not the common case, > unfortunately. Also, construction of "reachable" sets can be > optimized for the "neighbor_responder = all" case. But, for now, > only moved to a new structure for better readability. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- I left a small comment below, with that addressed feel free to add my ack to v2: Acked-by: Dumitru Ceara <dceara@redhat.com> > northd/northd.c | 156 +++++++++++++++++++++++++++++++++--------------- > northd/northd.h | 24 ++++---- > 2 files changed, 121 insertions(+), 59 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index a942346bd..4f8ffc124 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -818,16 +818,13 @@ lr_lb_address_set_ref(const struct ovn_datapath *od, int addr_family) > return lr_lb_address_set_name_(od, "$", addr_family); > } > > +static struct ovn_lb_ip_set *ovn_lb_ip_set_create(void); > +static void ovn_lb_ip_set_destroy(struct ovn_lb_ip_set *); > +static struct ovn_lb_ip_set *ovn_lb_ip_set_clone(struct ovn_lb_ip_set *); > + > static void > init_lb_for_datapath(struct ovn_datapath *od) > { > - sset_init(&od->lb_ips_v4); > - sset_init(&od->lb_ips_v4_routable); > - sset_init(&od->lb_ips_v4_reachable); > - sset_init(&od->lb_ips_v6); > - sset_init(&od->lb_ips_v6_routable); > - sset_init(&od->lb_ips_v6_reachable); > - > if (od->nbs) { > od->has_lb_vip = ls_has_lb_vip(od); > } else { > @@ -838,16 +835,12 @@ init_lb_for_datapath(struct ovn_datapath *od) > static void > destroy_lb_for_datapath(struct ovn_datapath *od) > { > + ovn_lb_ip_set_destroy(od->lb_ips); > + od->lb_ips = NULL; > + > if (!od->nbs && !od->nbr) { > return; > } > - > - sset_destroy(&od->lb_ips_v4); > - sset_destroy(&od->lb_ips_v4_routable); > - sset_destroy(&od->lb_ips_v4_reachable); > - sset_destroy(&od->lb_ips_v6); > - sset_destroy(&od->lb_ips_v6_routable); > - sset_destroy(&od->lb_ips_v6_reachable); > } > > /* A group of logical router datapaths which are connected - either > @@ -2818,20 +2811,20 @@ get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only, > if (include_lb_ips) { > const char *ip_address; > if (routable_only) { > - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4_routable) { > + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v4_routable) { > ds_put_format(&c_addresses, " %s", ip_address); > central_ip_address = true; > } > - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6_routable) { > + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v6_routable) { > ds_put_format(&c_addresses, " %s", ip_address); > central_ip_address = true; > } > } else { > - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4) { > + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v4) { > ds_put_format(&c_addresses, " %s", ip_address); > central_ip_address = true; > } > - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) { > + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v6) { > ds_put_format(&c_addresses, " %s", ip_address); > central_ip_address = true; > } > @@ -3822,29 +3815,76 @@ build_lb_vip_actions(struct ovn_lb_vip *lb_vip, > } > > static void > -build_lrouter_lb_ips(struct ovn_datapath *od, const struct ovn_northd_lb *lb) > +build_lrouter_lb_ips(struct ovn_lb_ip_set *lb_ips, > + const struct ovn_northd_lb *lb) > { > const char *ip_address; > > SSET_FOR_EACH (ip_address, &lb->ips_v4) { > - sset_add(&od->lb_ips_v4, ip_address); > + sset_add(&lb_ips->ips_v4, ip_address); > if (lb->routable) { > - sset_add(&od->lb_ips_v4_routable, ip_address); > + sset_add(&lb_ips->ips_v4_routable, ip_address); > } > } > SSET_FOR_EACH (ip_address, &lb->ips_v6) { > - sset_add(&od->lb_ips_v6, ip_address); > + sset_add(&lb_ips->ips_v6, ip_address); > if (lb->routable) { > - sset_add(&od->lb_ips_v6_routable, ip_address); > + sset_add(&lb_ips->ips_v6_routable, ip_address); > } > } > } > > +static struct ovn_lb_ip_set * > +ovn_lb_ip_set_create(void) > +{ > + struct ovn_lb_ip_set *lb_ip_set = xzalloc(sizeof *lb_ip_set); > + > + sset_init(&lb_ip_set->ips_v4); > + sset_init(&lb_ip_set->ips_v4_routable); > + sset_init(&lb_ip_set->ips_v4_reachable); > + sset_init(&lb_ip_set->ips_v6); > + sset_init(&lb_ip_set->ips_v6_routable); > + sset_init(&lb_ip_set->ips_v6_reachable); > + > + return lb_ip_set; > +} > + > +static void > +ovn_lb_ip_set_destroy(struct ovn_lb_ip_set *lb_ip_set) > +{ > + if (!lb_ip_set) { > + return; > + } > + sset_destroy(&lb_ip_set->ips_v4); > + sset_destroy(&lb_ip_set->ips_v4_routable); > + sset_destroy(&lb_ip_set->ips_v4_reachable); > + sset_destroy(&lb_ip_set->ips_v6); > + sset_destroy(&lb_ip_set->ips_v6_routable); > + sset_destroy(&lb_ip_set->ips_v6_reachable); > + free(lb_ip_set); > +} > + > +static struct ovn_lb_ip_set * > +ovn_lb_ip_set_clone(struct ovn_lb_ip_set *lb_ip_set) > +{ > + struct ovn_lb_ip_set *clone = ovn_lb_ip_set_create(); > + > + sset_clone(&clone->ips_v4, &lb_ip_set->ips_v4); > + sset_clone(&clone->ips_v4_routable, &lb_ip_set->ips_v4_routable); > + sset_clone(&clone->ips_v4_reachable, &lb_ip_set->ips_v4_reachable); > + sset_clone(&clone->ips_v6, &lb_ip_set->ips_v6); > + sset_clone(&clone->ips_v6_routable, &lb_ip_set->ips_v6_routable); > + sset_clone(&clone->ips_v6_reachable, &lb_ip_set->ips_v6_reachable); > + > + return clone; > +} > + > struct ovn_lb_group { > struct hmap_node hmap_node; > struct uuid uuid; > size_t n; > struct ovn_northd_lb **lbs; > + struct ovn_lb_ip_set *lb_ips; > size_t n_od; > struct ovn_datapath **ods; > }; > @@ -3889,11 +3929,13 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, > lb_group->n = nbrec_lb_group->n_load_balancer; > lb_group->lbs = xzalloc(lb_group->n * sizeof *lb_group->lbs); > lb_group->ods = xzalloc(hmap_count(datapaths) * sizeof *lb_group->ods); > + lb_group->lb_ips = ovn_lb_ip_set_create(); > > for (size_t i = 0; i < nbrec_lb_group->n_load_balancer; i++) { > const struct uuid *lb_uuid = > &nbrec_lb_group->load_balancer[i]->header_.uuid; > lb_group->lbs[i] = ovn_northd_lb_find(lbs, lb_uuid); > + build_lrouter_lb_ips(lb_group->lb_ips, lb_group->lbs[i]); > } > > hmap_insert(lb_groups, &lb_group->hmap_node, > @@ -3934,23 +3976,34 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, > continue; > } > > - for (size_t i = 0; i < od->nbr->n_load_balancer; i++) { > - const struct uuid *lb_uuid = > - &od->nbr->load_balancer[i]->header_.uuid; > - lb = ovn_northd_lb_find(lbs, lb_uuid); > - ovn_northd_lb_add_lr(lb, 1, &od); > - build_lrouter_lb_ips(od, lb); > - } > - > + /* Checking load balancer groups first to more efficiently copy > + * IP sets for large groups. */ > for (size_t i = 0; i < od->nbr->n_load_balancer_group; i++) { > nbrec_lb_group = od->nbr->load_balancer_group[i]; > lb_group = ovn_lb_group_find(lb_groups, > &nbrec_lb_group->header_.uuid); > lb_group->ods[lb_group->n_od++] = od; > - for (size_t j = 0; j < lb_group->n; j++) { > - build_lrouter_lb_ips(od, lb_group->lbs[j]); > + > + if (!od->lb_ips) { > + od->lb_ips = ovn_lb_ip_set_clone(lb_group->lb_ips); > + } else { > + for (size_t j = 0; j < lb_group->n; j++) { > + build_lrouter_lb_ips(od->lb_ips, lb_group->lbs[j]); > + } > } > } > + > + if (!od->lb_ips) { > + od->lb_ips = ovn_lb_ip_set_create(); > + } > + > + for (size_t i = 0; i < od->nbr->n_load_balancer; i++) { > + const struct uuid *lb_uuid = > + &od->nbr->load_balancer[i]->header_.uuid; > + lb = ovn_northd_lb_find(lbs, lb_uuid); > + ovn_northd_lb_add_lr(lb, 1, &od); > + build_lrouter_lb_ips(od->lb_ips, lb); > + } > } > > HMAP_FOR_EACH (lb_group, hmap_node, lb_groups) { > @@ -4012,9 +4065,9 @@ build_lrouter_lb_reachable_ips(struct ovn_datapath *od, > if (lb->neighbor_responder_mode == LB_NEIGH_RESPOND_ALL) { > for (size_t i = 0; i < lb->n_vips; i++) { > if (IN6_IS_ADDR_V4MAPPED(&lb->vips[i].vip)) { > - sset_add(&od->lb_ips_v4_reachable, lb->vips[i].vip_str); > + sset_add(&od->lb_ips->ips_v4_reachable, lb->vips[i].vip_str); > } else { > - sset_add(&od->lb_ips_v6_reachable, lb->vips[i].vip_str); > + sset_add(&od->lb_ips->ips_v6_reachable, lb->vips[i].vip_str); > } > } > return; > @@ -4030,7 +4083,8 @@ build_lrouter_lb_reachable_ips(struct ovn_datapath *od, > > LIST_FOR_EACH (op, dp_node, &od->port_list) { > if (lrouter_port_ipv4_reachable(op, vip_ip4)) { > - sset_add(&od->lb_ips_v4_reachable, lb->vips[i].vip_str); > + sset_add(&od->lb_ips->ips_v4_reachable, > + lb->vips[i].vip_str); > break; > } > } > @@ -4039,7 +4093,8 @@ build_lrouter_lb_reachable_ips(struct ovn_datapath *od, > > LIST_FOR_EACH (op, dp_node, &od->port_list) { > if (lrouter_port_ipv6_reachable(op, &lb->vips[i].vip)) { > - sset_add(&od->lb_ips_v6_reachable, lb->vips[i].vip_str); > + sset_add(&od->lb_ips->ips_v6_reachable, > + lb->vips[i].vip_str); > break; > } > } > @@ -7476,7 +7531,7 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, > */ > > const char *ip_addr; > - SSET_FOR_EACH (ip_addr, &op->od->lb_ips_v4) { > + SSET_FOR_EACH (ip_addr, &op->od->lb_ips->ips_v4) { > ovs_be32 ipv4_addr; > > /* Check if the ovn port has a network configured on which we could > @@ -7489,7 +7544,7 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, > stage_hint); > } > } > - SSET_FOR_EACH (ip_addr, &op->od->lb_ips_v6) { > + SSET_FOR_EACH (ip_addr, &op->od->lb_ips->ips_v6) { > struct in6_addr ipv6_addr; > > /* Check if the ovn port has a network configured on which we could > @@ -7519,13 +7574,13 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, > * expect ARP requests/NS for the DNAT external_ip. > */ > if (nat_entry_is_v6(nat_entry)) { > - if (!sset_contains(&op->od->lb_ips_v6, nat->external_ip)) { > + if (!sset_contains(&op->od->lb_ips->ips_v6, nat->external_ip)) { > build_lswitch_rport_arp_req_flow( > nat->external_ip, AF_INET6, sw_op, sw_od, 80, lflows, > stage_hint); > } > } else { > - if (!sset_contains(&op->od->lb_ips_v4, nat->external_ip)) { > + if (!sset_contains(&op->od->lb_ips->ips_v4, nat->external_ip)) { > build_lswitch_rport_arp_req_flow( > nat->external_ip, AF_INET, sw_op, sw_od, 80, lflows, > stage_hint); > @@ -12807,7 +12862,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, > &op->nbrp->header_, lflows); > } > > - if (sset_count(&op->od->lb_ips_v4_reachable)) { > + if (sset_count(&op->od->lb_ips->ips_v4_reachable)) { > ds_clear(match); > if (is_l3dgw_port(op)) { > ds_put_format(match, "is_chassis_resident(%s)", > @@ -12822,7 +12877,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, > free(lb_ips_v4_as); > } > > - if (sset_count(&op->od->lb_ips_v6_reachable)) { > + if (sset_count(&op->od->lb_ips->ips_v6_reachable)) { > ds_clear(match); > > if (is_l3dgw_port(op)) { > @@ -14635,23 +14690,25 @@ sync_address_sets(struct northd_input *input_data, > continue; > } > > - if (sset_count(&od->lb_ips_v4_reachable)) { > + if (sset_count(&od->lb_ips->ips_v4_reachable)) { > char *ipv4_addrs_name = lr_lb_address_set_name(od, AF_INET); > - const char **ipv4_addrs = sset_array(&od->lb_ips_v4_reachable); > + const char **ipv4_addrs = > + sset_array(&od->lb_ips->ips_v4_reachable); > > sync_address_set(ovnsb_txn, ipv4_addrs_name, ipv4_addrs, > - sset_count(&od->lb_ips_v4_reachable), > + sset_count(&od->lb_ips->ips_v4_reachable), > &sb_address_sets); > free(ipv4_addrs_name); > free(ipv4_addrs); > } > > - if (sset_count(&od->lb_ips_v6_reachable)) { > + if (sset_count(&od->lb_ips->ips_v6_reachable)) { > char *ipv6_addrs_name = lr_lb_address_set_name(od, AF_INET6); > - const char **ipv6_addrs = sset_array(&od->lb_ips_v6_reachable); > + const char **ipv6_addrs = > + sset_array(&od->lb_ips->ips_v6_reachable); > > sync_address_set(ovnsb_txn, ipv6_addrs_name, ipv6_addrs, > - sset_count(&od->lb_ips_v6_reachable), > + sset_count(&od->lb_ips->ips_v6_reachable), > &sb_address_sets); > free(ipv6_addrs_name); > free(ipv6_addrs); > @@ -15413,6 +15470,7 @@ northd_destroy(struct northd_data *data) > > struct ovn_lb_group *lb_group; > HMAP_FOR_EACH_POP (lb_group, hmap_node, &data->lb_groups) { > + ovn_lb_ip_set_destroy(lb_group->lb_ips); > free(lb_group->lbs); > free(lb_group->ods); > free(lb_group); > diff --git a/northd/northd.h b/northd/northd.h > index 8d299864f..16ca0e051 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -174,6 +174,17 @@ struct mcast_port_info { > * (e.g., IGMP join/leave). */ > }; > > +/* The "routable" ssets are subsets of the load balancer IPs for which IP > + * routes and ARP resolution flows are automatically added. */ > +struct ovn_lb_ip_set { > + struct sset ips_v4; > + struct sset ips_v4_routable; > + struct sset ips_v4_reachable; > + struct sset ips_v6; > + struct sset ips_v6_routable; > + struct sset ips_v6_reachable; > +}; > + Nit: I'd move this (and related functions) to lib/lb.[hc]. > /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or > * sb->external_ids:logical-switch. */ > struct ovn_datapath { > @@ -236,16 +247,9 @@ struct ovn_datapath { > struct lport_addresses dnat_force_snat_addrs; > struct lport_addresses lb_force_snat_addrs; > bool lb_force_snat_router_ip; > - /* The "routable" ssets are subsets of the load balancer > - * IPs for which IP routes and ARP resolution flows are automatically > - * added > - */ > - struct sset lb_ips_v4; > - struct sset lb_ips_v4_routable; > - struct sset lb_ips_v4_reachable; > - struct sset lb_ips_v6; > - struct sset lb_ips_v6_routable; > - struct sset lb_ips_v6_reachable; > + > + /* Load Balancer vIPs relevant for this datapath. */ > + struct ovn_lb_ip_set *lb_ips; > > struct ovn_port **localnet_ports; > size_t n_localnet_ports;
diff --git a/northd/northd.c b/northd/northd.c index a942346bd..4f8ffc124 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -818,16 +818,13 @@ lr_lb_address_set_ref(const struct ovn_datapath *od, int addr_family) return lr_lb_address_set_name_(od, "$", addr_family); } +static struct ovn_lb_ip_set *ovn_lb_ip_set_create(void); +static void ovn_lb_ip_set_destroy(struct ovn_lb_ip_set *); +static struct ovn_lb_ip_set *ovn_lb_ip_set_clone(struct ovn_lb_ip_set *); + static void init_lb_for_datapath(struct ovn_datapath *od) { - sset_init(&od->lb_ips_v4); - sset_init(&od->lb_ips_v4_routable); - sset_init(&od->lb_ips_v4_reachable); - sset_init(&od->lb_ips_v6); - sset_init(&od->lb_ips_v6_routable); - sset_init(&od->lb_ips_v6_reachable); - if (od->nbs) { od->has_lb_vip = ls_has_lb_vip(od); } else { @@ -838,16 +835,12 @@ init_lb_for_datapath(struct ovn_datapath *od) static void destroy_lb_for_datapath(struct ovn_datapath *od) { + ovn_lb_ip_set_destroy(od->lb_ips); + od->lb_ips = NULL; + if (!od->nbs && !od->nbr) { return; } - - sset_destroy(&od->lb_ips_v4); - sset_destroy(&od->lb_ips_v4_routable); - sset_destroy(&od->lb_ips_v4_reachable); - sset_destroy(&od->lb_ips_v6); - sset_destroy(&od->lb_ips_v6_routable); - sset_destroy(&od->lb_ips_v6_reachable); } /* A group of logical router datapaths which are connected - either @@ -2818,20 +2811,20 @@ get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only, if (include_lb_ips) { const char *ip_address; if (routable_only) { - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4_routable) { + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v4_routable) { ds_put_format(&c_addresses, " %s", ip_address); central_ip_address = true; } - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6_routable) { + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v6_routable) { ds_put_format(&c_addresses, " %s", ip_address); central_ip_address = true; } } else { - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4) { + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v4) { ds_put_format(&c_addresses, " %s", ip_address); central_ip_address = true; } - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) { + SSET_FOR_EACH (ip_address, &op->od->lb_ips->ips_v6) { ds_put_format(&c_addresses, " %s", ip_address); central_ip_address = true; } @@ -3822,29 +3815,76 @@ build_lb_vip_actions(struct ovn_lb_vip *lb_vip, } static void -build_lrouter_lb_ips(struct ovn_datapath *od, const struct ovn_northd_lb *lb) +build_lrouter_lb_ips(struct ovn_lb_ip_set *lb_ips, + const struct ovn_northd_lb *lb) { const char *ip_address; SSET_FOR_EACH (ip_address, &lb->ips_v4) { - sset_add(&od->lb_ips_v4, ip_address); + sset_add(&lb_ips->ips_v4, ip_address); if (lb->routable) { - sset_add(&od->lb_ips_v4_routable, ip_address); + sset_add(&lb_ips->ips_v4_routable, ip_address); } } SSET_FOR_EACH (ip_address, &lb->ips_v6) { - sset_add(&od->lb_ips_v6, ip_address); + sset_add(&lb_ips->ips_v6, ip_address); if (lb->routable) { - sset_add(&od->lb_ips_v6_routable, ip_address); + sset_add(&lb_ips->ips_v6_routable, ip_address); } } } +static struct ovn_lb_ip_set * +ovn_lb_ip_set_create(void) +{ + struct ovn_lb_ip_set *lb_ip_set = xzalloc(sizeof *lb_ip_set); + + sset_init(&lb_ip_set->ips_v4); + sset_init(&lb_ip_set->ips_v4_routable); + sset_init(&lb_ip_set->ips_v4_reachable); + sset_init(&lb_ip_set->ips_v6); + sset_init(&lb_ip_set->ips_v6_routable); + sset_init(&lb_ip_set->ips_v6_reachable); + + return lb_ip_set; +} + +static void +ovn_lb_ip_set_destroy(struct ovn_lb_ip_set *lb_ip_set) +{ + if (!lb_ip_set) { + return; + } + sset_destroy(&lb_ip_set->ips_v4); + sset_destroy(&lb_ip_set->ips_v4_routable); + sset_destroy(&lb_ip_set->ips_v4_reachable); + sset_destroy(&lb_ip_set->ips_v6); + sset_destroy(&lb_ip_set->ips_v6_routable); + sset_destroy(&lb_ip_set->ips_v6_reachable); + free(lb_ip_set); +} + +static struct ovn_lb_ip_set * +ovn_lb_ip_set_clone(struct ovn_lb_ip_set *lb_ip_set) +{ + struct ovn_lb_ip_set *clone = ovn_lb_ip_set_create(); + + sset_clone(&clone->ips_v4, &lb_ip_set->ips_v4); + sset_clone(&clone->ips_v4_routable, &lb_ip_set->ips_v4_routable); + sset_clone(&clone->ips_v4_reachable, &lb_ip_set->ips_v4_reachable); + sset_clone(&clone->ips_v6, &lb_ip_set->ips_v6); + sset_clone(&clone->ips_v6_routable, &lb_ip_set->ips_v6_routable); + sset_clone(&clone->ips_v6_reachable, &lb_ip_set->ips_v6_reachable); + + return clone; +} + struct ovn_lb_group { struct hmap_node hmap_node; struct uuid uuid; size_t n; struct ovn_northd_lb **lbs; + struct ovn_lb_ip_set *lb_ips; size_t n_od; struct ovn_datapath **ods; }; @@ -3889,11 +3929,13 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, lb_group->n = nbrec_lb_group->n_load_balancer; lb_group->lbs = xzalloc(lb_group->n * sizeof *lb_group->lbs); lb_group->ods = xzalloc(hmap_count(datapaths) * sizeof *lb_group->ods); + lb_group->lb_ips = ovn_lb_ip_set_create(); for (size_t i = 0; i < nbrec_lb_group->n_load_balancer; i++) { const struct uuid *lb_uuid = &nbrec_lb_group->load_balancer[i]->header_.uuid; lb_group->lbs[i] = ovn_northd_lb_find(lbs, lb_uuid); + build_lrouter_lb_ips(lb_group->lb_ips, lb_group->lbs[i]); } hmap_insert(lb_groups, &lb_group->hmap_node, @@ -3934,23 +3976,34 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, continue; } - for (size_t i = 0; i < od->nbr->n_load_balancer; i++) { - const struct uuid *lb_uuid = - &od->nbr->load_balancer[i]->header_.uuid; - lb = ovn_northd_lb_find(lbs, lb_uuid); - ovn_northd_lb_add_lr(lb, 1, &od); - build_lrouter_lb_ips(od, lb); - } - + /* Checking load balancer groups first to more efficiently copy + * IP sets for large groups. */ for (size_t i = 0; i < od->nbr->n_load_balancer_group; i++) { nbrec_lb_group = od->nbr->load_balancer_group[i]; lb_group = ovn_lb_group_find(lb_groups, &nbrec_lb_group->header_.uuid); lb_group->ods[lb_group->n_od++] = od; - for (size_t j = 0; j < lb_group->n; j++) { - build_lrouter_lb_ips(od, lb_group->lbs[j]); + + if (!od->lb_ips) { + od->lb_ips = ovn_lb_ip_set_clone(lb_group->lb_ips); + } else { + for (size_t j = 0; j < lb_group->n; j++) { + build_lrouter_lb_ips(od->lb_ips, lb_group->lbs[j]); + } } } + + if (!od->lb_ips) { + od->lb_ips = ovn_lb_ip_set_create(); + } + + for (size_t i = 0; i < od->nbr->n_load_balancer; i++) { + const struct uuid *lb_uuid = + &od->nbr->load_balancer[i]->header_.uuid; + lb = ovn_northd_lb_find(lbs, lb_uuid); + ovn_northd_lb_add_lr(lb, 1, &od); + build_lrouter_lb_ips(od->lb_ips, lb); + } } HMAP_FOR_EACH (lb_group, hmap_node, lb_groups) { @@ -4012,9 +4065,9 @@ build_lrouter_lb_reachable_ips(struct ovn_datapath *od, if (lb->neighbor_responder_mode == LB_NEIGH_RESPOND_ALL) { for (size_t i = 0; i < lb->n_vips; i++) { if (IN6_IS_ADDR_V4MAPPED(&lb->vips[i].vip)) { - sset_add(&od->lb_ips_v4_reachable, lb->vips[i].vip_str); + sset_add(&od->lb_ips->ips_v4_reachable, lb->vips[i].vip_str); } else { - sset_add(&od->lb_ips_v6_reachable, lb->vips[i].vip_str); + sset_add(&od->lb_ips->ips_v6_reachable, lb->vips[i].vip_str); } } return; @@ -4030,7 +4083,8 @@ build_lrouter_lb_reachable_ips(struct ovn_datapath *od, LIST_FOR_EACH (op, dp_node, &od->port_list) { if (lrouter_port_ipv4_reachable(op, vip_ip4)) { - sset_add(&od->lb_ips_v4_reachable, lb->vips[i].vip_str); + sset_add(&od->lb_ips->ips_v4_reachable, + lb->vips[i].vip_str); break; } } @@ -4039,7 +4093,8 @@ build_lrouter_lb_reachable_ips(struct ovn_datapath *od, LIST_FOR_EACH (op, dp_node, &od->port_list) { if (lrouter_port_ipv6_reachable(op, &lb->vips[i].vip)) { - sset_add(&od->lb_ips_v6_reachable, lb->vips[i].vip_str); + sset_add(&od->lb_ips->ips_v6_reachable, + lb->vips[i].vip_str); break; } } @@ -7476,7 +7531,7 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, */ const char *ip_addr; - SSET_FOR_EACH (ip_addr, &op->od->lb_ips_v4) { + SSET_FOR_EACH (ip_addr, &op->od->lb_ips->ips_v4) { ovs_be32 ipv4_addr; /* Check if the ovn port has a network configured on which we could @@ -7489,7 +7544,7 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, stage_hint); } } - SSET_FOR_EACH (ip_addr, &op->od->lb_ips_v6) { + SSET_FOR_EACH (ip_addr, &op->od->lb_ips->ips_v6) { struct in6_addr ipv6_addr; /* Check if the ovn port has a network configured on which we could @@ -7519,13 +7574,13 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, * expect ARP requests/NS for the DNAT external_ip. */ if (nat_entry_is_v6(nat_entry)) { - if (!sset_contains(&op->od->lb_ips_v6, nat->external_ip)) { + if (!sset_contains(&op->od->lb_ips->ips_v6, nat->external_ip)) { build_lswitch_rport_arp_req_flow( nat->external_ip, AF_INET6, sw_op, sw_od, 80, lflows, stage_hint); } } else { - if (!sset_contains(&op->od->lb_ips_v4, nat->external_ip)) { + if (!sset_contains(&op->od->lb_ips->ips_v4, nat->external_ip)) { build_lswitch_rport_arp_req_flow( nat->external_ip, AF_INET, sw_op, sw_od, 80, lflows, stage_hint); @@ -12807,7 +12862,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, &op->nbrp->header_, lflows); } - if (sset_count(&op->od->lb_ips_v4_reachable)) { + if (sset_count(&op->od->lb_ips->ips_v4_reachable)) { ds_clear(match); if (is_l3dgw_port(op)) { ds_put_format(match, "is_chassis_resident(%s)", @@ -12822,7 +12877,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, free(lb_ips_v4_as); } - if (sset_count(&op->od->lb_ips_v6_reachable)) { + if (sset_count(&op->od->lb_ips->ips_v6_reachable)) { ds_clear(match); if (is_l3dgw_port(op)) { @@ -14635,23 +14690,25 @@ sync_address_sets(struct northd_input *input_data, continue; } - if (sset_count(&od->lb_ips_v4_reachable)) { + if (sset_count(&od->lb_ips->ips_v4_reachable)) { char *ipv4_addrs_name = lr_lb_address_set_name(od, AF_INET); - const char **ipv4_addrs = sset_array(&od->lb_ips_v4_reachable); + const char **ipv4_addrs = + sset_array(&od->lb_ips->ips_v4_reachable); sync_address_set(ovnsb_txn, ipv4_addrs_name, ipv4_addrs, - sset_count(&od->lb_ips_v4_reachable), + sset_count(&od->lb_ips->ips_v4_reachable), &sb_address_sets); free(ipv4_addrs_name); free(ipv4_addrs); } - if (sset_count(&od->lb_ips_v6_reachable)) { + if (sset_count(&od->lb_ips->ips_v6_reachable)) { char *ipv6_addrs_name = lr_lb_address_set_name(od, AF_INET6); - const char **ipv6_addrs = sset_array(&od->lb_ips_v6_reachable); + const char **ipv6_addrs = + sset_array(&od->lb_ips->ips_v6_reachable); sync_address_set(ovnsb_txn, ipv6_addrs_name, ipv6_addrs, - sset_count(&od->lb_ips_v6_reachable), + sset_count(&od->lb_ips->ips_v6_reachable), &sb_address_sets); free(ipv6_addrs_name); free(ipv6_addrs); @@ -15413,6 +15470,7 @@ northd_destroy(struct northd_data *data) struct ovn_lb_group *lb_group; HMAP_FOR_EACH_POP (lb_group, hmap_node, &data->lb_groups) { + ovn_lb_ip_set_destroy(lb_group->lb_ips); free(lb_group->lbs); free(lb_group->ods); free(lb_group); diff --git a/northd/northd.h b/northd/northd.h index 8d299864f..16ca0e051 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -174,6 +174,17 @@ struct mcast_port_info { * (e.g., IGMP join/leave). */ }; +/* The "routable" ssets are subsets of the load balancer IPs for which IP + * routes and ARP resolution flows are automatically added. */ +struct ovn_lb_ip_set { + struct sset ips_v4; + struct sset ips_v4_routable; + struct sset ips_v4_reachable; + struct sset ips_v6; + struct sset ips_v6_routable; + struct sset ips_v6_reachable; +}; + /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or * sb->external_ids:logical-switch. */ struct ovn_datapath { @@ -236,16 +247,9 @@ struct ovn_datapath { struct lport_addresses dnat_force_snat_addrs; struct lport_addresses lb_force_snat_addrs; bool lb_force_snat_router_ip; - /* The "routable" ssets are subsets of the load balancer - * IPs for which IP routes and ARP resolution flows are automatically - * added - */ - struct sset lb_ips_v4; - struct sset lb_ips_v4_routable; - struct sset lb_ips_v4_reachable; - struct sset lb_ips_v6; - struct sset lb_ips_v6_routable; - struct sset lb_ips_v6_reachable; + + /* Load Balancer vIPs relevant for this datapath. */ + struct ovn_lb_ip_set *lb_ips; struct ovn_port **localnet_ports; size_t n_localnet_ports;
If the same load balancer group is attached to multiple routers, IP sets will be constructed for each of them by sequentially calling sset_add() for each IP for each load balancer for each router. Instead of doing that, we can create IP sets for load balancer groups and clone them. That will avoid extra sset_find__() call making the construction of IP sets about 2x faster. Only first load balancer group is cloned, the rest as well as standalone load balancers are still added one by one, because there is no more efficient way to merge sets. The order of processing changed to make sure that we're actually optimizing copy of a large group. The code can be optimized further by introduction of a reference counter and not copying at all if the router has no standalone load balancers and only one group, but that is not the common case, unfortunately. Also, construction of "reachable" sets can be optimized for the "neighbor_responder = all" case. But, for now, only moved to a new structure for better readability. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- northd/northd.c | 156 +++++++++++++++++++++++++++++++++--------------- northd/northd.h | 24 ++++---- 2 files changed, 121 insertions(+), 59 deletions(-)