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

[Bala Sankaran]

----- Original Message -----
> From: "Ian Stokes" <ian.stokes at intel.com>
> To: "Aaron Conole" <aconole at redhat.com>, dev at openvswitch.org
> Cc: "Ciara Loftus" <ciara.loftus at intel.com>, "Bala Sankaran" <bsankara at 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 at 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 at redhat.com>
> > Co-authored-by: Aaron Conole <aconole at redhat.com>
> > Signed-off-by: Aaron Conole <aconole at 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 at 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. at d
> > +\@Failed to enable flow control at d
> > +\@VHOST_CONFIG: recvmsg failed at d
> > +\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/vhu0: No such file or
> > directory at d
> > +\@Global register is changed during at d
> > +\@dpdkvhostuser ports are considered deprecated;  please migrate to
> > dpdkvhostuserclient ports. at d
> > +\@failed to enumerate system datapaths: No such file or directory at d
> > +\@EAL:   Invalid NUMA socket, default to 0 at d
> > +\@EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using
> > unreliable clock cycles !@d
> > +\@EAL: No free hugepages reported in hugepages-1048576kB at d"])
> > +AT_CLEANUP
> > +dnl
> > --------------------------------------------------------------------------
> > 
> 
>