On Tue, Jun 6, 2017 at 4:55 PM, Ben Pfaff <b...@ovn.org> wrote: > On Fri, May 26, 2017 at 08:47:44PM -0700, Andy Zhou wrote: >> There are three methods of translating openflow layer clone action. >> Using datapath clone action, datapath sample action or using actions >> to negating the any changes within a clone. Xlate logic selects >> a specific method depends on datapath features detected at the boot time. >> >> Currently only xlate_clone() implements the selection logic since it >> is the solo user, but patches will add more users. Introduce >> new APIs that hide the details of generating datapath clone action. >> >> Signed-off-by: Andy Zhou <az...@ovn.org> > > This adds a nice layer of abstraction. Thanks!
Thanks for the careful review and comments. > > I don't think it's necessary to use malloc and free to allocate these > structures, if the caller provides an instance of struct > compose_clone_info. That seems like a better choice, in my opinion. > Good point. Malloc is not essential here. Will drop. > I think that this implementation fails when it chooses > COMPOSE_CLONE_USING_ACTIONS, because in that case it does not save and > restore the base flow. > > A possible improvement to the implementation would be to keep track of > the nesting level and fall back from COMPOSE_CLONE_USING_SAMPLE to > COMPOSE_CLONE_USING_ACTIONS when it exceeds the datapath's capability. > If datapath is actually required, then the translation won't be correct. Should we just abort the translation and log an error? > The code might be a little more straightforward without breaking > compose_clone_method() into a separate function, because then there is > no need to have a separate switch statement to again discover what > method to use. But if you prefer this implementation, I understand that > too. It is likely I will drop the compose_clone_method() when the enhancements required to address your comment for the next patch. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
