diff mbox series

[ovs-dev,v2,branch-22.12,1/2] northd: Make the enclose handling less error prone

Message ID 20230330133247.446892-1-amusil@redhat.com
State Accepted
Headers show
Series [ovs-dev,v2,branch-22.12,1/2] northd: Make the enclose handling less error prone | 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

Ales Musil March 30, 2023, 1:32 p.m. UTC 
The enclose handling was working for all cases
except the drop. Make sure that the enclose is
properly defined for that case and make sure
that it is used properly.

Fixes: cf205ca0e52c ("northd: Fix missig "); " from LB flows")
Signed-off-by: Ales Musil <amusil@redhat.com>
---
 northd/northd.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Dumitru Ceara March 31, 2023, 11:40 a.m. UTC | #1 
On 3/30/23 15:32, Ales Musil wrote:
> The enclose handling was working for all cases
> except the drop. Make sure that the enclose is
> properly defined for that case and make sure
> that it is used properly.
> 
> Fixes: cf205ca0e52c ("northd: Fix missig "); " from LB flows")
> Signed-off-by: Ales Musil <amusil@redhat.com>
> ---

Thanks for the fix, Ales, applied to 22.12 and backported down to 21.12.

Just for future-me: this is only applies to branches <= 22.12 because in
23.03 the corresponding LB code has been changed and the bug indirectly
got fixed via 7b94d212f694 ("northd: Refactor
build_lrouter_nat_flows_for_lb function").

>  northd/northd.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/northd/northd.c b/northd/northd.c
> index 3165f4bc2..89bd9cbb3 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -10525,11 +10525,13 @@ build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
>                                         lb->selection_fields, false,
>                                         features->ct_no_masked_label);
>      bool drop = !!strncmp(ds_cstr(action), "ct_lb", strlen("ct_lb"));
> +    const char *enclose = drop ? "" : ");";
>      if (!drop) {
>          /* Remove the trailing ");". */
>          ds_truncate(action, action->length - 2);
>      }
>  
> +

I removed this extra newline.

Regards,
Dumitru
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 3165f4bc2..89bd9cbb3 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -10525,11 +10525,13 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
                                        lb->selection_fields, false,
                                        features->ct_no_masked_label);
     bool drop = !!strncmp(ds_cstr(action), "ct_lb", strlen("ct_lb"));
+    const char *enclose = drop ? "" : ");";
     if (!drop) {
         /* Remove the trailing ");". */
         ds_truncate(action, action->length - 2);
     }
 
+
     /* Higher priority rules are added for load-balancing in DNAT
      * table.  For every match (on a VIP[:port]), we add two flows.
      * One flow is for specific matching on ct.new with an action
@@ -10548,8 +10550,8 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
         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_new_action = xasprintf("flags.skip_snat_for_lb = 1; %s%s%s",
+                                         ds_cstr(action), skip_snat, enclose);
         skip_snat_est_action = xasprintf("flags.skip_snat_for_lb = 1; "
                                          "next;");
     }
@@ -10685,8 +10687,8 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
     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);
+    char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s%s%s",
+                                  ds_cstr(action), force_snat, enclose);
     build_gw_lrouter_nat_flows_for_lb(lb, gw_router_force_snat,
             n_gw_router_force_snat, reject, new_match,
             new_actions, est_match,
@@ -10701,9 +10703,7 @@  build_lrouter_nat_flows_for_lb(struct ovn_lb_vip *lb_vip,
                                lb_aff_force_snat_router,
                                n_lb_aff_force_snat_router);
 
-    if (!drop) {
-        ds_put_cstr(action, ");");
-    }
+    ds_put_cstr(action, enclose);
 
     build_gw_lrouter_nat_flows_for_lb(lb, gw_router, n_gw_router,
             reject, new_match, ds_cstr(action), est_match,