odp-execute: Consolidate callbacks.

Use one callback instead of many, helps in adding new functionality
later on.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 19fbdcf..0763697 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1345,6 +1345,16 @@ dpif_netdev_wait(struct dpif *dpif)
     ovs_mutex_unlock(&dp_netdev_mutex);
 }
 
+static void
+dp_netdev_output_port(struct dp_netdev *dp, struct ofpbuf *packet,
+                      odp_port_t out_port)
+{
+    struct dp_netdev_port *p = dp->ports[odp_to_u32(out_port)];
+    if (p) {
+        netdev_send(p->netdev, packet);
+    }
+}
+
 static int
 dp_netdev_output_userspace(struct dp_netdev *dp, struct ofpbuf *packet,
                            int queue_no, const struct flow *flow,
@@ -1397,35 +1407,42 @@ struct dp_netdev_execute_aux {
 };
 
 static void
-dp_netdev_action_output(void *aux_, struct ofpbuf *packet,
-                        const struct flow *flow OVS_UNUSED,
-                        odp_port_t out_port)
+dp_execute_cb(void *aux_, struct ofpbuf *packet, struct flow *flow OVS_UNUSED,
+              const struct nlattr *a, bool may_steal)
 {
     struct dp_netdev_execute_aux *aux = aux_;
-    struct dp_netdev_port *p = aux->dp->ports[odp_to_u32(out_port)];
-    if (p) {
-        netdev_send(p->netdev, packet);
-    }
-}
+    int type = nl_attr_type(a);
 
-static void
-dp_netdev_action_userspace(void *aux_, struct ofpbuf *packet,
-                           const struct flow *flow OVS_UNUSED,
-                           const struct nlattr *a, bool may_steal)
-{
-    struct dp_netdev_execute_aux *aux = aux_;
-    const struct nlattr *userdata;
+    switch ((enum ovs_action_attr)type) {
+    case OVS_ACTION_ATTR_OUTPUT:
+        dp_netdev_output_port(aux->dp, packet, u32_to_odp(nl_attr_get_u32(a)));
+        break;
+
+    case OVS_ACTION_ATTR_USERSPACE: {
+        const struct nlattr *userdata;
 
-    userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA);
+        userdata = nl_attr_find_nested(a, OVS_USERSPACE_ATTR_USERDATA);
 
-    /* Make a copy if we are not allowed to steal the packet's data. */
-    if (!may_steal) {
-        packet = ofpbuf_clone_with_headroom(packet, DP_NETDEV_HEADROOM);
+        /* Make a copy if we are not allowed to steal the packet's data. */
+        if (!may_steal) {
+            packet = ofpbuf_clone_with_headroom(packet, DP_NETDEV_HEADROOM);
+        }
+        dp_netdev_output_userspace(aux->dp, packet, DPIF_UC_ACTION, aux->key,
+                                   userdata);
+        if (!may_steal) {
+            ofpbuf_uninit(packet);
+        }
+        break;
     }
-    dp_netdev_output_userspace(aux->dp, packet, DPIF_UC_ACTION, aux->key,
-                               userdata);
-    if (!may_steal) {
-        ofpbuf_uninit(packet);
+    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_MAX:
+        OVS_NOT_REACHED();
     }
 }
 
@@ -1438,7 +1455,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp, const struct flow *key,
     struct flow md = *key;   /* Packet metadata, may be modified by actions. */
 
     odp_execute_actions(&aux, packet, &md, actions, actions_len,
-                        dp_netdev_action_output, dp_netdev_action_userspace);
+                        dp_execute_cb);
 }
 
 const struct dpif_class dpif_netdev_class = {

diff --git a/lib/dpif.c b/lib/dpif.c
index 6b519b8..e256558 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1091,28 +1091,29 @@ dpif_execute_helper_execute__(void *aux_, struct ofpbuf *packet,
     }
 }
 
+/* This is called for actions that need the context of the datapath to be
+ * meaningful. */
 static void
