There's no need to parse the IP sets every time we iterate through them. It's enough to parse them once for every main loop iteration.
Reported-at: https://bugzilla.redhat.com/1962338 Signed-off-by: Dumitru Ceara <[email protected]> --- lib/lb.c | 9 +++ lib/lb.h | 4 + northd/ovn-northd.c | 175 ++++++++++++++++++++++++--------------------------- 3 files changed, 95 insertions(+), 93 deletions(-) diff --git a/lib/lb.c b/lib/lb.c index 18dc226e9..4cb46b346 100644 --- a/lib/lb.c +++ b/lib/lb.c @@ -170,6 +170,8 @@ ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb) lb->n_vips = smap_count(&nbrec_lb->vips); lb->vips = xcalloc(lb->n_vips, sizeof *lb->vips); lb->vips_nb = xcalloc(lb->n_vips, sizeof *lb->vips_nb); + sset_init(&lb->ips_v4); + sset_init(&lb->ips_v6); struct smap_node *node; size_t n_vips = 0; @@ -184,6 +186,11 @@ ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb) } ovn_northd_lb_vip_init(lb_vip_nb, lb_vip, nbrec_lb, node->key, node->value); + if (IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)) { + sset_add(&lb->ips_v4, lb_vip->vip_str); + } else { + sset_add(&lb->ips_v6, lb_vip->vip_str); + } n_vips++; } @@ -247,6 +254,8 @@ ovn_northd_lb_destroy(struct ovn_northd_lb *lb) } free(lb->vips); free(lb->vips_nb); + sset_destroy(&lb->ips_v4); + sset_destroy(&lb->ips_v6); free(lb->selection_fields); free(lb->dps); free(lb); diff --git a/lib/lb.h b/lib/lb.h index 0486b3d89..58e6bb031 100644 --- a/lib/lb.h +++ b/lib/lb.h @@ -20,6 +20,7 @@ #include <sys/types.h> #include <netinet/in.h> #include "openvswitch/hmap.h" +#include "sset.h" #include "ovn-util.h" struct nbrec_load_balancer; @@ -38,6 +39,9 @@ struct ovn_northd_lb { struct ovn_northd_lb_vip *vips_nb; size_t n_vips; + struct sset ips_v4; + struct sset ips_v6; + size_t n_dps; size_t n_allocated_dps; const struct sbrec_datapath_binding **dps; diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 224ea9543..f59dba469 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -661,6 +661,8 @@ struct ovn_datapath { struct lport_addresses dnat_force_snat_addrs; struct lport_addresses lb_force_snat_addrs; bool lb_force_snat_router_ip; + struct sset lb_ips_v4; + struct sset lb_ips_v6; struct ovn_port **localnet_ports; size_t n_localnet_ports; @@ -827,6 +829,24 @@ destroy_nat_entries(struct ovn_datapath *od) } } +static void +init_lb_ips(struct ovn_datapath *od) +{ + sset_init(&od->lb_ips_v4); + sset_init(&od->lb_ips_v6); +} + +static void +destroy_lb_ips(struct ovn_datapath *od) +{ + if (!od->nbs && !od->nbr) { + return; + } + + sset_destroy(&od->lb_ips_v4); + sset_destroy(&od->lb_ips_v6); +} + /* A group of logical router datapaths which are connected - either * directly or indirectly. * Each logical router can belong to only one group. */ @@ -871,6 +891,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) destroy_ipam_info(&od->ipam_info); free(od->router_ports); destroy_nat_entries(od); + destroy_lb_ips(od); free(od->nat_entries); free(od->localnet_ports); ovn_ls_port_group_destroy(&od->nb_pgs); @@ -1193,6 +1214,7 @@ join_datapaths(struct northd_context *ctx, struct hmap *datapaths, init_ipam_info_for_datapath(od); init_mcast_info_for_datapath(od); + init_lb_ips(od); } const struct nbrec_logical_router *nbr; @@ -1224,6 +1246,7 @@ join_datapaths(struct northd_context *ctx, struct hmap *datapaths, } init_mcast_info_for_datapath(od); init_nat_entries(od); + init_lb_ips(od); ovs_list_push_back(lr_list, &od->lr_list); } } @@ -2437,46 +2460,6 @@ join_logical_ports(struct northd_context *ctx, } } -static void -get_router_load_balancer_ips(const struct ovn_datapath *od, - struct sset *all_ips_v4, struct sset *all_ips_v6) -{ - if (!od->nbr) { - return; - } - - for (int i = 0; i < od->nbr->n_load_balancer; i++) { - struct nbrec_load_balancer *lb = od->nbr->load_balancer[i]; - struct smap *vips = &lb->vips; - struct smap_node *node; - - SMAP_FOR_EACH (node, vips) { - /* node->key contains IP:port or just IP. */ - char *ip_address; - uint16_t port; - int addr_family; - - if (!ip_address_and_port_from_lb_key(node->key, &ip_address, &port, - &addr_family)) { - continue; - } - - struct sset *all_ips; - if (addr_family == AF_INET) { - all_ips = all_ips_v4; - } else { - all_ips = all_ips_v6; - } - - if (!sset_contains(all_ips, ip_address)) { - sset_add(all_ips, ip_address); - } - - free(ip_address); - } - } -} - /* Returns an array of strings, each consisting of a MAC address followed * by one or more IP addresses, and if the port is a distributed gateway * port, followed by 'is_chassis_resident("LPORT_NAME")', where the @@ -2561,22 +2544,15 @@ get_nat_addresses(const struct ovn_port *op, size_t *n) } } - /* Two sets to hold all load-balancer vips. */ - struct sset all_ips_v4 = SSET_INITIALIZER(&all_ips_v4); - struct sset all_ips_v6 = SSET_INITIALIZER(&all_ips_v6); - get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6); - const char *ip_address; - SSET_FOR_EACH (ip_address, &all_ips_v4) { + SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4) { ds_put_format(&c_addresses, " %s", ip_address); central_ip_address = true; } - SSET_FOR_EACH (ip_address, &all_ips_v6) { + SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) { ds_put_format(&c_addresses, " %s", ip_address); central_ip_address = true; } - sset_destroy(&all_ips_v4); - sset_destroy(&all_ips_v6); if (central_ip_address) { /* Gratuitous ARP for centralized NAT rules on distributed gateway @@ -3546,6 +3522,31 @@ build_ovn_lb_svcs(struct northd_context *ctx, struct hmap *ports, hmap_destroy(&monitor_map); } +static void +build_lrouter_lbs(struct hmap *datapaths, struct hmap *lbs) +{ + struct ovn_datapath *od; + + HMAP_FOR_EACH (od, key_node, datapaths) { + if (!od->nbr) { + continue; + } + + for (size_t i = 0; i < od->nbr->n_load_balancer; i++) { + struct ovn_northd_lb *lb = + ovn_northd_lb_find(lbs, + &od->nbr->load_balancer[i]->header_.uuid); + const char *ip_address; + SSET_FOR_EACH (ip_address, &lb->ips_v4) { + sset_add(&od->lb_ips_v4, ip_address); + } + SSET_FOR_EACH (ip_address, &lb->ips_v6) { + sset_add(&od->lb_ips_v6, ip_address); + } + } + } +} + static bool ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key) { @@ -6486,7 +6487,7 @@ build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op, * switching domain as regular broadcast. */ static void -build_lswitch_rport_arp_req_flow_for_ip(struct sset *ips, +build_lswitch_rport_arp_req_flow_for_ip(struct ds *ip_match, int addr_family, struct ovn_port *patch_op, struct ovn_datapath *od, @@ -6510,13 +6511,7 @@ build_lswitch_rport_arp_req_flow_for_ip(struct sset *ips, ds_put_cstr(&match, "nd_ns && nd.target == { "); } - const char *ip_address; - SSET_FOR_EACH (ip_address, ips) { - ds_put_format(&match, "%s, ", ip_address); - } - - ds_chomp(&match, ' '); - ds_chomp(&match, ','); + ds_put_cstr(&match, ds_cstr_ro(ip_match)); ds_put_cstr(&match, "}"); /* Send a the packet to the router pipeline. If the switch has non-router @@ -6566,14 +6561,12 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, * router port. * Priority: 80. */ - struct sset all_ips_v4 = SSET_INITIALIZER(&all_ips_v4); - struct sset all_ips_v6 = SSET_INITIALIZER(&all_ips_v6); - - get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6); + struct ds ips_v4_match = DS_EMPTY_INITIALIZER; + struct ds ips_v6_match = DS_EMPTY_INITIALIZER; const char *ip_addr; const char *ip_addr_next; - SSET_FOR_EACH_SAFE (ip_addr, ip_addr_next, &all_ips_v4) { + SSET_FOR_EACH_SAFE (ip_addr, ip_addr_next, &op->od->lb_ips_v4) { ovs_be32 ipv4_addr; /* Check if the ovn port has a network configured on which we could @@ -6581,12 +6574,10 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, */ if (ip_parse(ip_addr, &ipv4_addr) && lrouter_port_ipv4_reachable(op, ipv4_addr)) { - continue; + ds_put_format(&ips_v4_match, "%s, ", ip_addr); } - - sset_delete(&all_ips_v4, SSET_NODE_FROM_NAME(ip_addr)); } - SSET_FOR_EACH_SAFE (ip_addr, ip_addr_next, &all_ips_v6) { + SSET_FOR_EACH_SAFE (ip_addr, ip_addr_next, &op->od->lb_ips_v6) { struct in6_addr ipv6_addr; /* Check if the ovn port has a network configured on which we could @@ -6594,10 +6585,8 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, */ if (ipv6_parse(ip_addr, &ipv6_addr) && lrouter_port_ipv6_reachable(op, &ipv6_addr)) { - continue; + ds_put_format(&ips_v6_match, "%s, ", ip_addr); } - - sset_delete(&all_ips_v6, SSET_NODE_FROM_NAME(ip_addr)); } for (size_t i = 0; i < op->od->nbr->n_nat; i++) { @@ -6618,39 +6607,44 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, if (nat_entry_is_v6(nat_entry)) { struct in6_addr *addr = &nat_entry->ext_addrs.ipv6_addrs[0].addr; - if (lrouter_port_ipv6_reachable(op, addr)) { - sset_add(&all_ips_v6, nat->external_ip); + if (lrouter_port_ipv6_reachable(op, addr) && + !sset_contains(&op->od->lb_ips_v6, nat->external_ip)) { + ds_put_format(&ips_v6_match, "%s, ", nat->external_ip); } } else { ovs_be32 addr = nat_entry->ext_addrs.ipv4_addrs[0].addr; - if (lrouter_port_ipv4_reachable(op, addr)) { - sset_add(&all_ips_v4, nat->external_ip); + if (lrouter_port_ipv4_reachable(op, addr) && + !sset_contains(&op->od->lb_ips_v4, nat->external_ip)) { + ds_put_format(&ips_v4_match, "%s, ", nat->external_ip); } } } for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { - sset_add(&all_ips_v4, op->lrp_networks.ipv4_addrs[i].addr_s); + ds_put_format(&ips_v4_match, "%s, ", + op->lrp_networks.ipv4_addrs[i].addr_s); } for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { - sset_add(&all_ips_v6, op->lrp_networks.ipv6_addrs[i].addr_s); + ds_put_format(&ips_v6_match, "%s, ", + op->lrp_networks.ipv6_addrs[i].addr_s); } - if (!sset_is_empty(&all_ips_v4)) { - build_lswitch_rport_arp_req_flow_for_ip(&all_ips_v4, AF_INET, sw_op, + if (ds_last(&ips_v4_match) != EOF) { + ds_chomp(&ips_v4_match, ' '); + ds_chomp(&ips_v4_match, ','); + build_lswitch_rport_arp_req_flow_for_ip(&ips_v4_match, AF_INET, sw_op, sw_od, 80, lflows, stage_hint); } - if (!sset_is_empty(&all_ips_v6)) { - build_lswitch_rport_arp_req_flow_for_ip(&all_ips_v6, AF_INET6, sw_op, + if (ds_last(&ips_v6_match) != EOF) { + ds_chomp(&ips_v6_match, ' '); + ds_chomp(&ips_v6_match, ','); + build_lswitch_rport_arp_req_flow_for_ip(&ips_v6_match, AF_INET6, sw_op, sw_od, 80, lflows, stage_hint); } - sset_destroy(&all_ips_v4); - sset_destroy(&all_ips_v6); - /* Self originated ARP requests/ND need to be flooded as usual. * * However, if the switch doesn't have any non-router ports we shouldn't @@ -6661,6 +6655,8 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, if (sw_od->n_router_ports != sw_od->nbs->n_ports) { build_lswitch_rport_arp_req_self_orig_flow(op, 75, sw_od, lflows); } + ds_destroy(&ips_v4_match); + ds_destroy(&ips_v6_match); } static void @@ -11099,13 +11095,8 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, &op->nbrp->header_, lflows); } - /* A set to hold all load-balancer vips that need ARP responses. */ - struct sset all_ips_v4 = SSET_INITIALIZER(&all_ips_v4); - struct sset all_ips_v6 = SSET_INITIALIZER(&all_ips_v6); - get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6); - const char *ip_address; - if (sset_count(&all_ips_v4)) { + if (sset_count(&op->od->lb_ips_v4)) { ds_clear(match); if (op == op->od->l3dgw_port) { ds_put_format(match, "is_chassis_resident(%s)", @@ -11117,7 +11108,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, /* For IPv4 we can just create one rule with all required IPs. */ ds_put_cstr(&load_balancer_ips_v4, "{ "); ds_put_and_free_cstr(&load_balancer_ips_v4, - sset_join(&all_ips_v4, ", ", " }")); + sset_join(&op->od->lb_ips_v4, ", ", " }")); build_lrouter_arp_flow(op->od, op, ds_cstr(&load_balancer_ips_v4), REG_INPORT_ETH_ADDR, @@ -11125,7 +11116,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, ds_destroy(&load_balancer_ips_v4); } - SSET_FOR_EACH (ip_address, &all_ips_v6) { + SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) { ds_clear(match); if (op == op->od->l3dgw_port) { ds_put_format(match, "is_chassis_resident(%s)", @@ -11137,9 +11128,6 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, match, false, 90, NULL, lflows); } - sset_destroy(&all_ips_v4); - sset_destroy(&all_ips_v6); - if (!smap_get(&op->od->nbr->options, "chassis") && !op->od->l3dgw_port) { /* UDP/TCP/SCTP port unreachable. */ @@ -13347,6 +13335,7 @@ ovnnb_db_run(struct northd_context *ctx, build_datapaths(ctx, datapaths, lr_list); build_ovn_lbs(ctx, datapaths, &lbs); + build_lrouter_lbs(datapaths, &lbs); build_ports(ctx, sbrec_chassis_by_name, datapaths, ports); build_ovn_lb_svcs(ctx, ports, &lbs); build_ipam(datapaths, ports); _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
