dpif: Fix slow action handling for DP_HASH and RECIRC

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 <[email protected]> Signed-off-by: Andy Zhou <[email protected]> Acked-by: YAMAMOTO Takashi <[email protected]>
subashd · Jun 4, 2014 · c6bf49f · c6bf49f
1 parent 50a1a1d
commit c6bf49f
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git 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,15 +1078,14 @@ 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:
     case OVS_ACTION_ATTR_POP_MPLS:
     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
@@ -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);