On 9/10/24 19:25, Mark Michelson wrote:
> Excellent, thanks Lorenzo!
>
> Acked-by: Mark Michelson <[email protected]>
>
Thanks, Lorenzo, Ales and Mark! I applied this to main and 24.09 with a
small change to the test (I think the original version had a chance of
being flaky):
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index a43c6cffb7..a2e451880c 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -3145,13 +3145,16 @@ check_ct_zone_min() {
check_ct_zone_max() {
max_val=$1
- AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk '{printf
"%02d\n", $2}' | sort | tail -n1) -le ${max_val}])
+ OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
'{printf "%02d\n", $2}' | sort | tail -n1) -le ${max_val}])
}
net_add n1
sim_add hv1
as hv1
check ovs-vsctl add-br br-phys
+check ovs-appctl vlog/disable-rate-limit
+check ovs-appctl vlog/set vconn:DBG
+
ovn_attach n1 br-phys 192.168.0.1
check ovn-appctl -t ovn-controller vlog/set dbg:ct_zone
@@ -3184,18 +3187,21 @@ check ovn-nbctl lsp-set-options ls1-lr
router-port=lr-ls1
check ovn-nbctl lsp-add ls1 lsp1
check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:02 172.16.0.2"
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
-# Check regular boundaries
+AS_BOX([Check regular boundaries])
check_ct_zone_min 1
check_ct_zone_max 12
-# increase boundaries
+AS_BOX([Increase boundaries])
ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-30\"
+
check_ct_zone_min 10
check_ct_zone_max 22
-# Reset min boundary
-ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\"
+AS_BOX([Reset min boundary])
+check ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\"
# Add a new port to the ls0 switch
check ovs-vsctl add-port br-int lsp2 \
@@ -3206,13 +3212,14 @@ check_ct_zone_min 5
check_ct_zone_max 22
check ovn-nbctl set logical_router lr options:snat-ct-zone=2
+wait_for_ports_up
+check ovn-nbctl --wait=hv sync
+
check_ct_zone_min 2
check_ct_zone_max 22
-# Check lr-snat zone value
-AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
'/lr_snat/{print $2}') -eq 2])
-check ovs-appctl vlog/disable-rate-limit
-check ovs-appctl vlog/set vconn:DBG
+AS_BOX([Check LR snat requested zone 2])
+AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
'/lr_snat/{print $2}') -eq 2])
n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log)
check ovn-appctl -t ovn-controller exit --restart
@@ -3220,12 +3227,15 @@ check ovn-appctl -t ovn-controller exit --restart
OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
"running"])
start_daemon ovn-controller --verbose="ct_zone:dbg"
wait_for_ports_up
+check ovn-nbctl --wait=hv sync
# Check we do not have unexpected ct-flush restarting ovn-controller
AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}])
-# snat-ct-zone 0 is allowed
+AS_BOX([Check LR snat allowed requested zone 0])
check ovn-nbctl set logical_router lr options:snat-ct-zone=0
+check ovn-nbctl --wait=hv sync
+
check_ct_zone_min 0
check_ct_zone_max 22
AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
'/lr_snat/{print $2}') -eq 0])
@@ -3236,6 +3246,7 @@ check ovn-appctl -t ovn-controller exit --restart
OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
"running"])
start_daemon ovn-controller --verbose="ct_zone:dbg"
wait_for_ports_up
+check ovn-nbctl --wait=hv sync
# Check we do not have unexpected ct-flush restarting ovn-controller
AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}])
---
Regards,
Dumitru
> On 9/10/24 09:51, Lorenzo Bianconi wrote:
>> Do not check if snat-ct-zones are compliant with min/max boundary since
>> they have been explicitly requested by the CMS and this will trigger an
>> unnecessary NXT_CT_FLUSH_ZONE of that zones on ovn-controller restart.
>>
>> Fixes: f2363f49f6a4 ("controller: Add the capability to specify a
>> min/max value for ct_zone.")
>> Fixes: 493ef704a973 ("controller: Prepare structure around CT zone
>> limiting.")
>> Reported-at: https://issues.redhat.com/browse/FDP-773
>> Co-developed-by: Ales Musil <[email protected]>
>> Signed-off-by: Lorenzo Bianconi <[email protected]>
>> ---
>> controller/ct-zone.c | 14 ++++---
>> tests/ovn-controller.at | 89 ++++++++++++++++++++++++++++++-----------
>> 2 files changed, 75 insertions(+), 28 deletions(-)
>>
>> diff --git a/controller/ct-zone.c b/controller/ct-zone.c
>> index 77eb16ac9..469a8fc54 100644
>> --- a/controller/ct-zone.c
>> +++ b/controller/ct-zone.c
>> @@ -216,12 +216,15 @@ ct_zones_update(const struct sset *local_lports,
>> struct shash_node *node;
>> SHASH_FOR_EACH_SAFE (node, &ctx->current) {
>> struct ct_zone *ct_zone = node->data;
>> - if (!sset_contains(&all_users, node->name) ||
>> - ct_zone->zone < min_ct_zone || ct_zone->zone >
>> max_ct_zone) {
>> + if (!sset_contains(&all_users, node->name)) {
>> ct_zone_remove(ctx, node->name);
>> } else if (!simap_find(&req_snat_zones, node->name)) {
>> - bitmap_set1(unreq_snat_zones_map, ct_zone->zone);
>> - simap_put(&unreq_snat_zones, node->name, ct_zone->zone);
>> + if (ct_zone->zone < min_ct_zone || ct_zone->zone >
>> max_ct_zone) {
>> + ct_zone_remove(ctx, node->name);
>> + } else {
>> + bitmap_set1(unreq_snat_zones_map, ct_zone->zone);
>> + simap_put(&unreq_snat_zones, node->name, ct_zone->zone);
>> + }
>> }
>> }
>> @@ -249,10 +252,11 @@ ct_zones_update(const struct sset *local_lports,
>> struct ct_zone *ct_zone = shash_find_data(&ctx->current,
>> snat_req_node->name);
>> + bool flush = !(ct_zone && ct_zone->zone == snat_req_node->data);
>> if (ct_zone && ct_zone->zone != snat_req_node->data) {
>> ct_zone_remove(ctx, snat_req_node->name);
>> }
>> - ct_zone_add(ctx, snat_req_node->name, snat_req_node->data,
>> true);
>> + ct_zone_add(ctx, snat_req_node->name, snat_req_node->data,
>> flush);
>> }
>> /* xxx This is wasteful to assign a zone to each port--even if no
>> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
>> index dcab5f2e9..a43c6cffb 100644
>> --- a/tests/ovn-controller.at
>> +++ b/tests/ovn-controller.at
>> @@ -3140,12 +3140,12 @@ ovn_start
>> check_ct_zone_min() {
>> min_val=$1
>> - OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list
>> | awk '{print $2}' | sort | head -n1) -ge ${min_val}])
>> + OVS_WAIT_UNTIL([test $(ovn-appctl -t ovn-controller ct-zone-list
>> | awk '{printf "%02d\n", $2}' | sort | head -n1) -ge ${min_val}])
>> }
>> check_ct_zone_max() {
>> max_val=$1
>> - AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
>> '{print $2}' | sort | tail -n1) -le ${max_val}])
>> + AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
>> '{printf "%02d\n", $2}' | sort | tail -n1) -le ${max_val}])
>> }
>> net_add n1
>> @@ -3158,44 +3158,87 @@ check ovn-appctl -t ovn-controller vlog/set
>> dbg:ct_zone
>> check ovs-vsctl add-port br-int lsp0 \
>> -- set Interface lsp0 external-ids:iface-id=lsp0
>> +check ovs-vsctl add-port br-int lsp1 \
>> + -- set Interface lsp1 external-ids:iface-id=lsp1
>> check ovn-nbctl lr-add lr
>> +check ovn-nbctl ls-add ls0
>> +check ovn-nbctl ls-add ls1
>> -check ovn-nbctl ls-add ls
>> -check ovn-nbctl lsp-add ls ls-lr
>> -check ovn-nbctl lsp-set-type ls-lr router
>> -check ovn-nbctl lsp-set-addresses ls-lr router
>> -check ovn-nbctl lrp-add lr lr-ls 00:00:00:00:00:01 10.0.0.1
>> +check ovn-nbctl set logical_router lr options:chassis=hv1
>> +
>> +check ovn-nbctl lrp-add lr lr-ls0 00:00:00:00:ff:01 10.0.0.1/24
>> +check ovn-nbctl lsp-add ls0 ls0-lr
>> +check ovn-nbctl lsp-set-type ls0-lr router
>> +check ovn-nbctl lsp-set-addresses ls0-lr 00:00:00:00:ff:01
>> +check ovn-nbctl lsp-set-options ls0-lr router-port=lr-ls0
>> -check ovn-nbctl lsp-add ls lsp0
>> +check ovn-nbctl lsp-add ls0 lsp0
>> check ovn-nbctl lsp-set-addresses lsp0 "00:00:00:00:00:02 10.0.0.2"
>> -check ovn-nbctl lrp-add lr lrp-gw 01:00:00:00:00:01 172.16.0.1
>> -check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1
>> +check ovn-nbctl lrp-add lr lr-ls1 00:00:00:00:ff:02 172.16.0.1/24
>> +check ovn-nbctl lsp-add ls1 ls1-lr
>> +check ovn-nbctl lsp-set-type ls1-lr router
>> +check ovn-nbctl lsp-set-addresses ls1-lr 00:00:00:00:ff:02
>> +check ovn-nbctl lsp-set-options ls1-lr router-port=lr-ls1
>> +
>> +check ovn-nbctl lsp-add ls1 lsp1
>> +check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:02 172.16.0.2"
>> -# check regular boundaries
>> +# Check regular boundaries
>> check_ct_zone_min 1
>> -check_ct_zone_max 10
>> +check_ct_zone_max 12
>> # increase boundaries
>> -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-20\"
>> +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"10-30\"
>> check_ct_zone_min 10
>> -check_ct_zone_max 20
>> +check_ct_zone_max 22
>> -# reset min boundary
>> -ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-20\"
>> +# Reset min boundary
>> +ovs-vsctl set Open_vSwitch . external_ids:ct-zone-range=\"5-30\"
>> -# add a new port to the switch
>> -check ovs-vsctl add-port br-int lsp1 \
>> - -- set Interface lsp1 external-ids:iface-id=lsp1
>> -check ovn-nbctl lsp-add ls lsp1
>> -check ovn-nbctl lsp-set-addresses lsp1 "00:00:00:00:00:03 10.0.0.3"
>> +# Add a new port to the ls0 switch
>> +check ovs-vsctl add-port br-int lsp2 \
>> + -- set Interface lsp2 external-ids:iface-id=lsp2
>> +check ovn-nbctl lsp-add ls0 lsp2
>> +check ovn-nbctl lsp-set-addresses lsp2 "00:00:00:00:00:03 10.0.0.3"
>> check_ct_zone_min 5
>> -check_ct_zone_max 20
>> +check_ct_zone_max 22
>> check ovn-nbctl set logical_router lr options:snat-ct-zone=2
>> check_ct_zone_min 2
>> -check_ct_zone_max 20
>> +check_ct_zone_max 22
>> +# Check lr-snat zone value
>> +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
>> '/lr_snat/{print $2}') -eq 2])
>> +
>> +check ovs-appctl vlog/disable-rate-limit
>> +check ovs-appctl vlog/set vconn:DBG
>> +
>> +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log)
>> +check ovn-appctl -t ovn-controller exit --restart
>> +# Make sure ovn-controller stopped before restarting it
>> +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)"
>> != "running"])
>> +start_daemon ovn-controller --verbose="ct_zone:dbg"
>> +wait_for_ports_up
>> +
>> +# Check we do not have unexpected ct-flush restarting ovn-controller
>> +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}])
>> +
>> +# snat-ct-zone 0 is allowed
>> +check ovn-nbctl set logical_router lr options:snat-ct-zone=0
>> +check_ct_zone_min 0
>> +check_ct_zone_max 22
>> +AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk
>> '/lr_snat/{print $2}') -eq 0])
>> +
>> +n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log)
>> +check ovn-appctl -t ovn-controller exit --restart
>> +# Make sure ovn-controller stopped before restarting it
>> +OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)"
>> != "running"])
>> +start_daemon ovn-controller --verbose="ct_zone:dbg"
>> +wait_for_ports_up
>> +
>> +# Check we do not have unexpected ct-flush restarting ovn-controller
>> +AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}])
>> OVN_CLEANUP([hv1])
>> AT_CLEANUP
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
