diff mbox series

[ovs-dev] tests: fixed multiple flaky tests (not waiting for patch flows)

Message ID 20220622150232.3987202-1-xsimonar@redhat.com
State Superseded
Headers show
Series [ovs-dev] tests: fixed multiple flaky tests (not waiting for patch flows) | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Xavier Simonart June 22, 2022, 3:02 p.m. UTC
The following test cases were sometimes failing, (mainly) for the same reason
i.e. packet lost as being sent before patch ports were installed.
- 2 HVs, 2 LS, switching between multiple localnet ports with same tags
- VLAN transparency, passthru=true, ARP responder disabled
- send gratuitous arp for l3gateway only on selected chassis
- 4 HV, 3 LS, 2 LR, packet test with HA distributed router gateway port
- 1 LR with distributed router gateway port
- send gratuitous arp for NAT rules on distributed router
- send gratuitous ARP for NAT rules on HA distributed router
- localnet connectivity with multiple requested-chassis
- 2 HVs, 2 lports/HV, localnet ports, DVR chassis mac
- 2 HVs, 4 lports/HV, localnet ports
- 2 HVs, 2 LS, routing works for multiple collocated segments attached to different switches
- 2 HVs, 1 LS, no switching between multiple localnet ports with different tags

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
---
 tests/ovn.at | 70 +++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 22 deletions(-)

Comments

Mark Michelson June 23, 2022, 8:39 p.m. UTC | #1
Hi Xavier,

Thanks a bunch for fixing this. I have a small suggestion below

