diff mbox series

[ovs-dev,v4.1,4/7] tests: Eliminate most "sleep" calls.

Message ID 20201111012454.2574461-5-blp@ovn.org
State Superseded
Headers show
Series Add DDlog implementation of ovn-northd | expand

Commit Message

Ben Pfaff Nov. 11, 2020, 1:24 a.m. UTC 
Many of these could be replaced by "ovn-nbctl sync".  Some weren't
really needed at all because they were adjacent to something that itself
called sync or otherwise used --wait.  Some were more appropriately
done with explicit waits for what was really needed.

I left some "sleep"s.  Some were because they were "negative" sleeps:
they were giving time for something to happen that should *not* happen
(in other words, if you wait for it to happen, you'll wait forever,
unless there's a bug).  Some were because I didn't know how to properly
wait for what they were waiting for, or because I didn't understand
the circumstances deeply enough.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 tests/ovn-northd.at |  11 ++-
 tests/ovn.at        | 167 ++++++++++++--------------------------------
 2 files changed, 54 insertions(+), 124 deletions(-)

Comments

Dumitru Ceara Nov. 11, 2020, 9:23 a.m. UTC | #1 
On 11/11/20 2:24 AM, Ben Pfaff wrote:
> Many of these could be replaced by "ovn-nbctl sync".  Some weren't
> really needed at all because they were adjacent to something that itself
> called sync or otherwise used --wait.  Some were more appropriately
> done with explicit waits for what was really needed.
> 
> I left some "sleep"s.  Some were because they were "negative" sleeps:
> they were giving time for something to happen that should *not* happen
> (in other words, if you wait for it to happen, you'll wait forever,
> unless there's a bug).  Some were because I didn't know how to properly
> wait for what they were waiting for, or because I didn't understand
> the circumstances deeply enough.
> 
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---

As mentioned in the discussion on the V3 of this series, this also needs
the following incremental to make the "ovn -- /32 router IP address"
test less racy.

Thanks,
Dumtiru

diff --git a/tests/ovn.at b/tests/ovn.at
index d30f55622..e12f3b668 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -11074,6 +11074,9 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
 OVN_POPULATE_ARP

 # Allow some time for ovn-northd and ovn-controller to catch up.
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up foo1` = xup])
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up alice1` = xup])
+
 check ovn-nbctl --wait=hv sync

 ovn-sbctl dump-flows > sbflows
Dumitru Ceara Nov. 11, 2020, 9:41 a.m. UTC | #2 
On 11/11/20 10:23 AM, Dumitru Ceara wrote:
> On 11/11/20 2:24 AM, Ben Pfaff wrote:
>> Many of these could be replaced by "ovn-nbctl sync".  Some weren't
>> really needed at all because they were adjacent to something that itself
>> called sync or otherwise used --wait.  Some were more appropriately
>> done with explicit waits for what was really needed.
>>
>> I left some "sleep"s.  Some were because they were "negative" sleeps:
>> they were giving time for something to happen that should *not* happen
>> (in other words, if you wait for it to happen, you'll wait forever,
>> unless there's a bug).  Some were because I didn't know how to properly
>> wait for what they were waiting for, or because I didn't understand
>> the circumstances deeply enough.
>>
>> Signed-off-by: Ben Pfaff <blp@ovn.org>
>> ---
> 
> As mentioned in the discussion on the V3 of this series, this also needs
> the following incremental to make the "ovn -- /32 router IP address"
> test less racy.
> 
> Thanks,
> Dumtiru
> 
> diff --git a/tests/ovn.at b/tests/ovn.at
> index d30f55622..e12f3b668 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -11074,6 +11074,9 @@ ovs-vsctl -- add-port br-int hv2-vif1 -- \
>  OVN_POPULATE_ARP
> 
>  # Allow some time for ovn-northd and ovn-controller to catch up.
> +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up foo1` = xup])
> +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up alice1` = xup])
> +
>  check ovn-nbctl --wait=hv sync
> 
>  ovn-sbctl dump-flows > sbflows
> 

