[ovs-dev] [packet_in 12/13] ofproto-dpif: Implement PACKET_IN in userspace.

Ethan Jackson ethan at nicira.com
Thu Dec 29 01:52:39 UTC 2011


In future patches, PACKET_IN messages will include meta-data which
is only available in userspace during action translation.  Either,
this data needs to be stored until it's required by a userspace
datapath action, or the PACKET_IN messages must be sent at the time
the data is available.  This patch implements the latter.
---
 lib/odp-util.c         |   15 +----
 lib/odp-util.h         |    1 -
 ofproto/ofproto-dpif.c |  163 +++++++++++++++++------------------------------
 tests/ofproto-dpif.at  |    4 +-
 4 files changed, 62 insertions(+), 121 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 490d35e..bfc1937 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -189,9 +189,7 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
 
         memcpy(&cookie, &userdata, sizeof cookie);
 
-        if (cookie.type == USER_ACTION_COOKIE_CONTROLLER) {
-            ds_put_format(ds, ",controller,length=%"PRIu32, cookie.data);
-        } else if (cookie.type == USER_ACTION_COOKIE_SFLOW) {
+        if (cookie.type == USER_ACTION_COOKIE_SFLOW) {
             ds_put_format(ds, ",sFlow,n_output=%"PRIu8","
                           "vid=%"PRIu16",pcp=%"PRIu8",ifindex=%"PRIu32,
                           cookie.n_output, vlan_tci_to_vid(cookie.vlan_tci),
@@ -328,7 +326,6 @@ parse_odp_action(const char *s, const struct shash *port_names,
 
     {
         unsigned long long int pid;
-        unsigned long long int length;
         unsigned long long int ifindex;
         char userdata_s[32];
         int n_output;
@@ -338,16 +335,6 @@ parse_odp_action(const char *s, const struct shash *port_names,
         if (sscanf(s, "userspace(pid=%lli)%n", &pid, &n) > 0 && n > 0) {
             odp_put_userspace_action(pid, NULL, actions);
             return n;
-        } else if (sscanf(s, "userspace(pid=%lli,controller,length=%lli)%n",
-                          &pid, &length, &n) > 0 && n > 0) {
-            struct user_action_cookie cookie;
-
-            cookie.type = USER_ACTION_COOKIE_CONTROLLER;
-            cookie.n_output = 0;
-            cookie.vlan_tci = htons(0);
-            cookie.data = length;
-            odp_put_userspace_action(pid, &cookie, actions);
-            return n;
         } else if (sscanf(s, "userspace(pid=%lli,sFlow,n_output=%i,vid=%i,"
                           "pcp=%i,ifindex=%lli)%n", &pid, &n_output,
                           &vid, &pcp, &ifindex, &n) > 0 && n > 0) {
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 7c9b588..a6f8a30 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -117,7 +117,6 @@ enum odp_key_fitness odp_flow_key_to_flow(const struct nlattr *, size_t,
 
 enum user_action_cookie_type {
     USER_ACTION_COOKIE_UNSPEC,
-    USER_ACTION_COOKIE_CONTROLLER,   /* Packet for controller. */
     USER_ACTION_COOKIE_SFLOW,        /* Packet for sFlow sampling. */
 };
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 33d5c2e..d6e0b17 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -314,12 +314,6 @@ static struct facet *facet_lookup_valid(struct ofproto_dpif *,
                                         const struct flow *);
 static bool facet_revalidate(struct ofproto_dpif *, struct facet *);
 
-static bool execute_controller_action(struct ofproto_dpif *,
-                                      const struct flow *,
-                                      const struct nlattr *odp_actions,
-                                      size_t actions_len,
-                                      struct ofpbuf *packet, bool clone);
-
 static void facet_flush_stats(struct ofproto_dpif *, struct facet *);
 
 static void facet_update_time(struct ofproto_dpif *, struct facet *,
@@ -2416,34 +2410,6 @@ send_packet_in_miss(struct ofproto_dpif *ofproto, struct ofpbuf *packet,
                            clone ? NULL : packet);
 }
 
-/* Sends an OFPT_PACKET_IN message for 'packet' of type OFPR_ACTION to each
- * OpenFlow controller as necessary according to their individual
- * configurations.
- *
- * 'send_len' should be the number of bytes of 'packet' to send to the
- * controller, as specified in the action that caused the packet to be sent.
- *
- * If 'clone' is true, the caller retains ownership of 'upcall->packet'.
- * Otherwise, ownership is transferred to this function. */
-static void
-send_packet_in_action(struct ofproto_dpif *ofproto, struct ofpbuf *packet,
-                      uint64_t userdata, const struct flow *flow, bool clone)
-{
-    struct ofputil_packet_in pin;
-    struct user_action_cookie cookie;
-
-    memcpy(&cookie, &userdata, sizeof(cookie));
-
-    pin.in_port = flow->in_port;
-    pin.reason = OFPR_ACTION;
-    pin.buffer_id = 0;          /* not yet known */
-    pin.send_len = cookie.data;
-    pin.total_len = packet->size;
-    ofpbuf_use_const(&pin.packet, packet->data, packet->size);
-    connmgr_send_packet_in(ofproto->up.connmgr, &pin, flow,
-                           clone ? NULL : packet);
-}
-
 static bool
 process_special(struct ofproto_dpif *ofproto, const struct flow *flow,
                 const struct ofpbuf *packet)
@@ -2546,6 +2512,8 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
 
     LIST_FOR_EACH_SAFE (packet, next_packet, list_node, &miss->packets) {
         struct dpif_flow_stats stats;
+        struct flow_miss_op *op;
+        struct dpif_execute *execute;
 
         list_remove(&packet->list_node);
         ofproto->n_matches++;
@@ -2568,28 +2536,21 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
             subfacet_make_actions(ofproto, subfacet, packet);
         }
 
-        /* Credit statistics to subfacet for this packet.  We must do this now
-         * because execute_controller_action() below may destroy 'packet'. */
         dpif_flow_stats_extract(&facet->flow, packet, &stats);
         subfacet_update_stats(ofproto, subfacet, &stats);
 
-        if (!execute_controller_action(ofproto, &facet->flow,
-                                       subfacet->actions,
-                                       subfacet->actions_len, packet, true)) {
-            struct flow_miss_op *op = &ops[(*n_ops)++];
-            struct dpif_execute *execute = &op->dpif_op.execute;
-
-            op->subfacet = subfacet;
-            execute->type = DPIF_OP_EXECUTE;
-            execute->key = miss->key;
-            execute->key_len = miss->key_len;
-            execute->actions
-                = (facet->may_install
-                   ? subfacet->actions
-                   : xmemdup(subfacet->actions, subfacet->actions_len));
-            execute->actions_len = subfacet->actions_len;
-            execute->packet = packet;
-        }
+        op = &ops[(*n_ops)++];
+        execute = &op->dpif_op.execute;
+        op->subfacet = subfacet;
+        execute->type = DPIF_OP_EXECUTE;
+        execute->key = miss->key;
+        execute->key_len = miss->key_len;
+        execute->actions = (facet->may_install
+                            ? subfacet->actions
+                            : xmemdup(subfacet->actions,
+                                      subfacet->actions_len));
+        execute->actions_len = subfacet->actions_len;
+        execute->packet = packet;
     }
 
     if (facet->may_install && subfacet->key_fitness != ODP_FIT_TOO_LITTLE) {
@@ -2761,10 +2722,6 @@ handle_userspace_upcall(struct ofproto_dpif *ofproto,
                                 &cookie);
         }
         ofpbuf_delete(upcall->packet);
-    } else if (cookie.type == USER_ACTION_COOKIE_CONTROLLER) {
-        COVERAGE_INC(ofproto_dpif_ctlr_action);
-        send_packet_in_action(ofproto, upcall->packet, upcall->userdata,
-                              &flow, false);
     } else {
         VLOG_WARN_RL(&rl, "invalid user cookie : 0x%"PRIx64, upcall->userdata);
         ofpbuf_delete(upcall->packet);
@@ -3101,39 +3058,6 @@ facet_free(struct facet *facet)
     free(facet);
 }
 
-/* If the 'actions_len' bytes of actions in 'odp_actions' are just a single
- * OVS_ACTION_ATTR_USERSPACE action, executes it internally and returns true.
- * Otherwise, returns false without doing anything.
- *
- * If 'clone' is true, the caller always retains ownership of 'packet'.
- * Otherwise, ownership is transferred to this function if it returns true. */
-static bool
-execute_controller_action(struct ofproto_dpif *ofproto,
-                          const struct flow *flow,
-                          const struct nlattr *odp_actions, size_t actions_len,
-                          struct ofpbuf *packet, bool clone)
-{
-    if (actions_len
-        && odp_actions->nla_type == OVS_ACTION_ATTR_USERSPACE
-        && NLA_ALIGN(odp_actions->nla_len) == actions_len) {
-        /* As an optimization, avoid a round-trip from userspace to kernel to
-         * userspace.  This also avoids possibly filling up kernel packet
-         * buffers along the way.
-         *
-         * This optimization will not accidentally catch sFlow
-         * OVS_ACTION_ATTR_USERSPACE actions, since those are encapsulated
-         * inside OVS_ACTION_ATTR_SAMPLE. */
-        const struct nlattr *nla;
-
-        nla = nl_attr_find_nested(odp_actions, OVS_USERSPACE_ATTR_USERDATA);
-        send_packet_in_action(ofproto, packet, nl_attr_get_u64(nla), flow,
-                              clone);
-        return true;
-    } else {
-        return false;
-    }
-}
-
 /* Executes, within 'ofproto', the 'n_actions' actions in 'actions' on
  * 'packet', which arrived on 'in_port'.
  *
@@ -3147,11 +3071,6 @@ execute_odp_actions(struct ofproto_dpif *ofproto, const struct flow *flow,
     struct ofpbuf key;
     int error;
 
-    if (execute_controller_action(ofproto, flow, odp_actions, actions_len,
-                                  packet, false)) {
-        return true;
-    }
-
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
     odp_flow_key_from_flow(&key, flow);
 
@@ -4286,16 +4205,52 @@ flood_packets(struct action_xlate_ctx *ctx, bool all)
 }
 
 static void
-compose_controller_action(struct action_xlate_ctx *ctx, int len)
+execute_controller_action(struct action_xlate_ctx *ctx, int len)
 {
-    struct user_action_cookie cookie;
+    struct ofputil_packet_in pin;
+    struct ofpbuf *packet;
 
-    commit_odp_actions(&ctx->flow, &ctx->base_flow, ctx->odp_actions);
-    cookie.type = USER_ACTION_COOKIE_CONTROLLER;
-    cookie.data = len;
-    cookie.n_output = 0;
-    cookie.vlan_tci = 0;
-    put_userspace_action(ctx->ofproto, ctx->odp_actions, &ctx->flow, &cookie);
+    ctx->may_set_up_flow = false;
+    if (!ctx->packet) {
+        return;
+    }
+
+    packet = ofpbuf_clone(ctx->packet);
+
+    if (packet->l3) {
+        struct eth_header *eh;
+
+        eth_pop_vlan(packet);
+        eh = packet->l2;
+        assert(eh->eth_type == ctx->flow.dl_type);
+        memcpy(eh->eth_src, ctx->flow.dl_src, sizeof eh->eth_src);
+        memcpy(eh->eth_dst, ctx->flow.dl_dst, sizeof eh->eth_dst);
+
+        if (ctx->flow.vlan_tci & htons(VLAN_CFI)) {
+            eth_push_vlan(packet, ctx->flow.vlan_tci);
+        }
+    }
+
+    if (ctx->flow.dl_type == htons(ETH_TYPE_IP) && packet->l4) {
+        packet_set_ipv4(packet, ctx->flow.nw_src, ctx->flow.nw_dst,
+                        ctx->flow.nw_tos, ctx->flow.nw_ttl);
+    }
+
+    if (packet->l7) {
+        if (ctx->flow.nw_proto == IPPROTO_TCP) {
+            packet_set_tcp_port(packet, ctx->flow.tp_src, ctx->flow.tp_dst);
+        } else if (ctx->flow.nw_proto == IPPROTO_UDP) {
+            packet_set_udp_port(packet, ctx->flow.tp_src, ctx->flow.tp_dst);
+        }
+    }
+
+    ofpbuf_use_const(&pin.packet, packet->data, packet->size);
+    pin.in_port = ctx->flow.in_port;
+    pin.reason = OFPR_ACTION;
+    pin.buffer_id = 0;
+    pin.send_len = len;
+    pin.total_len = packet->size;
+    connmgr_send_packet_in(ctx->ofproto->up.connmgr, &pin, &ctx->flow, packet);
 }
 
 static void
@@ -4323,7 +4278,7 @@ xlate_output_action__(struct action_xlate_ctx *ctx,
         flood_packets(ctx, true);
         break;
     case OFPP_CONTROLLER:
-        compose_controller_action(ctx, max_len);
+        execute_controller_action(ctx, max_len);
         break;
     case OFPP_LOCAL:
         compose_output_action(ctx, OFPP_LOCAL);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 73bca1d..f17c882 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -317,9 +317,9 @@ priority:0,tunnel:0,in_port:0000,tci(vlan:80,pcp:0) mac(81:81:81:81:81:81->82:82
 ])
 
 AT_CHECK([ovs-ofctl dump-flows br0 | sed 's/duration=[[0-9]]*\.[[0-9]]*s,/duration=\<omitted\>,/' | sort], [0], [dnl
- cookie=0x0, duration=<omitted>, table=0, n_packets=1, n_bytes=60, dl_src=11:11:11:11:11:11 actions=CONTROLLER:65535
+ cookie=0x0, duration=<omitted>, table=0, n_packets=3, n_bytes=180, dl_src=11:11:11:11:11:11 actions=CONTROLLER:65535
  cookie=0x1, duration=<omitted>, table=0, n_packets=2, n_bytes=120, dl_src=22:22:22:22:22:22 actions=CONTROLLER:65535,resubmit:80
- cookie=0x2, duration=<omitted>, table=0, n_packets=1, n_bytes=60, dl_src=33:33:33:33:33:33 actions=mod_vlan_vid:15,CONTROLLER:65535
+ cookie=0x2, duration=<omitted>, table=0, n_packets=3, n_bytes=180, dl_src=33:33:33:33:33:33 actions=mod_vlan_vid:15,CONTROLLER:65535
  cookie=0x3, duration=<omitted>, table=0, n_packets=2, n_bytes=120, in_port=80 actions=mod_vlan_vid:80,CONTROLLER:65535,resubmit:81
  cookie=0x4, duration=<omitted>, table=0, n_packets=2, n_bytes=120, in_port=81 actions=mod_dl_src:81:81:81:81:81:81,CONTROLLER:65535,resubmit:82
  cookie=0x5, duration=<omitted>, table=0, n_packets=2, n_bytes=120, in_port=82 actions=mod_dl_dst:82:82:82:82:82:82,CONTROLLER:65535,resubmit:83
-- 
1.7.7.1




More information about the dev mailing list