Skip to content

Commit

Permalink
controller: Add config option per LB to enable/disable CT flush
Browse files Browse the repository at this point in the history
The CT flush was enabled by default for every LB, add
config option called "ct_flush" that allows
users to enable/disable the CT flush. The CT flush option
is set to "false" by default.

Reported-at: https://bugzilla.redhat.com/2178962
Acked-by: Numan Siddique <numans@ovn.org>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ales Musil <amusil@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
almusil authored and dceara committed Mar 20, 2023
1 parent 763e05c commit 1ad7791
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 11 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ Post v23.03.0
-------------
- Enhance LSP.options:arp_proxy to support IPv6, configurable MAC
addresses and CIDRs.
- CT entries are not flushed by default anymore whenever a load balancer
backend is removed. A new, per-LB, option 'ct_flush' can be used to
restore the previous behavior. Disabled by default.

OVN v23.03.0 - 03 Mar 2023
--------------------------
Expand Down
6 changes: 4 additions & 2 deletions controller/ovn-controller.c
Original file line number Diff line number Diff line change
Expand Up @@ -2697,7 +2697,8 @@ static void
lb_data_removed_five_tuples_add(struct ed_type_lb_data *lb_data,
const struct ovn_controller_lb *lb)
{
if (!ovs_feature_is_supported(OVS_CT_TUPLE_FLUSH_SUPPORT)) {
if (!ovs_feature_is_supported(OVS_CT_TUPLE_FLUSH_SUPPORT) ||
!lb->ct_flush) {
return;
}

Expand All @@ -2716,7 +2717,8 @@ static void
lb_data_removed_five_tuples_remove(struct ed_type_lb_data *lb_data,
const struct ovn_controller_lb *lb)
{
if (!ovs_feature_is_supported(OVS_CT_TUPLE_FLUSH_SUPPORT)) {
if (!ovs_feature_is_supported(OVS_CT_TUPLE_FLUSH_SUPPORT) ||
!lb->ct_flush) {
return;
}

Expand Down
1 change: 1 addition & 0 deletions lib/lb.c
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ ovn_controller_lb_create(const struct sbrec_load_balancer *sbrec_lb,
lb->hairpin_orig_tuple = smap_get_bool(&sbrec_lb->options,
"hairpin_orig_tuple",
false);
lb->ct_flush = smap_get_bool(&sbrec_lb->options, "ct_flush", false);
ovn_lb_get_hairpin_snat_ip(&sbrec_lb->header_.uuid, &sbrec_lb->options,
&lb->hairpin_snat_ips);
return lb;
Expand Down
1 change: 1 addition & 0 deletions lib/lb.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ struct ovn_controller_lb {
bool hairpin_orig_tuple; /* True if ovn-northd stores the original
* destination tuple in registers.
*/
bool ct_flush; /* True if we should flush CT after backend removal. */

struct lport_addresses hairpin_snat_ips; /* IP (v4 and/or v6) to be used
* as source for hairpinned
Expand Down
8 changes: 8 additions & 0 deletions ovn-nb.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2041,6 +2041,14 @@ or
the affinity timeslot. Max supported affinity_timeout is 65535
seconds.
</column>

<column name="options" key="ct_flush" type='{"type": "boolean"}'>
The value indicates whether ovn-controller should flush CT entries
that are related to this LB. The flush happens if the LB is removed,
any of the backends is updated/removed or the LB is not considered
local anymore by the ovn-controller. This option is set to
<code>false</code> by default.
</column>
</group>
</table>

Expand Down
28 changes: 25 additions & 3 deletions tests/ovn.at
Original file line number Diff line number Diff line change
Expand Up @@ -34997,7 +34997,8 @@ check ovs-vsctl add-port br-int p1 -- set interface p1 external_ids:iface-id=lsp
wait_for_ports_up
ovn-nbctl --wait=hv sync

check ovn-nbctl lb-add lb1 "192.168.0.10" "192.168.10.10,192.168.10.20"
check ovn-nbctl lb-add lb1 "192.168.0.10" "192.168.10.10,192.168.10.20" \
-- set load_balancer lb1 options:ct_flush="true"
check ovn-nbctl ls-lb-add sw lb1

# Remove a single backend
Expand All @@ -35020,7 +35021,8 @@ AT_CHECK([grep -q "Flushing CT for 5-tuple: vip=192.168.0.10:0, backend=192.168.
AT_CHECK([grep -q "Flushing CT for 5-tuple: vip=192.168.0.10:0, backend=192.168.10.30:0, protocol=0" hv1/ovn-controller.log], [0])

# Check flush for LB with port and protocol
check ovn-nbctl lb-add lb1 "192.168.30.10:80" "192.168.40.10:8080,192.168.40.20:8090" udp
check ovn-nbctl lb-add lb1 "192.168.30.10:80" "192.168.40.10:8080,192.168.40.20:8090" udp \
-- set load_balancer lb1 options:ct_flush="true"
check ovn-nbctl ls-lb-add sw lb1
check ovn-nbctl lb-del lb1
check ovn-nbctl --wait=hv sync
Expand All @@ -35029,7 +35031,8 @@ AT_CHECK([grep -q "Flushing CT for 5-tuple: vip=192.168.30.10:80, backend=192.16
AT_CHECK([grep -q "Flushing CT for 5-tuple: vip=192.168.30.10:80, backend=192.168.40.20:8090, protocol=17" hv1/ovn-controller.log], [0])

# Check recompute when LB is no longer local
check ovn-nbctl lb-add lb1 "192.168.50.10:80" "192.168.60.10:8080"
check ovn-nbctl lb-add lb1 "192.168.50.10:80" "192.168.60.10:8080" \
-- set load_balancer lb1 options:ct_flush="true"
check ovn-nbctl ls-lb-add sw lb1
check ovs-vsctl remove interface p1 external_ids iface-id
check ovn-appctl inc-engine/recompute
Expand All @@ -35039,6 +35042,25 @@ AT_CHECK([grep -q "Flushing CT for 5-tuple: vip=192.168.50.10:80, backend=192.16

AT_CHECK([test "$(grep -c "Flushing CT for 5-tuple" hv1/ovn-controller.log)" = "6"], [0])

# Check if CT flush is disabled by default
check ovn-nbctl lb-del lb1
check ovn-nbctl lb-add lb1 "192.168.70.10:80" "192.168.80.10:8080,192.168.90.10:8080"
check ovn-nbctl ls-lb-add sw lb1
check ovs-vsctl set interface p1 external_ids:iface-id=lsp1
check ovn-nbctl --wait=hv sync

AT_CHECK([test "$(grep -c "Flushing CT for 5-tuple" hv1/ovn-controller.log)" = "6"], [0])

# Remove one backend
check ovn-nbctl --wait=hv set load_balancer lb1 vips='"192.168.70.10:80"="192.168.80.10:8080"'

AT_CHECK([grep -q "Flushing CT for 5-tuple: vip=192.168.70.10:80, backend=192.168.90.10:8080, protocol=6" hv1/ovn-controller.log], [1])
AT_CHECK([test "$(grep -c "Flushing CT for 5-tuple" hv1/ovn-controller.log)" = "6"], [0])

check ovn-nbctl --wait=hv lb-del lb1
AT_CHECK([grep -q "Flushing CT for 5-tuple: vip=192.168.70.10:80, backend=192.168.80.10:8080, protocol=6" hv1/ovn-controller.log], [1])
AT_CHECK([test "$(grep -c "Flushing CT for 5-tuple" hv1/ovn-controller.log)" = "6"], [0])

OVN_CLEANUP([hv1])
AT_CLEANUP
])
36 changes: 30 additions & 6 deletions tests/system-ovn.at
Original file line number Diff line number Diff line change
Expand Up @@ -9993,11 +9993,13 @@ check ovn-nbctl lsp-add bar bar3 \
-- lsp-set-addresses bar3 "f0:00:0f:01:02:05 172.16.1.4"

# Config OVN load-balancer with a VIP.
check ovn-nbctl lb-add lb1 30.0.0.1 "172.16.1.2,172.16.1.3,172.16.1.4"
check ovn-nbctl lb-add lb1 30.0.0.1 "172.16.1.2,172.16.1.3,172.16.1.4" \
-- set load_balancer lb1 options:ct_flush="true"
check ovn-nbctl ls-lb-add foo lb1

# Create another load-balancer with another VIP.
lb2_uuid=`ovn-nbctl create load_balancer name=lb2 vips:30.0.0.3="172.16.1.2,172.16.1.3,172.16.1.4"`
check ovn-nbctl set load_balancer lb2 options:ct_flush="true"
check ovn-nbctl ls-lb-add foo lb2

# Config OVN load-balancer with another VIP (this time with ports).
Expand All @@ -10013,16 +10015,18 @@ OVS_START_L7([bar1], [http])
OVS_START_L7([bar2], [http])
OVS_START_L7([bar3], [http])

OVS_WAIT_FOR_OUTPUT([
for i in `seq 1 20`; do
ip netns exec foo1 wget 30.0.0.1 -t 5 -T 1 --retry-connrefused -v -o wget$i.log;
done
ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
m4_define([LB1_CT_ENTRIES], [dnl
tcp,orig=(src=192.168.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=172.16.1.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
tcp,orig=(src=192.168.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=172.16.1.3,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
tcp,orig=(src=192.168.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=172.16.1.4,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
])

OVS_WAIT_FOR_OUTPUT([
for i in `seq 1 20`; do
ip netns exec foo1 wget 30.0.0.1 -t 5 -T 1 --retry-connrefused -v -o wget$i.log;
done
ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [LB1_CT_ENTRIES])

OVS_WAIT_FOR_OUTPUT([
for i in `seq 1 20`; do
ip netns exec foo1 wget 30.0.0.2:8000 -t 5 -T 1 --retry-connrefused -v -o wget$i.log;
Expand Down Expand Up @@ -10096,6 +10100,26 @@ check ovn-nbctl lb-del lb2

OVS_WAIT_UNTIL([test "$(ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.3) | wc -l)" = "0"])

# Check that LB has CT flush disabled by default
check ovn-nbctl lb-add lb1 30.0.0.1 "172.16.1.2,172.16.1.3,172.16.1.4"
check ovn-nbctl ls-lb-add foo lb1

OVS_WAIT_FOR_OUTPUT([
for i in `seq 1 20`; do
ip netns exec foo1 wget 30.0.0.1 -t 5 -T 1 --retry-connrefused -v -o wget$i.log;
done
ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [LB1_CT_ENTRIES])

# Remove one backend
check ovn-nbctl --wait=hv set load_balancer lb1 vips='"30.0.0.1"="172.16.1.2,172.16.1.3"'

AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [LB1_CT_ENTRIES])

# Remove whole LB
check ovn-nbctl --wait=hv lb-del lb1

AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [LB1_CT_ENTRIES])

OVS_APP_EXIT_AND_WAIT([ovn-controller])

as ovn-sb
Expand Down

0 comments on commit 1ad7791

Please sign in to comment.