diff mbox series

[ovs-dev,v2,2/5] northd: move nat_flows_for_lb for force_nat in a dedicated routine

Message ID e86a92c77eaa2790b1ed55ecbfe53481073e1a92.1652459689.git.lorenzo.bianconi@redhat.com
State Accepted
Headers show
Series optimize lb lflow generation for gw router | 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

Lorenzo Bianconi May 13, 2022, 5:03 p.m. UTC 
Similar to skip_snat counterpart, move force_nat gw router where a
given lb is installed in a dedicated routine.
This is a preliminary patch to reduce load balancer logical flows
computation cost.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 northd/northd.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

Comments

Dumitru Ceara May 17, 2022, 11:13 a.m. UTC | #1 
On 5/13/22 19:03, Lorenzo Bianconi wrote:
> Similar to skip_snat counterpart, move force_nat gw router where a
> given lb is installed in a dedicated routine.
> This is a preliminary patch to reduce load balancer logical flows
> computation cost.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara <dceara@redhat.com>
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 361529be1..4efe6b810 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9979,12 +9979,18 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
             skip_snat_new_action, est_match,
             skip_snat_est_action, lflows, prio, meter_groups);
 
+    char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s",
+                                  ds_cstr(action));
+    build_gw_lrouter_nat_flows_for_lb(lb, gw_router_force_snat,
+            n_gw_router_force_snat, reject, new_match,
+            new_actions, est_match,
+            "flags.force_snat_for_lb = 1; next;",
+            lflows, prio, meter_groups);
 
     for (size_t i = 0; i < lb->n_nb_lr; i++) {
         struct ovn_datapath *od = lb->nb_lr[i];
         char *new_match_p = new_match;
         char *est_match_p = est_match;
-        char *est_actions = NULL;
         const char *meter = NULL;
         bool is_dp_lb_force_snat =
             !lport_addresses_is_empty(&od->lb_force_snat_addrs) ||
@@ -10031,18 +10037,15 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
                                         &lb->nlb->header_);
             }
         } else if (is_dp_lb_force_snat) {
-            char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s",
-                                          ds_cstr(action));
-            ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT, prio,
-                                      new_match_p, new_actions, NULL,
-                                      meter, &lb->nlb->header_);
-            free(new_actions);
-
-            est_actions = xasprintf("flags.force_snat_for_lb = 1; "
-                                    "next;");
-            ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, prio,
-                                    est_match_p, est_actions,
-                                    &lb->nlb->header_);
+            if (od->n_l3dgw_ports) {
+                ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT, prio,
+                                          new_match_p, new_actions, NULL,
+                                          meter, &lb->nlb->header_);
+                ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, prio,
+                                        est_match_p,
+                                        "flags.force_snat_for_lb = 1; next;",
+                                        &lb->nlb->header_);
+            }
         } else {
             ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT, prio,
                                       new_match_p, ds_cstr(action), NULL,
@@ -10060,7 +10063,7 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
         }
 
         if (!od->n_l3dgw_ports || !lb_vip->n_backends) {
-            goto next;
+            continue;
         }
 
         char *undnat_match_p = xasprintf(
@@ -10074,7 +10077,8 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
                                     &lb->nlb->header_);
         } else if (is_dp_lb_force_snat) {
             ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
-                                    undnat_match_p, est_actions,
+                                    undnat_match_p,
+                                    "flags.force_snat_for_lb = 1; next;",
                                     &lb->nlb->header_);
         } else {
             ovn_lflow_add_with_hint(
@@ -10083,8 +10087,6 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
                 &lb->nlb->header_);
         }
         free(undnat_match_p);
-next:
-        free(est_actions);
     }
 
     ds_destroy(&unsnat_match);
@@ -10094,6 +10096,7 @@  next:
     free(skip_snat_est_action);
     free(est_match);
     free(new_match);
+    free(new_actions);
 
     free(gw_router_force_snat);
     free(gw_router_skip_snat);