> From: Seamus Ryan <[email protected]> > > This adds 4 new unit tests to the 'check-dpdk' subsystem that will > test rate limiting functionality. >
Thanks for the patch Michael, a few comments below. I'm still conducting some testing so may have some follow up comments for full review. > Signed-off-by: Seamus Ryan <[email protected]> Probably for the v2 you should add yourself as a Co-author and add your own sign off tag as well. > --- > NEWS | 3 +- > tests/system-dpdk.at | 128 > +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 130 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index 8fa57836a..e1fbc39ed 100644 > --- a/NEWS > +++ b/NEWS > @@ -3,7 +3,8 @@ Post-v2.17.0 > - OVSDB: > * 'relay' service model now supports transaction history, i.e. honors > the > 'last-txn-id' field in 'monitor_cond_since' requests from clients. > - No need to remove the white space above, You can just add the DPDK section straight after the last line above. There should still be 2 white spaces between this section and the v2.17 section below. > + - DPDK: > + * Add rate limiting unit tests to DPDK testsuite Minor, missing period at end of the sentence above. > > v2.17.0 - 17 Feb 2022 > --------------------- > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at > index 7d2715c4a..6d4707f84 100644 > --- a/tests/system-dpdk.at > +++ b/tests/system-dpdk.at > @@ -222,6 +222,134 @@ OVS_VSWITCHD_STOP("m4_join([], > [SYSTEM_DPDK_ALLOWED_LOGS], [ > AT_CLEANUP > dnl > -------------------------------------------------------------------------- > Just to keep with the existing style for the majority of the unit tests please ensure 3 whitespaces between the dnl above and below here. > +dnl > -------------------------------------------------------------------------- > +dn1 Rate create delete phy port Should this not be dnl instead of dn1? As this is ingress policing I think it would be better to reflect that in the text throughout, you can have rate limiting technically on the QoS (Egress) and on ingress policing also so it would be good to reflect which is being checked here. > +AT_SETUP([OVS-DPDK - Rate create delete phy port]) > +AT_KEYWORDS([dpdk]) > + > +OVS_DPDK_PRE_PHY_SKIP() > +OVS_DPDK_START() > + > +dnl Add userspace bridge and attach it to OVS and add policer > +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 ingress_policing_rate=10000 > ingress_policing_burst=1000]) > +AT_CHECK([ovs-vsctl show], [], [stdout]) What's the purpose of this check? I'm guessing that you are probably looking to confirm the rate and burst levels but I don't see an explicit check for it? If so maybe you need to search for the required fields explicitly rather than just the show command itself? > +sleep 2 > + > +dn1 remove policer dn1 should be dnl > +AT_CHECK([ovs-vsctl set interface phy0 ingress_policing_rate=0 > ingress_policing_burst=0]) > + > +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 > +])") Not sure why we need the OVS_VSWITCHD_STOP command above to check for vhost client connection error, this test only looks at using a standard DPDK PHY device so we shouldn't see this message would be my thinking as we are not adding a vhost port? > +AT_CLEANUP > +dnl > -------------------------------------------------------------------------- > + > +dnl > -------------------------------------------------------------------------- > +dn1 Rate create delete vport port dn1 -> dnl > +AT_SETUP([OVS-DPDK - Rate create delete vport port]) > +AT_KEYWORDS([dpdk]) > + > +OVS_DPDK_PRE_CHECK() > +OVS_DPDK_START() > + > +dnl Add userspace bridge and attach it to OVS and add policer > +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 > ingress_policing_rate=10000 ingress_policing_burst=1000]) > +AT_CHECK([ovs-vsctl show], [], [stdout]) > +sleep 2 > + > +dn1 remove policer dn1 -> dnl > +AT_CHECK([ovs-vsctl set interface dpdkvhostuserclient0 > ingress_policing_rate=0 ingress_policing_burst=0]) > + > +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 > -------------------------------------------------------------------------- > + 3 whitspaces between dnl above and below to separate the unit tests > +dnl > -------------------------------------------------------------------------- > +dn1 Rate no policing rate dn1 - > dnl > +AT_SETUP([OVS-DPDK - Rate no policing rate]) > +AT_KEYWORDS([dpdk]) > + > +OVS_DPDK_PRE_CHECK() > +OVS_DPDK_START() > + > +dnl Add userspace bridge and attach it to OVS and add policer > +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 > ingress_policing_burst=1000]) > +AT_CHECK([ovs-vsctl show], [], [stdout]) > +sleep 2 > + > +dn1 check policer was created correctly dn1 -> dnl > +AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout]) > +AT_CHECK([egrep 'ingress_policing_burst: 1000' stdout], [], [stdout]) > + > +AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout]) > +AT_CHECK([egrep 'ingress_policing_rate: 0' 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 > -------------------------------------------------------------------------- > + 3 whitspaces between dnl above and below to separate the unit tests > +dnl > -------------------------------------------------------------------------- > +dn1 Rate no policing burst > +AT_SETUP([OVS-DPDK - Rate no policing burst]) > +AT_KEYWORDS([dpdk]) > + > +OVS_DPDK_PRE_CHECK() > +OVS_DPDK_START() > + > +dnl Add userspace bridge and attach it to OVS and add policer > +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 > ingress_policing_rate=10000]) > +AT_CHECK([ovs-vsctl show], [], [stdout]) > +sleep 2 > + > +dn1 check policer was created correctly > +AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout]) > +AT_CHECK([egrep 'ingress_policing_burst: 0' stdout], [], [stdout]) > + > +AT_CHECK([ovs-vsctl list interface dpdkvhostuserclient0], [], [stdout]) > +AT_CHECK([egrep 'ingress_policing_rate: 10000' stdout], [], [stdout]) > + So when burst is not defined it should default to 8000 from what I remember, it would be good if we could check this and confirm it in the test? Thanks Ian > +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 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
