To make it easier to add flows to this stage, refactor the function.
This also has the benefit that we should see fewer allocations due to
rearranging how we create flows and how we manipulate the match string.

Signed-off-by: Ales Musil <amu...@redhat.com>
---
v5: Rebase on top of current main.
    Add missing ");" if ct_lb_related feature is not supported.
v6: Rebase on top of current main.
    Fix wrong enclosing for reject action and add missing test case.
---
 northd/northd.c     | 483 ++++++++++++++++++++------------------------
 tests/ovn-northd.at |  69 ++++++-
 2 files changed, 280 insertions(+), 272 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 0944a7b56..4b0d37375 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3878,10 +3878,13 @@ static bool
 build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
                      struct ovn_northd_lb_vip *lb_vip_nb,
                      struct ds *action, char *selection_fields,
-                     bool ls_dp, bool ct_lb_mark)
+                     struct ds *skip_snat_action, struct ds *force_snat_action,
+                     bool ls_dp, const struct chassis_features *features)
 {
-    const char *ct_lb_action = ct_lb_mark ? "ct_lb_mark" : "ct_lb";
-    bool skip_hash_fields = false, reject = false;
+    const char *ct_lb_action =
+        features->ct_no_masked_label ? "ct_lb_mark" : "ct_lb";
+    bool reject = !lb_vip->n_backends && lb_vip->empty_backend_rej;
+    bool drop = false;
 
     if (lb_vip_nb->lb_health_check) {
         ds_put_format(action, "%s(backends=", ct_lb_action);
@@ -3902,23 +3905,12 @@ build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
             ds_put_format(action, "%s:%"PRIu16",",
                           backend->ip_str, backend->port);
         }
+        ds_chomp(action, ',');
 
-        if (!n_active_backends) {
-            if (!lb_vip->empty_backend_rej) {
-                ds_clear(action);
-                ds_put_cstr(action, debug_drop_action());
-                skip_hash_fields = true;
-            } else {
-                reject = true;
-            }
-        } else {
-            ds_chomp(action, ',');
-            ds_put_cstr(action, ");");
-        }
-    } else if (lb_vip->empty_backend_rej && !lb_vip->n_backends) {
-        reject = true;
+        drop = !n_active_backends && !lb_vip->empty_backend_rej;
+        reject = !n_active_backends && lb_vip->empty_backend_rej;
     } else {
-        ds_put_format(action, "%s(backends=%s);", ct_lb_action,
+        ds_put_format(action, "%s(backends=%s", ct_lb_action,
                       lb_vip_nb->backend_ips);
     }
 
@@ -3927,12 +3919,29 @@ build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
                           : ovn_stage_get_table(S_ROUTER_OUT_SNAT);
         ds_clear(action);
         ds_put_format(action, "reg0 = 0; reject { outport <-> inport; "
-                      "next(pipeline=egress,table=%d);};", stage);
-    } else if (!skip_hash_fields && selection_fields && selection_fields[0]) {
-        ds_chomp(action, ';');
-        ds_chomp(action, ')');
-        ds_put_format(action, "; hash_fields=\"%s\");", selection_fields);
+                              "next(pipeline=egress,table=%d);};", stage);
+    } else if (drop) {
+        ds_clear(action);
+        ds_put_cstr(action, debug_drop_action());
+    } else if (selection_fields && selection_fields[0]) {
+        ds_put_format(action, "; hash_fields=\"%s\"", selection_fields);
+    }
+
+    bool is_lb_action = !(reject || drop);
+    const char *enclose = is_lb_action ? ");" : "";
+
+    if (!ls_dp) {
+        bool flag_supported = is_lb_action && features->ct_lb_related;
+        ds_put_format(skip_snat_action, "flags.skip_snat_for_lb = 1; %s%s",
+                      ds_cstr(action),
+                      flag_supported ? "; skip_snat);" : enclose);
+        ds_put_format(force_snat_action, "flags.force_snat_for_lb = 1; %s%s",
+                      ds_cstr(action),
+                      flag_supported ? "; force_snat);" : enclose);
     }
+
+    ds_put_cstr(action, enclose);
+
     return reject;
 }
 
@@ -7502,9 +7511,9 @@ build_lb_affinity_default_flows(struct ovn_datapath *od, 
struct hmap *lflows)
 }
 
 static void
