[ovs-dev] [recirc datapath V2 1/3] dpif-netdev: Move hash function out of the recirc action, into its own action

Pravin Shelar pshelar at nicira.com
Wed Apr 16 16:53:03 UTC 2014


On Wed, Apr 16, 2014 at 6:51 AM, Andy Zhou <azhou at nicira.com> wrote:
> Currently recirculation action can optionally compute hash. This patch
> adds a hash action that is independent of the recirc action, which
> no longer computes hash.  For megaflow bond with recirc, the output
> to a bond port action will look like:
>
>     hash(hash_l4(0)), recric(<recirc_id>)
>
> Obviously, when a recirculation application that does not depend on
> hash value can just use the recirc action alone.
>
> Signed-off-by: Andy Zhou <azhou at nicira.com>

Looks good to me.
Acked-by: Pravin B Shelar <pshelar at nicira.com>

Thanks.

> ---
>  include/linux/openvswitch.h  | 21 +++++++++------------
>  lib/dpif-netdev.c            | 35 ++++++++++++++++++++++-------------
>  lib/dpif.c                   |  1 +
>  lib/odp-execute.c            |  1 +
>  lib/odp-util.c               | 29 ++++++++++++++++++++---------
>  ofproto/ofproto-dpif-xlate.c | 19 ++++++++++++-------
>  6 files changed, 65 insertions(+), 41 deletions(-)
>
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index b3977d1..e17f802 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -542,26 +542,22 @@ struct ovs_action_push_vlan {
>
>  /* Data path hash algorithm for computing Datapath hash.
>   *
> - * The Algorithm type only specifies the fields in a flow
> + * The algorithm type only specifies the fields in a flow
>   * will be used as part of the hash. Each datapath is free
>   * to use its own hash algorithm. The hash value will be
>   * opaque to the user space daemon.
>   */
> -enum ovs_recirc_hash_alg {
> -       OVS_RECIRC_HASH_ALG_NONE,
> -       OVS_RECIRC_HASH_ALG_L4,
> +enum ovs_hash_alg {
> +       OVS_HASH_ALG_L4,
>  };
>  /*
> - * struct ovs_action_recirc - %OVS_ACTION_ATTR_RECIRC action argument.
> - * @recirc_id: The Recirculation label, Zero is invalid.
> + * struct ovs_action_hash - %OVS_ACTION_ATTR_HASH action argument.
>   * @hash_alg: Algorithm used to compute hash prior to recirculation.
> - * @hash_bias: bias used for computing hash.  used to compute hash prior to
> - *             recirculation.
> + * @hash_bias: bias used for computing hash.
>   */
> -struct ovs_action_recirc {
> -       uint32_t  hash_alg;     /* One of ovs_dp_hash_alg. */
> +struct ovs_action_hash {
> +       uint32_t  hash_alg;     /* One of ovs_hash_alg. */
>         uint32_t  hash_bias;
> -       uint32_t  recirc_id;    /* Recirculation label. */
>  };
>
>  /**
> @@ -604,7 +600,8 @@ enum ovs_action_attr {
>         OVS_ACTION_ATTR_SAMPLE,       /* Nested OVS_SAMPLE_ATTR_*. */
>         OVS_ACTION_ATTR_PUSH_MPLS,    /* struct ovs_action_push_mpls. */
>         OVS_ACTION_ATTR_POP_MPLS,     /* __be16 ethertype. */
> -       OVS_ACTION_ATTR_RECIRC,       /* struct ovs_action_recirc. */
> +       OVS_ACTION_ATTR_RECIRC,       /* u32 recirc_id. */
> +       OVS_ACTION_ATTR_HASH,         /* struct ovs_action_hash. */
>         __OVS_ACTION_ATTR_MAX
>  };
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9ad9386..76141e3 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2138,25 +2138,34 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
>          break;
>      }
>
> +    case OVS_ACTION_ATTR_HASH: {
> +        const struct ovs_action_hash *hash_act;
> +        uint32_t hash;
> +
> +        hash_act = nl_attr_get(a);
> +        if (hash_act->hash_alg == OVS_HASH_ALG_L4) {
> +
> +            hash = flow_hash_symmetric_l4(aux->key, hash_act->hash_bias);
> +            if (!hash) {
> +                hash = 1; /* 0 is not valid */
> +            }
> +
> +        } else {
> +            VLOG_WARN("Unknown hash algorithm specified for the hash action.");
> +            hash = 2;
> +        }
> +
> +        md->dp_hash = hash;
> +        break;
> +    }
> +
>      case OVS_ACTION_ATTR_RECIRC:
>          if (*depth < MAX_RECIRC_DEPTH) {
>              struct pkt_metadata recirc_md = *md;
>              struct ofpbuf *recirc_packet;
> -            const struct ovs_action_recirc *act;
>
>              recirc_packet = may_steal ? packet : ofpbuf_clone(packet);
> -
> -            act = nl_attr_get(a);
> -            recirc_md.recirc_id = act->recirc_id;
> -            recirc_md.dp_hash = 0;
> -
> -            if (act->hash_alg == OVS_RECIRC_HASH_ALG_L4) {
> -                recirc_md.dp_hash = flow_hash_symmetric_l4(aux->key,
> -                                                           act->hash_bias);
> -                if (!recirc_md.dp_hash) {
> -                    recirc_md.dp_hash = 1;  /* 0 is not valid */
> -                }
> -            }
> +            recirc_md.recirc_id = nl_attr_get_u32(a);
>
>              (*depth)++;
>              dp_netdev_input(aux->dp, recirc_packet, &recirc_md);
> diff --git a/lib/dpif.c b/lib/dpif.c
> index a8bd674..41b8eb7 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1134,6 +1134,7 @@ dpif_execute_helper_cb(void *aux_, struct ofpbuf *packet,
>      case OVS_ACTION_ATTR_SAMPLE:
>      case OVS_ACTION_ATTR_UNSPEC:
>      case OVS_ACTION_ATTR_RECIRC:
> +    case OVS_ACTION_ATTR_HASH:
>      case __OVS_ACTION_ATTR_MAX:
>          OVS_NOT_REACHED();
>      }
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 37e44e3..12ad679 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -208,6 +208,7 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, bool steal,
>          case OVS_ACTION_ATTR_OUTPUT:
>          case OVS_ACTION_ATTR_USERSPACE:
>          case OVS_ACTION_ATTR_RECIRC:
> +        case OVS_ACTION_ATTR_HASH:
>              if (dp_execute_action) {
>                  /* Allow 'dp_execute_action' to steal the packet data if we do
>                   * not need it any more. */
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index cc73413..0969ce8 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -79,7 +79,8 @@ odp_action_len(uint16_t type)
>      case OVS_ACTION_ATTR_POP_VLAN: return 0;
>      case OVS_ACTION_ATTR_PUSH_MPLS: return sizeof(struct ovs_action_push_mpls);
>      case OVS_ACTION_ATTR_POP_MPLS: return sizeof(ovs_be16);
> -    case OVS_ACTION_ATTR_RECIRC: return sizeof(struct ovs_action_recirc);
> +    case OVS_ACTION_ATTR_RECIRC: return sizeof(uint32_t);
> +    case OVS_ACTION_ATTR_HASH: return sizeof(struct ovs_action_hash);
>      case OVS_ACTION_ATTR_SET: return -2;
>      case OVS_ACTION_ATTR_SAMPLE: return -2;
>
> @@ -387,16 +388,23 @@ format_mpls(struct ds *ds, const struct ovs_key_mpls *mpls_key,
>  }
>
>  static void
> -format_odp_recirc_action(struct ds *ds,
> -                         const struct ovs_action_recirc *act)
> +format_odp_recirc_action(struct ds *ds, uint32_t recirc_id)
>  {
> -    ds_put_format(ds, "recirc(");
> +    ds_put_format(ds, "recirc(%"PRIu32")", recirc_id);
> +}
>
> -    if (act->hash_alg == OVS_RECIRC_HASH_ALG_L4) {
> -        ds_put_format(ds, "hash_l4(%"PRIu32"), ", act->hash_bias);
> -    }
> +static void
> +format_odp_hash_action(struct ds *ds, const struct ovs_action_hash *hash_act)
> +{
> +    ds_put_format(ds, "hash(");
>
> -    ds_put_format(ds, "%"PRIu32")", act->recirc_id);
> +    if (hash_act->hash_alg == OVS_HASH_ALG_L4) {
> +        ds_put_format(ds, "hash_l4(%"PRIu32")", hash_act->hash_bias);
> +    } else {
> +        ds_put_format(ds, "Unknown hash algorithm(%"PRIu32")",
> +                      hash_act->hash_alg);
> +    }
> +    ds_put_format(ds, ")");
>  }
>
>  static void
> @@ -422,7 +430,10 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
>          format_odp_userspace_action(ds, a);
>          break;
>      case OVS_ACTION_ATTR_RECIRC:
> -        format_odp_recirc_action(ds, nl_attr_get(a));
> +        format_odp_recirc_action(ds, nl_attr_get_u32(a));
> +        break;
> +    case OVS_ACTION_ATTR_HASH:
> +        format_odp_hash_action(ds, nl_attr_get(a));
>          break;
>      case OVS_ACTION_ATTR_SET:
>          ds_put_cstr(ds, "set(");
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 91ce7b7..04998b9 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -1152,7 +1152,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
>
>              if (ctx->xout->use_recirc) {
>                  /* Only TCP mode uses recirculation. */
> -                xr->hash_alg = OVS_RECIRC_HASH_ALG_L4;
> +                xr->hash_alg = OVS_HASH_ALG_L4;
>                  bond_update_post_recirc_rules(out_xbundle->bond, false);
>
>                  /* Recirculation does not require unmasking hash fields. */
> @@ -1843,14 +1843,19 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
>                                                &ctx->xout->wc);
>
>          if (ctx->xout->use_recirc) {
> -            struct ovs_action_recirc *act_recirc;
> +            struct ovs_action_hash *act_hash;
>              struct xlate_recirc *xr = &ctx->xout->recirc;
>
> -            act_recirc = nl_msg_put_unspec_uninit(&ctx->xout->odp_actions,
> -                               OVS_ACTION_ATTR_RECIRC, sizeof *act_recirc);
> -            act_recirc->recirc_id = xr->recirc_id;
> -            act_recirc->hash_alg = xr->hash_alg;
> -            act_recirc->hash_bias = xr->hash_bias;
> +            /* Hash action. */
> +            act_hash = nl_msg_put_unspec_uninit(&ctx->xout->odp_actions,
> +                                                OVS_ACTION_ATTR_HASH,
> +                                                sizeof *act_hash);
> +            act_hash->hash_alg = xr->hash_alg;
> +            act_hash->hash_bias = xr->hash_bias;
> +
> +            /* Recirc action. */
> +            nl_msg_put_u32(&ctx->xout->odp_actions, OVS_ACTION_ATTR_RECIRC,
> +                           xr->recirc_id);
>          } else {
>              nl_msg_put_odp_port(&ctx->xout->odp_actions, OVS_ACTION_ATTR_OUTPUT,
>                                  out_port);
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list