[ovs-dev,v2,6/6] system-dpdk: Execute testpmd on the background

From: Bala Sankaran <bsankara@redhat.com>

This adds a new test to the 'check-dpdk' subsystem that will exercise
allocations, PMDs, and the vhost-user code path.

Signed-off-by: Bala Sankaran <bsankara@redhat.com>
Co-authored-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 tests/system-dpdk.at | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

On 8/22/2018 2:37 PM, Aaron Conole wrote:
> From: Bala Sankaran <bsankara@redhat.com>
> 
> This adds a new test to the 'check-dpdk' subsystem that will exercise
> allocations, PMDs, and the vhost-user code path.
> 
> Signed-off-by: Bala Sankaran <bsankara@redhat.com>
> Co-authored-by: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>   tests/system-dpdk.at | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 77 insertions(+)
> 
> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index 58dc8aaae..914a1b644 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -1,3 +1,6 @@
> +m4_define([CONFIGURE_VETH_OFFLOADS],
> +   [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
> +
>   AT_BANNER([OVS-DPDK unit tests])
>   
>   dnl --------------------------------------------------------------------------
> @@ -74,3 +77,77 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is probably
>   \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
>   AT_CLEANUP
>   dnl --------------------------------------------------------------------------
> +
> +
> +
> +dnl --------------------------------------------------------------------------
> +dnl Ping vhost-user-client port
This test uses vhost user server so above should be changed to reflect this.

> +AT_SETUP([OVS-DPDK datapath - ping vhost-user-client ports])
> +AT_KEYWORDS([dpdk])
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START()
> +
> +dnl Add userspace bridge and attach it to OVS
> +AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
> +AT_CHECK([ovs-vsctl add-port br10 vhu0 -- set Interface vhu0 \

I'd like to keep the name of the vhost user interfaces uniform across 
the tests. Can we change the interface name to dpdkvhostuser0 instead of 
vhu0? This is in keeping with the existing vhost tests and the OVS DPDK 
documentation.

> +          type=dpdkvhostuser], [],
> +         [stdout], [stderr])
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +
> +dnl Parse log file
> +AT_CHECK([grep "VHOST_CONFIG: vhost-user server: socket created" \
> +          ovs-vswitchd.log], [], [stdout])
> +AT_CHECK([grep "Socket $OVS_RUNDIR/vhu0 created for vhost-user port vhu0" \
> +          ovs-vswitchd.log], [], [stdout])
> +AT_CHECK([grep "VHOST_CONFIG: bind to $OVS_RUNDIR/vhu0" ovs-vswitchd.log], [],
> +         [stdout])
> +
> +dnl Set up namespaces
> +ADD_NAMESPACES(ns1, ns2)
> +
> +dnl execute testpmd in background

To be uniform the first word after each dnl should be capitalized 
(applies to a few of the other dnl added in this test also).

> +on_exit "pkill -f -x -9 'tail -f /dev/null'"
> +tail -f /dev/null | testpmd --socket-mem=512 \
> +           --vdev="net_virtio_user,path=$OVS_RUNDIR/vhu0" \
> +           --vdev="net_tap0,iface=tap0" --file-prefix page0 \
> +           --single-file-segments -- -a >$OVS_RUNDIR/testpmd-vhu0.log 2>&1 &

I have a few queries as regards running testpmd.

Is the assumption that testpmd is a recognized command?

How exactly were you testing this? I believe DPDK 18.08 is required in 
the case of testpmd but 17.11 is still linked against for OVS? Ideally 
I'd like to remove the dependency on 18.08. What issues were seen when 
using testpmd in 17.11?

Is it expected that DPDK has been installed from a repo and in a default 
location?

If it is then I think we should provision for an environmental variable 
specifying the path to testpmd also. It could be the case someone is 
building DPDK from source and the testpmd executable is elsewhere.

Ian
> +
> +dnl add veth device
> +ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
> +
> +dnl give settling time to the testpmd processes - NOTE: this is bad form.
> +sleep 10
> +
> +dnl move the tap devices to the namespaces
> +AT_CHECK([ps aux | grep testpmd], [], [stdout], [stderr])
> +AT_CHECK([ip link show], [], [stdout], [stderr])
> +AT_CHECK([ip link set tap0 netns ns1], [], [stdout], [stderr])
> +
> +AT_CHECK([ip netns exec ns1 ip link show], [], [stdout], [stderr])
> +AT_CHECK([ip netns exec ns1 ip link show | grep tap0], [], [stdout], [stderr])
> +AT_CHECK([ip netns exec ns1 ip link set tap0 up], [], [stdout], [stderr])
> +AT_CHECK([ip netns exec ns1 ip addr add 172.31.110.11/24 dev tap0], [],
> +         [stdout], [stderr])
> +
> +AT_CHECK([ip netns exec ns1 ip link show], [], [stdout], [stderr])
> +AT_CHECK([ip netns exec ns2 ip link show], [], [stdout], [stderr])
> +AT_CHECK([ip netns exec ns1 arping -c 4 -I tap0 172.31.110.12], [], [stdout],
> +         [stderr])
> +
> +dnl clean up the testpmd now
> +pkill -f -x -9 'tail -f /dev/null'
> +
> +dnl Clean up
> +AT_CHECK([ovs-vsctl del-port br10 vhu0], [], [stdout], [stderr])
> +OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is probably not loaded.@d
> +\@Failed to enable flow control@d
> +\@VHOST_CONFIG: recvmsg failed@d
> +\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/vhu0: No such file or directory@d
> +\@Global register is changed during@d
> +\@dpdkvhostuser ports are considered deprecated;  please migrate to dpdkvhostuserclient ports.@d
> +\@failed to enumerate system datapaths: No such file or directory@d
> +\@EAL:   Invalid NUMA socket, default to 0@d
> +\@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable clock cycles !@d
> +\@EAL: No free hugepages reported in hugepages-1048576kB@d"])
> +AT_CLEANUP
> +dnl --------------------------------------------------------------------------
>

----- Original Message -----
> From: "Ian Stokes" <ian.stokes@intel.com>
> To: "Aaron Conole" <aconole@redhat.com>, dev@openvswitch.org
> Cc: "Ciara Loftus" <ciara.loftus@intel.com>, "Bala Sankaran" <bsankara@redhat.com>
> Sent: Monday, 27 August, 2018 7:25:48 AM
> Subject: Re: [PATCH v2 6/6] system-dpdk: Execute testpmd on the background
> 
> On 8/22/2018 2:37 PM, Aaron Conole wrote:
> > From: Bala Sankaran <bsankara@redhat.com>
> > 
> > This adds a new test to the 'check-dpdk' subsystem that will exercise
> > allocations, PMDs, and the vhost-user code path.
> > 
> > Signed-off-by: Bala Sankaran <bsankara@redhat.com>
> > Co-authored-by: Aaron Conole <aconole@redhat.com>
> > Signed-off-by: Aaron Conole <aconole@redhat.com>
> > ---
> >   tests/system-dpdk.at | 77
> >   ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 77 insertions(+)
> > 
> > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> > index 58dc8aaae..914a1b644 100644
> > --- a/tests/system-dpdk.at
> > +++ b/tests/system-dpdk.at
> > @@ -1,3 +1,6 @@
> > +m4_define([CONFIGURE_VETH_OFFLOADS],
> > +   [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])])
> > +
> >   AT_BANNER([OVS-DPDK unit tests])
> >   
> >   dnl
> >   --------------------------------------------------------------------------
> > @@ -74,3 +77,77 @@ OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch
> > kernel module is probably
> >   \@EAL: No free hugepages reported in hugepages-1048576kB@d"])
> >   AT_CLEANUP
> >   dnl
> >   --------------------------------------------------------------------------
> > +
> > +
> > +
> > +dnl
> > --------------------------------------------------------------------------
> > +dnl Ping vhost-user-client port
> This test uses vhost user server so above should be changed to reflect this.