-build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, bool ct_lb_mark,
-               struct ds *match, struct ds *action,
-               const struct shash *meter_groups)
+build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb,
+               const struct chassis_features *features, struct ds *match,
+               struct ds *action, const struct shash *meter_groups)
 {
     for (size_t i = 0; i < lb->n_vips; i++) {
         struct ovn_lb_vip *lb_vip = &lb->vips[i];
@@ -7527,8 +7536,8 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb 
*lb, bool ct_lb_mark,
         /* New connections in Ingress table. */
         const char *meter = NULL;
         bool reject = build_lb_vip_actions(lb_vip, lb_vip_nb, action,
-                                           lb->selection_fields, true,
-                                           ct_lb_mark);
+                                           lb->selection_fields, NULL,
+                                           NULL, true, features);
 
         ds_put_format(match, "ct.new && %s.dst == %s", ip_match,
                       lb_vip->vip_str);
@@ -10465,49 +10474,123 @@ get_force_snat_ip(struct ovn_datapath *od, const 
char *key_type,
     return true;
 }
 
+#define LROUTER_NAT_LB_FLOW_INIT(MATCH, ACTION, PRIO) \
+        (struct lrouter_nat_lb_flow)                  \
+        { .action = (ACTION), .lflow_ref = NULL,      \
+          .hash = ovn_logical_flow_hash(              \
+          ovn_stage_get_table(S_ROUTER_IN_DNAT),      \
+          ovn_stage_get_pipeline(S_ROUTER_IN_DNAT),   \
+          (PRIO), ds_cstr(MATCH), (ACTION)) }
+
+enum lrouter_nat_lb_flow_type {
+    LROUTER_NAT_LB_FLOW_NORMAL = 0,
+    LROUTER_NAT_LB_FLOW_SKIP_SNAT,
+    LROUTER_NAT_LB_FLOW_FORCE_SNAT,
+    LROUTER_NAT_LB_FLOW_MAX,
+};
+
+struct lrouter_nat_lb_flow {
+    char *action;
+    struct ovn_lflow *lflow_ref;
+
+    uint32_t hash;
+};
+
+struct lrouter_nat_lb_flows_ctx {
+    struct lrouter_nat_lb_flow new[LROUTER_NAT_LB_FLOW_MAX];
+    struct lrouter_nat_lb_flow est[LROUTER_NAT_LB_FLOW_MAX];
+
+    struct ds *new_match;
+    struct ds *est_match;
+    struct ds *undnat_match;
+
+    struct ovn_lb_vip *lb_vip;
+    struct ovn_northd_lb *lb;
+    bool reject;
+
+    int prio;
+
+    struct hmap *lflows;
+    const struct shash *meter_groups;
+};
+
 static void
-build_gw_lrouter_nat_flows_for_lb(struct ovn_northd_lb *lb,
-                                  struct ovn_datapath **dplist, int n_dplist,
-                                  bool reject, char *new_match,
-                                  char *new_action, char *est_match,
-                                  char *est_action, struct hmap *lflows,
-                                  int prio, const struct shash *meter_groups)
-{
-    if (!n_dplist) {
+build_distr_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx,
+                                     enum lrouter_nat_lb_flow_type type,
+                                     struct ovn_datapath *od)
+{
+    char *gw_action = od->is_gw_router ? "ct_dnat;" : "ct_dnat_in_czone;";
+    /* Store the match lengths, so we can reuse the ds buffer. */
+    size_t new_match_len = ctx->new_match->length;
+    size_t est_match_len = ctx->est_match->length;
+    size_t undnat_match_len = ctx->undnat_match->length;
+
+
+    const char *meter = NULL;
+
+    if (ctx->reject) {
+        meter = copp_meter_get(COPP_REJECT, od->nbr->copp, ctx->meter_groups);
+    }
+
+    if (ctx->lb_vip->n_backends || !ctx->lb_vip->empty_backend_rej) {
+        ds_put_format(ctx->new_match, " && is_chassis_resident(%s)",
+                      od->l3dgw_ports[0]->cr_port->json_key);
+        ds_put_format(ctx->est_match, " && is_chassis_resident(%s)",
+                      od->l3dgw_ports[0]->cr_port->json_key);
+    }
+
+    ovn_lflow_add_with_hint__(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio,
+                              ds_cstr(ctx->new_match), ctx->new[type].action,
+                              NULL, meter, &ctx->lb->nlb->header_);
+    ovn_lflow_add_with_hint(ctx->lflows, od, S_ROUTER_IN_DNAT, ctx->prio,
+                            ds_cstr(ctx->est_match), ctx->est[type].action,
+                            &ctx->lb->nlb->header_);
+
+    ds_truncate(ctx->new_match, new_match_len);
+    ds_truncate(ctx->est_match, est_match_len);
+
+    if (!ctx->lb_vip->n_backends) {
         return;
     }
 
-    struct ovn_lflow *lflow_ref_new = NULL, *lflow_ref_est = NULL;
-    uint32_t hash_new = ovn_logical_flow_hash(
-            ovn_stage_get_table(S_ROUTER_IN_DNAT),
-            ovn_stage_get_pipeline(S_ROUTER_IN_DNAT),
-            prio, new_match, new_action);
-    uint32_t hash_est = ovn_logical_flow_hash(
-            ovn_stage_get_table(S_ROUTER_IN_DNAT),
-            ovn_stage_get_pipeline(S_ROUTER_IN_DNAT),
-            prio, est_match, est_action);
+    char *action = type == LROUTER_NAT_LB_FLOW_NORMAL
+                   ? gw_action : ctx->est[type].action;
 
-    for (size_t i = 0; i < n_dplist; i++) {
-        struct ovn_datapath *od = dplist[i];
-        const char *meter = NULL;
+    ds_put_format(ctx->undnat_match,
+                  ") && outport == %s && is_chassis_resident(%s)",
+                  od->l3dgw_ports[0]->json_key,
+                  od->l3dgw_ports[0]->cr_port->json_key);
+    ovn_lflow_add_with_hint(ctx->lflows, od, S_ROUTER_OUT_UNDNAT, 120,
+                            ds_cstr(ctx->undnat_match), action,
+                            &ctx->lb->nlb->header_);
+    ds_truncate(ctx->undnat_match, undnat_match_len);
+}
 
-        if (reject) {
-            meter = copp_meter_get(COPP_REJECT, od->nbr->copp, meter_groups);
-        }
-        if (meter || !ovn_dp_group_add_with_reference(lflow_ref_new, od)) {
-            struct ovn_lflow *lflow = ovn_lflow_add_at_with_hash(lflows, od,
-                    S_ROUTER_IN_DNAT, prio, new_match, new_action,
-                    NULL, meter, &lb->nlb->header_, OVS_SOURCE_LOCATOR,
-                    hash_new);
-            lflow_ref_new = meter ? NULL : lflow;
-        }
+static void
+build_gw_lrouter_nat_flows_for_lb(struct lrouter_nat_lb_flows_ctx *ctx,
+                                  enum lrouter_nat_lb_flow_type type,
+                                  struct ovn_datapath *od)
+{
+    const char *meter = NULL;
+    struct lrouter_nat_lb_flow *new_flow = &ctx->new[type];
+    struct lrouter_nat_lb_flow *est_flow = &ctx->est[type];
 
-        if (!ovn_dp_group_add_with_reference(lflow_ref_est, od)) {
-            lflow_ref_est = ovn_lflow_add_at_with_hash(lflows, od,
-                    S_ROUTER_IN_DNAT, prio, est_match, est_action,
-                    NULL, NULL, &lb->nlb->header_,
-                    OVS_SOURCE_LOCATOR, hash_est);
-        }
+    if (ctx->reject) {
+        meter = copp_meter_get(COPP_REJECT, od->nbr->copp, ctx->meter_groups);
+    }
+    if (meter || !ovn_dp_group_add_with_reference(new_flow->lflow_ref, od)) {
+        struct ovn_lflow *lflow = ovn_lflow_add_at_with_hash(ctx->lflows, od,
+                S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->new_match),
+                new_flow->action, NULL, meter, &ctx->lb->nlb->header_,
+                OVS_SOURCE_LOCATOR, new_flow->hash);
+        new_flow->lflow_ref = meter ? NULL : lflow;
+    }
+
+    if (!ovn_dp_group_add_with_reference(est_flow->lflow_ref, od)) {
+        est_flow->lflow_ref = ovn_lflow_add_at_with_hash(ctx->lflows, od,
+                S_ROUTER_IN_DNAT, ctx->prio, ds_cstr(ctx->est_match),
+                est_flow->action, NULL, NULL, &ctx->lb->nlb->header_,
+                OVS_SOURCE_LOCATOR, est_flow->hash);
     }
 }
 
@@ -10523,22 +10606,25 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
     const char *ct_natted = features->ct_no_masked_label
                             ? "ct_mark.natted"
                             : "ct_label.natted";
-    char *skip_snat_new_action = NULL;
-    char *skip_snat_est_action = NULL;
-    char *new_match;
-    char *est_match;
+
+    bool ipv4 = lb_vip->address_family == AF_INET;
+    const char *ip_match = ipv4 ? "ip4" : "ip6";
+    const char *ip_reg = ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6;
+
+    int prio = 110;
+
+    struct ds skip_snat_act = DS_EMPTY_INITIALIZER;
+    struct ds force_snat_act = DS_EMPTY_INITIALIZER;
+    struct ds est_match = DS_EMPTY_INITIALIZER;
+    struct ds undnat_match = DS_EMPTY_INITIALIZER;
+    struct ds unsnat_match = DS_EMPTY_INITIALIZER;
 
     ds_clear(match);
     ds_clear(action);
 
     bool reject = build_lb_vip_actions(lb_vip, vips_nb, action,
-                                       lb->selection_fields, false,
-                                       features->ct_no_masked_label);
-    bool drop = !!strncmp(ds_cstr(action), "ct_lb", strlen("ct_lb"));
-    if (!drop) {
-        /* Remove the trailing ");". */
-        ds_truncate(action, action->length - 2);
-    }
+                                       lb->selection_fields, &skip_snat_act,
+                                       &force_snat_act, false, features);
 
     /* Higher priority rules are added for load-balancing in DNAT
      * table.  For every match (on a VIP[:port]), we add two flows.
@@ -10546,52 +10632,23 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
      * of "ct_lb_mark($targets);". The other flow is for ct.est with
      * an action of "next;".
      */
-    if (lb_vip->address_family == AF_INET) {
-        ds_put_format(match, "ip4 && "REG_NEXT_HOP_IPV4" == %s",
-                      lb_vip->vip_str);
-    } else {
-        ds_put_format(match, "ip6 && "REG_NEXT_HOP_IPV6" == %s",
-                      lb_vip->vip_str);
-    }
-
-    if (lb->skip_snat) {
-        const char *skip_snat = features->ct_lb_related && !drop
-                                ? "; skip_snat);"
-                                : "";
-        skip_snat_new_action = xasprintf("flags.skip_snat_for_lb = 1; %s%s",
-                                         ds_cstr(action), skip_snat);
-        skip_snat_est_action = xasprintf("flags.skip_snat_for_lb = 1; "
-                                         "next;");
-    }
-
-    int prio = 110;
-    if (lb_vip->port_str) {
+    ds_put_format(match, "ct.new && !ct.rel && %s && %s == %s",
+                  ip_match, ip_reg, lb_vip->vip_str);
+    if (lb_vip->vip_port) {
         prio = 120;
-        new_match = xasprintf("ct.new && !ct.rel && %s && %s && "
-                              REG_ORIG_TP_DPORT_ROUTER" == %s",
-                              ds_cstr(match), lb->proto, lb_vip->port_str);
-        est_match = xasprintf("ct.est && !ct.rel && %s && %s && "
-                              REG_ORIG_TP_DPORT_ROUTER" == %s && %s == 1",
-                              ds_cstr(match), lb->proto, lb_vip->port_str,
-                              ct_natted);
-    } else {
-        new_match = xasprintf("ct.new && !ct.rel && %s", ds_cstr(match));
-        est_match = xasprintf("ct.est && !ct.rel && %s && %s == 1",
-                          ds_cstr(match), ct_natted);
+        ds_put_format(match, " && %s && "REG_ORIG_TP_DPORT_ROUTER" == %d",
+                      lb->proto, lb_vip->vip_port);
     }
 
-    const char *ip_match = NULL;
-    if (lb_vip->address_family == AF_INET) {
-        ip_match = "ip4";
-    } else {
-        ip_match = "ip6";
-    }
+    ds_put_cstr(&est_match, "ct.est");
+    /* Clone the match after initial "ct.new" (6 bytes). */
+    ds_put_cstr(&est_match, ds_cstr(match) + 6);
+    ds_put_format(&est_match, " && %s == 1", ct_natted);
 
     /* Add logical flows to UNDNAT the load balanced reverse traffic in
      * the router egress pipleine stage - S_ROUTER_OUT_UNDNAT if the logical
      * router has a gateway router port associated.
      */
-    struct ds undnat_match = DS_EMPTY_INITIALIZER;
     ds_put_format(&undnat_match, "%s && (", ip_match);
 
     for (size_t i = 0; i < lb_vip->n_backends; i++) {
@@ -10606,12 +10663,9 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
             ds_put_cstr(&undnat_match, ") || ");
         }
     }
-    ds_chomp(&undnat_match, ' ');
-    ds_chomp(&undnat_match, '|');
-    ds_chomp(&undnat_match, '|');
-    ds_chomp(&undnat_match, ' ');
+    /* Remove the trailing " || ". */
+    ds_truncate(&undnat_match, undnat_match.length - 4);
 
-    struct ds unsnat_match = DS_EMPTY_INITIALIZER;
     ds_put_format(&unsnat_match, "%s && %s.dst == %s && %s",
                   ip_match, ip_match, lb_vip->vip_str, lb->proto);
     if (lb_vip->port_str) {
@@ -10619,21 +10673,34 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
                       lb_vip->port_str);
     }
 
