[ovs-dev] [slow path HASH and RECIRC action v2] dpif: Fix slow action handling for DP_HASH and RECIRC

[Andy Zhou]

In case DP_HASH and RECIRC actions need to be executed in slow path,
current implementation simply don't handle them -- vswitchd simply
crashes. This patch fixes them by supply an implementation for them.

RECIRC will be handled by the datapath, same as the output action.

DP_HASH, on the other hand, is handled in the user space. Although the
resulting hash values may not match those computed by the datapath, it
is less expensive; current use case (bonding) does not require a strict
match to work properly.

Reported-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
Signed-off-by: Andy Zhou <azhou at nicira.com>

---
v1->v2:   Addresses Ben's review feedback
          fix the comment on why execute dp_hash in user space.
	  Add assert for unknown hash algorithm specification.
---
 lib/dpif.c        |  4 ++--
 lib/odp-execute.c | 23 ++++++++++++++++++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/lib/dpif.c b/lib/dpif.c
index 84dba28..ac73be1 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1069,6 +1069,7 @@ dpif_execute_helper_cb(void *aux_, struct ofpbuf *packet,
     switch ((enum ovs_action_attr)type) {
     case OVS_ACTION_ATTR_OUTPUT:
     case OVS_ACTION_ATTR_USERSPACE:
+    case OVS_ACTION_ATTR_RECIRC:
         execute.actions = action;
         execute.actions_len = NLA_ALIGN(action->nla_len);
         execute.packet = packet;
@@ -1077,6 +1078,7 @@ dpif_execute_helper_cb(void *aux_, struct ofpbuf *packet,
         aux->error = aux->dpif->dpif_class->execute(aux->dpif, &execute);
         break;
 
+    case OVS_ACTION_ATTR_HASH:
     case OVS_ACTION_ATTR_PUSH_VLAN:
     case OVS_ACTION_ATTR_POP_VLAN:
     case OVS_ACTION_ATTR_PUSH_MPLS:
@@ -1084,8 +1086,6 @@ dpif_execute_helper_cb(void *aux_, struct ofpbuf *packet,
     case OVS_ACTION_ATTR_SET:
     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 12ad679..cc18536 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -26,6 +26,7 @@
 #include "ofpbuf.h"
 #include "odp-util.h"
 #include "packets.h"
+#include "flow.h"
 #include "unaligned.h"
 #include "util.h"
 
@@ -208,7 +209,6 @@ 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. */
@@ -219,6 +219,27 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, bool steal,
             }
             break;
 
+        case OVS_ACTION_ATTR_HASH: {
+            const struct ovs_action_hash *hash_act = nl_attr_get(a);
+
+            /* Calculate a hash value directly.  This might not match the
+             * value computed by the datapath, but it is much less expensive,
+             * and the current use case (bonding) does not require a strict
+             * match to work properly. */
+            if (hash_act->hash_alg == OVS_HASH_ALG_L4) {
+                struct flow flow;
+                uint32_t hash;
+
+                flow_extract(packet, md, &flow);
+                hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
+                md->dp_hash = hash ? hash : 1;
+            } else {
+                /* Assert on unknown hash algorithm.  */
+                OVS_NOT_REACHED();
+            }
+            break;
+        }
+
         case OVS_ACTION_ATTR_PUSH_VLAN: {
             const struct ovs_action_push_vlan *vlan = nl_attr_get(a);
             eth_push_vlan(packet, htons(ETH_TYPE_VLAN), vlan->vlan_tci);
-- 
1.9.1