> -----Original Message----- > From: Stokes, Ian <[email protected]> > Sent: Tuesday 28 June 2022 15:09 > To: Phelan, Michael <[email protected]>; [email protected] > Cc: [email protected]; [email protected]; [email protected] > Subject: RE: [PATCH] tests: Add OVS-DPDK MTU unit tests. > > > > > -----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.
Hey Ian, Thanks for the review, I have left some responses inline 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? Yep, I think that makes sense, no need for the extra check in that case. I will remove it in the next version. > > > + > > +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. I might need to play around with triggering those warnings to see if the variables change across machines but other than that I think it will be good to include a check for each of them. I will add them in the next version also. Thanks, Michael. > > 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
