| Message ID | 20210427172007.1416154-1-mmichels@redhat.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [ovs-dev] tests: Fix inconsistent "ACL Conjunction" test. | expand |
Bleep bloop. Greetings Mark Michelson, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: error: sha1 information is lacking or useless (tests/ovn-macros.at). error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 tests: Fix inconsistent "ACL Conjunction" test. When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On Tue, Apr 27, 2021 at 1:20 PM Mark Michelson <mmichels@redhat.com> wrote: > > The ACL Conjunction test would occasionally fail during automated test > runs. During the test, we send a packet on a netdev-dummy interface and > check the associated pcap file to ensure the packet is sent where we > expect and that it has the expected contents. Looking at logs from > failed runs, it appeared that the pcap file was unpopulated. This likely > was because we were attempting to dump the contents of the pcap file > before the packet had been processed and added to the pcap file. > > This patch aims to fix the problem by blocking until the pcap file has > been modified when sending the packet to the netdev-dummy interface. > Since this could be a useful thing for other tests, this new method of > blocking has been added to ovn-macros.at. > > Signed-off-by: Mark Michelson <mmichels@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> Numan > --- > tests/ovn-macros.at | 23 +++++++++++++++++++++++ > tests/ovn.at | 8 ++++---- > 2 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index bd227215a..94fba405e 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -509,6 +509,29 @@ options:rxq_pcap=${pcap_file}-rx.pcap > OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " " -f1)]) > } > > +# Receive a packet on a dummy netdev interface. If we expect packets to be > +# recorded, then wait until the pcap file reflects the change. > +netdev_dummy_receive() { > + local interface="$1" > + local packet="$2" > + local hv="$3" > + local pcap_file="$4" > + > + if test -n "pcap_file" ; then > + ts_old=$(stat -c %y "$pcap_file") > + fi > + if test -n "$hv" ; then > + as "$hv" ovs-appctl netdev-dummy/receive "$interface" "$packet" > + else > + ovs-appctl netdev-dummy/receive "$interface" "$packet" > + fi > + if test -n "$pcap_file" ; then > + OVS_WAIT_WHILE( > + [ts_new=$(stat -c %y "$pcap_file") > + test "$ts_new" = "$ts_old"]) > + fi > +} > + > 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 9f38ec6ec..c1158f5d0 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -14016,11 +14016,11 @@ check ovn-nbctl --wait=hv acl-add ls1 to-lport 1001 \ > # port numbers, e.g. 11 for vif11. > test_ip() { > # This packet has bad checksums but logical L3 routing doesn't check. > - local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 > + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 pcap_file=$6 > local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\ > ${dst_ip}0035111100080000 > - shift; shift; shift; shift; shift > - as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet > + shift; shift; shift; shift; shift; shift > + netdev_dummy_receive hv1-vif1 $packet hv1 "$pcap_file" > for outport; do > echo $packet >> $outport.expected > done > @@ -14040,7 +14040,7 @@ options:rxq_pcap=${pcap_file}-rx.pcap > sip=`ip_to_hex 10 0 0 4` > dip=`ip_to_hex 10 0 0 6` > > -test_ip 1 f00000000001 f00000000002 $sip $dip 2 > +test_ip 1 f00000000001 f00000000002 $sip $dip hv1/vif2-tx.pcap 2 > > cat 2.expected > expout > $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets > -- > 2.29.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 4/27/21 6:29 PM, Numan Siddique wrote: > On Tue, Apr 27, 2021 at 1:20 PM Mark Michelson <mmichels@redhat.com> wrote: >> >> The ACL Conjunction test would occasionally fail during automated test >> runs. During the test, we send a packet on a netdev-dummy interface and >> check the associated pcap file to ensure the packet is sent where we >> expect and that it has the expected contents. Looking at logs from >> failed runs, it appeared that the pcap file was unpopulated. This likely >> was because we were attempting to dump the contents of the pcap file >> before the packet had been processed and added to the pcap file. >> >> This patch aims to fix the problem by blocking until the pcap file has >> been modified when sending the packet to the netdev-dummy interface. >> Since this could be a useful thing for other tests, this new method of >> blocking has been added to ovn-macros.at. >> >> Signed-off-by: Mark Michelson <mmichels@redhat.com> > > Acked-by: Numan Siddique <numans@ovn.org> > > Numan Thanks Numan, I pushed this change to master, branch-21.03, and branch-20.12. > >> --- >> tests/ovn-macros.at | 23 +++++++++++++++++++++++ >> tests/ovn.at | 8 ++++---- >> 2 files changed, 27 insertions(+), 4 deletions(-) >> >> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at >> index bd227215a..94fba405e 100644 >> --- a/tests/ovn-macros.at >> +++ b/tests/ovn-macros.at >> @@ -509,6 +509,29 @@ options:rxq_pcap=${pcap_file}-rx.pcap >> OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " " -f1)]) >> } >> >> +# Receive a packet on a dummy netdev interface. If we expect packets to be >> +# recorded, then wait until the pcap file reflects the change. >> +netdev_dummy_receive() { >> + local interface="$1" >> + local packet="$2" >> + local hv="$3" >> + local pcap_file="$4" >> + >> + if test -n "pcap_file" ; then >> + ts_old=$(stat -c %y "$pcap_file") >> + fi >> + if test -n "$hv" ; then >> + as "$hv" ovs-appctl netdev-dummy/receive "$interface" "$packet" >> + else >> + ovs-appctl netdev-dummy/receive "$interface" "$packet" >> + fi >> + if test -n "$pcap_file" ; then >> + OVS_WAIT_WHILE( >> + [ts_new=$(stat -c %y "$pcap_file") >> + test "$ts_new" = "$ts_old"]) >> + fi >> +} >> + >> 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 9f38ec6ec..c1158f5d0 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -14016,11 +14016,11 @@ check ovn-nbctl --wait=hv acl-add ls1 to-lport 1001 \ >> # port numbers, e.g. 11 for vif11. >> test_ip() { >> # This packet has bad checksums but logical L3 routing doesn't check. >> - local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 >> + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 pcap_file=$6 >> local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\ >> ${dst_ip}0035111100080000 >> - shift; shift; shift; shift; shift >> - as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet >> + shift; shift; shift; shift; shift; shift >> + netdev_dummy_receive hv1-vif1 $packet hv1 "$pcap_file" >> for outport; do >> echo $packet >> $outport.expected >> done >> @@ -14040,7 +14040,7 @@ options:rxq_pcap=${pcap_file}-rx.pcap >> sip=`ip_to_hex 10 0 0 4` >> dip=`ip_to_hex 10 0 0 6` >> >> -test_ip 1 f00000000001 f00000000002 $sip $dip 2 >> +test_ip 1 f00000000001 f00000000002 $sip $dip hv1/vif2-tx.pcap 2 >> >> cat 2.expected > expout >> $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets >> -- >> 2.29.2 >> >> _______________________________________________ >> 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 bd227215a..94fba405e 100644 --- a/tests/ovn-macros.at +++ b/tests/ovn-macros.at @@ -509,6 +509,29 @@ options:rxq_pcap=${pcap_file}-rx.pcap OVS_WAIT_WHILE([test 24 = $(wc -c ${pcap_file}-tx.pcap | cut -d " " -f1)]) } +# Receive a packet on a dummy netdev interface. If we expect packets to be +# recorded, then wait until the pcap file reflects the change. +netdev_dummy_receive() { + local interface="$1" + local packet="$2" + local hv="$3" + local pcap_file="$4" + + if test -n "pcap_file" ; then + ts_old=$(stat -c %y "$pcap_file") + fi + if test -n "$hv" ; then + as "$hv" ovs-appctl netdev-dummy/receive "$interface" "$packet" + else + ovs-appctl netdev-dummy/receive "$interface" "$packet" + fi + if test -n "$pcap_file" ; then + OVS_WAIT_WHILE( + [ts_new=$(stat -c %y "$pcap_file") + test "$ts_new" = "$ts_old"]) + fi +} + 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 9f38ec6ec..c1158f5d0 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -14016,11 +14016,11 @@ check ovn-nbctl --wait=hv acl-add ls1 to-lport 1001 \ # port numbers, e.g. 11 for vif11. test_ip() { # This packet has bad checksums but logical L3 routing doesn't check. - local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 pcap_file=$6 local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\ ${dst_ip}0035111100080000 - shift; shift; shift; shift; shift - as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet + shift; shift; shift; shift; shift; shift + netdev_dummy_receive hv1-vif1 $packet hv1 "$pcap_file" for outport; do echo $packet >> $outport.expected done @@ -14040,7 +14040,7 @@ options:rxq_pcap=${pcap_file}-rx.pcap sip=`ip_to_hex 10 0 0 4` dip=`ip_to_hex 10 0 0 6` -test_ip 1 f00000000001 f00000000002 $sip $dip 2 +test_ip 1 f00000000001 f00000000002 $sip $dip hv1/vif2-tx.pcap 2 cat 2.expected > expout $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
The ACL Conjunction test would occasionally fail during automated test runs. During the test, we send a packet on a netdev-dummy interface and check the associated pcap file to ensure the packet is sent where we expect and that it has the expected contents. Looking at logs from failed runs, it appeared that the pcap file was unpopulated. This likely was because we were attempting to dump the contents of the pcap file before the packet had been processed and added to the pcap file. This patch aims to fix the problem by blocking until the pcap file has been modified when sending the packet to the netdev-dummy interface. Since this could be a useful thing for other tests, this new method of blocking has been added to ovn-macros.at. Signed-off-by: Mark Michelson <mmichels@redhat.com> --- tests/ovn-macros.at | 23 +++++++++++++++++++++++ tests/ovn.at | 8 ++++---- 2 files changed, 27 insertions(+), 4 deletions(-)