ofp-util: Add struct ofputil_packet_out, helper functions, and use it all.
authorBen Pfaff <blp@nicira.com>
Mon, 6 Feb 2012 22:17:49 +0000 (14:17 -0800)
committerBen Pfaff <blp@nicira.com>
Thu, 9 Feb 2012 21:23:36 +0000 (13:23 -0800)
This makes the ofp-util support for packet_out better match the support
that ofp-util has for other OpenFlow messages.  It also prepares for an
upcoming patch that adds a new piece of code that generates packet_out
messages.

Signed-off-by: Ben Pfaff <blp@nicira.com>
include/openflow/openflow.h
lib/learning-switch.c
lib/ofp-print.c
lib/ofp-util.c
lib/ofp-util.h
ofproto/ofproto.c
tests/ofp-print.at

index cd30d325edf70ceafa9378c0917ac697062c6886..a707087b95ef8012d81bdfd6a99fcccdb00c4385 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -450,13 +450,15 @@ OFP_ASSERT(sizeof(union ofp_action) == 8);
 /* Send packet (controller -> datapath). */
 struct ofp_packet_out {
     struct ofp_header header;
-    ovs_be32 buffer_id;           /* ID assigned by datapath (-1 if none). */
+    ovs_be32 buffer_id;           /* ID assigned by datapath or UINT32_MAX. */
     ovs_be16 in_port;             /* Packet's input port (OFPP_NONE if none). */
     ovs_be16 actions_len;         /* Size of action array in bytes. */
-    struct ofp_action_header actions[0]; /* Actions. */
-    /* uint8_t data[0]; */        /* Packet data.  The length is inferred
-                                     from the length field in the header.
-                                     (Only meaningful if buffer_id == -1.) */
+    /* Followed by:
+     *   - Exactly 'actions_len' bytes (possibly 0 bytes, and always a multiple
+     *     of 8) containing actions.
+     *   - If 'buffer_id' == UINT32_MAX, packet data to fill out the remainder
+     *     of the message length.
+     */
 };
 OFP_ASSERT(sizeof(struct ofp_packet_out) == 16);
 
index 922db543e46f48ea2b2702a876f84c9eb56d0c98..81aadeb40feda69d8f8b4f9f013a964466471802 100644 (file)
@@ -410,6 +410,8 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn,
     struct ofp_action_header actions[2];
     size_t actions_len;
 
+    struct ofputil_packet_out po;
+
     size_t pkt_ofs, pkt_len;
     struct ofpbuf pkt;
     struct flow flow;
@@ -458,6 +460,19 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn,
     }
     assert(actions_len <= sizeof actions);
 
+    /* Prepare packet_out in case we need one. */
+    po.buffer_id = ntohl(opi->buffer_id);
+    if (po.buffer_id == UINT32_MAX) {
+        po.packet = pkt.data;
+        po.packet_len = pkt.size;
+    } else {
+        po.packet = NULL;
+        po.packet_len = 0;
+    }
+    po.in_port = in_port;
+    po.actions = (union ofp_action *) actions;
+    po.n_actions = actions_len / sizeof *actions;
+
     /* Send the packet, and possibly the whole flow, to the output port. */
     if (sw->max_idle >= 0 && (!sw->ml || out_port != OFPP_FLOOD)) {
         struct ofpbuf *buffer;
@@ -473,17 +488,13 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn,
 
         /* If the switch didn't buffer the packet, we need to send a copy. */
         if (ntohl(opi->buffer_id) == UINT32_MAX && actions_len > 0) {
-            queue_tx(sw, rconn,
-                     make_packet_out(&pkt, UINT32_MAX, in_port,
-                                     actions, actions_len / sizeof *actions));
+            queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
         }
     } else {
         /* We don't know that MAC, or we don't set up flows.  Send along the
          * packet without setting up a flow. */
         if (ntohl(opi->buffer_id) != UINT32_MAX || actions_len > 0) {
-            queue_tx(sw, rconn,
-                     make_packet_out(&pkt, ntohl(opi->buffer_id), in_port,
-                                     actions, actions_len / sizeof *actions));
+            queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
         }
     }
 }
