Hello Ilya,

On Tue, Jan 4, 2022 at 12:52 PM Ilya Maximets <[email protected]> wrote:
> > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> > index 5c988da280..ecce9e3c9b 100644
> > --- a/tests/system-dpdk.at
> > +++ b/tests/system-dpdk.at
> > @@ -226,22 +226,29 @@ dnl 
> > --------------------------------------------------------------------------
> >  dnl Add standard DPDK PHY port
> >  AT_SETUP([OVS-DPDK - MFEX Autovalidator])
> >  AT_KEYWORDS([dpdk])
> > +AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], [])
> >
> >  OVS_DPDK_START()
> >
> >  dnl Add userspace bridge and attach it to OVS
> >  AT_CHECK([ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev])
> > -AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dpdk 
> > options:dpdk-devargs=net_pcap1,rx_pcap=$srcdir/pcap/mfex_test.pcap,infinite_rx=1],
> >  [], [stdout], [stderr])
> > +AT_CHECK([ovs-vsctl add-port br0 p1 -- set interface p1 type=dummy-pmd])
> >  AT_CHECK([ovs-vsctl show], [], [stdout])
> >
> >  AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | grep 
> > "True"], [], [dnl
> >  ])
> >
> > +on_exit "pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1'"
> > +($PYTHON3 $srcdir/genpkts.py -1 | while read pkt; do
> > +     ovs-appctl netdev-dummy/receive p1 "$pkt" || break
> > + done) &
>
> With this pattern I'm having issues running tests in parallel (in the
> main testsuite with the next patch applied).  One of the tests always
> fails and succedes on re-check.  I have an impression that script breaks
> or getting killed before sending all the packets (cross-fire from the
> other test?).  In general, the pattern here doesn't seem CPU friendly

It's likely a problem for parallel tests, yes.


> and that might be an issue for CPU-usage-restricted test environments.
>
> Suggesting a following change for the test:
>
> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
> index fbb8fe9a7..5f6a07151 100644
> --- a/tests/dpif-netdev.at
> +++ b/tests/dpif-netdev.at
> @@ -645,17 +645,16 @@ OVS_VSWITCHD_START(
>  AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | grep 
> "True"], [], [dnl
>  ])
>
> -on_exit "pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1'"
> -($PYTHON3 $srcdir/genpkts.py -1 | while read pkt; do
> -     ovs-appctl netdev-dummy/receive p1 "$pkt" || break
> - done) &
> -
>  AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], [0], 
> [dnl
>  Miniflow extract implementation set to autovalidator.
>  ])
>
> +AT_SKIP_IF([! $PYTHON3 $srcdir/genpkts.py 1000 > packets])
> +cat packets | while read pkt; do
> +  AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$pkt"], [0], [ignore])
> +done
> +
>  OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics:rx_packets` -ge 
> 1000])
> -pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1'
>
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> ---
>
> Same for the fuzzy test below.  This will save a lot of CPU cycles as
> we will not have parallel thread that generates packets, and will also
> not need to kill it.  This also removes the race between packet generation
> and enabling of the autovalidator.
>
> I also added the AT_SKIP_IF for the actual genpkts.py call, because I have
> scapy crashing on one of my systems during the packet generation.  Looks
> like some internal bug.  So, it's better to skip instead of failing in this
> case.  I kept the import check, because it's more visual for the user, i.e.
> it clearly suggests a missing dependency.
>
> What do you think?

>From a CI pov, your proposal is saner.

Just one concern is that we lose a check that changing parameters is
fine while processing packets in pmd.
Not sure if this was something intended or desirable in the first place.


>
> > +
> >  AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], [0], 
> > [dnl
> >  Miniflow extract implementation set to autovalidator.
> >  ])
> >
> > -OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep -oP 
> > 'rx_packets=\s*\K\d+'` -ge 1000])
> > +OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics:rx_packets` 
> > -ge 1000])
> > +pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1'
> >
> >  dnl Clean up
> >  AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr])
> > @@ -254,22 +261,27 @@ dnl Add standard DPDK PHY port
> >  AT_SETUP([OVS-DPDK - MFEX Autovalidator Fuzzy])
> >  AT_KEYWORDS([dpdk])
> >  AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], [])
> > -AT_CHECK([$PYTHON3 $srcdir/mfex_fuzzy.py $srcdir], [], [stdout])
> >  OVS_DPDK_START()
> >
> >  dnl Add userspace bridge and attach it to OVS
> >  AT_CHECK([ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev])
> > -AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dpdk 
> > options:dpdk-devargs=net_pcap1,rx_pcap=$srcdir/pcap/fuzzy.pcap,infinite_rx=1],
> >  [], [stdout], [stderr])
> > +AT_CHECK([ovs-vsctl add-port br0 p1 -- set interface p1 type=dummy-pmd])
> >  AT_CHECK([ovs-vsctl show], [], [stdout])
> >
> >  AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | grep 
> > "True"], [], [dnl
> >  ])
> >
> > +on_exit "pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1 fuzz'"
> > +($PYTHON3 $srcdir/genpkts.py -1 fuzz | while read pkt; do
> > +     ovs-appctl netdev-dummy/receive p1 "$pkt" || break
> > + done) &
> > +
> >  AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], [0], 
> > [dnl
> >  Miniflow extract implementation set to autovalidator.
> >  ])
> >
> > -OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep -oP 
> > 'rx_packets=\s*\K\d+'` -ge 100000])
> > +OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics:rx_packets` 
> > -ge 1000])
> > +pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1 fuzz'
> >
> >  dnl Clean up
> >  AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr])
> > @@ -280,13 +292,20 @@ dnl 
> > --------------------------------------------------------------------------
> >  dnl 
> > --------------------------------------------------------------------------
> >  AT_SETUP([OVS-DPDK - MFEX Configuration])
> >  AT_KEYWORDS([dpdk])
> > +AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], [])
> > +
> >  OVS_DPDK_START()
> >  AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . 
> > other_config:pmd-cpu-mask=0xC])
> >  dnl Add userspace bridge and attach it to OVS
> >  AT_CHECK([ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev])
> > -AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dpdk 
> > options:dpdk-devargs=net_pcap1,rx_pcap=$srcdir/pcap/mfex_test.pcap,infinite_rx=1],
> >  [], [stdout], [stderr])
> > +AT_CHECK([ovs-vsctl add-port br0 p1 -- set interface p1 type=dummy-pmd])
> >  AT_CHECK([ovs-vsctl show], [], [stdout])
> >
> > +on_exit "pkill -f -x -9 '$PYTHON3 $srcdir/genpkts.py -1'"
> > +($PYTHON3 $srcdir/genpkts.py -1 | while read pkt; do
> > +     ovs-appctl netdev-dummy/receive p1 "$pkt" || break
> > + done) &
>
> This configuration test doesn't seem to need any packets generated.
> And so it doesn't seem to need a scapy dependency.  Or am I missing
> something?

Idem, maybe the intent in this test was to provide packets while
changing params.


-- 
David Marchand

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to