| Message ID | 20210417203952.1447048-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] tests: Fix frequent failure of "4 HV, 1 LS, 1 LR, packet test with HA distributed router gateway port:". | expand |
Looks good and passes for me regularly. I'm curious if this issue with the pcap file is common across other failing tests. Acked-by: Mark Michelson <mmichels@redhat.com> On 4/17/21 4:39 PM, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > This test is failing quite often due to timing issues. The failures are > mainly due to the packet not received by the other side of the tunnel. > The test case resets the pcap file and then injects the packet. And > this packet gets lost sometimes if the pcap files are not reset yet. > To fix this issue the test now ensures that ovs-vswitchd resets the > pcap file before injecting the packet. The test also now adds > some waits to make sure flows related to tunnels are programmed > when the chassisresident port moves from one gw chassis to other. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > tests/ovn-macros.at | 18 ++++++++++++++++++ > tests/ovn.at | 31 +++++++++++++++++++------------ > 2 files changed, 37 insertions(+), 12 deletions(-) > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index 25f3dbe348..9a05359ba5 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -487,6 +487,24 @@ wait_for_ports_up() { > done > fi > } > + > +# reset_pcap_file iface pcap_file > +# Resets the pcap file associates with OVS interface. should be used > +# with dummy datapath. > +reset_iface_pcap_file() { > + local iface=$1 > + local pcap_file=$2 > + check rm -f dummy-*.pcap > + check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ > +options:rxq_pcap=dummy-rx.pcap > + OVS_WAIT_WHILE([test 24 = $(wc -c dummy-tx.pcap | cut -d " " -f1)]) > + check rm -f ${pcap_file}*.pcap > + check ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ > +options:rxq_pcap=${pcap_file}-rx.pcap > + > + OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " " -f1)]) > +} > + > OVS_END_SHELL_HELPERS > > m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) > diff --git a/tests/ovn.at b/tests/ovn.at > index 4c3d76d573..71cc7118ba 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -10096,15 +10096,12 @@ AT_CHECK([ovn-nbctl --wait=sb sync], [0], [ignore]) > ovn-sbctl dump-flows > sbflows > AT_CAPTURE_FILE([sbflows]) > > -reset_pcap_file() { > - local iface=$1 > - local pcap_file=$2 > - check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ > -options:rxq_pcap=dummy-rx.pcap > - rm -f ${pcap_file}*.pcap > - check ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ > -options:rxq_pcap=${pcap_file}-rx.pcap > -} > +hv1_gw1_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw1-0) > +hv1_gw2_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw2-0) > + > +OVS_WAIT_UNTIL([ > + test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=37 | grep -c "active_backup,ofport,members:$hv1_gw1_ofport,$hv1_gw2_ofport") > +]) > > test_ip_packet() > { > @@ -10150,13 +10147,13 @@ test_ip_packet() > echo $expected > ext1-vif1.expected > exp_gw_ip_garp=ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > echo $exp_gw_ip_garp >> ext1-vif1.expected > - as $active_gw reset_pcap_file br-phys_n1 $active_gw/br-phys_n1 > + as $active_gw reset_iface_pcap_file br-phys_n1 $active_gw/br-phys_n1 > > if test $backup_vswitchd_dead != 1; then > # Reset the file only if vswitchd in backup gw is alive > - as $backup_gw reset_pcap_file br-phys_n1 $backup_gw/br-phys_n1 > + as $backup_gw reset_iface_pcap_file br-phys_n1 $backup_gw/br-phys_n1 > fi > - as ext1 reset_pcap_file ext1-vif1 ext1/vif1 > + as ext1 reset_iface_pcap_file ext1-vif1 ext1/vif1 > > # Resend packet from foo1 to outside1 > check as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > @@ -10208,6 +10205,10 @@ AT_CHECK( > <1> > ]) > > +OVS_WAIT_UNTIL([ > + test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=37 | grep -c "active_backup,ofport,members:$hv1_gw2_ofport,$hv1_gw1_ofport") > +]) > + > test_ip_packet gw2 gw1 0 > > # Get the claim count of both gw1 and gw2. > @@ -10228,6 +10229,12 @@ OVS_WAIT_UNTIL([test $gw1_claim_ct = `cat gw1/ovn-controller.log \ > AT_CHECK([test $gw2_claim_ct = `cat gw2/ovn-controller.log | \ > grep -c "cr-alice: Claiming"`]) > > +OVS_WAIT_UNTIL([ > + bfd_status=$(as hv1 ovs-vsctl get interface ovn-gw2-0 bfd_status:state) > + echo "bfd status = $bfd_status" > + test "$bfd_status" = "down" > +]) > + > test_ip_packet gw1 gw2 1 > > as gw2 >
On Tue, Apr 20, 2021 at 1:55 PM Mark Michelson <mmichels@redhat.com> wrote: > > Looks good and passes for me regularly. I'm curious if this issue with > the pcap file is common across other failing tests. It could be. That's why I thought adding the reset pcap file function as a common function. We should change the existing tests to use this new function. > > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks. I applied this patch to the main branch. Numan > > On 4/17/21 4:39 PM, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > This test is failing quite often due to timing issues. The failures are > > mainly due to the packet not received by the other side of the tunnel. > > The test case resets the pcap file and then injects the packet. And > > this packet gets lost sometimes if the pcap files are not reset yet. > > To fix this issue the test now ensures that ovs-vswitchd resets the > > pcap file before injecting the packet. The test also now adds > > some waits to make sure flows related to tunnels are programmed > > when the chassisresident port moves from one gw chassis to other. > > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > tests/ovn-macros.at | 18 ++++++++++++++++++ > > tests/ovn.at | 31 +++++++++++++++++++------------ > > 2 files changed, 37 insertions(+), 12 deletions(-) > > > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > > index 25f3dbe348..9a05359ba5 100644 > > --- a/tests/ovn-macros.at > > +++ b/tests/ovn-macros.at > > @@ -487,6 +487,24 @@ wait_for_ports_up() { > > done > > fi > > } > > + > > +# reset_pcap_file iface pcap_file > > +# Resets the pcap file associates with OVS interface. should be used > > +# with dummy datapath. > > +reset_iface_pcap_file() { > > + local iface=$1 > > + local pcap_file=$2 > > + check rm -f dummy-*.pcap > > + check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ > > +options:rxq_pcap=dummy-rx.pcap > > + OVS_WAIT_WHILE([test 24 = $(wc -c dummy-tx.pcap | cut -d " " -f1)]) > > + check rm -f ${pcap_file}*.pcap > > + check ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ > > +options:rxq_pcap=${pcap_file}-rx.pcap > > + > > + OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " " -f1)]) > > +} > > + > > OVS_END_SHELL_HELPERS > > > > m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 4c3d76d573..71cc7118ba 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -10096,15 +10096,12 @@ AT_CHECK([ovn-nbctl --wait=sb sync], [0], [ignore]) > > ovn-sbctl dump-flows > sbflows > > AT_CAPTURE_FILE([sbflows]) > > > > -reset_pcap_file() { > > - local iface=$1 > > - local pcap_file=$2 > > - check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ > > -options:rxq_pcap=dummy-rx.pcap > > - rm -f ${pcap_file}*.pcap > > - check ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ > > -options:rxq_pcap=${pcap_file}-rx.pcap > > -} > > +hv1_gw1_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw1-0) > > +hv1_gw2_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw2-0) > > + > > +OVS_WAIT_UNTIL([ > > + test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=37 | grep -c "active_backup,ofport,members:$hv1_gw1_ofport,$hv1_gw2_ofport") > > +]) > > > > test_ip_packet() > > { > > @@ -10150,13 +10147,13 @@ test_ip_packet() > > echo $expected > ext1-vif1.expected > > exp_gw_ip_garp=ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 > > echo $exp_gw_ip_garp >> ext1-vif1.expected > > - as $active_gw reset_pcap_file br-phys_n1 $active_gw/br-phys_n1 > > + as $active_gw reset_iface_pcap_file br-phys_n1 $active_gw/br-phys_n1 > > > > if test $backup_vswitchd_dead != 1; then > > # Reset the file only if vswitchd in backup gw is alive > > - as $backup_gw reset_pcap_file br-phys_n1 $backup_gw/br-phys_n1 > > + as $backup_gw reset_iface_pcap_file br-phys_n1 $backup_gw/br-phys_n1 > > fi > > - as ext1 reset_pcap_file ext1-vif1 ext1/vif1 > > + as ext1 reset_iface_pcap_file ext1-vif1 ext1/vif1 > > > > # Resend packet from foo1 to outside1 > > check as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > > @@ -10208,6 +10205,10 @@ AT_CHECK( > > <1> > > ]) > > > > +OVS_WAIT_UNTIL([ > > + test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=37 | grep -c "active_backup,ofport,members:$hv1_gw2_ofport,$hv1_gw1_ofport") > > +]) > > + > > test_ip_packet gw2 gw1 0 > > > > # Get the claim count of both gw1 and gw2. > > @@ -10228,6 +10229,12 @@ OVS_WAIT_UNTIL([test $gw1_claim_ct = `cat gw1/ovn-controller.log \ > > AT_CHECK([test $gw2_claim_ct = `cat gw2/ovn-controller.log | \ > > grep -c "cr-alice: Claiming"`]) > > > > +OVS_WAIT_UNTIL([ > > + bfd_status=$(as hv1 ovs-vsctl get interface ovn-gw2-0 bfd_status:state) > > + echo "bfd status = $bfd_status" > > + test "$bfd_status" = "down" > > +]) > > + > > test_ip_packet gw1 gw2 1 > > > > as gw2 > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at index 25f3dbe348..9a05359ba5 100644 --- a/tests/ovn-macros.at +++ b/tests/ovn-macros.at @@ -487,6 +487,24 @@ wait_for_ports_up() { done fi } + +# reset_pcap_file iface pcap_file +# Resets the pcap file associates with OVS interface. should be used +# with dummy datapath. +reset_iface_pcap_file() { + local iface=$1 + local pcap_file=$2 + check rm -f dummy-*.pcap + check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ +options:rxq_pcap=dummy-rx.pcap + OVS_WAIT_WHILE([test 24 = $(wc -c dummy-tx.pcap | cut -d " " -f1)]) + check rm -f ${pcap_file}*.pcap + check ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ +options:rxq_pcap=${pcap_file}-rx.pcap + + OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " " -f1)]) +} + OVS_END_SHELL_HELPERS m4_define([OVN_POPULATE_ARP], [AT_CHECK(ovn_populate_arp__, [0], [ignore])]) diff --git a/tests/ovn.at b/tests/ovn.at index 4c3d76d573..71cc7118ba 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -10096,15 +10096,12 @@ AT_CHECK([ovn-nbctl --wait=sb sync], [0], [ignore]) ovn-sbctl dump-flows > sbflows AT_CAPTURE_FILE([sbflows]) -reset_pcap_file() { - local iface=$1 - local pcap_file=$2 - check ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ -options:rxq_pcap=dummy-rx.pcap - rm -f ${pcap_file}*.pcap - check ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ -options:rxq_pcap=${pcap_file}-rx.pcap -} +hv1_gw1_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw1-0) +hv1_gw2_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw2-0) + +OVS_WAIT_UNTIL([ + test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=37 | grep -c "active_backup,ofport,members:$hv1_gw1_ofport,$hv1_gw2_ofport") +]) test_ip_packet() { @@ -10150,13 +10147,13 @@ test_ip_packet() echo $expected > ext1-vif1.expected exp_gw_ip_garp=ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101 echo $exp_gw_ip_garp >> ext1-vif1.expected - as $active_gw reset_pcap_file br-phys_n1 $active_gw/br-phys_n1 + as $active_gw reset_iface_pcap_file br-phys_n1 $active_gw/br-phys_n1 if test $backup_vswitchd_dead != 1; then # Reset the file only if vswitchd in backup gw is alive - as $backup_gw reset_pcap_file br-phys_n1 $backup_gw/br-phys_n1 + as $backup_gw reset_iface_pcap_file br-phys_n1 $backup_gw/br-phys_n1 fi - as ext1 reset_pcap_file ext1-vif1 ext1/vif1 + as ext1 reset_iface_pcap_file ext1-vif1 ext1/vif1 # Resend packet from foo1 to outside1 check as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet @@ -10208,6 +10205,10 @@ AT_CHECK( <1> ]) +OVS_WAIT_UNTIL([ + test 1 = $(as hv1 ovs-ofctl dump-flows br-int table=37 | grep -c "active_backup,ofport,members:$hv1_gw2_ofport,$hv1_gw1_ofport") +]) + test_ip_packet gw2 gw1 0 # Get the claim count of both gw1 and gw2. @@ -10228,6 +10229,12 @@ OVS_WAIT_UNTIL([test $gw1_claim_ct = `cat gw1/ovn-controller.log \ AT_CHECK([test $gw2_claim_ct = `cat gw2/ovn-controller.log | \ grep -c "cr-alice: Claiming"`]) +OVS_WAIT_UNTIL([ + bfd_status=$(as hv1 ovs-vsctl get interface ovn-gw2-0 bfd_status:state) + echo "bfd status = $bfd_status" + test "$bfd_status" = "down" +]) + test_ip_packet gw1 gw2 1 as gw2