Agreed, changes made.

> 
> > +AT_SETUP([OVS-DPDK datapath - ping vhost-user-client ports])
> > +AT_KEYWORDS([dpdk])
> > +OVS_DPDK_PRE_CHECK()
> > +OVS_DPDK_START()
> > +
> > +dnl Add userspace bridge and attach it to OVS
> > +AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
> > +AT_CHECK([ovs-vsctl add-port br10 vhu0 -- set Interface vhu0 \
> 
> I'd like to keep the name of the vhost user interfaces uniform across
> the tests. Can we change the interface name to dpdkvhostuser0 instead of
> vhu0? This is in keeping with the existing vhost tests and the OVS DPDK
> documentation.

This has been noted, I have made the necessary changes.

> 
> > +          type=dpdkvhostuser], [],
> > +         [stdout], [stderr])
> > +AT_CHECK([ovs-vsctl show], [], [stdout])
> > +
> > +dnl Parse log file
> > +AT_CHECK([grep "VHOST_CONFIG: vhost-user server: socket created" \
> > +          ovs-vswitchd.log], [], [stdout])
> > +AT_CHECK([grep "Socket $OVS_RUNDIR/vhu0 created for vhost-user port vhu0"
> > \
> > +          ovs-vswitchd.log], [], [stdout])
> > +AT_CHECK([grep "VHOST_CONFIG: bind to $OVS_RUNDIR/vhu0" ovs-vswitchd.log],
> > [],
> > +         [stdout])
> > +
> > +dnl Set up namespaces
> > +ADD_NAMESPACES(ns1, ns2)
> > +
> > +dnl execute testpmd in background
> 
> To be uniform the first word after each dnl should be capitalized
> (applies to a few of the other dnl added in this test also).