index 9a50c3392a8e44f385e6f231937f8fa182db8392..b1c6f97b2f3a9053689d8528feac8f3f829c69b4 100644 (file)
@@ -388,36 +388,31 @@ static void
 ofp_print_packet_out(struct ds *string, const struct ofp_packet_out *opo,
                      int verbosity)
 {
-    size_t len = ntohs(opo->header.length);
-    size_t actions_len = ntohs(opo->actions_len);
-
-    ds_put_cstr(string, " in_port=");
-    ofputil_format_port(ntohs(opo->in_port), string);
+    struct ofputil_packet_out po;
+    enum ofperr error;
 
-    ds_put_format(string, " actions_len=%zu ", actions_len);
-    if (actions_len > (ntohs(opo->header.length) - sizeof *opo)) {
-        ds_put_format(string, "***packet too short for action length***\n");
+    error = ofputil_decode_packet_out(&po, opo);
+    if (error) {
+        ofp_print_error(string, error);
         return;
     }
-    if (actions_len % sizeof(union ofp_action)) {
-        ds_put_format(string, "***action length not a multiple of %zu***\n",
-                      sizeof(union ofp_action));
-    }
-    ofp_print_actions(string, (const union ofp_action *) opo->actions,
-                      actions_len / sizeof(union ofp_action));
 
-    if (ntohl(opo->buffer_id) == UINT32_MAX) {
-        int data_len = len - sizeof *opo - actions_len;
-        ds_put_format(string, " data_len=%d", data_len);
-        if (verbosity > 0 && len > sizeof *opo) {
-            char *packet = ofp_packet_to_string(
-                    (uint8_t *) opo->actions + actions_len, data_len);
+    ds_put_cstr(string, " in_port=");
+    ofputil_format_port(po.in_port, string);
+
+    ds_put_char(string, ' ');
+    ofp_print_actions(string, po.actions, po.n_actions);
+
+    if (po.buffer_id == UINT32_MAX) {
+        ds_put_format(string, " data_len=%d", po.packet_len);
+        if (verbosity > 0 && po.packet_len > 0) {
+            char *packet = ofp_packet_to_string(po.packet, po.packet_len);
             ds_put_char(string, '\n');
             ds_put_cstr(string, packet);
             free(packet);
         }
     } else {
-        ds_put_format(string, " buffer=0x%08"PRIx32, ntohl(opo->buffer_id));
+        ds_put_format(string, " buffer=0x%08"PRIx32, po.buffer_id);
     }
     ds_put_char(string, '\n');
 }
index ed98251e1610bdd3ec3304ba99ea1aadd279f449..6f8b033eceda8b9936b15af3add5f40911b7c5f9 100644 (file)
@@ -1780,6 +1780,70 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
     return packet;
 }
 
+enum ofperr
+ofputil_decode_packet_out(struct ofputil_packet_out *po,
+                          const struct ofp_packet_out *opo)
+{
+    enum ofperr error;
+    struct ofpbuf b;
+
+    po->buffer_id = ntohl(opo->buffer_id);
+    po->in_port = ntohs(opo->in_port);
+    if (po->in_port >= OFPP_MAX && po->in_port != OFPP_LOCAL
+        && po->in_port != OFPP_NONE) {
+        VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out has bad input port %#"PRIx16,
+                     po->in_port);
+        return OFPERR_NXBRC_BAD_IN_PORT;
+    }
+
+    ofpbuf_use_const(&b, opo, ntohs(opo->header.length));
+    ofpbuf_pull(&b, sizeof *opo);
+
+    error = ofputil_pull_actions(&b, ntohs(opo->actions_len),
+                                 &po->actions, &po->n_actions);
+    if (error) {
+        return error;
+    }
+
+    if (po->buffer_id == UINT32_MAX) {
+        po->packet = b.data;
+        po->packet_len = b.size;
+    } else {
+        po->packet = NULL;
+        po->packet_len = 0;
+    }
+
+    return 0;
+}
+
+struct ofpbuf *
+ofputil_encode_packet_out(const struct ofputil_packet_out *po)
+{
+    struct ofp_packet_out *opo;
+    size_t actions_len;
+    struct ofpbuf *msg;
+    size_t size;
+
+    actions_len = po->n_actions * sizeof *po->actions;
+    size = sizeof *opo + actions_len;
+    if (po->buffer_id == UINT32_MAX) {
+        size += po->packet_len;
+    }
+
+    msg = ofpbuf_new(size);
+    opo = put_openflow(sizeof *opo, OFPT_PACKET_OUT, msg);
+    opo->buffer_id = htonl(po->buffer_id);
+    opo->in_port = htons(po->in_port);
+    opo->actions_len = htons(actions_len);
+    ofpbuf_put(msg, po->actions, actions_len);
+    if (po->buffer_id == UINT32_MAX) {
+        ofpbuf_put(msg, po->packet, po->packet_len);
+    }
+    update_openflow_length(msg);
+
+    return msg;
+}
+
 /* Returns a string representing the message type of 'type'.  The string is the
  * enumeration constant for the type, e.g. "OFPT_HELLO".  For statistics
  * messages, the constant is followed by "request" or "reply",
@@ -2158,59 +2222,6 @@ make_packet_in(uint32_t buffer_id, uint16_t in_port, uint8_t reason,
     return buf;
 }
 
-struct ofpbuf *
-make_packet_out(const struct ofpbuf *packet, uint32_t buffer_id,
-                uint16_t in_port,
-                const struct ofp_action_header *actions, size_t n_actions)
-{
-    size_t actions_len = n_actions * sizeof *actions;
-    struct ofp_packet_out *opo;
-    size_t size = sizeof *opo + actions_len + (packet ? packet->size : 0);
-    struct ofpbuf *out = ofpbuf_new(size);
-
-    opo = ofpbuf_put_uninit(out, sizeof *opo);
-    opo->header.version = OFP_VERSION;
-    opo->header.type = OFPT_PACKET_OUT;
-    opo->header.length = htons(size);
-    opo->header.xid = htonl(0);
-    opo->buffer_id = htonl(buffer_id);
-    opo->in_port = htons(in_port);
-    opo->actions_len = htons(actions_len);
-    ofpbuf_put(out, actions, actions_len);
-    if (packet) {
-        ofpbuf_put(out, packet->data, packet->size);
-    }
-    return out;
-}
-
-struct ofpbuf *
-make_unbuffered_packet_out(const struct ofpbuf *packet,
-                           uint16_t in_port, uint16_t out_port)
-{
-    struct ofp_action_output action;
-    action.type = htons(OFPAT_OUTPUT);
-    action.len = htons(sizeof action);
-    action.port = htons(out_port);
-    return make_packet_out(packet, UINT32_MAX, in_port,
-                           (struct ofp_action_header *) &action, 1);
-}
-
-struct ofpbuf *
-make_buffered_packet_out(uint32_t buffer_id,
-                         uint16_t in_port, uint16_t out_port)
-{
-    if (out_port != OFPP_NONE) {
-        struct ofp_action_output action;
-        action.type = htons(OFPAT_OUTPUT);
-        action.len = htons(sizeof action);
-        action.port = htons(out_port);
-        return make_packet_out(NULL, buffer_id, in_port,
-                               (struct ofp_action_header *) &action, 1);
-    } else {
-        return make_packet_out(NULL, buffer_id, in_port, NULL, 0);
-    }
-}
-
 /* Creates and returns an OFPT_ECHO_REQUEST message with an empty payload. */
 struct ofpbuf *
 make_echo_request(void)
index f9885a3c243de2737ec7613fd5cd8984d2bf26dc..e2b32ddc1a0ce2e194fdfd2c9af3cefe59261ae9 100644 (file)
@@ -249,6 +249,20 @@ struct ofpbuf *ofputil_encode_packet_in(const struct ofputil_packet_in *,
 int ofputil_decode_packet_in(struct ofputil_packet_in *pi,
                              const struct ofp_header *oh);
 
+/* Abstract packet-out message. */
+struct ofputil_packet_out {
+    const void *packet;         /* Packet data, if buffer_id == UINT32_MAX. */
+    size_t packet_len;          /* Length of packet data in bytes. */
+    uint32_t buffer_id;         /* Buffer id or UINT32_MAX if no buffer. */
+    uint16_t in_port;           /* Packet's input port or OFPP_NONE. */
+    union ofp_action *actions;  /* Actions. */
+    size_t n_actions;           /* Number of elements in 'actions' array. */
+};
+
+enum ofperr ofputil_decode_packet_out(struct ofputil_packet_out *,
+                                      const struct ofp_packet_out *);
+struct ofpbuf *ofputil_encode_packet_out(const struct ofputil_packet_out *);
+
 /* OpenFlow protocol utility functions. */
 void *make_openflow(size_t openflow_len, uint8_t type, struct ofpbuf **);
 void *make_nxmsg(size_t openflow_len, uint32_t subtype, struct ofpbuf **);
@@ -296,14 +310,6 @@ struct ofpbuf *make_add_simple_flow(const struct cls_rule *,
 struct ofpbuf *make_packet_in(uint32_t buffer_id, uint16_t in_port,
                               uint8_t reason,
                               const struct ofpbuf *payload, int max_send_len);
-struct ofpbuf *make_packet_out(const struct ofpbuf *packet, uint32_t buffer_id,
-                               uint16_t in_port,
-                               const struct ofp_action_header *,
-                               size_t n_actions);
-struct ofpbuf *make_buffered_packet_out(uint32_t buffer_id,
-                                        uint16_t in_port, uint16_t out_port);
-struct ofpbuf *make_unbuffered_packet_out(const struct ofpbuf *packet,
-                                          uint16_t in_port, uint16_t out_port);
 struct ofpbuf *make_echo_request(void);
 struct ofpbuf *make_echo_reply(const struct ofp_header *rq);
 
index fe3b6206cae20f373efe77cf5ef456c2c6c049b6..92744a21650aabfa7ed4e863bc47e4976746f2a1 100644 (file)
@@ -1953,17 +1953,13 @@ reject_slave_controller(struct ofconn *ofconn)
 }
 
 static enum ofperr
-handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
+handle_packet_out(struct ofconn *ofconn, const struct ofp_packet_out *opo)
 {
     struct ofproto *p = ofconn_get_ofproto(ofconn);
-    struct ofp_packet_out *opo;
-    struct ofpbuf payload, *buffer;
-    union ofp_action *ofp_actions;
-    struct ofpbuf request;
+    struct ofputil_packet_out po;
+    struct ofpbuf *payload;
     struct flow flow;
-    size_t n_ofp_actions;
     enum ofperr error;
-    uint16_t in_port;
 
     COVERAGE_INC(ofproto_packet_out);
 
@@ -1972,45 +1968,28 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
         return error;
     }
 
-    /* Get ofp_packet_out. */
-    ofpbuf_use_const(&request, oh, ntohs(oh->length));
-    opo = ofpbuf_pull(&request, offsetof(struct ofp_packet_out, actions));
-
-    /* Get actions. */
-    error = ofputil_pull_actions(&request, ntohs(opo->actions_len),
-                                 &ofp_actions, &n_ofp_actions);
+    /* Decode message. */
+    error = ofputil_decode_packet_out(&po, opo);
     if (error) {
         return error;
     }
 
     /* Get payload. */
-    if (opo->buffer_id != htonl(UINT32_MAX)) {
-        error = ofconn_pktbuf_retrieve(ofconn, ntohl(opo->buffer_id),
-                                       &buffer, NULL);
-        if (error || !buffer) {
+    if (po.buffer_id != UINT32_MAX) {
+        error = ofconn_pktbuf_retrieve(ofconn, po.buffer_id, &payload, NULL);
+        if (error || !payload) {
             return error;
         }
-        payload = *buffer;
     } else {
-        payload = request;
-        buffer = NULL;
-    }
-
-    /* Get in_port and partially validate it.
-     *
-     * We don't know what range of ports the ofproto actually implements, but
-     * we do know that only certain reserved ports (numbered OFPP_MAX and
-     * above) are valid. */
-    in_port = ntohs(opo->in_port);
-    if (in_port >= OFPP_MAX && in_port != OFPP_LOCAL && in_port != OFPP_NONE) {
-        return OFPERR_NXBRC_BAD_IN_PORT;
+        payload = xmalloc(sizeof *payload);
+        ofpbuf_use_const(payload, po.packet, po.packet_len);
     }
 
     /* Send out packet. */
-    flow_extract(&payload, 0, 0, in_port, &flow);
-    error = p->ofproto_class->packet_out(p, &payload, &flow,
-                                         ofp_actions, n_ofp_actions);
-    ofpbuf_delete(buffer);
+    flow_extract(payload, 0, 0, po.in_port, &flow);
+    error = p->ofproto_class->packet_out(p, payload, &flow,
+                                         po.actions, po.n_actions);
+    ofpbuf_delete(payload);
 
     return error;
 }
@@ -3180,7 +3159,7 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
         return handle_set_config(ofconn, msg->data);
 
     case OFPUTIL_OFPT_PACKET_OUT:
-        return handle_packet_out(ofconn, oh);
+        return handle_packet_out(ofconn, msg->data);
 
     case OFPUTIL_OFPT_PORT_MOD:
         return handle_port_mod(ofconn, oh);
index 259590030f46b4e0c53b8a86c9386a58c90ee0cb..09c9e835fb69b9b44bfb222c6cdad26be1873160 100644 (file)
@@ -315,7 +315,7 @@ b9 7c c0 a8 00 02 c0 a8 00 01 00 00 2b 60 00 00 \
 00 00 6a 4f 2b 58 50 14 00 00 6d 75 00 00 00 00 \
 00 00 00 00 \
 "], [0], [dnl
-OFPT_PACKET_OUT (xid=0x0): in_port=1 actions_len=8 actions=output:3 buffer=0x00000114
+OFPT_PACKET_OUT (xid=0x0): in_port=1 actions=output:3 buffer=0x00000114
 ])
 AT_CLEANUP