From 8c5c3ca0fc5c28aac937868e9ce5b11010e693f5 Mon Sep 17 00:00:00 2001

Message-Id: <8c5c3ca0fc5c28aac937868e9ce5b11010e693f5.1605177623.git.lorenzo.bianconi@redhat.com>

From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Date: Wed, 11 Nov 2020 13:39:17 +0100

Subject: [PATCH ovn 20.09] northd: Fix lb_action when there are no active backends

 for lb health_check.

Fix the following warning reported by ovn-controller when there are no

active backends for lb health_check and selection_fields has been

configured for hash computation.

flow|WARN|error parsing actions "drop; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");":

Syntax error at `hash_fields' expecting end of input.

Tested-by: Flavio Fernandes <flavio@flaviof.com>

Fixes: 5af304e747 ("Support selection fields in load balancer.")

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

Signed-off-by: Numan Siddique <numans@ovn.org>

---

 northd/ovn-northd.c | 5 ++++-

 tests/ovn.at        | 6 ++++--

 2 files changed, 8 insertions(+), 3 deletions(-)

--- a/northd/ovn-northd.c

+++ b/northd/ovn-northd.c

@@ -3613,6 +3613,8 @@ static void build_lb_vip_ct_lb_actions(s

                                        struct ds *action,

                                        char *selection_fields)

 {

+    bool skip_hash_fields = false;

+

     if (lb_vip->health_check) {

         ds_put_cstr(action, "ct_lb(backends=");

@@ -3631,6 +3633,7 @@ static void build_lb_vip_ct_lb_actions(s

         }

         if (!n_active_backends) {

+            skip_hash_fields = true;

             ds_clear(action);

             ds_put_cstr(action, "drop;");

         } else {

@@ -3641,7 +3644,7 @@ static void build_lb_vip_ct_lb_actions(s

         ds_put_format(action, "ct_lb(backends=%s);", lb_vip->backend_ips);

     }

-    if (selection_fields && selection_fields[0]) {

+    if (!skip_hash_fields && selection_fields && selection_fields[0]) {

         ds_chomp(action, ';');

         ds_chomp(action, ')');

         ds_put_format(action, "; hash_fields=\"%s\");", selection_fields);

--- a/tests/ovn.at

+++ b/tests/ovn.at

@@ -19601,6 +19601,8 @@ ovn-nbctl lsp-set-addresses sw1-lr0 rout

 ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1

 ovn-nbctl lb-add lb1 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80

+OVN_LB_ID=$(ovn-nbctl --bare --column _uuid find load_balancer name=lb1)

+ovn-nbctl set load_balancer ${OVN_LB_ID} selection_fields="ip_dst,ip_src,tp_dst,tp_src"

 ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:10.0.0.3=sw0-p1:10.0.0.2

 ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:20.0.0.3=sw1-p1:20.0.0.2

@@ -19637,12 +19639,12 @@ service_monitor | sed '/^$/d' | wc -l`])

 ovn-sbctl dump-flows sw0 | grep ct_lb | grep priority=120 > lflows.txt

 AT_CHECK([cat lflows.txt], [0], [dnl

-  table=11(ls_in_stateful     ), priority=120  , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)

+  table=11(ls_in_stateful     ), priority=120  , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)

 ])

 ovn-sbctl dump-flows lr0 | grep ct_lb | grep priority=120 > lflows.txt

 AT_CHECK([cat lflows.txt], [0], [dnl

-  table=6 (lr_in_dnat         ), priority=120  , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80 && is_chassis_resident("cr-lr0-public")), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80);)

+  table=6 (lr_in_dnat         ), priority=120  , match=(ct.new && ip && ip4.dst == 10.0.0.10 && tcp && tcp.dst == 80 && is_chassis_resident("cr-lr0-public")), action=(ct_lb(backends=10.0.0.3:80,20.0.0.3:80; hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)

 ])

 # get the svc monitor mac.