[ovs-dev] [PATCH v11 2/6] Add set skb_mark, set_priority and tunnel support to execute_set_action

[Ben Pfaff]

On Wed, May 29, 2013 at 03:06:39PM +0900, Simon Horman wrote:
> Add set skb_mark support to execute_set_action.

Now renamed odp_execute_actions().

> This also adds support for the user-space datapath
> to honour such actions if they occur before recirculation,
> which will be added by a subsequent patch.
> 
> This is in preparation for using execute_set_action()

Ditto.

> to handle recirculation.
> 
> Signed-off-by: Simon Horman <horms at verge.net.au>


> index bd00f18..a2a8ed4 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -159,6 +159,10 @@ static void dp_netdev_execute_actions(struct dp_netdev *,
>                                        struct ofpbuf *, struct flow *,
>                                        const struct nlattr *actions,
>                                        size_t actions_len);
> +static void dp_netdev_port_input(struct dp_netdev *dp,
> +                                 struct dp_netdev_port *port,
> +                                 struct ofpbuf *packet, uint32_t skb_priority,
> +                                 uint32_t skb_mark, const struct flow_tnl *tnl);
>  
>  static struct dpif_netdev *
>  dpif_netdev_cast(const struct dpif *dpif)
> @@ -1032,20 +1036,21 @@ dp_netdev_flow_used(struct dp_netdev_flow *flow, const struct ofpbuf *packet)
>  
>  static void
>  dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port,
> -                     struct ofpbuf *packet)
> +                     struct ofpbuf *packet, uint32_t skb_priority,
> +                     uint32_t skb_mark, const struct flow_tnl *tnl)
>  {
> -    struct dp_netdev_flow *flow;
> +    struct dp_netdev_flow *dp_flow;

Any particular reason to rename flow to dp_flow here?  It makes the
patch larger and thus less obviously correct.

>      struct flow key;
>  
>      if (packet->size < ETH_HEADER_LEN) {
>          return;
>      }
> -    flow_extract(packet, 0, 0, NULL, port->port_no, &key);
> -    flow = dp_netdev_lookup_flow(dp, &key);
> -    if (flow) {
> -        dp_netdev_flow_used(flow, packet);
> +    flow_extract(packet, skb_priority, skb_mark, tnl, port->port_no, &key);
> +    dp_flow = dp_netdev_lookup_flow(dp, &key);
> +    if (dp_flow) {
> +        dp_netdev_flow_used(dp_flow, packet);
>          dp_netdev_execute_actions(dp, packet, &key,
> -                                  flow->actions, flow->actions_len);
> +                                  dp_flow->actions, dp_flow->actions_len);
>          dp->n_hit++;
>      } else {
>          dp->n_missed++;
> @@ -1071,7 +1076,7 @@ dpif_netdev_run(struct dpif *dpif)
>  
>          error = port->rx ? netdev_rx_recv(port->rx, &packet) : EOPNOTSUPP;
>          if (!error) {
> -            dp_netdev_port_input(dp, port, &packet);
> +            dp_netdev_port_input(dp, port, &packet, 0, 0, NULL);
>          } else if (error != EAGAIN && error != EOPNOTSUPP) {
>              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>              VLOG_ERR_RL(&rl, "error receiving data from %s: %s",

...

> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index a981d17..3635591 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -87,6 +87,8 @@ struct odputil_keybuf {
>      uint32_t keybuf[DIV_ROUND_UP(ODPUTIL_FLOW_KEY_BYTES, 4)];
>  };
>  
> +enum odp_key_fitness tun_key_from_attr(const struct nlattr *, struct flow_tnl *);
> +

Most of the other functions exported by the odp-util module have names
that begin with "odp_".  I'd like to maintain that here.

Otherwise this looks good, thanks.