On 6/22/22 11:02, Xavier Simonart wrote:
> The following test cases were sometimes failing, (mainly) for the same reason
> i.e. packet lost as being sent before patch ports were installed.
> - 2 HVs, 2 LS, switching between multiple localnet ports with same tags
> - VLAN transparency, passthru=true, ARP responder disabled
> - send gratuitous arp for l3gateway only on selected chassis
> - 4 HV, 3 LS, 2 LR, packet test with HA distributed router gateway port
> - 1 LR with distributed router gateway port
> - send gratuitous arp for NAT rules on distributed router
> - send gratuitous ARP for NAT rules on HA distributed router
> - localnet connectivity with multiple requested-chassis
> - 2 HVs, 2 lports/HV, localnet ports, DVR chassis mac
> - 2 HVs, 4 lports/HV, localnet ports
> - 2 HVs, 2 LS, routing works for multiple collocated segments attached to different switches
> - 2 HVs, 1 LS, no switching between multiple localnet ports with different tags
> 
> Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
> ---
>   tests/ovn.at | 70 +++++++++++++++++++++++++++++++++++-----------------
>   1 file changed, 48 insertions(+), 22 deletions(-)
> 
> diff --git a/tests/ovn.at b/tests/ovn.at
> index bfaa41962..5913537bc 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -76,19 +76,21 @@ m4_divert_text([PREPARE_TESTS],
>      }
>   
>      ovn_wait_patch_port_flows () {
> -     patch_port=$1
> -     hv=$2
> -     echo "$3: waiting for flows for $patch_port on $hv"
> -     # Patch port might be created after ports are reported up
> -     OVS_WAIT_UNTIL([
> -         test 1 = $(as $hv ovs-vsctl show | grep "Port $patch_port" | wc -l)
> -     ])
> -     # Wait for a flow outputing to patch port
> -     OVS_WAIT_UNTIL([
> -         hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport find Interface name=$patch_port)
> -         echo "$patch_port=$hv_patch_ofport"
> -         test 1 = $(as $hv ovs-ofctl dump-flows br-int | grep -c "output:$hv_patch_ofport")
> -     ])
> +     for patch_port in $1; do
> +       for hv in $2; do
> +         echo "$3: waiting for flows for $patch_port on $hv"
> +         # Patch port might be created after ports are reported up
> +         OVS_WAIT_UNTIL([
> +             test 1 = $(as $hv ovs-vsctl show | grep "Port \b$patch_port\b" | wc -l)
> +         ])
> +         # Wait for a flow outputing to patch port
> +         OVS_WAIT_UNTIL([
> +             hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport find Interface name=$patch_port)
> +             echo "$patch_port=$hv_patch_ofport"
> +             test 1 -le $(as $hv ovs-ofctl dump-flows br-int | grep -c "output:\b$hv_patch_ofport\b")
> +         ])
> +       done
> +     done
>      }
>   
>      ovn_wait_remote_output_flows () {
> @@ -2952,6 +2954,8 @@ for i in 1 2; do
>       done
>   done
>   wait_for_ports_up
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1" "patch-br-int-to-ln2" "patch-br-int-to-ln3"], ["hv1"] ["hv2"])

What are the odds that we will want to wait on a patch port that is not 
connected to br-int? Would it make things easier if just the port name 
is used as the argument and OVN_WAIT_PATCH_PORT_FLOWS prepends 
"patch-br-int-to-" to the port name?

Using the above as an example, we would instead have:

OVN_WAIT_PATCH_PORT_FLOWS(["ln1" "ln2" "ln3"], ["hv1"] ["hv2"])

Does this seem reasonable?

> +
>   ovn-nbctl --wait=sb sync
>   ovn-sbctl dump-flows > sbflows
>   AT_CAPTURE_FILE([sbflows])
> @@ -3122,6 +3126,7 @@ done
>   
>   wait_for_ports_up
>   ovn-nbctl --wait=sb sync
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-11" "patch-br-int-to-ln-21"], ["hv-1" "hv-2"])
>   
>   ovn-sbctl dump-flows > sbflows
>   AT_CAPTURE_FILE([sbflows])
> @@ -3394,6 +3399,7 @@ for i in 1 2; do
>       ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:0$i
>   
>       OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
> +    OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-$i-20"], ["hv-$i"])
>   done
>   
>   wait_for_ports_up
> @@ -3501,7 +3507,7 @@ done
>   
>   wait_for_ports_up
>   
> -# Remote output flows are setup whe pb of remote is received
> +# Remote output flows are setup when pb of remote is received
>   # Hence they can be setup after both ports have been reported up.
>   OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-1"],["hv-2"])
>   OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-2"],["hv-1"])
> @@ -4021,6 +4027,7 @@ for tag in 10 20; do
>           ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:0$i:$tag
>   
>           OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
> +        OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-$tag"], ["hv-$tag-$i"])
>       done
>   done
>   wait_for_ports_up
> @@ -8856,14 +8863,14 @@ AT_CAPTURE_FILE([sbflows])
>   
>   # Wait until the patch ports are created in hv1 and hv2 to connect br-int to br-eth0
>   AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
> -OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-vsctl show | \
> -grep "Port patch-br-int-to-ln_port" | wc -l`])
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv1"])
>   AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
> -OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
> -grep "Port patch-br-int-to-ln_port" | wc -l`])
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"])
>   
>   # Temporarily remove lr0 chassis
> -AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
> +# Wait for hv confirmation to make sure chassis is removed before we reset pcap
> +# Otherwise a garp might be sent after pcap have been reset but before chassis is removed
> +AT_CHECK([ovn-nbctl --wait=hv remove logical_router lr0 options chassis])
>   
>   reset_pcap_file() {
>       local hv=$1
> @@ -11324,6 +11331,8 @@ check as ext1 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
>   
>   wait_for_ports_up
>   check ovn-nbctl --wait=sb sync
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-alice"], ["gw1"] ["gw2"])
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-outside"], ["ext1"])
>   
>   ovn-sbctl dump-flows > sbflows
>   AT_CAPTURE_FILE([sbflows])
> @@ -11540,6 +11549,7 @@ check as hv3 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
>   dnl Allow some time for ovn-northd and ovn-controller to catch up.
>   wait_for_ports_up
>   ovn-nbctl --wait=hv sync
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-outside"], ["hv3"])
>   
>   (echo "---------NB dump-----"
>    ovn-nbctl show
> @@ -11733,6 +11743,7 @@ ovn-nbctl lsp-add foo foo2 \
>   # Allow some time for ovn-northd and ovn-controller to catch up.
>   wait_for_ports_up
>   check ovn-nbctl --wait=hv sync
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-alice"], ["hv1"])
>   
>   : > hv1-vif2.expected
>   
> @@ -11831,8 +11842,7 @@ OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap], [packets])
>   AT_CHECK([as hv2 ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=physnet1:br-phys])
>   
>   # Wait until the patch ports are created in hv2 to connect br-int to br-phys
> -OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
> -grep "Port patch-br-int-to-ln_port" | wc -l`])
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"])
>   
>   # Wait for packets to be received.
>   OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100])
> @@ -13550,6 +13560,7 @@ AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1])
>   # wait for earlier changes to take effect
>   wait_for_ports_up
>   check ovn-nbctl --wait=hv sync
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"] ["hv3"])
>   
>   reset_pcap_file() {
>       local iface=$1
> @@ -14762,6 +14773,13 @@ reset_env
>   # Start port migration hv1 -> hv2: both hypervisors are now bound
>   check ovn-nbctl lsp-set-options migrator requested-chassis=hv1,hv2
>   wait_for_ports_up
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-public"], ["hv1"])
> +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-public"], ["hv2"])
> +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv1"],["hv2"])
> +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv2"],["hv1"])
> +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv3"],[hv1"])
> +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv3"],[hv2"])
> +
>   wait_column "$hv1_uuid" Port_Binding chassis logical_port=migrator
>   wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=migrator
>   wait_column "$hv2_uuid" Port_Binding additional_chassis logical_port=migrator
> @@ -20079,6 +20097,11 @@ ovn-nbctl lsp-add ls2 ls2-to-router -- set Logical_Switch_Port ls2-to-router typ
>   wait_for_ports_up
>   ovn-nbctl --wait=sb sync
>   #ovn-sbctl dump-flows
> +for i in 1 2; do
> +    for j in 1 2; do
> +        OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln$j"], ["hv$i"])
> +    done
> +done
>   
>   ovn-nbctl show
>   ovn-sbctl show
> @@ -22484,6 +22507,8 @@ m4_define([DVR_N_S_ARP_HANDLING],
>      ovn-nbctl lrp-set-gateway-chassis router-to-underlay hv3
>      wait_for_ports_up
>      ovn-nbctl --wait=sb sync
> +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1"] ["patch-br-int-to-ln2"], ["hv1"] ["hv2"] ["hv3"])
> +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln4"], ["hv4"])
>   
>      wait_row_count Port_Binding 1 logical_port=cr-router-to-underlay
>   
> @@ -22704,7 +22729,8 @@ m4_define([DVR_N_S_PING],
>   
>      wait_for_ports_up
>      ovn-nbctl --wait=sb sync
> -
> +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1"] ["patch-br-int-to-ln2"], ["hv1"] ["hv2"] ["hv3"])
> +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln4"], ["hv4"])
>   
>      OVN_POPULATE_ARP
>   
>
Xavier Simonart June 24, 2022, 3:44 p.m. UTC | #2
Hi Mark

Thanks for the suggestion, it makes sense as it makes the test easier to
read.
I'll send a v2

Thanks
Xavier

On Thu, Jun 23, 2022 at 10:39 PM Mark Michelson <mmichels@redhat.com> wrote:

> Hi Xavier,
>
> Thanks a bunch for fixing this. I have a small suggestion below
>
> On 6/22/22 11:02, Xavier Simonart wrote:
> > The following test cases were sometimes failing, (mainly) for the same
> reason
> > i.e. packet lost as being sent before patch ports were installed.
> > - 2 HVs, 2 LS, switching between multiple localnet ports with same tags
> > - VLAN transparency, passthru=true, ARP responder disabled
> > - send gratuitous arp for l3gateway only on selected chassis
> > - 4 HV, 3 LS, 2 LR, packet test with HA distributed router gateway port
> > - 1 LR with distributed router gateway port
> > - send gratuitous arp for NAT rules on distributed router
> > - send gratuitous ARP for NAT rules on HA distributed router
> > - localnet connectivity with multiple requested-chassis
> > - 2 HVs, 2 lports/HV, localnet ports, DVR chassis mac
> > - 2 HVs, 4 lports/HV, localnet ports
> > - 2 HVs, 2 LS, routing works for multiple collocated segments attached
> to different switches
> > - 2 HVs, 1 LS, no switching between multiple localnet ports with
> different tags
> >
> > Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
> > ---
> >   tests/ovn.at | 70 +++++++++++++++++++++++++++++++++++-----------------
> >   1 file changed, 48 insertions(+), 22 deletions(-)
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index bfaa41962..5913537bc 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -76,19 +76,21 @@ m4_divert_text([PREPARE_TESTS],
> >      }
> >
> >      ovn_wait_patch_port_flows () {
> > -     patch_port=$1
> > -     hv=$2
> > -     echo "$3: waiting for flows for $patch_port on $hv"
> > -     # Patch port might be created after ports are reported up
> > -     OVS_WAIT_UNTIL([
> > -         test 1 = $(as $hv ovs-vsctl show | grep "Port $patch_port" |
> wc -l)
> > -     ])
> > -     # Wait for a flow outputing to patch port
> > -     OVS_WAIT_UNTIL([
> > -         hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport
> find Interface name=$patch_port)
> > -         echo "$patch_port=$hv_patch_ofport"
> > -         test 1 = $(as $hv ovs-ofctl dump-flows br-int | grep -c
> "output:$hv_patch_ofport")
> > -     ])
> > +     for patch_port in $1; do
> > +       for hv in $2; do
> > +         echo "$3: waiting for flows for $patch_port on $hv"
> > +         # Patch port might be created after ports are reported up
> > +         OVS_WAIT_UNTIL([
> > +             test 1 = $(as $hv ovs-vsctl show | grep "Port
> \b$patch_port\b" | wc -l)
> > +         ])
> > +         # Wait for a flow outputing to patch port
> > +         OVS_WAIT_UNTIL([
> > +             hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport
> find Interface name=$patch_port)
> > +             echo "$patch_port=$hv_patch_ofport"
> > +             test 1 -le $(as $hv ovs-ofctl dump-flows br-int | grep -c
> "output:\b$hv_patch_ofport\b")
> > +         ])
> > +       done
> > +     done
> >      }
> >
> >      ovn_wait_remote_output_flows () {
> > @@ -2952,6 +2954,8 @@ for i in 1 2; do
> >       done
> >   done
> >   wait_for_ports_up
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1" "patch-br-int-to-ln2"
> "patch-br-int-to-ln3"], ["hv1"] ["hv2"])
>
> What are the odds that we will want to wait on a patch port that is not
> connected to br-int? Would it make things easier if just the port name
> is used as the argument and OVN_WAIT_PATCH_PORT_FLOWS prepends
> "patch-br-int-to-" to the port name?
>
> Using the above as an example, we would instead have:
>
> OVN_WAIT_PATCH_PORT_FLOWS(["ln1" "ln2" "ln3"], ["hv1"] ["hv2"])
>
> Does this seem reasonable?
>
> > +
> >   ovn-nbctl --wait=sb sync
> >   ovn-sbctl dump-flows > sbflows
> >   AT_CAPTURE_FILE([sbflows])
> > @@ -3122,6 +3126,7 @@ done
> >
> >   wait_for_ports_up
> >   ovn-nbctl --wait=sb sync
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-11"
> "patch-br-int-to-ln-21"], ["hv-1" "hv-2"])
> >
> >   ovn-sbctl dump-flows > sbflows
> >   AT_CAPTURE_FILE([sbflows])
> > @@ -3394,6 +3399,7 @@ for i in 1 2; do
> >       ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:0$i
> >
> >       OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
> > +    OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-$i-20"], ["hv-$i"])
> >   done
> >
> >   wait_for_ports_up
> > @@ -3501,7 +3507,7 @@ done
> >
> >   wait_for_ports_up
> >
> > -# Remote output flows are setup whe pb of remote is received
> > +# Remote output flows are setup when pb of remote is received
> >   # Hence they can be setup after both ports have been reported up.
> >   OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-1"],["hv-2"])
> >   OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-2"],["hv-1"])
> > @@ -4021,6 +4027,7 @@ for tag in 10 20; do
> >           ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:0$i:$tag
> >
> >           OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
> > +        OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-$tag"],
> ["hv-$tag-$i"])
> >       done
> >   done
> >   wait_for_ports_up
> > @@ -8856,14 +8863,14 @@ AT_CAPTURE_FILE([sbflows])
> >
> >   # Wait until the patch ports are created in hv1 and hv2 to connect
> br-int to br-eth0
> >   AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
> > -OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-vsctl show | \
> > -grep "Port patch-br-int-to-ln_port" | wc -l`])
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv1"])
> >   AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
> > -OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
> > -grep "Port patch-br-int-to-ln_port" | wc -l`])
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"])
> >
> >   # Temporarily remove lr0 chassis
> > -AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
> > +# Wait for hv confirmation to make sure chassis is removed before we
> reset pcap
> > +# Otherwise a garp might be sent after pcap have been reset but before
> chassis is removed
> > +AT_CHECK([ovn-nbctl --wait=hv remove logical_router lr0 options
> chassis])
> >
> >   reset_pcap_file() {
> >       local hv=$1
> > @@ -11324,6 +11331,8 @@ check as ext1 ovs-vsctl set open .
> external-ids:ovn-bridge-mappings=phys:br-phys
> >
> >   wait_for_ports_up
> >   check ovn-nbctl --wait=sb sync
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-alice"], ["gw1"] ["gw2"])
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-outside"], ["ext1"])
> >
> >   ovn-sbctl dump-flows > sbflows
> >   AT_CAPTURE_FILE([sbflows])
> > @@ -11540,6 +11549,7 @@ check as hv3 ovs-vsctl set open .
> external-ids:ovn-bridge-mappings=phys:br-phys
> >   dnl Allow some time for ovn-northd and ovn-controller to catch up.
> >   wait_for_ports_up
> >   ovn-nbctl --wait=hv sync
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-outside"], ["hv3"])
> >
> >   (echo "---------NB dump-----"
> >    ovn-nbctl show
> > @@ -11733,6 +11743,7 @@ ovn-nbctl lsp-add foo foo2 \
> >   # Allow some time for ovn-northd and ovn-controller to catch up.
> >   wait_for_ports_up
> >   check ovn-nbctl --wait=hv sync
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-alice"], ["hv1"])
> >
> >   : > hv1-vif2.expected
> >
> > @@ -11831,8 +11842,7 @@ OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap],
> [packets])
> >   AT_CHECK([as hv2 ovs-vsctl set Open_vSwitch .
> external-ids:ovn-bridge-mappings=physnet1:br-phys])
> >
> >   # Wait until the patch ports are created in hv2 to connect br-int to
> br-phys
> > -OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
> > -grep "Port patch-br-int-to-ln_port" | wc -l`])
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"])
> >
> >   # Wait for packets to be received.
> >   OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100])
> > @@ -13550,6 +13560,7 @@ AT_CHECK([ovn-nbctl lsp-set-options ln_port
> network_name=physnet1])
> >   # wait for earlier changes to take effect
> >   wait_for_ports_up
> >   check ovn-nbctl --wait=hv sync
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"] ["hv3"])
> >
> >   reset_pcap_file() {
> >       local iface=$1
> > @@ -14762,6 +14773,13 @@ reset_env
> >   # Start port migration hv1 -> hv2: both hypervisors are now bound
> >   check ovn-nbctl lsp-set-options migrator requested-chassis=hv1,hv2
> >   wait_for_ports_up
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-public"], ["hv1"])
> > +OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-public"], ["hv2"])
> > +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv1"],["hv2"])
> > +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv2"],["hv1"])
> > +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv3"],[hv1"])
> > +OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv3"],[hv2"])
> > +
> >   wait_column "$hv1_uuid" Port_Binding chassis logical_port=migrator
> >   wait_column "$hv1_uuid" Port_Binding requested_chassis
> logical_port=migrator
> >   wait_column "$hv2_uuid" Port_Binding additional_chassis
> logical_port=migrator
> > @@ -20079,6 +20097,11 @@ ovn-nbctl lsp-add ls2 ls2-to-router -- set
> Logical_Switch_Port ls2-to-router typ
> >   wait_for_ports_up
> >   ovn-nbctl --wait=sb sync
> >   #ovn-sbctl dump-flows
> > +for i in 1 2; do
> > +    for j in 1 2; do
> > +        OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln$j"], ["hv$i"])
> > +    done
> > +done
> >
> >   ovn-nbctl show
> >   ovn-sbctl show
> > @@ -22484,6 +22507,8 @@ m4_define([DVR_N_S_ARP_HANDLING],
> >      ovn-nbctl lrp-set-gateway-chassis router-to-underlay hv3
> >      wait_for_ports_up
> >      ovn-nbctl --wait=sb sync
> > +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1"]
> ["patch-br-int-to-ln2"], ["hv1"] ["hv2"] ["hv3"])
> > +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln4"], ["hv4"])
> >
> >      wait_row_count Port_Binding 1 logical_port=cr-router-to-underlay
> >
> > @@ -22704,7 +22729,8 @@ m4_define([DVR_N_S_PING],
> >
> >      wait_for_ports_up
> >      ovn-nbctl --wait=sb sync
> > -
> > +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1"]
> ["patch-br-int-to-ln2"], ["hv1"] ["hv2"] ["hv3"])
> > +   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln4"], ["hv4"])
> >
> >      OVN_POPULATE_ARP
> >
> >
>
>
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index bfaa41962..5913537bc 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -76,19 +76,21 @@  m4_divert_text([PREPARE_TESTS],
    }
 
    ovn_wait_patch_port_flows () {
-     patch_port=$1
-     hv=$2
-     echo "$3: waiting for flows for $patch_port on $hv"
-     # Patch port might be created after ports are reported up
-     OVS_WAIT_UNTIL([
-         test 1 = $(as $hv ovs-vsctl show | grep "Port $patch_port" | wc -l)
-     ])
-     # Wait for a flow outputing to patch port
-     OVS_WAIT_UNTIL([
-         hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport find Interface name=$patch_port)
-         echo "$patch_port=$hv_patch_ofport"
-         test 1 = $(as $hv ovs-ofctl dump-flows br-int | grep -c "output:$hv_patch_ofport")
-     ])
+     for patch_port in $1; do
+       for hv in $2; do
+         echo "$3: waiting for flows for $patch_port on $hv"
+         # Patch port might be created after ports are reported up
+         OVS_WAIT_UNTIL([
+             test 1 = $(as $hv ovs-vsctl show | grep "Port \b$patch_port\b" | wc -l)
+         ])
+         # Wait for a flow outputing to patch port
+         OVS_WAIT_UNTIL([
+             hv_patch_ofport=$(as $hv ovs-vsctl --bare --columns ofport find Interface name=$patch_port)
+             echo "$patch_port=$hv_patch_ofport"
+             test 1 -le $(as $hv ovs-ofctl dump-flows br-int | grep -c "output:\b$hv_patch_ofport\b")
+         ])
+       done
+     done
    }
 
    ovn_wait_remote_output_flows () {
@@ -2952,6 +2954,8 @@  for i in 1 2; do
     done
 done
 wait_for_ports_up
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1" "patch-br-int-to-ln2" "patch-br-int-to-ln3"], ["hv1"] ["hv2"])
+
 ovn-nbctl --wait=sb sync
 ovn-sbctl dump-flows > sbflows
 AT_CAPTURE_FILE([sbflows])
@@ -3122,6 +3126,7 @@  done
 
 wait_for_ports_up
 ovn-nbctl --wait=sb sync
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-11" "patch-br-int-to-ln-21"], ["hv-1" "hv-2"])
 
 ovn-sbctl dump-flows > sbflows
 AT_CAPTURE_FILE([sbflows])
@@ -3394,6 +3399,7 @@  for i in 1 2; do
     ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:00:0$i
 
     OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
+    OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-$i-20"], ["hv-$i"])
 done
 
 wait_for_ports_up
@@ -3501,7 +3507,7 @@  done
 
 wait_for_ports_up
 
-# Remote output flows are setup whe pb of remote is received
+# Remote output flows are setup when pb of remote is received
 # Hence they can be setup after both ports have been reported up.
 OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-1"],["hv-2"])
 OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv-2"],["hv-1"])
@@ -4021,6 +4027,7 @@  for tag in 10 20; do
         ovn-nbctl lsp-set-port-security $lsp_name f0:00:00:00:0$i:$tag
 
         OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lsp_name` = xup])
+        OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-$tag"], ["hv-$tag-$i"])
     done
 done
 wait_for_ports_up
@@ -8856,14 +8863,14 @@  AT_CAPTURE_FILE([sbflows])
 
 # Wait until the patch ports are created in hv1 and hv2 to connect br-int to br-eth0
 AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
-OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-vsctl show | \
-grep "Port patch-br-int-to-ln_port" | wc -l`])
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv1"])
 AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
-OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
-grep "Port patch-br-int-to-ln_port" | wc -l`])
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"])
 
 # Temporarily remove lr0 chassis
-AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
+# Wait for hv confirmation to make sure chassis is removed before we reset pcap
+# Otherwise a garp might be sent after pcap have been reset but before chassis is removed
+AT_CHECK([ovn-nbctl --wait=hv remove logical_router lr0 options chassis])
 
 reset_pcap_file() {
     local hv=$1
@@ -11324,6 +11331,8 @@  check as ext1 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
 
 wait_for_ports_up
 check ovn-nbctl --wait=sb sync
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-alice"], ["gw1"] ["gw2"])
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-outside"], ["ext1"])
 
 ovn-sbctl dump-flows > sbflows
 AT_CAPTURE_FILE([sbflows])
@@ -11540,6 +11549,7 @@  check as hv3 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
 dnl Allow some time for ovn-northd and ovn-controller to catch up.
 wait_for_ports_up
 ovn-nbctl --wait=hv sync
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-outside"], ["hv3"])
 
 (echo "---------NB dump-----"
  ovn-nbctl show
@@ -11733,6 +11743,7 @@  ovn-nbctl lsp-add foo foo2 \
 # Allow some time for ovn-northd and ovn-controller to catch up.
 wait_for_ports_up
 check ovn-nbctl --wait=hv sync
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln-alice"], ["hv1"])
 
 : > hv1-vif2.expected
 
@@ -11831,8 +11842,7 @@  OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap], [packets])
 AT_CHECK([as hv2 ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=physnet1:br-phys])
 
 # Wait until the patch ports are created in hv2 to connect br-int to br-phys
-OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
-grep "Port patch-br-int-to-ln_port" | wc -l`])
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"])
 
 # Wait for packets to be received.
 OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 100])