-    struct ovn_datapath **gw_router_skip_snat =
-        xcalloc(lb->n_nb_lr, sizeof *gw_router_skip_snat);
-    int n_gw_router_skip_snat = 0;
+    struct lrouter_nat_lb_flows_ctx ctx = {
+        .lb_vip = lb_vip,
+        .lb = lb,
+        .reject = reject,
+        .prio = prio,
+        .lflows = lflows,
+        .meter_groups = meter_groups,
+        .new_match = match,
+        .est_match = &est_match,
+        .undnat_match = &undnat_match
+    };
 
-    struct ovn_datapath **gw_router_force_snat =
-        xcalloc(lb->n_nb_lr, sizeof *gw_router_force_snat);
-    int n_gw_router_force_snat = 0;
+    ctx.new[LROUTER_NAT_LB_FLOW_NORMAL] =
+        LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(action), prio);
+    ctx.est[LROUTER_NAT_LB_FLOW_NORMAL] =
+        LROUTER_NAT_LB_FLOW_INIT(&est_match, "next;", prio);
 
-    struct ovn_datapath **gw_router =
-        xcalloc(lb->n_nb_lr, sizeof *gw_router);
-    int n_gw_router = 0;
+    ctx.new[LROUTER_NAT_LB_FLOW_SKIP_SNAT] =
+        LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(&skip_snat_act), prio);
+    ctx.est[LROUTER_NAT_LB_FLOW_SKIP_SNAT] =
+        LROUTER_NAT_LB_FLOW_INIT(&est_match,
+                                 "flags.skip_snat_for_lb = 1; next;", prio);
 
