On 25/08/2021 17:36, Kumar Amber wrote: Please shorten the commit title as it is too long and should have "." at the end.
> Added 2 separate test-cases for DPCLS and DPIF commands: > 1018: PMD - DPCLS Configuration > 1017: PMD - DPIF Configuration > > Signed-off-by: Kumar Amber <[email protected]> > > --- > v2: > - Moved the test-cases to pmd.at from dpdk suit. > - Removed avx512 specific set commands as per discussion. > --- > tests/pmd.at | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/tests/pmd.at b/tests/pmd.at > index 225d4ee3a..09ae5bcd6 100644 > --- a/tests/pmd.at > +++ b/tests/pmd.at > @@ -1068,3 +1068,39 @@ AT_CHECK([ovs-appctl dpctl/del-dp dummy@dp0], [0], [dnl > > OVS_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([PMD - DPIF Configuration]) minor comment: the other pmd tests avoid using capitals (after "PMD -") and sentence case, so for consistency maybe it can be "PMD - dpif configuration". At least make "configuration" lower case. > +OVS_VSWITCHD_START( > + [], [], [], [--dummy-numa 0,0]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) Are dbg level logs needed? > +AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy-pmd]) > + > +AT_CHECK([ovs-vsctl show], [], [stdout]) > +AT_CHECK([ovs-appctl dpif-netdev/dpif-impl-get], [], [stdout]) > + This ensures that the dpif-impl-get command succeeded. Why not also check the output to ensure it is correct. > +AT_CHECK([ovs-appctl dpif-netdev/dpif-impl-set dpif_scalar], [0], [dnl > +DPIF implementation set to dpif_scalar. > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([PMD - DPCLS Configuration]) same comment as above re naming > +OVS_VSWITCHD_START( > + [], [], [], [--dummy-numa 0,0]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > +AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy-pmd]) > + > +AT_CHECK([ovs-vsctl show], [], [stdout]) > +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-get], [], [stdout]) > + This ensures that the command succeeds. Why not also check the output to ensure correct options and default priorities (maybe it should only check for presence of autovalidator and generic) > +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set autovalidator 3], > [0], [dnl > +Lookup priority change affected 0 dpcls ports and 0 subtables. > +]) > + As above, you can use dpif-netdev/subtable-lookup-prio-get to check it has been set correctly. > +AT_CHECK([ovs-appctl dpif-netdev/subtable-lookup-prio-set generic 4], [0], > [dnl > +Lookup priority change affected 0 dpcls ports and 0 subtables. > +]) > + As above, you can use dpif-netdev/subtable-lookup-prio-get to check it has been set correctly. You could check the behaviour in additional ways. e.g. - Check priorities can be changed again - Check if priorities can be the same - Check working with min prio, check behaviour with below prio - Check working with max prio, check behaviour with above prio > +OVS_VSWITCHD_STOP > +AT_CLEANUP > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
