From 7aa75981dfc17eb7f0ac9ee7300e346f3b6a0c8e Mon Sep 17 00:00:00 2001

From: Numan Siddique <numans@ovn.org>

Date: Tue, 7 Jul 2020 18:30:20 +0530

Subject: [PATCH 1/5] ovn-northd: Fix the missing lflow issue in LS_OUT_PRE_LB.

When load balancer(s) configured with VIPs are associated to a logical switch,

then ovn-northd adds the below logical flow so that the packets in the egress

switch pipeline enter the conntrack.

table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]] = 1; next;)

ovn-northd maintains a local boolean variable 'vip_configured' in

build_pre_lb() and adds the above lflow if this is true at the end.

But this variable is overriden as -> vip_configured = !!lb->n_vips;

when it loops through every load balancer associated with the logical switch.

This is wrong and this patch fixes this issue.

A test case is addd to test this scenario and the test case fails without the

fix in this patch.

Fixes: bb9f2b9ce56c("ovn-northd: Consider load balancer active backends in router pipeline")

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1849162

Reported-by: Tim Rozet <trozet@redhat.com>

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

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

(cherry-picked from master commit 59af6f9048946e16813ad7ad4e453b85989670e4)

---

 northd/ovn-northd.c |  2 +-

 tests/ovn-northd.at | 73 +++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c

index 8a809d020..2b891c68f 100644

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

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

@@ -4932,7 +4932,7 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows,

              * table, we will eventually look at L4 information. */

         }

-        vip_configured = !!lb->n_vips;

+        vip_configured = (vip_configured || lb->n_vips);

     }

     /* 'REGBIT_CONNTRACK_DEFRAG' is set to let the pre-stateful table send

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at

index 37805d3d8..842800b90 100644

--- a/tests/ovn-northd.at

+++ b/tests/ovn-northd.at

@@ -1485,3 +1485,76 @@ lsp2

 ])

 AT_CLEANUP

+

+# This test case tests that when a logical switch has load balancers associated

+# (with VIPs configured), the below logical flow is added by ovn-northd.

+# table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]] = 1; next;)

+# This test case is added for the BZ -

+# https://bugzilla.redhat.com/show_bug.cgi?id=1849162

+#

+# ovn-northd was not adding the above lflow if the last load balancer associated

+# to the logical switch doesn't have the VIP configured even if other load

+# balancers before the last one in the last have VIPs configured.

+# So make sure that the above lflow is added even if one load balancer has VIP

+# associated.

+

+AT_SETUP([ovn -- Load balancer - missing ls_out_pre_lb flows])

+ovn_start

+

+ovn-nbctl ls-add sw0

+ovn-nbctl lsp-add sw0 sw0-p1

+

+ovn-nbctl lb-add lb1 "10.0.0.10" "10.0.0.3"

+ovn-nbctl lb-add lb2 "10.0.0.11" "10.0.0.4"

+

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+])

+

+ovn-nbctl ls-lb-add sw0 lb1

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+  table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]] = 1; next;)

+])

+

+ovn-nbctl ls-lb-add sw0 lb2

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+  table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]] = 1; next;)

+])

+

+lb1_uuid=$(ovn-nbctl --bare --columns _uuid find load_balancer name=lb1)

+lb2_uuid=$(ovn-nbctl --bare --columns _uuid find load_balancer name=lb2)

+

+ovn-nbctl clear load_balancer $lb1_uuid vips

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+  table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]] = 1; next;)

+])

+

+ovn-nbctl clear load_balancer $lb2_uuid vips

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+])

+

+ovn-nbctl set load_balancer $lb1_uuid vips:"10.0.0.10"="10.0.0.3"

+ovn-nbctl set load_balancer $lb2_uuid vips:"10.0.0.11"="10.0.0.4"

+

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+  table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]] = 1; next;)

+])

+

+# Now reverse the order of clearing the vip.

+ovn-nbctl clear load_balancer $lb2_uuid vips

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+  table=0 (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]] = 1; next;)

+])

+

+ovn-nbctl clear load_balancer $lb1_uuid vips

+ovn-nbctl --wait=sb sync

+AT_CHECK([ovn-sbctl lflow-list | grep "ls_out_pre_lb.*priority=100" | grep reg0 | sort], [0], [dnl

+])

+

+AT_CLEANUP

-- 

2.26.2