On Wed, May 15, 2013 at 1:31 AM, Simon Horman <ho...@verge.net.au> wrote: > This moves generic action execution code out of lib/dpif-netedev.c > and into a new file, lib/execute-actions.c. > > This is in preparation for using execute_set_action() > in lib/odp-util.c to handle recirculation/ > > Signed-off-by: Simon Horman <ho...@verge.net.au>
Here's a combined set of comments for all of the patches in the execute-actions code: * The convention used other places in the code when passing around void pointers and casting them is to have the variable named with an underscore be the void *. * I would extract the userdata for userspace actions in execute_actions() and pass it directly to the userspace function pointer. * I don't think there is a lot of value in the asserts in execute_actions(). I think it's valid to pass NULL for 'dp' and 'key' in these situations if you want. The function pointers can't be NULL but that will stop the program just as quickly as an assert anyways. * The later patches pass around a huge number of arguments. We should find some way of encapsulating them in a struct. It might even be possible to store them directly in the flow. It deviates slightly from other behavior because we don't update the flow when we change the packet but it would clean a lot of other things up. * We already do some action execution in userspace when we send packets to the controller. It would be good to see if we can start using this code there as well. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
