diff mbox series

[ovs-dev,v3,3/4] northd: Retrieve load balancer options only once.

Message ID 20220909213223.824013-4-i.maximets@ovn.org
State Accepted
Headers show
Series northd: Optimize preparation of load balancers. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Ilya Maximets Sept. 9, 2022, 9:32 p.m. UTC
Options like 'add_route' and 'neighbor_responder' are looked up
in the smap for each load balancer for each datapath and it takes
significant amount of time.  Checking them only once to speed up
processing.  'skip_snat' is not that critical, but it's better to
get all of them in a single place.

Also using enum for the neighbor responder mode to avoid costly
string comparison on a hot path.

Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 lib/lb.c        |  7 +++++++
 lib/lb.h        | 10 ++++++++++
 northd/northd.c | 38 ++++++++++++++++----------------------
 3 files changed, 33 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/lib/lb.c b/lib/lb.c
index 0b229f828..fa1a66d82 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -173,6 +173,13 @@  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);
+    lb->controller_event = smap_get_bool(&nbrec_lb->options, "event", false);
+    lb->routable = smap_get_bool(&nbrec_lb->options, "add_route", false);
+    lb->skip_snat = smap_get_bool(&nbrec_lb->options, "skip_snat", false);
+    const char *mode =
+        smap_get_def(&nbrec_lb->options, "neighbor_responder", "reachable");
+    lb->neigh_mode = strcmp(mode, "all") ? LB_NEIGH_RESPOND_REACHABLE
+                                         : LB_NEIGH_RESPOND_ALL;
     sset_init(&lb->ips_v4);
     sset_init(&lb->ips_v6);
     struct smap_node *node;
diff --git a/lib/lb.h b/lib/lb.h
index fbd4f19e8..e0b153fb3 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -32,6 +32,11 @@  struct ovn_datapath;
 struct ovn_port;
 struct uuid;
 