Actually, I think something like this might need to be done for more
tests in ovn.at now that the "sleep"s are removed.  It's not really a
problem introduced by your patch but if we apply it as is I'm afraid we
make the tests racier than they already are.

Thanks,
Dumitru
Ben Pfaff Nov. 12, 2020, 12:33 a.m. UTC | #3 
On Wed, Nov 11, 2020 at 10:23:02AM +0100, Dumitru Ceara wrote:
> On 11/11/20 2:24 AM, Ben Pfaff wrote:
> > Many of these could be replaced by "ovn-nbctl sync".  Some weren't
> > really needed at all because they were adjacent to something that itself
> > called sync or otherwise used --wait.  Some were more appropriately
> > done with explicit waits for what was really needed.
> > 
> > I left some "sleep"s.  Some were because they were "negative" sleeps:
> > they were giving time for something to happen that should *not* happen
> > (in other words, if you wait for it to happen, you'll wait forever,
> > unless there's a bug).  Some were because I didn't know how to properly
> > wait for what they were waiting for, or because I didn't understand
> > the circumstances deeply enough.
> > 
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > ---
> 
> As mentioned in the discussion on the V3 of this series, this also needs
> the following incremental to make the "ovn -- /32 router IP address"
> test less racy.

Oh, thanks!

I actually this we could use a way to wait for ports to come up (and
then use it many places).  Here's one way:

    # wait_for_ports_up [PORT...]
    #
    # With arguments, waits for specified Logical_Switch_Ports to come up.
    # Without arguments, waits for all Logical_Switch_Ports (except
    # localnet, localport, and virtual ports) to come up.
    wait_for_ports_up() {
        if test $# = 0; then
            wait_row_count nb:Logical_Switch_Port 0 up!=true type!=localnet type!=localport type!=virtual
        else
            for port; do
                wait_row_count nb:Logical_Switch_Port 1 up=true name=$port
            done
        fi
    }

I'm going to fold this into my series.
Ben Pfaff Nov. 12, 2020, 1:46 a.m. UTC | #4 
On Wed, Nov 11, 2020 at 04:33:19PM -0800, Ben Pfaff wrote:
> On Wed, Nov 11, 2020 at 10:23:02AM +0100, Dumitru Ceara wrote:
> > On 11/11/20 2:24 AM, Ben Pfaff wrote:
> > > Many of these could be replaced by "ovn-nbctl sync".  Some weren't
> > > really needed at all because they were adjacent to something that itself
> > > called sync or otherwise used --wait.  Some were more appropriately
> > > done with explicit waits for what was really needed.
> > > 
> > > I left some "sleep"s.  Some were because they were "negative" sleeps:
> > > they were giving time for something to happen that should *not* happen
> > > (in other words, if you wait for it to happen, you'll wait forever,
> > > unless there's a bug).  Some were because I didn't know how to properly
> > > wait for what they were waiting for, or because I didn't understand
> > > the circumstances deeply enough.
> > > 
> > > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > > ---
> > 
> > As mentioned in the discussion on the V3 of this series, this also needs
> > the following incremental to make the "ovn -- /32 router IP address"
> > test less racy.
> 
> Oh, thanks!
> 
> I actually this we could use a way to wait for ports to come up (and
> then use it many places).  Here's one way:
> 
>     # wait_for_ports_up [PORT...]
>     #
>     # With arguments, waits for specified Logical_Switch_Ports to come up.
>     # Without arguments, waits for all Logical_Switch_Ports (except
>     # localnet, localport, and virtual ports) to come up.
>     wait_for_ports_up() {
>         if test $# = 0; then
>             wait_row_count nb:Logical_Switch_Port 0 up!=true type!=localnet type!=localport type!=virtual
>         else
>             for port; do
>                 wait_row_count nb:Logical_Switch_Port 1 up=true name=$port
>             done
>         fi
>     }
> 
> I'm going to fold this into my series.

I took care of this in the v5 that I just posted.
diff mbox series

Patch

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 949a8eee054e..78b1ff728af3 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -878,7 +878,7 @@  uuid=$(fetch_column Port_Binding _uuid logical_port=cr-DR-S1)
 echo "CR-LRP UUID is: " $uuid
 
 check ovn-nbctl set Logical_Router $cr_uuid options:chassis=gw1
-check ovn-nbctl --wait=hv sync
+check ovn-nbctl --wait=sb sync
 
 ovn-nbctl create Address_Set name=allowed_range addresses=\"1.1.1.1\"
 ovn-nbctl create Address_Set name=disallowed_range addresses=\"2.2.2.2\"
@@ -1120,6 +1120,7 @@  ovn-nbctl --wait=sb -- --id=@hc create \
 Load_Balancer_Health_Check vip="10.0.0.10\:80" -- add Load_Balancer . \
 health_check @hc
 wait_row_count Service_Monitor 2
+check ovn-nbctl --wait=sb sync
 
 ovn-sbctl dump-flows sw0 | grep ct_lb | grep priority=120 > lflows.txt
 AT_CHECK([cat lflows.txt | sed 's/table=..//'], [0], [dnl
@@ -1139,6 +1140,7 @@  OVS_WAIT_FOR_OUTPUT(
 # Set the service monitor for sw1-p1 to offline
 check ovn-sbctl set service_monitor sw1-p1 status=offline
 wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=offline
+check ovn-nbctl --wait=sb sync
 
 AT_CAPTURE_FILE([sbflows4])
 OVS_WAIT_FOR_OUTPUT(
@@ -1150,6 +1152,7 @@  OVS_WAIT_FOR_OUTPUT(
 ovn-sbctl set service_monitor $sm_sw0_p1 status=offline
 
 wait_row_count Service_Monitor 1 logical_port=sw0-p1 status=offline
+check ovn-nbctl --wait=sb sync
 
 AT_CAPTURE_FILE([sbflows5])
 OVS_WAIT_FOR_OUTPUT(
@@ -1166,6 +1169,7 @@  ovn-sbctl set service_monitor $sm_sw0_p1 status=online
 ovn-sbctl set service_monitor $sm_sw1_p1 status=online
 
 wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=online
+check ovn-nbctl --wait=sb sync
 
 AT_CAPTURE_FILE([sbflows7])
 OVS_WAIT_FOR_OUTPUT(
@@ -1176,6 +1180,7 @@  OVS_WAIT_FOR_OUTPUT(
 # Set the service monitor for sw1-p1 to error
 ovn-sbctl set service_monitor $sm_sw1_p1 status=error
 wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=error
+check ovn-nbctl --wait=sb sync
 
 ovn-sbctl dump-flows sw0 | grep "ip4.dst == 10.0.0.10 && tcp.dst == 80" \
 | grep priority=120 > lflows.txt
@@ -1214,6 +1219,7 @@  OVS_WAIT_FOR_OUTPUT(
 check ovn-sbctl set service_monitor sw1-p1 status=online
 
 wait_row_count Service_Monitor 1 logical_port=sw1-p1 status=online
+check ovn-nbctl --wait=sb sync
 
 AT_CAPTURE_FILE([sbflows10])
 OVS_WAIT_FOR_OUTPUT(
@@ -1244,6 +1250,7 @@  AT_CHECK([ovn-nbctl -- --id=@hc create Load_Balancer_Health_Check vip="10.0.0.10
 
 check ovn-nbctl ls-lb-add sw0 lb2
 check ovn-nbctl ls-lb-add sw1 lb2
+check ovn-nbctl --wait=sb sync
 
 wait_row_count Service_Monitor 5
 
@@ -1756,7 +1763,7 @@  check ovn-nbctl pg-add pg0 sw0-p1 sw1-p1
 check ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4 && tcp && tcp.dst == 80" reject
 check ovn-nbctl acl-add pg0 to-lport 1003 "outport == @pg0 && ip6 && udp" reject
 
-check ovn-nbctl --wait=hv sync
+check ovn-nbctl --wait=sb sync
 
 AS_BOX([1])
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 2648cdd6cd07..5d9b3b1a4fa9 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1798,10 +1798,6 @@  check ovn-nbctl --wait=hv sync
 # for ARP resolution).
 OVN_POPULATE_ARP
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
-
 # Make sure there is no attempt to adding duplicated flows by ovn-controller
 AT_FAIL_IF([test -n "`grep duplicate hv1/ovn-controller.log`"])
 AT_FAIL_IF([test -n "`grep duplicate hv2/ovn-controller.log`"])
@@ -1996,11 +1992,7 @@  done
 
 # set address for lp13 with invalid characters.
 # lp13 should be configured with only 192.168.0.13.
-ovn-nbctl lsp-set-addresses lp13 "f0:00:00:00:00:13 192.168.0.13 invalid 192.169.0.13"
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+ovn-nbctl --wait=hv lsp-set-addresses lp13 "f0:00:00:00:00:13 192.168.0.13 invalid 192.169.0.13"
 
 sip=`ip_to_hex 192 168 0 11`
 tip=`ip_to_hex 192 168 0 13`
@@ -2075,7 +2067,11 @@  for i in 1 2; do
     done
 done
 
-sleep 1
+# Wait for bindings to take effect.
+wait_row_count Port_Binding 1 logical_port=lp11 'encap!=[[]]'
+wait_row_count Port_Binding 1 logical_port=lp12 'encap!=[[]]'
+wait_row_count Port_Binding 1 logical_port=lp21 'encap!=[[]]'
+wait_row_count Port_Binding 1 logical_port=lp22 'encap!=[[]]'
 
 # dump port bindings; since we have vxlan and geneve tunnels, we expect the
 # ports to be bound to geneve tunnels.
@@ -2095,9 +2091,7 @@  check_row_count Port_Binding 1 logical_port=lp22 encap=$encap_rec
 # for ARP resolution).
 OVN_POPULATE_ARP
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Make sure there is no attempt to adding duplicated flows by ovn-controller
 AT_FAIL_IF([test -n "`grep duplicate hv1/ovn-controller.log`"])
@@ -3213,9 +3207,7 @@  ovs-vsctl add-port br-phys vif3 -- set Interface vif3 options:tx_pcap=hv3/vif3-t
 # for ARP resolution).
 OVN_POPULATE_ARP
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # test_packet INPORT DST SRC ETHTYPE OUTPORT...
 #
@@ -3381,9 +3373,7 @@  ovs-vsctl add-port br-phys vif3 -- set Interface vif3 options:tx_pcap=hv3/vif3-t
 # for ARP resolution).
 OVN_POPULATE_ARP
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # test_packet INPORT DST SRC ETHTYPE OUTPORT...
 #
@@ -3581,8 +3571,7 @@  check ovn-nbctl --wait=hv sync
 # for ARP resolution).
 OVN_POPULATE_ARP
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
+check ovn-nbctl --wait=hv sync
 
 # test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
 #
@@ -3982,8 +3971,7 @@  done
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
 #
@@ -4157,8 +4145,7 @@  done
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Given the name of a logical port, prints the name of the hypervisor
 # on which it is located.
@@ -4592,8 +4579,7 @@  ovs-vsctl -- add-port br-int hv2-vif1 -- \
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Packet to send.
 packet="inport==\"ls1-lp1\" && eth.src==$ls1_lp1_mac && eth.dst==$rp_ls1_mac &&
@@ -4704,9 +4690,7 @@  ovs-vsctl -- add-port br-int vif2 -- \
     ofport-request=1
 
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Send ip packets between the two ports.
 
@@ -4738,11 +4722,7 @@  as hv1 ovs-ofctl dump-flows br-int
 
 
 #Disable router R1
-ovn-nbctl set Logical_Router R1 enabled=false
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+ovn-nbctl --wait=hv set Logical_Router R1 enabled=false
 
 echo "---------SB dump-----"
 ovn-sbctl list datapath_binding
@@ -4818,10 +4798,8 @@  ovs-vsctl -- add-port br-int vif2 -- \
     options:rxq_pcap=hv1/vif2-rx.pcap \
     ofport-request=1
 
+check ovn-nbctl --wait=hv sync
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
 
 # Send ip packets between the two ports.
 
@@ -4852,7 +4830,7 @@  echo "------ hv1 dump ----------"
 as hv1 ovs-ofctl dump-flows br-int
 
 #Disable router R1
-ovn-nbctl set Logical_Router R1 enabled=false
+ovn-nbctl --wait=hv set Logical_Router R1 enabled=false
 
 echo "---------SB dump-----"
 ovn-sbctl list datapath_binding
@@ -4863,9 +4841,6 @@  echo "---------------------"
 echo "------ hv1 dump ----------"
 as hv1 ovs-ofctl dump-flows br-int
 
-# Allow some time for the disabling of logical router R1 to propagate.
-# XXX This should be more systematic.
-sleep 1
 
 as hv1 ovs-appctl netdev-dummy/receive vif1 $packet
 
@@ -4970,8 +4945,7 @@  ovs-vsctl -- add-port br-int hv2-vif1 -- \
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Send ip packets between foo1 and alice1
 src_mac="f00000010203"
@@ -5193,8 +5167,7 @@  ovs-vsctl -- add-port br-int hv2-vif1 -- \
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Send ip packets between foo1 and alice1
 src_mac="f00000010203"
@@ -5331,7 +5304,7 @@  as hv1 ovs-appctl vlog/set dbg
 
 OVN_POPULATE_ARP
 
-sleep 2
+check ovn-nbctl --wait=hv sync
 
 as hv1 ovs-vsctl show
 
@@ -5996,7 +5969,7 @@  ovs-vsctl -- add-port br-int hv1-vif5 -- \
 
 OVN_POPULATE_ARP
 
-sleep 2
+check ovn-nbctl --wait=hv sync
 
 trim_zeros() {
     sed 's/\(00\)\{1,\}$//'
@@ -6278,10 +6251,7 @@  ovn-nbctl lsp-add foo foo1 \
 ovn-nbctl lsp-add alice alice1 \
 -- lsp-set-addresses alice1 "f0:00:00:01:02:04 172.16.1.2"
 
-
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 2
+check ovn-nbctl --wait=hv sync
 
 # Send ip packets between foo1 and alice1
 src_mac="f00000010203"
@@ -6344,7 +6314,7 @@  ip_prefix=192.168.1.0/24 nexthop=20.0.0.1 -- add Logical_Router \
 R2 static_routes @lrt
 
 # Wait for ovn-controller to catch up.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Send the packet again.
 as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
@@ -6416,10 +6386,8 @@  ovs-vsctl -- add-port br-int vif2 -- \
     options:rxq_pcap=hv1/vif2-rx.pcap \
     ofport-request=1
 
-
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 
 for i in 1 2; do
@@ -6577,10 +6545,6 @@  ovs-vsctl -- add-port br-int vif3 -- \
     options:rxq_pcap=pbr-hv/vif3-rx.pcap \
     ofport-request=1
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
-
 ls1_ro_mac=00:00:00:01:02:f1
 ls1_ro_ip=192.168.1.1
 
@@ -6767,10 +6731,6 @@  ovs-vsctl -- add-port br-int vif3 -- \
     options:rxq_pcap=pbr-hv/vif3-rx.pcap \
     ofport-request=1
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
-
 ls1_ro_mac=00:00:00:01:02:f1
 ls1_ro_ip=2001::1
 
@@ -7025,14 +6985,11 @@  ovn-nbctl acl-add lsw0 to-lport 1002 'outport == "lp1" && ip6 && icmp6'  allow-r
 ovn-nbctl acl-add lsw0 to-lport 1002 'outport == "lp2" && ip6 && icmp6'  allow-related
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
+# XXX The "sleep" here seems to be essential for ovn-northd-ddlog,
+# which may indicate that it needs improvement.
+check ovn-nbctl --wait=hv sync
 sleep 1
 
-# Given the name of a logical port, prints the name of the hypervisor
-# on which it is located.
-vif_to_hv() {
-    echo hv1${1%?}
-}
 for i in 1 2; do
     : > $i.expected
 done
@@ -7073,9 +7030,7 @@  ovn_attach n1 br-phys 192.168.0.1
 
 row=`ovn-nbctl create Address_Set name=set1 addresses=\"1.1.1.1\"`
 ovn-nbctl set Address_Set $row name=set1 addresses=\"1.1.1.1,1.1.1.2\"
-ovn-nbctl destroy Address_Set $row
-
-sleep 1
+ovn-nbctl --wait=hv destroy Address_Set $row
 
 # A bug previously existed in the address set support code
 # that caused ovn-controller to crash after an address set
@@ -7463,8 +7418,7 @@  ovs-vsctl -- add-port br-int hv1-vif3 -- \
     ofport-request=3
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Send ip packets between foo1 and foo2
 src_mac="0a0000a80103"
@@ -7675,8 +7629,7 @@  ovs-vsctl -- add-port br-int hv1-ls2lp2 -- \
     ofport-request=2
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 echo "---------NB dump-----"
 ovn-nbctl show
@@ -8805,8 +8758,7 @@  ovs-vsctl -- add-port br-int vm2 -- \
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Test that ovn-controllers create ct-zone entry for container ports.
 foo1_zoneid=$(as hv1 ovs-vsctl get bridge br-int external_ids:ct-zone-foo1)
@@ -9114,8 +9066,7 @@  ovn-nbctl --wait=hv lsp-add bob bob1 \
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 trim_zeros() {
     sed 's/\(00\)\{1,\}$//'
@@ -9224,7 +9175,7 @@  ovs-vsctl -- add-port br-int hv1-vif2 -- \
     ofport-request=2
 
 OVN_POPULATE_ARP
-sleep 2
+check ovn-nbctl --wait=hv sync
 as hv1 ovs-vsctl show
 
 echo "*************************"
@@ -9786,13 +9737,9 @@  test_ip_packet()
     fi
     as ext1 reset_pcap_file ext1-vif1 ext1/vif1
 
-    sleep 1
-
     # Resend packet from foo1 to outside1
     check as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
 
-    sleep 1
-
     AT_CAPTURE_FILE([exp])
     AT_CAPTURE_FILE([rcv])
     check_packets() {
@@ -9996,8 +9943,7 @@  hv1_ch_uuid=$(fetch_column Chassis _uuid name=hv1)
 wait_column "$hv1_ch_uuid" HA_Chassis_Group ref_chassis
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 2
+check ovn-nbctl --wait=hv sync
 
 reset_pcap_file() {
     local iface=$1
@@ -10389,8 +10335,7 @@  ovn-nbctl lsp-add foo foo2 \
 -- lsp-set-addresses foo2 "f0:00:00:01:02:06 192.168.1.3"
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 : > hv1-vif2.expected
 
@@ -10481,10 +10426,6 @@  AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown])
 AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet])
 AT_CHECK([ovn-nbctl --wait=hv lsp-set-options ln_port network_name=physnet1])
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 2
-
 # Expect no packets when hv2 bridge-mapping is not present
 : > packets
 OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap], [packets])
@@ -10706,16 +10647,15 @@  ovn-nbctl lsp-set-addresses ln-outside unknown
 ovn-nbctl lsp-set-type ln-outside localnet
 ovn-nbctl lsp-set-options ln-outside network_name=phys
 
-# Allow some time for ovn-northd and ovn-controller to catch up.
-check ovn-nbctl --wait=hv sync
-
 # Check that there is a logical flow in logical switch foo's pipeline
 # to set the outport to rp-foo (which is expected).
 OVS_WAIT_UNTIL([test 1 = `ovn-sbctl dump-flows foo | grep ls_in_l2_lkup | \
 grep rp-foo | grep -v is_chassis_resident | grep priority=50 -c`])
 
 # Set the option 'reside-on-redirect-chassis' for foo
-check ovn-nbctl --wait=hv set logical_router_port foo options:reside-on-redirect-chassis=true
+check ovn-nbctl set logical_router_port foo options:reside-on-redirect-chassis=true
+check ovn-nbctl --wait=hv sync
+
 # Check that there is a logical flow in logical switch foo's pipeline
 # to set the outport to rp-foo with the condition is_chassis_redirect.
 ovn-sbctl dump-flows foo
@@ -10944,8 +10884,7 @@  ovs-vsctl -- add-port br-int hv1-vif3 -- \
     ofport-request=3
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 reset_pcap_file() {
     local iface=$1
@@ -11200,8 +11139,7 @@  ovs-vsctl -- add-port br-int hv2-vif1 -- \
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Send ip packets between foo1 and alice1
 src_mac="f00000010203"
@@ -12785,8 +12723,6 @@  for i in 1 2 3; do
 done
 
 OVN_POPULATE_ARP
-# allow some time for ovn-northd and ovn-controller to catch up.
-sleep 1
 
 for i in 1 2 3; do
     : > vif${i}1.expected
@@ -12945,8 +12881,7 @@  done
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
 #
@@ -13025,10 +12960,6 @@  for is in 1 2 3; do
   done
 done
 
-# Allow some time for packet forwarding.
-# XXX This can be improved.
-sleep 1
-
 # Now check the packets actually received against the ones expected.
 for i in 1 2 3; do
     for j in 1 2 3; do
@@ -13169,8 +13100,7 @@  done
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 lsp_to_mac() {
     echo f0:00:00:00:0${1:0:1}:${1:1:2}
@@ -13276,10 +13206,6 @@  for is in 1 2 3; do
   done
 done
 
-# Allow some time for packet forwarding.
-# XXX This can be improved.
-sleep 1
-
 # Now check the packets actually received against the ones expected.
 for i in 1 2 3; do
     for j in 1 2 3; do
@@ -13751,8 +13677,6 @@  reset_pcap_file hv1-vif2 hv1/vif2
 rm -f 2.packets
 > 2.expected
 
-#sleep infinity
-
 # Remove the first less restrictive allow ACL.
 ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1'
 ovn-nbctl --wait=hv sync
@@ -13878,8 +13802,7 @@  ovn-nbctl create Address_Set name=set1 addresses=\"f0:00:00:00:00:11\",\"f0:00:0
 OVN_POPULATE_ARP
 
 # Allow some time for ovn-northd and ovn-controller to catch up.
-# XXX This should be more systematic.
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 # Make sure there is no attempt to adding duplicated flows by ovn-controller
 AT_FAIL_IF([test -n "`grep duplicate hv1/ovn-controller.log`"])
@@ -14340,7 +14263,7 @@  ovs-vsctl -- add-port br-int hv2-vif1 -- \
 
 OVN_POPULATE_ARP
 
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 packet="inport==\"sw1-p1\" && eth.src==$sw1_p1_mac && eth.dst==$sw1_ro_mac &&
        ip4 && ip.ttl==64 && ip4.src==$sw1_p1_ip && ip4.dst==$sw2_p1_ip &&
@@ -15264,7 +15187,7 @@  ovs-vsctl -- add-port br-int hv2-vif1 -- \
 
 OVN_POPULATE_ARP
 
-sleep 1
+check ovn-nbctl --wait=hv sync
 
 packet="inport==\"sw1-p1\" && eth.src==$sw1_p1_mac && eth.dst==$sw1_ro_mac &&
        ip4 && ip.ttl==64 && ip4.src==$sw1_p1_ip && ip4.dst==$sw2_p1_ip &&