> > > > 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
