> > > > From: Seamus Ryan <seamus.r...@intel.com> > > > > > > > > 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. > > > > Hi Michael, had some more time for testing today and have a few more > > follow up comments inline below for the v2. > > Hey Ian, > > Thanks for the review, I will incorporate your comments and push a V2 ASAP. > > I've also left a couple of comments below on things I wasn't sure on that you > might be able to help clarify.
Sure, responses inline. <snip> > > > 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? > > > > So just to confirm, show command here wont provide useful infor for the > > You're right in saying that the show command doesn't show any specific > information relating to the ingress policer but it appears in all other tests > after > creating a port on a bridge so the style was copied for these tests. > > Do you know of a reason to do this or is it unnecessary? Hmmm, I'm thinking this is a step to confirm the port was added as expected in the other tests cases. In this case I don't think it will do any harm to keep it as it is following the existing design/steps the unit tests implement. <snip> > > > 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? > > > > Actually I'm not sure how we can check this as the value will be 0 in the > > database so its difficult to confirm (I believe from the code the default > > value > > will be set to 8000 but this will not be reflected back in the database). > > Yes, I couldn't find any command which showed that the burst had been set to > 8000 as a default value, is it best to leave it as is in that case? For the moment yes, unless we have a different method to confirm which I cant think of at the moment I think it's safe to leave the check for burst value out here. Thanks Ian > > Thanks, > Michael. > > > > > Thanks > > Ian > > > > > > > > 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev