[ovs-dev] [async-msgs 05/13] ofp-util: Add struct ofputil_packet_out, helper functions, and use it all.
Ben Pfaff
blp at nicira.com
Thu Jan 26 23:53:43 UTC 2012
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 at nicira.com>
---
include/openflow/openflow.h | 10 ++--
lib/learning-switch.c | 23 ++++++--
lib/ofp-print.c | 39 ++++++--------
lib/ofp-util.c | 116 +++++++++++++++++++++++-------------------
lib/ofp-util.h | 24 ++++++---
ofproto/ofproto.c | 45 ++++++----------
tests/ofp-print.at | 2 +-
7 files changed, 136 insertions(+), 123 deletions(-)
diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h
index cd30d32..e624450 100644
--- a/include/openflow/openflow.h
+++ b/include/openflow/openflow.h
@@ -453,10 +453,12 @@ struct ofp_packet_out {
ovs_be32 buffer_id; /* ID assigned by datapath (-1 if none). */
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' != -1, packet data to fill out the remainder of the
+ * message length.
+ */
};
OFP_ASSERT(sizeof(struct ofp_packet_out) == 16);
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 3bcb961..b19d8bb 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -407,6 +407,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;
@@ -455,6 +457,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;
@@ -470,17 +485,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));
}
}
}
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 7bc26c9..9112c4b 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -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');
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6fe1611..8bfaddf 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1734,6 +1734,69 @@ 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_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",
@@ -2112,59 +2175,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)
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 422c14a..427a749 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -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.
@@ -245,6 +245,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 **);
@@ -292,14 +306,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);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c78aa39..b7b0d63 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1809,17 +1809,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);
@@ -1828,28 +1824,21 @@ 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;
+ payload = xmalloc(sizeof *payload);
+ ofpbuf_use_const(payload, po.packet, po.packet_len);
}
/* Get in_port and partially validate it.
@@ -1857,16 +1846,16 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
* 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) {
+ if (po.in_port >= OFPP_MAX && po.in_port != OFPP_LOCAL
+ && po.in_port != OFPP_NONE) {
return OFPERR_NXBRC_BAD_IN_PORT;
}
/* 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;
}
@@ -2932,7 +2921,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);
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 85562b6..cfa8d83 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -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
--
1.7.2.5
More information about the dev
mailing list