dpif: Fix slow action handling for DP_HASH and RECIRC
authorAndy Zhou <azhou@nicira.com>
Thu, 29 May 2014 00:00:48 +0000 (17:00 -0700)
committerAndy Zhou <azhou@nicira.com>
Wed, 4 Jun 2014 21:08:21 +0000 (14:08 -0700)
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@valinux.co.jp>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>
lib/dpif.c
lib/odp-execute.c

index 41b8eb7..48b2d1e 100644 (file)
@@ -1118,6 +1118,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;
@@ -1126,6 +1127,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:
@@ -1133,8 +1135,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();
     }
index 12ad679..cc18536 100644 (file)
@@ -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);