> -----Original Message----- > From: William Tu <[email protected]> > Sent: Saturday, June 27, 2020 7:27 PM > To: Van Haaren, Harry <[email protected]> > Cc: ovs-dev <[email protected]>; Stokes, Ian <[email protected]>; > Ilya Maximets <[email protected]>; Federico Iezzi <[email protected]> > Subject: Re: [PATCH v4 1/7] dpif-netdev: implement subtable lookup validation. > > On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren > <[email protected]> wrote: > > > > This commit refactors the existing dpif subtable function pointer > > infrastructure, and implements an autovalidator component. > > > > The refactoring of the existing dpcls subtable lookup function > > handling, making it more generic, and cleaning up how to enable > > more implementations in future. > > > > In order to ensure all implementations provide identical results, > > the autovalidator is added. The autovalidator itself implements > > the subtable lookup function prototype, but internally iterates > > over all other available implementations. The end result is that > > testing of each implementation becomes automatic, when the auto- > > validator implementation is selected. > > > > Signed-off-by: Harry van Haaren <[email protected]> > > > > --- > > > > v4: > > - Fix automake .c file order (William Tu) > > - Fix include file ordering: netdev-lookup before netdev-perf (William Tu) > > - Fix Typos (William Tu) > > - Add --enable-autovalidator compile time flag to set the DPCLS > > Autovalidator > > to the highest priority by default. This is required to run the unit tests > > with all DPCLS implementations without changing every test-case. > > Backwards compatibility is kept with OVS 2.13. > > Why adding this option at compile time? > To run the unit test, we can simply use the set-prio command to set > the autovalidator to the highest before starting the unit test, right?
As you noted later in the patchset (https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/372114.html) the goal is to enable autovalidator *by default*. This is not the default behavior we want in packaged versions of OVS, only for DPCLS test builds. By making this compile-time instead of runtime, it ensures any test that exists, or is developed in future can be tested with the autovalidator without any extra manual effort to enable autovalidation. > > - Add check to ensure autovalidator is 0th item in lookup array > > What's your concern here? Users can only change the prio, not > the .probe function. So autovalidator is always at 0th item. > If you worry about code being changed accidentally in the future, > how about using BUILD_ASSERT_DECL? Yes - good improvement. > The rest looks good to me. > Thanks! Thanks for review. <snip patch> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
