Hi Mike, 

Thanks for adding these tests, few comments inline.

<snipped>
 
> +dnl
> +-----------------------------------------------------------------------
> +---
> +dnl MTU increase phy port
> +AT_SETUP([OVS-DPDK - MTU increase phy port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_PHY_SKIP()
> +OVS_DPDK_START()
> +
> +dnl First set MTU to its default value and confirm that value, then
> +increase the MTU value and confirm the new value
> +
> +dnl Add userspace bridge and attach it to OVS with default MTU value
> +AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10
> +datapath_type=netdev]) AT_CHECK([ovs-vsctl add-port br10 phy0 -- set
> +Interface phy0 type=dpdk options:dpdk-devargs=$(cat PCI_ADDR)], [],
> +[stdout], [stderr]) AT_CHECK([ovs-vsctl show], [], [stdout]) sleep 2
> +
> +dnl check default MTU value in the datapath AT_CHECK([ovs-appctl
> +dpctl/show], [], [stdout]) AT_CHECK([egrep 'mtu=1500,' stdout], [],
> +[stdout])
> +
> +dnl increase MTU value and check in the datapath AT_CHECK(ovs-vsctl set
> +Interface phy0 mtu_request=9000)

To be consistent with the rest of the file, please add "[]" inside AT_CHECK. 
(Applies to all test cases in the patch)

> +
> +dnl Fail if MTU is not supported
> +AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration"
> +ovs-vswitchd.log], [], [stdout])
> +
> +dnl Fail if error is encountered during MTU setup AT_FAIL_IF([grep
> +"Interface phy0 MTU (9000) setup error" ovs-vswitchd.log], [],
> +[stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) AT_CHECK([egrep
> +'mtu=9000,' stdout], [], [stdout])

Do we need to check for the "," above along with mtu=val ? we can probably drop 
it. [applies to all test cases in the patch]

<snipped>

> +
> +dnl
> +-----------------------------------------------------------------------
> +---
> +dnl MTU increase vport port
> +AT_SETUP([OVS-DPDK - MTU increase vport port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START()
> +
> +dnl Add userspace bridge and attach it to OVS with default MTU value
> +AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10
> +datapath_type=netdev]) AT_CHECK([ovs-vsctl add-port br10
> +dpdkvhostuserclient0 -- set Interface dpdkvhostuserclient0
> +type=dpdkvhostuserclient
> +options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [], [stdout],
> +[stderr]) AT_CHECK([ovs-vsctl show], [], [stdout]) sleep 2
> +
> +dnl check default MTU value in the datapath AT_CHECK([ovs-appctl
> +dpctl/show], [], [stdout]) AT_CHECK([egrep 'mtu=1500,' stdout], [],
> +[stdout])
> +
> +dnl increase MTU value and check in the datapath AT_CHECK(ovs-vsctl set
> +Interface dpdkvhostuserclient0 mtu_request=9000)
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout]) AT_CHECK([egrep
> +'mtu=9000,' stdout], [], [stdout])
> +
> +
> +dnl Parse log file
> +AT_CHECK([grep "VHOST_CONFIG: vhost-user client: socket created"
> +ovs-vswitchd.log], [], [stdout]) AT_CHECK([grep "vHost User device
> +'dpdkvhostuserclient0' created in 'client' mode, using client socket"
> +ovs-vswitchd.log], [], [stdout]) AT_CHECK([grep "VHOST_CONFIG:
> +$OVS_RUNDIR/dpdkvhostclient0: reconnecting..." ovs-vswitchd.log], [],
> +[stdout])
> +
> +dnl Clean up
> +AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout],
> +[stderr]) OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> +\@VHOST_CONFIG: failed to connect to $OVS_RUNDIR/dpdkvhostclient0: No
> +such file or directory@d
> +])")
> +AT_CLEANUP
> +dnl
> +-----------------------------------------------------------------------



Now that the following commit is merged:

0dd409c2a netdev-dpdk: Delay vhost mempool creation.

AFAIK, we would require changing all the vhost related testcases in this patch 
a bit to configure the mempool, mtu etc by establishing connection with vhost 
front end (virtio-user) using testpmd first.
As an example, for the above test case : "OVS-DPDK - MTU increase vport port" 
the diff would be like:


 OVS_DPDK_PRE_CHECK()
 OVS_DPDK_START()

+dnl Find number of sockets
+AT_CHECK([lscpu], [], [stdout])
+AT_CHECK([cat stdout | grep "NUMA node(s)" | awk '{c=1; while (c++<$(3)) 
{printf "512,"}; print "512"}' > NUMA_NODE])
+
 dnl Add userspace bridge and attach it to OVS with default MTU value
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
 AT_CHECK([ovs-vsctl add-port br10 dpdkvhostuserclient0 -- set Interface 
dpdkvhostuserclient0 type=dpdkvhostuserclient 
options:vhost-server-path=$OVS_RUNDIR/dpdkvhostclient0], [], [stdout], [stderr])
 AT_CHECK([ovs-vsctl show], [], [stdout])
 sleep 2

+dnl Parse log file
+AT_CHECK([grep "VHOST_CONFIG: vhost-user client: socket created" 
ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' 
mode, using client socket" ovs-vswitchd.log], [], [stdout])
+AT_CHECK([grep "VHOST_CONFIG: $OVS_RUNDIR/dpdkvhostclient0: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
+
+dnl Execute testpmd in background
+on_exit "pkill -f -x -9 'tail -f /dev/null'"
+tail -f /dev/null | dpdk-testpmd --socket-mem="$(cat NUMA_NODE)" --no-pci\
+           --vdev="net_virtio_user,path=$OVS_RUNDIR/dpdkvhostclient0,server=1" 
\
+           --vdev="net_tap0,iface=tap0" --file-prefix page0 \
+           --single-file-segments -- -a 
>$OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 &
+
+OVS_WAIT_UNTIL([grep "virtio is now ready for processing" ovs-vswitchd.log])
+
 dnl check default MTU value in the datapath
 AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
-AT_CHECK([egrep 'mtu=1500,' stdout], [], [stdout])
+AT_CHECK([egrep 'mtu=1500' stdout], [], [stdout])

 dnl increase MTU value and check in the datapath
-AT_CHECK(ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=9000)
+AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=9000])

 AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
-AT_CHECK([egrep 'mtu=9000,' stdout], [], [stdout])
-
-
-dnl Parse log file
-AT_CHECK([grep "VHOST_CONFIG: vhost-user client: socket created" 
ovs-vswitchd.log], [], [stdout])
-AT_CHECK([grep "vHost User device 'dpdkvhostuserclient0' created in 'client' 
mode, using client socket" ovs-vswitchd.log], [], [stdout])
-AT_CHECK([grep "VHOST_CONFIG: $OVS_RUNDIR/dpdkvhostclient0: reconnecting..." 
ovs-vswitchd.log], [], [stdout])
+AT_CHECK([egrep 'mtu=9000' stdout], [], [stdout])

 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], 
[stderr])
        

Thanks and regards,
Sunil

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

Reply via email to