Noted, I've applied the changes all over.
> 
> > +on_exit "pkill -f -x -9 'tail -f /dev/null'"
> > +tail -f /dev/null | testpmd --socket-mem=512 \
> > +           --vdev="net_virtio_user,path=$OVS_RUNDIR/vhu0" \
> > +           --vdev="net_tap0,iface=tap0" --file-prefix page0 \
> > +           --single-file-segments -- -a >$OVS_RUNDIR/testpmd-vhu0.log 2>&1
> > &
> 
> I have a few queries as regards running testpmd.
> 
> Is the assumption that testpmd is a recognized command?

Yes, I assumed that testpmd is a recognized command while doing the tests,
because testpmd is an ancillary tool that comes with DPDK installation.

> 
> How exactly were you testing this? I believe DPDK 18.08 is required in
> the case of testpmd but 17.11 is still linked against for OVS? Ideally
> I'd like to remove the dependency on 18.08. What issues were seen when
> using testpmd in 17.11?

Issues I saw were that options/ arguments for testpmd like: 
single-file-segments were unavailable in the DPDK-17.11 version. I am not 
aware if we would be able to remove the dependency on 18.08.

> 
> Is it expected that DPDK has been installed from a repo and in a default
> location?

Yes, I assumed it is expected to be so.

> 
> If it is then I think we should provision for an environmental variable
> specifying the path to testpmd also. It could be the case someone is
> building DPDK from source and the testpmd executable is elsewhere.

The environment variable $PATH is already set and suffices this, I believe.

Thank you for your suggestions and letting me know your requirements. I've
made the necessary changes and I will be testing it at my end before I submit
a v3 of the set of patches. 

Let me know if you've got any more questions.

Thanks,
Bala.

> 
> Ian
> > +
> > +dnl add veth device
> > +ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
> > +
> > +dnl give settling time to the testpmd processes - NOTE: this is bad form.
> > +sleep 10
> > +
> > +dnl move the tap devices to the namespaces
> > +AT_CHECK([ps aux | grep testpmd], [], [stdout], [stderr])
> > +AT_CHECK([ip link show], [], [stdout], [stderr])
> > +AT_CHECK([ip link set tap0 netns ns1], [], [stdout], [stderr])
> > +
> > +AT_CHECK([ip netns exec ns1 ip link show], [], [stdout], [stderr])
> > +AT_CHECK([ip netns exec ns1 ip link show | grep tap0], [], [stdout],
> > [stderr])
> > +AT_CHECK([ip netns exec ns1 ip link set tap0 up], [], [stdout], [stderr])
> > +AT_CHECK([ip netns exec ns1 ip addr add 172.31.110.11/24 dev tap0], [],
> > +         [stdout], [stderr])
> > +
> > +AT_CHECK([ip netns exec ns1 ip link show], [], [stdout], [stderr])
> > +AT_CHECK([ip netns exec ns2 ip link show], [], [stdout], [stderr])
> > +AT_CHECK([ip netns exec ns1 arping -c 4 -I tap0 172.31.110.12], [],
> > [stdout],
> > +         [stderr])
> > +
> > +dnl clean up the testpmd now
> > +pkill -f -x -9 'tail -f /dev/null'
> > +
> > +dnl Clean up
> > +AT_CHECK([ovs-vsctl del-port br10 vhu0], [], [stdout], [stderr])
> > +OVS_VSWITCHD_STOP(["\@does not exist. The Open vSwitch kernel module is
> > probably not loaded.@d
> > +\@Failed to enable flow control@d
> > +\@VHOST_CONFIG: recvmsg failed@d
> > +\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/vhu0: No such file or
> > directory@d
> > +\@Global register is changed during@d
> > +\@dpdkvhostuser ports are considered deprecated;  please migrate to
> > dpdkvhostuserclient ports.@d
> > +\@failed to enumerate system datapaths: No such file or directory@d
> > +\@EAL:   Invalid NUMA socket, default to 0@d
> > +\@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using
> > unreliable clock cycles !@d
> > +\@EAL: No free hugepages reported in hugepages-1048576kB@d"])
> > +AT_CLEANUP
> > +dnl
> > --------------------------------------------------------------------------
> > 
> 
>