-    struct ovn_datapath **distributed_router =
-        xcalloc(lb->n_nb_lr, sizeof *distributed_router);
-    int n_distributed_router = 0;
+    ctx.new[LROUTER_NAT_LB_FLOW_FORCE_SNAT] =
+        LROUTER_NAT_LB_FLOW_INIT(match, ds_cstr(&force_snat_act), prio);
+    ctx.est[LROUTER_NAT_LB_FLOW_FORCE_SNAT] =
+        LROUTER_NAT_LB_FLOW_INIT(&est_match,
+                                 "flags.force_snat_for_lb = 1; next;", prio);
 
     struct ovn_datapath **lb_aff_force_snat_router =
         xcalloc(lb->n_nb_lr, sizeof *lb_aff_force_snat_router);
@@ -10647,18 +10714,22 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
      * lflow generation for them.
      */
     for (size_t i = 0; i < lb->n_nb_lr; i++) {
+        enum lrouter_nat_lb_flow_type type;
         struct ovn_datapath *od = lb->nb_lr[i];
+
+        if (lb->skip_snat) {
+            type = LROUTER_NAT_LB_FLOW_SKIP_SNAT;
+        } else if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) ||
+                   od->lb_force_snat_router_ip) {
+            type = LROUTER_NAT_LB_FLOW_FORCE_SNAT;
+        } else {
+            type = LROUTER_NAT_LB_FLOW_NORMAL;
+        }
+
         if (!od->n_l3dgw_ports) {
-            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) {
-                gw_router_force_snat[n_gw_router_force_snat++] = od;
-            } else {
-                gw_router[n_gw_router++] = od;
-            }
+            build_gw_lrouter_nat_flows_for_lb(&ctx, type, od);
         } else {
-            distributed_router[n_distributed_router++] = od;
+            build_distr_lrouter_nat_flows_for_lb(&ctx, type, od);
         }
 
         if (!lport_addresses_is_empty(&od->lb_force_snat_addrs) ||
@@ -10680,149 +10751,36 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip 
*lb_vip,
              * unsnat stage, the conntrack flags are not set properly, and
              * it doesn't hit the established state flows in
              * S_ROUTER_IN_DNAT stage. */
-             ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_UNSNAT, 120,
-                                     ds_cstr(&unsnat_match), "next;",
-                                     &lb->nlb->header_);
+            ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_UNSNAT, 120,
+                                    ds_cstr(&unsnat_match), "next;",
+                                    &lb->nlb->header_);
         }
     }
 
