----- Original Message -----
> From: "Tiago Lam" <tiago....@intel.com>
> To: "Bala Sankaran" <bsank...@redhat.com>, d...@openvswitch.org
> Sent: Wednesday, 29 August, 2018 1:36:13 PM
> Subject: Re: [ovs-dev] [PATCH v3 6/6] system-dpdk: Connect network namespaces 
> via dpdkvhostuser ports
> 
> Hi Bala,
> 
> Thanks to both you and Aaron for working on this. Seems to be a great
> addition.
> 
> As a general comment I agree with Ian that running everything on v17.11
> would be preferable, as this would enable us to run this test on any
> given system, and not only when v18.11 is installed. But after reading
> through your thread on the DPDK users list on the 2MB hugepages
> limitations around virtio_user, it seems this will have to be a
> dependency until OvS-DPDK moves to v18.11.
Hello Tiago,

I agree, I did not happen to notice a workaround for this.

> 
> On 28/08/2018 18:47, Bala Sankaran wrote:
> > 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 <bsank...@redhat.com>
> > Co-authored-by: Aaron Conole <acon...@redhat.com>
> > Signed-off-by: Aaron Conole <acon...@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
> > +AT_SETUP([OVS-DPDK datapath - ping vhost-user-client ports])
> 
> Any reason why you're using vhost-user instead of vhost-user-client? If
> we change it to "type=dpdkvhostuserclient" in the vhu0 interface added
> to OvS and append ",server=1" to the net_virtio_user --vdev in the
> testpmd arguments, doesn't it just work the same?

I believe I encountered an error while running the tests with a vhost-user 
client ports. That's when I switched over to vhost-user instead. I do not 
remember the error at this moment, but now that you mentioned it, I am thinking
of adding another unit test that use vhost-user-client port which would give 
me the error and then skip to vhost-user instead.

> 
> > +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 \
> > +          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
> > +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've seen your reply to Ian's comment on the $PATH environment variable
> on v2; That could be enough if there wasn't a requirement for DPDK
> v18.11 for the `testpmd` bin. Since there is a separate environment
> variable will be needed to guarantee we're executing the correct one
> (and not the v17.11 that's currently linked with OvS, as an example). It
> would also enable us to detect if the variable is set or not, and if
> not, skip the test altogether. At the moment the test will just fail, if
> the binary doesn't exist.

I do not have complete familiarity on this note, I have copied Aaron along
in this email. I suppose that he would be able to comment on this.
 
> 
> 
> > +dnl add veth device
> > +ADD_VETH(tap1, ns2, br10, "172.31.110.12/24")
> 
> The ADD_VETH() macro skips the test if $1 already exists. It would be
> better to set up tap1 at the beginning, thus skipping the test if it
> can't b set up, and this way it wouldn't be starting the `testpmd`
> process unnecessarily.

I had given it some thought too. I will test it out at my end and make the 
change if deemed necessary.

> 
> > +
> > +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])
> 
> Any specific requirement on arping? I for one didn't have it installed
> on my system.

Agreed, we could either replace arping with ping, or rather add 
functionality to install arping. The former would be preferred as
the user might not want the tests to install tools unnecessarily.

I am glad you noticed our work. I appreciate your feedback and I 
look to working towards it. 

Thanks,
Bala.

> 
> 
> Tiago.
> 
> > +
> > +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
> > --------------------------------------------------------------------------
> > 
> 
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to