From 7aa75981dfc17eb7f0ac9ee7300e346f3b6a0c8e Mon Sep 17 00:00:00 2001 From: Numan Siddique 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 Acked-by: Dumitru Ceara Signed-off-by: Numan Siddique (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