-    /* GW router logic */
-    build_gw_lrouter_nat_flows_for_lb(lb, gw_router_skip_snat,
-            n_gw_router_skip_snat, reject, new_match,
-            skip_snat_new_action, est_match,
-            skip_snat_est_action, lflows, prio, meter_groups);
-
-    const char *force_snat = features->ct_lb_related && !drop
-                             ? "; force_snat);"
-                             : "";
-    char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s%s",
-                                  ds_cstr(action), force_snat);
-    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);
-
     /* LB affinity flows for datapaths where CMS has specified
      * force_snat_for_lb floag option.
      */
-    build_lb_affinity_lr_flows(lflows, lb, lb_vip, new_match,
+    build_lb_affinity_lr_flows(lflows, lb, lb_vip, ds_cstr(match),
                                "flags.force_snat_for_lb = 1; ",
                                lb_aff_force_snat_router,
                                n_lb_aff_force_snat_router);
 
-    if (!drop) {
-        ds_put_cstr(action, ");");
-    }
-
-    build_gw_lrouter_nat_flows_for_lb(lb, gw_router, n_gw_router,
-            reject, new_match, ds_cstr(action), est_match,
-            "next;", lflows, prio, meter_groups);
-
     /* LB affinity flows for datapaths where CMS has specified
      * skip_snat_for_lb floag option or regular datapaths.
      */
     char *lb_aff_action =
         lb->skip_snat ? "flags.skip_snat_for_lb = 1; " : NULL;
-    build_lb_affinity_lr_flows(lflows, lb, lb_vip, new_match, lb_aff_action,
-                               lb_aff_router, n_lb_aff_router);
-
-    /* Distributed router logic */
-    for (size_t i = 0; i < n_distributed_router; i++) {
-        struct ovn_datapath *od = distributed_router[i];
-        char *new_match_p = new_match;
-        char *est_match_p = est_match;
-        const char *meter = NULL;
-        bool is_dp_lb_force_snat =
-            !lport_addresses_is_empty(&od->lb_force_snat_addrs) ||
-            od->lb_force_snat_router_ip;
-
-        if (reject) {
-            meter = copp_meter_get(COPP_REJECT, od->nbr->copp, meter_groups);
-        }
-
-        if (lb_vip->n_backends || !lb_vip->empty_backend_rej) {
-            new_match_p = xasprintf("%s && is_chassis_resident(%s)",
-                                    new_match,
-                                    od->l3dgw_ports[0]->cr_port->json_key);
-            est_match_p = xasprintf("%s && is_chassis_resident(%s)",
-                                    est_match,
-                                    od->l3dgw_ports[0]->cr_port->json_key);
-        }
-
-        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_);
-            ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, prio,
-                                    est_match_p, skip_snat_est_action,
-                                    &lb->nlb->header_);
-        } else if (is_dp_lb_force_snat) {
-            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,
-                                      meter, &lb->nlb->header_);
-            ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, prio,
-                                    est_match_p, "next;",
-                                    &lb->nlb->header_);
-        }
-
-        if (new_match_p != new_match) {
-            free(new_match_p);
-        }
-        if (est_match_p != est_match) {
-            free(est_match_p);
-        }
-
-        if (!lb_vip->n_backends) {
-            continue;
-        }
-
-        char *undnat_match_p = xasprintf(
-            "%s) && outport == %s && is_chassis_resident(%s)",
-            ds_cstr(&undnat_match),
-            od->l3dgw_ports[0]->json_key,
-            od->l3dgw_ports[0]->cr_port->json_key);
-        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_);
-        } else if (is_dp_lb_force_snat) {
-            ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
-                                    undnat_match_p,
-                                    "flags.force_snat_for_lb = 1; next;",
-                                    &lb->nlb->header_);
-        } else {
-            ovn_lflow_add_with_hint(
-                lflows, od, S_ROUTER_OUT_UNDNAT, 120, undnat_match_p,
-                od->is_gw_router ? "ct_dnat;" : "ct_dnat_in_czone;",
-                &lb->nlb->header_);
-        }
-        free(undnat_match_p);
-    }
+    build_lb_affinity_lr_flows(lflows, lb, lb_vip, ds_cstr(match),
+                               lb_aff_action, lb_aff_router, n_lb_aff_router);
 
     ds_destroy(&unsnat_match);
     ds_destroy(&undnat_match);
