On 7 Jul 2023, at 13:08, Ilya Maximets wrote:
> On 7/5/23 21:47, Eelco Chaudron wrote: >> >> >> On Wed, Jul 5, 2023 at 6:46 PM Eric Garver <e...@garver.life> wrote: >> >> On Wed, Jul 05, 2023 at 04:58:47PM +0200, Eelco Chaudron wrote: >> > >> > >> > On 30 Jun 2023, at 21:05, Eric Garver wrote: >> > Hi Eric, >> > >> > I started reviewing the series, and this test was failing every other >> run for me on ‘check-system-userspace’. I ended up making the following >> additional change: >> > >> > diff --git a/tests/ofproto-macros.at <http://ofproto-macros.at> >> b/tests/ofproto-macros.at <http://ofproto-macros.at> >> > index d2e6ac768..573ecdd0f 100644 >> > --- a/tests/ofproto-macros.at <http://ofproto-macros.at> >> > +++ b/tests/ofproto-macros.at <http://ofproto-macros.at> >> > @@ -120,7 +120,7 @@ strip_xids () { >> > >> > # Changes all 'used:...' to say 'used:0.0', to make output easier to >> compare. >> > strip_used () { >> > - sed 's/used:[[0-9]]\.[[0-9]]*/used:0.0/' >> > + sed 's/used:\([[0-9]]\.[[0-9]]*s\|never\)/used:0.0s/' >> > } >> >> In this test case I expect the flow to be used. My local test runs do >> not yield "never". >> >> Maybe we need a "ovs-appctl time/warp 5000" before dumping the flow? >> This is used in tests/drop-stats.at <http://drop-stats.at> . >> >> Tried that but did not work, if you look at the actual number of packets >> received by the flow entry, it varies quite a lot from 0 (only learning) to >> 5 when I run the test multiple times. >> >> I tried changing the following to generate more traffic: >> >> -dnl generate some traffic >> -NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2], [1], [ignore]) >> +dnl Generate some traffic. >> +NS_CHECK_EXEC([at_ns0], [ping -q -c 100 -i 0.01 -w 1 10.1.1.2], [1], >> [ignore]) >> >> And then run it 100 times, and the earlier problem did not happen: >> >> sudo bash -c 'for i in {1..100}; do make check-system-userspace >> TESTSUITEFLAGS="-k drop -v" || break; echo "EC_DONE: $i"; done' >> >> However, I have some times it fails with no traffic: >> >> >> --- - 2023-07-05 21:46:59.860127196 +0200 >> +++ >> /home/echaudron/Documents/review/ovs_eric_DROP/tests/system-userspace-testsuite.dir/at-groups/45/stdout >> 2023-07-05 21:46:59.857641231 +0200 >> @@ -1,2 +1 @@ >> -recirc_id(<recirc>),in_port(2),eth_type(0x0800),ipv4(frag=no), packets:0, >> bytes:0, used:0.0s, actions:drop >> >> Maybe this test needs some more love ;) > > The test likely needs a revalidator/wait call, since you're > checking datapath flows. Otherwise, it'll be time sensitive. > Stats are gathered during revalidation. That was might thought initially, and I tried it (no luck), but it’s dp flows, not openflow. >> >> > >> > This is also the reason why the intel tests are failing in patchwork; >> https://mail.openvswitch.org/pipermail/ovs-build/2023-June/032011.html >> <https://mail.openvswitch.org/pipermail/ovs-build/2023-June/032011.html> >> > >> > I still need to review the other patches in the series, but some small >> comments while testing the patch set: >> > >> > We are trying to align all commit messages to start with a Capital and >> end with a dot. So in your case: >> > >> > ‘tests: system-traffic: Add coverage for drop action.’ >> >> ACK. I'll address this in v3. >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
