[ovs-dev] [PATCH/RFC repost 4/8] datapath: execution of select group action

[Thomas Graf]

On 09/24/14 at 03:01pm, Simon Horman wrote:
> > > +	/* Only possible type of attributes is OVS_SELECT_GROUP_ATTR_BUCKET */
> > > +	for (bucket = nla_data(attr), rem = nla_len(attr); rem > 0;
> > > +	     bucket = nla_next(bucket, &rem)) {
> > > +		uint16_t weight = bucket_weight(bucket);
> > 
> > I think we should validate only once when we copy then assume it is
> > correct.

> That is the intention of this code, is it doing something else?

I must have treated the BUG_ON as validation when I wrote that
comment ;-)

> Nice idea. I think that would work out quite well.
> 
> The main question for me would be where to store such a table,
> which comes back to my remark above about more storing a more
> efficient efficient form of the action.

I had the same issue when working on zone support for the 
conntrack action that is in the works. It needs to keep a ref to
a ct template which is registered with the conntrack engine. I
ended up converting the Netlink attributes to a struct and storing
that in an attribute in sf_acts then converting it back.

You can find the WIP code in the following branch:
https://github.com/tgraf/ovs/tree/nat

datapath: Use central function to free sw_flow_actions 
RFC: Add support for connection tracking. 
datapath: Add zone support

> > Do we need a recursion limit here?
> 
> I believe that is already handled by the depth check that occurs
> when the actions are copied.

Right, I can see it now. Thanks.