+enum lb_neighbor_responder_mode {
+    LB_NEIGH_RESPOND_REACHABLE,
+    LB_NEIGH_RESPOND_ALL,
+};
+
 struct ovn_northd_lb {
     struct hmap_node hmap_node;
 
@@ -43,6 +48,11 @@  struct ovn_northd_lb {
     struct ovn_northd_lb_vip *vips_nb;
     size_t n_vips;
 
+    enum lb_neighbor_responder_mode neigh_mode;
+    bool controller_event;
+    bool routable;
+    bool skip_snat;
+
     struct sset ips_v4;
     struct sset ips_v6;
 
diff --git a/northd/northd.c b/northd/northd.c
index b08ad932b..a4aed7f96 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3824,18 +3824,17 @@  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)
 {
-    bool is_routable = smap_get_bool(&lb->nlb->options, "add_route",  false);
     const char *ip_address;
 
     SSET_FOR_EACH (ip_address, &lb->ips_v4) {
         sset_add(&od->lb_ips_v4, ip_address);
-        if (is_routable) {
+        if (lb->routable) {
             sset_add(&od->lb_ips_v4_routable, ip_address);
         }
     }
     SSET_FOR_EACH (ip_address, &lb->ips_v6) {
         sset_add(&od->lb_ips_v6, ip_address);
-        if (is_routable) {
+        if (lb->routable) {
             sset_add(&od->lb_ips_v6_routable, ip_address);
         }
     }
@@ -3972,13 +3971,10 @@  static void
 build_lrouter_lb_reachable_ips(struct ovn_datapath *od,
                                const struct ovn_northd_lb *lb)
 {
-    const char *neighbor_responder_mode =
-        smap_get_def(&lb->nlb->options, "neighbor_responder", "reachable");
-
     /* If configured to reply to neighbor requests for all VIPs force them
      * all to be considered "reachable".
      */
-    if (!strcmp(neighbor_responder_mode, "all")) {
+    if (lb->neigh_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);
@@ -5758,10 +5754,10 @@  ls_has_dns_records(const struct nbrec_logical_switch *nbs)
 
 static bool
 build_empty_lb_event_flow(struct ovn_lb_vip *lb_vip,
-                          const struct nbrec_load_balancer *lb,
+                          const struct ovn_northd_lb *lb,
                           struct ds *match, struct ds *action)
 {
-    bool controller_event = smap_get_bool(&lb->options, "event", false) ||
+    bool controller_event = lb->controller_event ||
                             controller_event_en; /* deprecated */
     if (!controller_event || lb_vip->n_backends ||
         lb_vip->empty_backend_rej) {
@@ -5774,12 +5770,11 @@  build_empty_lb_event_flow(struct ovn_lb_vip *lb_vip,
     bool ipv4 = IN6_IS_ADDR_V4MAPPED(&lb_vip->vip);
 
     ds_put_format(match, "ip%s.dst == %s && %s",
-                  ipv4 ? "4": "6", lb_vip->vip_str, lb->protocol);
+                  ipv4 ? "4": "6", lb_vip->vip_str, lb->proto);
 
     char *vip = lb_vip->vip_str;
     if (lb_vip->vip_port) {
-        ds_put_format(match, " && %s.dst == %u", lb->protocol,
-                      lb_vip->vip_port);
+        ds_put_format(match, " && %s.dst == %u", lb->proto, lb_vip->vip_port);
         vip = xasprintf("%s%s%s:%u", ipv4 ? "" : "[", lb_vip->vip_str,
                         ipv4 ? "" : "]", lb_vip->vip_port);
     }
@@ -5790,8 +5785,8 @@  build_empty_lb_event_flow(struct ovn_lb_vip *lb_vip,
                   "protocol = \"%s\", "
                   "load_balancer = \"" UUID_FMT "\");",
                   event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
-                  vip, lb->protocol,
-                  UUID_ARGS(&lb->header_.uuid));
+                  vip, lb->proto,
+                  UUID_ARGS(&lb->nlb->header_.uuid));
     if (lb_vip->vip_port) {
         free(vip);
     }
@@ -9950,8 +9945,7 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
                       lb_vip->vip_str);
     }
 
-    bool lb_skip_snat = smap_get_bool(&lb->nlb->options, "skip_snat", false);
-    if (lb_skip_snat) {
+    if (lb->skip_snat) {
         skip_snat_new_action = xasprintf("flags.skip_snat_for_lb = 1; %s",
                                          ds_cstr(action));
         skip_snat_est_action = xasprintf("flags.skip_snat_for_lb = 1; "
@@ -10035,7 +10029,7 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
     for (size_t i = 0; i < lb->n_nb_lr; i++) {
         struct ovn_datapath *od = lb->nb_lr[i];
         if (!od->n_l3dgw_ports) {
-            if (lb_skip_snat) {
+            if (lb->skip_snat) {
                 gw_router_skip_snat[n_gw_router_skip_snat++] = od;
             } else if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) ||
                        od->lb_force_snat_router_ip) {
@@ -10106,7 +10100,7 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
                                     od->l3dgw_ports[0]->cr_port->json_key);
         }
 
-        if (lb_skip_snat) {
+        if (lb->skip_snat) {
             ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT, prio,
                                       new_match_p, skip_snat_new_action,
                                       NULL, meter, &lb->nlb->header_);
@@ -10146,7 +10140,7 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
             ds_cstr(&undnat_match),
             od->l3dgw_ports[0]->json_key,
             od->l3dgw_ports[0]->cr_port->json_key);
-        if (lb_skip_snat) {
+        if (lb->skip_snat) {
             ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
                                     undnat_match_p, skip_snat_est_action,
                                     &lb->nlb->header_);
@@ -10193,7 +10187,7 @@  build_lswitch_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows,
         struct ovn_lb_vip *lb_vip = &lb->vips[i];
 
         /* pre-stateful lb */
-        if (!build_empty_lb_event_flow(lb_vip, lb->nlb, match, action)) {
+        if (!build_empty_lb_event_flow(lb_vip, lb, match, action)) {
             continue;
         }
         for (size_t j = 0; j < lb->n_nb_ls; j++) {
@@ -10307,7 +10301,7 @@  build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows,
                                        lflows, match, action, meter_groups,
                                        features->ct_no_masked_label);
 
-        if (!build_empty_lb_event_flow(lb_vip, lb->nlb, match, action)) {
+        if (!build_empty_lb_event_flow(lb_vip, lb, match, action)) {
             continue;
         }
         for (size_t j = 0; j < lb->n_nb_lr; j++) {
@@ -10322,7 +10316,7 @@  build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows,
         }
     }
 
-    if (smap_get_bool(&lb->nlb->options, "skip_snat", false)) {
+    if (lb->skip_snat) {
         for (size_t i = 0; i < lb->n_nb_lr; i++) {
             ovn_lflow_add(lflows, lb->nb_lr[i], S_ROUTER_OUT_SNAT, 120,
                           "flags.skip_snat_for_lb == 1 && ip", "next;");