On Wed, Sep 24, 2014 at 09:19:42AM +0100, Thomas Graf wrote:
> 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
Thanks, I think something along those lines could work out
quite well for a select group action.
> > > 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.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
