[ovs-dev] [PATCH 1/2] xlate: Add datapath clone generation API

[Ben Pfaff]

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 <azhou at ovn.org>

This adds a nice layer of abstraction.  Thanks!

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.

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.

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.