dpif-netdev: Move hash function out of the recirc action, into its own action

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)), recirc(<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@nicira.com>
Reviewed-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
Acked-by: Pravin B Shelar <pshelar@nicira.com

diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index b3977d1..e17f802 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 cc4be6d..1ba5462 100644 (file)
--- 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);