From c6a93eb711322474e0e8db07a6c53e1f6c61c56b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 6 Feb 2012 14:17:49 -0800 Subject: [PATCH] ofp-util: Add struct ofputil_packet_out, helper functions, and use it all. 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 --- include/openflow/openflow.h | 14 +++-- lib/learning-switch.c | 23 +++++-- lib/ofp-print.c | 37 +++++------- lib/ofp-util.c | 117 ++++++++++++++++++++---------------- lib/ofp-util.h | 22 ++++--- ofproto/ofproto.c | 51 +++++----------- tests/ofp-print.at | 2 +- 7 files changed, 135 insertions(+), 131 deletions(-) diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h index cd30d325..a707087b 100644 --- a/include/openflow/openflow.h +++ b/include/openflow/openflow.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. @@ -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); diff --git a/lib/learning-switch.c b/lib/learning-switch.c index 922db543..81aadeb4 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -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)); } } } diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 9a50c339..b1c6f97b 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 ed98251e..6f8b033e 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -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) diff --git a/lib/ofp-util.h b/lib/ofp-util.h index f9885a3c..e2b32ddc 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -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); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index fe3b6206..92744a21 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -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); diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 25959003..09c9e835 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 -- 2.30.2