> -----Original Message-----
> From: Phelan, Michael <[email protected]>
> Sent: Tuesday, June 21, 2022 11:21 AM
> To: [email protected]
> Cc: Stokes, Ian <[email protected]>; [email protected];
> [email protected]; [email protected]; Phelan, Michael
> <[email protected]>
> Subject: [PATCH] tests: Add OVS-DPDK MTU unit tests.
> 
> This adds 8 new unit tests to the 'check-dpdk' subsystem that will
> test Maximum Transmission Unit (MTU) functionality.
> 

Hi Michael,

Thanks for the patch, a few comments below.


> Signed-off-by: Michael Phelan <[email protected]>
> ---
>  tests/system-dpdk.at | 344
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 344 insertions(+)
> 
> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index 7d2715c4a..6a2c5f79f 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -222,6 +222,350 @@ OVS_VSWITCHD_STOP("m4_join([],
> [SYSTEM_DPDK_ALLOWED_LOGS], [
>  AT_CLEANUP
>  dnl 
> --------------------------------------------------------------------------
> 
> +
> +
> +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 defalt MTU value in DB and datapath
> +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [], [stdout])
> +AT_CHECK([egrep '1500' stdout], [], [stdout])


I'm not sure is there a need to confirm the MTU in the DB here,  I'm thinking 
is it enough to confirm the value in the datapath, the test itself is to 
confirm we can set the MTU for the device, checking the DB MTU value doesn't 
confirm what's set on the device, only the value stored in the DB?

As such we could possibly remove the DB check unless there is a specific reason 
to have it? @Ilya/Aaron, what are your thoughts?

> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=1500,' stdout], [], [stdout])
> +
> +dnl increase MTU value and check in DB and datapath
> +AT_CHECK(ovs-vsctl set Interface phy0 mtu_request=9000)
> +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [], [stdout])
> +AT_CHECK([egrep '9000' stdout], [], [stdout])

So when we attempt to set the MTU for an ethdev there are 2 cases we should 
check for

1. a device may not support MTU configuration, we should check for that when 
configuring ethdev ports as the test will be unable to increase/decrease MTU.

This warning is flagged specifically in the dpdk_eth_dev_port_config()

                VLOG_WARN("Interface %s does not support MTU configuration, "
                          "max packet size supported is %"PRIu16".",
                          dev->up.name, conf_mtu);

2. There could also be an error for one reason or another while setting he MTU 
for the ethdev device, we should check that this error occurs or not and fail 
if it does occur. Again in dpdk_eth_dev_port_config()

VLOG_ERR("Interface %s MTU (%d) setup error: %s",
                         dev->up.name, dev->mtu, rte_strerror(-diag));

So it would be good to handle these cases for the ethdev (phy ports) unit tests.

Thanks
Ian

> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=9000,' stdout], [], [stdout])
> +
> +
> +dnl Clean up
> +AT_CHECK([ovs-vsctl del-port br10 phy0], [], [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 
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl 
> --------------------------------------------------------------------------
> +dnl MTU decrease phy port
> +AT_SETUP([OVS-DPDK - MTU decrease phy port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_PHY_SKIP()
> +OVS_DPDK_START()
> +
> +dnl First set an increased MTU value and confirm that value, then decrease
> the MTU value and confirm the new value
> +
> +dnl Add userspace bridge and attach it to OVS and modify 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 set Interface phy0 mtu_request=9000)
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +sleep 2
> +
> +dnl check MTU value in DB and datapath
> +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [], [stdout])
> +AT_CHECK([egrep '9000' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=9000,' stdout], [], [stdout])
> +
> +dnl decrease MTU value and check in DB and datapath
> +AT_CHECK(ovs-vsctl set Interface phy0 mtu_request=2000)
> +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [], [stdout])
> +AT_CHECK([egrep '2000' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=2000,' stdout], [], [stdout])
> +
> +
> +dnl Clean up
> +AT_CHECK([ovs-vsctl del-port br10 phy0], [], [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 
> --------------------------------------------------------------------------
> +
> +
> +
> +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 DB and datapath
> +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [], [stdout])
> +AT_CHECK([egrep '1500' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=1500,' stdout], [], [stdout])
> +
> +dnl increase MTU value and check in DB and datapath
> +AT_CHECK(ovs-vsctl set Interface dpdkvhostuserclient0
> mtu_request=9000)
> +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [], [stdout])
> +AT_CHECK([egrep '9000' stdout], [], [stdout])
> +
> +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 
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl 
> --------------------------------------------------------------------------
> +dnl MTU decrease vport port
> +AT_SETUP([OVS-DPDK - MTU decrease vport port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START()
> +
> +dnl Add userspace bridge and attach it to OVS and modify 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 set Interface dpdkvhostuserclient0
> mtu_request=9000)
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +sleep 2
> +
> +dnl check MTU value in DB and datapath
> +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [], [stdout])
> +AT_CHECK([egrep '9000' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=9000,' stdout], [], [stdout])
> +
> +dnl decrease MTU value and check in DB and datapath
> +AT_CHECK(ovs-vsctl set Interface dpdkvhostuserclient0
> mtu_request=2000)
> +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [], [stdout])
> +AT_CHECK([egrep '2000' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=2000,' 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 
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl 
> --------------------------------------------------------------------------
> +dnl MTU upper bound phy port
> +AT_SETUP([OVS-DPDK - MTU upper bound phy port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_PHY_SKIP()
> +OVS_DPDK_START()
> +
> +dnl Add userspace bridge and attach it to OVS and set MTU value to max
> upper bound
> +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 set Interface phy0 mtu_request=9702)
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +sleep 2
> +
> +dnl check MTU value in DB and datapath
> +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [], [stdout])
> +AT_CHECK([egrep '9702' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=9702,' stdout], [], [stdout])
> +
> +dnl set MTU value above upper bound and check for error
> +AT_CHECK(ovs-vsctl set Interface phy0 mtu_request=9711)
> +AT_CHECK([grep "phy0: unsupported MTU 9711" ovs-vswitchd.log], [],
> [stdout])
> +
> +
> +dnl Clean up
> +AT_CHECK([ovs-vsctl del-port br10 phy0], [], [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
> +\@phy0: unsupported MTU 9711@d
> +\@failed to set MTU for network device phy0: Invalid argument@d
> +])")
> +AT_CLEANUP
> +dnl 
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl 
> --------------------------------------------------------------------------
> +dnl MTU lower bound phy port
> +AT_SETUP([OVS-DPDK - MTU lower bound phy port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_PHY_SKIP()
> +OVS_DPDK_START()
> +
> +dnl Add userspace bridge and attach it to OVS and set MTU value to min
> lower bound
> +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 set Interface phy0 mtu_request=68)
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +sleep 2
> +
> +dnl check MTU value in DB and datapath
> +AT_CHECK([ovs-vsctl get Interface phy0 mtu], [], [stdout])
> +AT_CHECK([egrep '68' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=68,' stdout], [], [stdout])
> +
> +dnl set MTU value below lower bound and check for error
> +AT_CHECK(ovs-vsctl set Interface phy0 mtu_request=67)
> +AT_CHECK([grep "phy0: unsupported MTU 67" ovs-vswitchd.log], [],
> [stdout])
> +
> +
> +dnl Clean up
> +AT_CHECK([ovs-vsctl del-port br10 phy0], [], [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
> +\@phy0: unsupported MTU 67@d
> +\@failed to set MTU for network device phy0: Invalid argument@d
> +])")
> +AT_CLEANUP
> +dnl 
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl 
> --------------------------------------------------------------------------
> +dnl MTU upper bound vport port
> +AT_SETUP([OVS-DPDK - MTU upper bound vport port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START()
> +
> +dnl Add userspace bridge and attach it to OVS and set MTU value to max
> upper bound
> +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 set Interface dpdkvhostuserclient0
> mtu_request=9702)
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +sleep 2
> +
> +dnl check MTU value in DB and datapath
> +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [], [stdout])
> +AT_CHECK([egrep '9702' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=9702,' stdout], [], [stdout])
> +
> +dnl set MTU value above upper bound and check for error
> +AT_CHECK(ovs-vsctl set Interface dpdkvhostuserclient0
> mtu_request=9711)
> +AT_CHECK([grep "dpdkvhostuserclient0: unsupported MTU 9711" ovs-
> vswitchd.log], [], [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
> +\@dpdkvhostuserclient0: unsupported MTU 9711@d
> +\@failed to set MTU for network device dpdkvhostuserclient0: Invalid
> argument@d
> +])")
> +AT_CLEANUP
> +dnl 
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl 
> --------------------------------------------------------------------------
> +dnl MTU lower bound vport port
> +AT_SETUP([OVS-DPDK - MTU lower bound vport port])
> +AT_KEYWORDS([dpdk])
> +
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START()
> +
> +dnl Add userspace bridge and attach it to OVS and set MTU value to min
> lower bound
> +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 set Interface dpdkvhostuserclient0 mtu_request=68)
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +sleep 2
> +
> +dnl check MTU value in DB and datapath
> +AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [], [stdout])
> +AT_CHECK([egrep '68' stdout], [], [stdout])
> +
> +AT_CHECK([ovs-appctl dpctl/show], [], [stdout])
> +AT_CHECK([egrep 'mtu=68,' stdout], [], [stdout])
> +
> +dnl set MTU value below lower bound and check for error
> +AT_CHECK(ovs-vsctl set Interface dpdkvhostuserclient0 mtu_request=67)
> +AT_CHECK([grep "dpdkvhostuserclient0: unsupported MTU 67" ovs-
> vswitchd.log], [], [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
> +\@dpdkvhostuserclient0: unsupported MTU 67@d
> +\@failed to set MTU for network device dpdkvhostuserclient0: Invalid
> argument@d
> +])")
> +AT_CLEANUP
> +dnl 
> --------------------------------------------------------------------------
> +
> +
> +
>  dnl 
> --------------------------------------------------------------------------
>  dnl Add standard DPDK PHY port
>  AT_SETUP([OVS-DPDK - MFEX Autovalidator])
> --
> 2.25.1

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

Reply via email to