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 <[email protected]> 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 <[email protected]> > > --- > > 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 > > > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
