On Wed, Jul 27, 2011 at 5:58 AM, pravin shelar <[email protected]> wrote:
>
> ODP_ACTION_ATTR_CONTROLLER in the kernel actually sends packets to userspace,
> not the controller.
> To make it generic rename this action to ODP_ACTION_ATTR_USERSPACE.
>
> Signed-off-by: pravin b shelar <[email protected]>
This looks most good, just a few comments:
Can you cleanup the commit message a bit by wrapping at 70 characters
and capitalizing your name?
grep returns a few instances of comments that still refer to the old action:
jesse@umstead:~/openvswitch$ grep -r ODP_ACTION_ATTR_CONTROLLER *
lib/dpif.h: DPIF_UC_ACTION, /*
ODP_ACTION_ATTR_CONTROLLER action. */
lib/dpif.h: uint64_t userdata; /* Argument to
ODP_ACTION_ATTR_CONTROLLER. */
> diff --git a/include/openvswitch/datapath-protocol.h
> b/include/openvswitch/datapath-protocol.h
> index e7708ef..faaf064 100644
> --- a/include/openvswitch/datapath-protocol.h
> +++ b/include/openvswitch/datapath-protocol.h
> @@ -409,7 +409,7 @@ enum odp_flow_attr {
> enum odp_action_type {
> ODP_ACTION_ATTR_UNSPEC,
> ODP_ACTION_ATTR_OUTPUT, /* Output to switch port. */
> - ODP_ACTION_ATTR_CONTROLLER, /* Send copy to controller. */
> + ODP_ACTION_ATTR_USERSPACE, /* Send copy to userpace. */
There's a typo in userspace.
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 3b93a4c..771773e 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1299,7 +1299,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
> dp_netdev_output_port(dp, packet, nl_attr_get_u32(a));
> break;
>
> - case ODP_ACTION_ATTR_CONTROLLER:
> + case ODP_ACTION_ATTR_USERSPACE:
> dp_netdev_output_control(dp, packet, DPIF_UC_ACTION,
> key, nl_attr_get_u64(a));
Should we rename dp_netdev_output_control() to match the kernel, even
if it is all in userspace?
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index d7a3118..b59ecaf 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -99,7 +99,7 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
> case ODP_ACTION_ATTR_OUTPUT:
> ds_put_format(ds, "%"PRIu16, nl_attr_get_u32(a));
> break;
> - case ODP_ACTION_ATTR_CONTROLLER:
> + case ODP_ACTION_ATTR_USERSPACE:
> ds_put_format(ds, "ctl(%"PRIu64")", nl_attr_get_u64(a));
We should change 'ctl' to something like 'userspace' to reflect the new name.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