+    ds_destroy(&est_match);
+    ds_destroy(&skip_snat_act);
+    ds_destroy(&force_snat_act);
 
-    free(skip_snat_new_action);
-    free(skip_snat_est_action);
-    free(est_match);
-    free(new_match);
-    free(new_actions);
-
-    free(gw_router_force_snat);
-    free(gw_router_skip_snat);
-    free(distributed_router);
     free(lb_aff_force_snat_router);
     free(lb_aff_router);
-    free(gw_router);
 }
 
 static void
@@ -10867,8 +10825,7 @@ build_lswitch_flows_for_lb(struct ovn_northd_lb *lb, 
struct hmap *lflows,
      * REGBIT_CONNTRACK_COMMIT. */
     build_lb_rules_pre_stateful(lflows, lb, features->ct_no_masked_label,
                                 match, action);
-    build_lb_rules(lflows, lb, features->ct_no_masked_label,
-                   match, action, meter_groups);
+    build_lb_rules(lflows, lb, features, match, action, meter_groups);
 }
 
 /* If there are any load balancing rules, we should send the packet to
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 5d5c32ee3..3fa02d2b3 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3773,15 +3773,6 @@ AT_CHECK([grep "lr_in_dnat" lr0flows | sort], [0], [dnl
   table=7 (lr_in_dnat         ), priority=70   , match=(ct.rel && !ct.est && 
!ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = 1; 
ct_commit_nat;)
 ])
 
-AT_CHECK([grep "lr_out_undnat" lr0flows | sed 's/table=./table=?/' | sort], 
[0], [dnl
-  table=? (lr_out_undnat      ), priority=0    , match=(1), action=(next;)
-  table=? (lr_out_undnat      ), priority=50   , match=(ip), 
action=(flags.loopback = 1; ct_dnat;)
-])
-
-AT_CHECK([grep "lr_out_post_undnat" lr0flows | sed 's/table=./table=?/' | 
sort], [0], [dnl
-  table=? (lr_out_post_undnat ), priority=0    , match=(1), action=(next;)
-  table=? (lr_out_post_undnat ), priority=50   , match=(ip && ct.new), 
action=(ct_commit { } ; next; )
-])
 
 check ovn-nbctl --wait=sb set logical_router lr0 
options:lb_force_snat_ip="20.0.0.4 aef0::4"
 
@@ -5616,6 +5607,66 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed 
's/table=./table=?/' | sort], [0], [
   table=? (lr_out_snat        ), priority=120  , match=(nd_ns), action=(next;)
 ])
 
+# LB with reject configured
+check ovn-nbctl --wait=sb remove logical_router lr0 options lb_force_snat_ip
+check ovn-nbctl --wait=sb sync
+
+check ovn-nbctl lr-lb-del lr0
+check ovn-nbctl lsp-add sw0 vip1
+check ovn-nbctl lsp-add sw0 vip2
+check ovn-nbctl --reject lb-add lb5 172.168.10.10 10.0.20.10,10.0.20.20
+check ovn-nbctl --wait=sb set load_balancer lb5 
ip_port_mappings:10.0.20.10=vip1:10.0.0.2
+check ovn-nbctl --wait=sb set load_balancer lb5 
ip_port_mappings:10.0.20.20=vip2:20.0.0.2
+
+check ovn-nbctl --wait=sb lr-lb-add lr0 lb5
+AT_CHECK([ovn-nbctl --wait=sb -- --id=@hc create \
+Load_Balancer_Health_Check vip="172.168.10.10" -- add Load_Balancer lb5 \
+health_check @hc | uuidfilt], [0], [<0>
+])
+wait_row_count Service_Monitor 2
+
+# Set the service monitor for vip1 and vip2 to offline
+sm_vip1=$(fetch_column Service_Monitor _uuid logical_port=vip1)
+sm_vip2=$(fetch_column Service_Monitor _uuid logical_port=vip2)
+
+ovn-sbctl set service_monitor $sm_vip1 status=offline
+ovn-sbctl set service_monitor $sm_vip2 status=offline
+
+AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat" | sort], [0], [dnl
+  table=7 (lr_in_dnat         ), priority=0    , match=(1), action=(next;)
+  table=7 (lr_in_dnat         ), priority=110  , match=(ct.est && !ct.rel && 
ip4 && reg0 == 172.168.10.10 && ct_mark.natted == 1), action=(next;)
+  table=7 (lr_in_dnat         ), priority=110  , match=(ct.new && !ct.rel && 
ip4 && reg0 == 172.168.10.10), action=(reg0 = 0; reject { outport <-> inport; 
next(pipeline=egress,table=3);};)
+  table=7 (lr_in_dnat         ), priority=50   , match=(ct.rel && !ct.est && 
!ct.new), action=(ct_commit_nat;)
+  table=7 (lr_in_dnat         ), priority=70   , match=(ct.rel && !ct.est && 
!ct.new && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb = 1; 
ct_commit_nat;)
+  table=7 (lr_in_dnat         ), priority=70   , match=(ct.rel && !ct.est && 
!ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = 1; 
ct_commit_nat;)
+])
+
+# LB with reject and skip_snat
+check ovn-nbctl --wait=sb set load_balancer lb5 options:skip_snat=true
+
+AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat" | sort], [0], [dnl
+  table=7 (lr_in_dnat         ), priority=0    , match=(1), action=(next;)
+  table=7 (lr_in_dnat         ), priority=110  , match=(ct.est && !ct.rel && 
ip4 && reg0 == 172.168.10.10 && ct_mark.natted == 1), 
action=(flags.skip_snat_for_lb = 1; next;)
+  table=7 (lr_in_dnat         ), priority=110  , match=(ct.new && !ct.rel && 
ip4 && reg0 == 172.168.10.10), action=(flags.skip_snat_for_lb = 1; reg0 = 0; 
reject { outport <-> inport; next(pipeline=egress,table=3);};)
+  table=7 (lr_in_dnat         ), priority=50   , match=(ct.rel && !ct.est && 
!ct.new), action=(ct_commit_nat;)
+  table=7 (lr_in_dnat         ), priority=70   , match=(ct.rel && !ct.est && 
!ct.new && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb = 1; 
ct_commit_nat;)
+  table=7 (lr_in_dnat         ), priority=70   , match=(ct.rel && !ct.est && 
!ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = 1; 
ct_commit_nat;)
+])
+
+check ovn-nbctl --wait=sb remove load_balancer lb5 options skip_snat
+
+# LB with reject and force_snat
+check ovn-nbctl --wait=sb set logical_router lr0 
options:lb_force_snat_ip="router_ip"
+
+AT_CHECK([ovn-sbctl dump-flows lr0 | grep "lr_in_dnat" | sort], [0], [dnl
+  table=7 (lr_in_dnat         ), priority=0    , match=(1), action=(next;)
+  table=7 (lr_in_dnat         ), priority=110  , match=(ct.est && !ct.rel && 
ip4 && reg0 == 172.168.10.10 && ct_mark.natted == 1), 
action=(flags.force_snat_for_lb = 1; next;)
+  table=7 (lr_in_dnat         ), priority=110  , match=(ct.new && !ct.rel && 
ip4 && reg0 == 172.168.10.10), action=(flags.force_snat_for_lb = 1; reg0 = 0; 
reject { outport <-> inport; next(pipeline=egress,table=3);};)
+  table=7 (lr_in_dnat         ), priority=50   , match=(ct.rel && !ct.est && 
!ct.new), action=(ct_commit_nat;)
+  table=7 (lr_in_dnat         ), priority=70   , match=(ct.rel && !ct.est && 
!ct.new && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb = 1; 
ct_commit_nat;)
+  table=7 (lr_in_dnat         ), priority=70   , match=(ct.rel && !ct.est && 
!ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = 1; 
ct_commit_nat;)
+])
+
 AT_CLEANUP
 ])
 
-- 
2.39.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to