On 15 July 2014 12:11, Ben Pfaff <b...@nicira.com> wrote:
> dpif_netdev_flow_get() calls dp_netdev_flow_get_actions() twice. I
> did not check whether the actions could change in between. Please
> change the code to just retrieve the actions once.
>
> Acked-by: Ben Pfaff <b...@nicira.com>
>
> As an additional optimization I think it would be reasonable for
> dpif_netdev_flow_get() to return the actions from the
> dp_netdev_actions directly, without copying them, since they are
> immutable and RCU-protected from destruction, something like this:
>
> if (actionsp) {
> struct dp_netdev_actions *actions;
>
> actions = dp_netdev_flow_get_actions(netdev_flow);
> *actionsp = actions->actions;
> *actions_len = actions->size;
> }
>
Thanks, this cleans the code up quite a bit. I plan to push this soon.
I notice that dpif_netdev_flow_dump_next() also does this optimization for
actions, but there's no mention of RCU in the dpif_flow_dump_next() API. Do
you think we should add a comment like this to dpif_flow_get() and
dpif_flow_dump_next()?
"Implementations may opt to point flow->mask and/or flow->actions at
RCU-protected data rather than making a copy of them. Therefore, callers
that wish to hold these over quiesce periods must make a copy of these
fields before quiescing."
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
