[ovs-dev] [PATCH ovn v2 3/3] northd: Precompute load balancer IP sets.
Dumitru Ceara
dceara at redhat.com
Thu Jun 10 12:48:09 UTC 2021
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 <dceara at redhat.com>
---
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);
More information about the dev
mailing list