-dpif_execute_helper_output_cb(void *aux, struct ofpbuf *packet,
-                              const struct flow *flow, odp_port_t out_port)
-{
-    uint64_t actions_stub[DIV_ROUND_UP(NL_A_U32_SIZE, 8)];
-    struct ofpbuf actions;
-
-    ofpbuf_use_stack(&actions, actions_stub, sizeof actions_stub);
-    nl_msg_put_u32(&actions, OVS_ACTION_ATTR_OUTPUT, odp_to_u32(out_port));
-
-    dpif_execute_helper_execute__(aux, packet, flow,
-                                  actions.data, actions.size);
-}
-
-static void
-dpif_execute_helper_userspace_cb(void *aux, struct ofpbuf *packet,
-                                 const struct flow *flow,
-                                 const struct nlattr *action,
-                                 bool may_steal OVS_UNUSED)
-{
-    dpif_execute_helper_execute__(aux, packet, flow,
-                                  action, NLA_ALIGN(action->nla_len));
+dpif_execute_helper_cb(void *aux, struct ofpbuf *packet, struct flow *flow,
+                       const struct nlattr *action, bool may_steal OVS_UNUSED)
+{
+    int type = nl_attr_type(action);
+    switch ((enum ovs_action_attr)type) {
+    case OVS_ACTION_ATTR_OUTPUT:
+    case OVS_ACTION_ATTR_USERSPACE:
+        dpif_execute_helper_execute__(aux, packet, flow,
+                                      action, NLA_ALIGN(action->nla_len));
+        break;
+    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_MAX:
+        OVS_NOT_REACHED();
+    }
 }
 
 /* Executes 'execute' by performing most of the actions in userspace and
@@ -1139,8 +1140,7 @@ dpif_execute_with_help(struct dpif *dpif, const struct dpif_execute *execute)
 
     odp_execute_actions(&aux, execute->packet, &flow,
                         execute->actions, execute->actions_len,
-                        dpif_execute_helper_output_cb,
-                        dpif_execute_helper_userspace_cb);
+                        dpif_execute_helper_cb);
     return aux.error;
 }
 

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 2487118..7a96ca0 100644 (file)
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -29,7 +29,8 @@
 #include "util.h"
 
 static void
-odp_eth_set_addrs(struct ofpbuf *packet, const struct ovs_key_ethernet *eth_key)
+odp_eth_set_addrs(struct ofpbuf *packet,
+                  const struct ovs_key_ethernet *eth_key)
 {
     struct eth_header *eh = packet->l2;
 
@@ -60,7 +61,7 @@ set_arp(struct ofpbuf *packet, const struct ovs_key_arp *arp_key)
 
 static void
 odp_execute_set_action(struct ofpbuf *packet, const struct nlattr *a,
-                       struct flow *flow)
+                       struct flow *md)
 {
     enum ovs_key_attr type = nl_attr_type(a);
     const struct ovs_key_ipv4 *ipv4_key;
@@ -71,15 +72,15 @@ odp_execute_set_action(struct ofpbuf *packet, const struct nlattr *a,
 
     switch (type) {
     case OVS_KEY_ATTR_PRIORITY:
-        flow->skb_priority = nl_attr_get_u32(a);
+        md->skb_priority = nl_attr_get_u32(a);
         break;
 
     case OVS_KEY_ATTR_TUNNEL:
-        odp_set_tunnel_action(a, &flow->tunnel);
+        odp_set_tunnel_action(a, &md->tunnel);
         break;
 
     case OVS_KEY_ATTR_SKB_MARK:
-        flow->pkt_mark = nl_attr_get_u32(a);
+        md->pkt_mark = nl_attr_get_u32(a);
         break;
 
     case OVS_KEY_ATTR_ETHERNET:
@@ -141,13 +142,12 @@ odp_execute_set_action(struct ofpbuf *packet, const struct nlattr *a,
 static void
 odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key,
                       const struct nlattr *actions, size_t actions_len,
-                      odp_output_cb output, odp_userspace_cb userspace,
-                      bool more_actions);
+                      odp_execute_cb dp_execute_action, bool more_actions);
 
 static void
-odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *key,
-                   const struct nlattr *action, odp_output_cb output,
-                   odp_userspace_cb userspace, bool more_actions)
+odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *md,
+                   const struct nlattr *action,
+                   odp_execute_cb dp_execute_action, bool more_actions)
 {
     const struct nlattr *subactions = NULL;
     const struct nlattr *a;
@@ -174,16 +174,15 @@ odp_execute_sample(void *dp, struct ofpbuf *packet, struct flow *key,
         }
     }
 
-    odp_execute_actions__(dp, packet, key, nl_attr_get(subactions),
-                          nl_attr_get_size(subactions), output, userspace,
+    odp_execute_actions__(dp, packet, md, nl_attr_get(subactions),
+                          nl_attr_get_size(subactions), dp_execute_action,
                           more_actions);
 }
 
 static void
-odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key,
+odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *md,
                       const struct nlattr *actions, size_t actions_len,
-                      odp_output_cb output, odp_userspace_cb userspace,
-                      bool more_actions)
+                      odp_execute_cb dp_execute_action, bool more_actions)
 {
     const struct nlattr *a;
     unsigned int left;
@@ -192,21 +191,16 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key,
         int type = nl_attr_type(a);
 
         switch ((enum ovs_action_attr) type) {
+            /* These only make sense in the context of a datapath. */
         case OVS_ACTION_ATTR_OUTPUT:
-            if (output) {
-                output(dp, packet, key, u32_to_odp(nl_attr_get_u32(a)));
-            }
-            break;
-
-        case OVS_ACTION_ATTR_USERSPACE: {
-            if (userspace) {
-                /* Allow 'userspace' to steal the packet data if we do not
-                 * need it any more. */
+        case OVS_ACTION_ATTR_USERSPACE:
+            if (dp_execute_action) {
+                /* Allow 'dp_execute_action' to steal the packet data if we do
+                 * not need it any more. */
                 bool steal = !more_actions && left <= NLA_ALIGN(a->nla_len);
-                userspace(dp, packet, key, a, steal);
+                dp_execute_action(dp, packet, md, a, steal);
             }
             break;
-        }
 
         case OVS_ACTION_ATTR_PUSH_VLAN: {
             const struct ovs_action_push_vlan *vlan = nl_attr_get(a);
@@ -229,11 +223,11 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key,
             break;
 
         case OVS_ACTION_ATTR_SET:
-            odp_execute_set_action(packet, nl_attr_get(a), key);
+            odp_execute_set_action(packet, nl_attr_get(a), md);
             break;
 
         case OVS_ACTION_ATTR_SAMPLE:
-            odp_execute_sample(dp, packet, key, a, output, userspace,
+            odp_execute_sample(dp, packet, md, a, dp_execute_action,
                                more_actions || left > NLA_ALIGN(a->nla_len));
             break;
 
@@ -245,10 +239,10 @@ odp_execute_actions__(void *dp, struct ofpbuf *packet, struct flow *key,
 }
 
 void
-odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *key,
+odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *md,
                     const struct nlattr *actions, size_t actions_len,
-                    odp_output_cb output, odp_userspace_cb userspace)
+                    odp_execute_cb dp_execute_action)
 {
-    odp_execute_actions__(dp, packet, key, actions, actions_len, output,
-                          userspace, false);
+    odp_execute_actions__(dp, packet, md, actions, actions_len,
+                          dp_execute_action, false);
 }

diff --git a/lib/odp-execute.h b/lib/odp-execute.h
index 90a6788..993ff47 100644 (file)
--- a/lib/odp-execute.h
+++ b/lib/odp-execute.h
@@ -27,14 +27,16 @@ struct flow;
 struct nlattr;
 struct ofpbuf;
 
-typedef void (*odp_output_cb)(void *dp, struct ofpbuf *packet,
-                              const struct flow *key, odp_port_t out_port);
-typedef void (*odp_userspace_cb)(void *dp, struct ofpbuf *packet,
-                                 const struct flow *key,
-                                 const struct nlattr *action, bool may_steal);
+typedef void (*odp_execute_cb)(void *dp, struct ofpbuf *packet,
+                               struct flow *metadata,
+                               const struct nlattr *action, bool may_steal);
 
+/* Actions that need to be executed in the context of a datapath are handed
+ * to 'dp_execute_action', if non-NULL.  Currently this is called only for
+ * actions OVS_ACTION_ATTR_OUTPUT and OVS_ACTION_ATTR_USERSPACE so
+ * 'dp_execute_action' needs to handle only these. */
 void
-odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *key,
+odp_execute_actions(void *dp, struct ofpbuf *packet, struct flow *metadata,
                     const struct nlattr *actions, size_t actions_len,
-                    odp_output_cb output, odp_userspace_cb userspace);
+                    odp_execute_cb dp_execute_action);
 #endif

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 558598c..11e0855 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2093,7 +2093,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
                                           &ctx->mpls_depth_delta);
 
     odp_execute_actions(NULL, packet, &key, ctx->xout->odp_actions.data,
-                        ctx->xout->odp_actions.size, NULL, NULL);
+                        ctx->xout->odp_actions.size, NULL);
 
     pin = xmalloc(sizeof *pin);
     pin->up.packet_len = packet->size;