ofproto: Refactor handle_packet_out().

An upcoming commit will require the flow to be passed in as part of
OpenFlow action validation, but handle_packet_out() has until now been
structured to make this difficult.  This commit refactors it to better
suit this purpose.

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index a3b8e20abe41c1ce512e7164fefb2992bb551d8d..b1678ad8a778419df9a59fb784c5c789a6be428e 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -415,50 +415,6 @@ check_ofp_message_array(const struct ofp_header *msg, uint8_t type,
     return 0;
 }
 
-int
-check_ofp_packet_out(const struct ofp_header *oh, struct ofpbuf *data,
-                     int *n_actionsp, int max_ports)
-{
-    const struct ofp_packet_out *opo;
-    unsigned int actions_len, n_actions;
-    size_t extra;
-    int error;
-
-    *n_actionsp = 0;
-    error = check_ofp_message_array(oh, OFPT_PACKET_OUT,
-                                    sizeof *opo, 1, &extra);
-    if (error) {
-        return error;
-    }
-    opo = (const struct ofp_packet_out *) oh;
-
-    actions_len = ntohs(opo->actions_len);
-    if (actions_len > extra) {
-        VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out claims %u bytes of actions "
-                     "but message has room for only %zu bytes",
-                     actions_len, extra);
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
-    }
-    if (actions_len % sizeof(union ofp_action)) {
-        VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out claims %u bytes of actions, "
-                     "which is not a multiple of %zu",
-                     actions_len, sizeof(union ofp_action));
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
-    }
-
-    n_actions = actions_len / sizeof(union ofp_action);
-    error = validate_actions((const union ofp_action *) opo->actions,
-                             n_actions, max_ports);
-    if (error) {
-        return error;
-    }
-
-    data->data = (void *) &opo->actions[n_actions];
-    data->size = extra - actions_len;
-    *n_actionsp = n_actions;
-    return 0;
-}
-
 const struct ofp_flow_stats *
 flow_stats_first(struct flow_stats_iterator *iter,
                  const struct ofp_stats_reply *osr)

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 62548e52cb96f5893b3ed5404b32d0e90b5f7b22..28bce5f89bd853fbe300f237595cc149b1f21b5e 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -62,8 +62,6 @@ int check_ofp_message(const struct ofp_header *, uint8_t type, size_t size);
 int check_ofp_message_array(const struct ofp_header *, uint8_t type,
                             size_t size, size_t array_elt_size,
                             size_t *n_array_elts);
-int check_ofp_packet_out(const struct ofp_header *, struct ofpbuf *data,
-                         int *n_actions, int max_ports);
 
 struct flow_stats_iterator {
     const uint8_t *pos, *end;

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 618e8a56bc848051d07085a71a1d970648b4c9da..36a23d956812874e19e125c302ad73be212a8a9c 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2884,24 +2884,37 @@ handle_packet_out(struct ofconn *ofconn, struct ofp_header *oh)
     struct ofproto *p = ofconn->ofproto;
     struct ofp_packet_out *opo;
     struct ofpbuf payload, *buffer;
-    struct odp_actions actions;
+    union ofp_action *ofp_actions;
+    struct odp_actions odp_actions;
+    struct ofpbuf request;
     struct flow flow;
-    int n_actions;
+    size_t n_ofp_actions;
     uint16_t in_port;
     int error;
 
+    COVERAGE_INC(ofproto_packet_out);
+
     error = reject_slave_controller(ofconn, "OFPT_PACKET_OUT");
     if (error) {
         return error;
     }
 
-    error = check_ofp_packet_out(oh, &payload, &n_actions, p->max_ports);
+    /* Get ofp_packet_out. */
+    request.data = oh;
+    request.size = ntohs(oh->length);
+    opo = ofpbuf_try_pull(&request, offsetof(struct ofp_packet_out, actions));
+    if (!opo) {
+        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
+    }
+
+    /* Get actions. */
+    error = ofputil_pull_actions(&request, ntohs(opo->actions_len),
+                                 &ofp_actions, &n_ofp_actions);
     if (error) {
         return error;
     }
-    opo = (struct ofp_packet_out *) oh;
 
-    COVERAGE_INC(ofproto_packet_out);
+    /* Get payload. */
     if (opo->buffer_id != htonl(UINT32_MAX)) {
         error = pktbuf_retrieve(ofconn->pktbuf, ntohl(opo->buffer_id),
                                 &buffer, &in_port);
@@ -2910,18 +2923,29 @@ handle_packet_out(struct ofconn *ofconn, struct ofp_header *oh)
         }
         payload = *buffer;
     } else {
+        payload = request;
         buffer = NULL;
     }
 
-    flow_extract(&payload, 0, ofp_port_to_odp_port(ntohs(opo->in_port)), &flow);
-    error = xlate_actions((const union ofp_action *) opo->actions, n_actions,
-                          &flow, p, &payload, &actions, NULL, NULL, NULL);
+    /* Extract flow, check actions. */
+    flow_extract(&payload, 0, ofp_port_to_odp_port(ntohs(opo->in_port)),
+                 &flow);
+    error = validate_actions(ofp_actions, n_ofp_actions, p->max_ports);
+    if (error) {
+        goto exit;
+    }
+
+    /* Send. */
+    error = xlate_actions(ofp_actions, n_ofp_actions, &flow, p, &payload,
+                          &odp_actions, NULL, NULL, NULL);
     if (!error) {
-        dpif_execute(p->dpif, actions.actions, actions.n_actions, &payload);
+        dpif_execute(p->dpif, odp_actions.actions, odp_actions.n_actions,
+                     &payload);
     }
-    ofpbuf_delete(buffer);
 
-    return error;
+exit:
+    ofpbuf_delete(buffer);
+    return 0;
 }
 
 static void