Fix the follwoing 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 <[email protected]>
Fixes: 5af304e747 ("Support selection fields in load balancer.")
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Changes since v1:
- introduce unit-test
---
 northd/ovn-northd.c | 5 ++++-
 tests/ovn.at        | 6 ++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 996aebeb6..b039d1851 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3600,6 +3600,8 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip 
*lb_vip,
                                        struct ds *action,
                                        char *selection_fields)
 {
+    bool skip_hash_fields = false;
+
     if (lb_vip->health_check) {
         ds_put_cstr(action, "ct_lb(backends=");
 
@@ -3618,6 +3620,7 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip 
*lb_vip,
         }
 
         if (!n_active_backends) {
+            skip_hash_fields = true;
             ds_clear(action);
             ds_put_cstr(action, "drop;");
         } else {
@@ -3628,7 +3631,7 @@ static void build_lb_vip_ct_lb_actions(struct lb_vip 
*lb_vip,
         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);
diff --git a/tests/ovn.at b/tests/ovn.at
index 94bd379b4..4c770ce79 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -19326,6 +19326,8 @@ check ovn-nbctl lsp-set-addresses sw1-lr0 router
 check ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
 
 check 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)
+check ovn-nbctl set load_balancer ${OVN_LB_ID} 
selection_fields="ip_dst,ip_src,tp_dst,tp_src"
 
 check ovn-nbctl --wait=sb set load_balancer . 
ip_port_mappings:10.0.0.3=sw0-p1:10.0.0.2
 check ovn-nbctl --wait=sb set load_balancer . 
ip_port_mappings:20.0.0.3=sw1-p1:20.0.0.2
@@ -19365,14 +19367,14 @@ AT_CAPTURE_FILE([sbflows])
 OVS_WAIT_FOR_OUTPUT(
   [ovn-sbctl dump-flows > sbflows
    ovn-sbctl dump-flows sw0 | grep ct_lb | grep priority=120 | sed 
's/table=..//'], 0,
-  [  (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);)
+  [  (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");)
 ])
 
 AT_CAPTURE_FILE([sbflows2])
 OVS_WAIT_FOR_OUTPUT(
   [ovn-sbctl dump-flows > sbflows2
    ovn-sbctl dump-flows lr0 | grep ct_lb | grep priority=120 | sed 
's/table=..//'], 0,
-  [  (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);)
+  [  (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.
-- 
2.26.2

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to