@@ -13550,6 +13560,7 @@  AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=physnet1])
 # wait for earlier changes to take effect
 wait_for_ports_up
 check ovn-nbctl --wait=hv sync
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln_port"], ["hv2"] ["hv3"])
 
 reset_pcap_file() {
     local iface=$1
@@ -14762,6 +14773,13 @@  reset_env
 # Start port migration hv1 -> hv2: both hypervisors are now bound
 check ovn-nbctl lsp-set-options migrator requested-chassis=hv1,hv2
 wait_for_ports_up
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-public"], ["hv1"])
+OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-public"], ["hv2"])
+OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv1"],["hv2"])
+OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv2"],["hv1"])
+OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv3"],[hv1"])
+OVN_WAIT_REMOTE_OUTPUT_FLOWS(["hv3"],[hv2"])
+
 wait_column "$hv1_uuid" Port_Binding chassis logical_port=migrator
 wait_column "$hv1_uuid" Port_Binding requested_chassis logical_port=migrator
 wait_column "$hv2_uuid" Port_Binding additional_chassis logical_port=migrator
@@ -20079,6 +20097,11 @@  ovn-nbctl lsp-add ls2 ls2-to-router -- set Logical_Switch_Port ls2-to-router typ
 wait_for_ports_up
 ovn-nbctl --wait=sb sync
 #ovn-sbctl dump-flows
+for i in 1 2; do
+    for j in 1 2; do
+        OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln$j"], ["hv$i"])
+    done
+done
 
 ovn-nbctl show
 ovn-sbctl show
@@ -22484,6 +22507,8 @@  m4_define([DVR_N_S_ARP_HANDLING],
    ovn-nbctl lrp-set-gateway-chassis router-to-underlay hv3
    wait_for_ports_up
    ovn-nbctl --wait=sb sync
+   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1"] ["patch-br-int-to-ln2"], ["hv1"] ["hv2"] ["hv3"])
+   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln4"], ["hv4"])
 
    wait_row_count Port_Binding 1 logical_port=cr-router-to-underlay
 
@@ -22704,7 +22729,8 @@  m4_define([DVR_N_S_PING],
 
    wait_for_ports_up
    ovn-nbctl --wait=sb sync
-
+   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln1"] ["patch-br-int-to-ln2"], ["hv1"] ["hv2"] ["hv3"])
+   OVN_WAIT_PATCH_PORT_FLOWS(["patch-br-int-to-ln4"], ["hv4"])
 
    OVN_POPULATE_ARP