From: Andre Mantas <aman...@lasige.di.fc.ul.pt>
This patch allows ofp_packet_out messages to be added to bundles. In a
multi controller scenario, packet_out messages inside bundles can serve
as a commit_reply for other controllers - since the original commit_reply
is only forwarded to the controller that sent the commit_request.
Tested with make check and external controller adding packet_out messages
to a bundle with different destinations (hosts and controllers). Also tested
grouping packet_out with port_mod messages in the same bundle. After
committing the bundle, the destinations received the packet_out.
Signed-off-by: Andre Mantas <aman...@lasige.di.fc.ul.pt>
---
lib/ofp-util.c | 2 +-
ofproto/bundles.h | 5 ++++
ofproto/ofproto-provider.h | 7 +++++
ofproto/ofproto.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 81 insertions(+), 3 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index c5353cc..6725220 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -9578,6 +9578,7 @@ ofputil_is_bundlable(enum ofptype type)
/* Minimum required by OpenFlow 1.4. */
case OFPTYPE_PORT_MOD:
case OFPTYPE_FLOW_MOD:
+ case OFPTYPE_PACKET_OUT:
return true;
/* Nice to have later. */
@@ -9585,7 +9586,6 @@ ofputil_is_bundlable(enum ofptype type)
case OFPTYPE_GROUP_MOD:
case OFPTYPE_TABLE_MOD:
case OFPTYPE_METER_MOD:
- case OFPTYPE_PACKET_OUT:
case OFPTYPE_NXT_TLV_TABLE_MOD:
/* Not to be bundlable. */
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index d045031..61c1b48 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -22,6 +22,7 @@
#include <sys/types.h>
#include "connmgr.h"
+#include "dp-packet.h"
#include "ofproto-provider.h"
#include "openvswitch/ofp-msgs.h"
#include "openvswitch/ofp-util.h"
@@ -37,6 +38,7 @@ struct ofp_bundle_entry {
union {
struct ofproto_flow_mod ofm; /* ofm.fm.ofpacts must be malloced. */
struct ofproto_port_mod opm;
+ struct ofproto_packet_out opo; /* opo.po.ofpacts must be malloced */
};
/* OpenFlow header and some of the message contents for error reporting. */
@@ -89,6 +91,9 @@ ofp_bundle_entry_free(struct ofp_bundle_entry *entry)
if (entry) {
if (entry->type == OFPTYPE_FLOW_MOD) {
free(entry->ofm.fm.ofpacts);
+ } else if (entry->type == OFPTYPE_PACKET_OUT) {
+ dp_packet_delete(entry->opo.payload);
+ free(entry->opo.po.ofpacts);
}
free(entry);
}
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index daa0077..c98f110 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1814,6 +1814,13 @@ struct ofproto_port_mod {
struct ofport *port; /* Affected port. */
};
+/* packet_out with execution context. */
+struct ofproto_packet_out {
+ struct ofputil_packet_out po;
+ struct dp_packet *payload;
+ struct flow flow;
+};
+
enum ofperr ofproto_flow_mod(struct ofproto *, struct ofproto_flow_mod *)
OVS_EXCLUDED(ofproto_mutex);
void ofproto_add_flow(struct ofproto *, const struct match *, int priority,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ff6affd..c5a6097 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -6996,6 +6996,8 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
* effect. */
be->ofm.version = version;
error = ofproto_flow_mod_start(ofproto, &be->ofm);
+ } else if (be->type == OFPTYPE_PACKET_OUT) {
+ prev_is_port_mod = false;
} else {
OVS_NOT_REACHED();
}
@@ -7016,7 +7018,7 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
if (be->type == OFPTYPE_FLOW_MOD) {
ofproto_flow_mod_revert(ofproto, &be->ofm);
}
- /* Nothing needs to be reverted for a port mod. */
+ /* Nothing needs to be reverted for a port mod or packet out.
*/
}
} else {
/* 4. Finish. */
@@ -7042,6 +7044,18 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
* also from OVSDB changes asynchronously to all upcall
* processing. */
port_mod_finish(ofconn, &be->opm.pm, be->opm.port);
+ } else if (be->type == OFPTYPE_PACKET_OUT) {
+ /* Try to send the packet. The bundle is committed
+ * regardless of errors */
+ error = ofproto->ofproto_class->packet_out(ofproto,
+ be->opo.payload,
+ &(be->opo.flow),
+ be->opo.po.ofpacts,
+
be->opo.po.ofpacts_len);
+ if (error) {
+ VLOG_INFO("Error sending packet out while committing a
"
+ "bundle: %d", error);
+ }
}
}
}
@@ -7150,6 +7164,58 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
error = ofproto_check_ofpacts(ofproto, bmsg->ofm.fm.ofpacts,
bmsg->ofm.fm.ofpacts_len);
}
+ } else if (type == OFPTYPE_PACKET_OUT) {
+ uint64_t ofpacts_stub[1024 / 8];
+ struct ofpbuf ofpacts;
+
+ /* Decode message. */
+ ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
+ error = ofputil_decode_packet_out(&(bmsg->opo.po), badd.msg, &ofpacts);
+ /* Same validation steps as in handle_packet_out */
+ if (error) {
+ goto exit;
+ }
+
+ /* Move actions to heap. */
+ bmsg->opo.po.ofpacts = ofpbuf_steal_data(&ofpacts);
+
+ if (ofp_to_u16(bmsg->opo.po.in_port) >= ofproto->max_ports
+ && ofp_to_u16(bmsg->opo.po.in_port) < ofp_to_u16(OFPP_MAX)) {
+ error = OFPERR_OFPBRC_BAD_PORT;
+ goto exit;
+ }
+
+ /* Get payload. */
+ if (bmsg->opo.po.buffer_id != UINT32_MAX) {
+ error = ofconn_pktbuf_retrieve(ofconn, bmsg->opo.po.buffer_id,
+ &(bmsg->opo.payload), NULL);
+ if (error) {
+ goto exit;
+ }
+ } else {
+ /* Ensure that the L3 header is 32-bit aligned. */
+ bmsg->opo.payload = dp_packet_clone_data_with_headroom(
+ bmsg->opo.po.packet,
+
bmsg->opo.po.packet_len,
+ 2);
+ }
+
+ /* Verify actions against packet */
+ flow_extract(bmsg->opo.payload, &(bmsg->opo.flow));
+ bmsg->opo.flow.in_port.ofp_port = bmsg->opo.po.in_port;
+ /* Check actions like for flow mods. */
+ error = ofpacts_check_consistency(bmsg->opo.po.ofpacts,
+ bmsg->opo.po.ofpacts_len,
+ &(bmsg->opo.flow),
+ u16_to_ofp(ofproto->max_ports),
+ 0, ofproto->n_tables,
+ ofconn_get_protocol(ofconn));
+ if (error) {
+ goto exit;
+ }
+
+ error = ofproto_check_ofpacts(ofproto, bmsg->opo.po.ofpacts,
+ bmsg->opo.po.ofpacts_len);
} else {
OVS_NOT_REACHED();
}
@@ -7158,7 +7224,7 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
bmsg);
}
-
+exit:
if (error) {
ofp_bundle_entry_free(bmsg);
}
--
2.5.0
From aedda156635261c896826b7751acfff7d99d83cb Mon Sep 17 00:00:00 2001
From: Andre Mantas <andremant...@gmail.com>
Date: Sat, 16 Jul 2016 22:26:50 +0100
Subject: [PATCH 2/2] ofproto: ofp_packet_out messages in bundles
Added new helper function to refactor duplicated code used in
handle_packet_out() and bundle_add() functions.
Tested with make check and external controller adding packet_out messages
to a bundle with different destinations (hosts and controllers).
Signed-off-by: Andre Mantas <aman...@lasige.di.fc.ul.pt>
---
lib/ofp-util.c | 1 +
ofproto/ofproto.c | 166 +++++++++++++++++++++++++-----------------------------
2 files changed, 77 insertions(+), 90 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 5495771..a16239a 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -9863,6 +9863,7 @@ ofputil_is_bundlable(enum ofptype type)
/* Minimum required by OpenFlow 1.4. */
case OFPTYPE_PORT_MOD:
case OFPTYPE_FLOW_MOD:
+ /* Optional */
case OFPTYPE_PACKET_OUT:
return true;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 8fbfa6c..9c4f0cf 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -324,6 +324,14 @@ static void update_mtu(struct ofproto *, struct ofport *);
static void update_mtu_ofproto(struct ofproto *);
static void meter_delete(struct ofproto *, uint32_t first, uint32_t last);
static void meter_insert_rule(struct rule *);
+static enum ofperr ofproto_extract_packet_out(struct ofproto *p,
+ struct ofconn *ofconn,
+ const struct ofp_header *oh,
+ uint64_t *ofpacts_stub,
+ struct ofpbuf *ofpacts,
+ struct ofputil_packet_out *po,
+ struct dp_packet **payload,
+ struct flow *flow);
/* unixctl. */
static void ofproto_unixctl_init(void);
@@ -3388,6 +3396,66 @@ reject_slave_controller(struct ofconn *ofconn)
}
}
+/**
+ * Extracts, validates and initializes all the required fields to send a
+ * PacketOut to the datapath. The caller must free ofpacts and payload.
+ *
+ * Used by handle_packet_out() and handle_bundle_add()
+ *
+ * Returns 0 if successful, otherwise an OpenFlow error. */
+static enum ofperr
+ofproto_extract_packet_out(struct ofproto *p, struct ofconn *ofconn,
+ const struct ofp_header *oh,
+ uint64_t *ofpacts_stub, struct ofpbuf *ofpacts,
+ struct ofputil_packet_out *po,
+ struct dp_packet **payload, struct flow *flow)
+{
+ enum ofperr error;
+
+ /* Decode message. */
+ ofpbuf_use_stub(ofpacts, ofpacts_stub, sizeof ofpacts_stub);
+ error = ofputil_decode_packet_out(po, oh, ofpacts);
+ if (error) {
+ return error;
+ }
+ if (ofp_to_u16(po->in_port) >= p->max_ports
+ && ofp_to_u16(po->in_port) < ofp_to_u16(OFPP_MAX)) {
+ error = OFPERR_OFPBRC_BAD_PORT;
+ }
+
+ /* Get payload. */
+ if (po->buffer_id != UINT32_MAX) {
+ error = ofconn_pktbuf_retrieve(ofconn, po->buffer_id, payload, NULL);
+ if (error) {
+ goto exit;
+ }
+ } else {
+ /* Ensure that the L3 header is 32-bit aligned. */
+ *payload = dp_packet_clone_data_with_headroom(po->packet,
+ po->packet_len, 2);
+ }
+
+ /* Verify actions against packet, then send packet if successful. */
+ flow_extract(*payload, flow);
+ flow->in_port.ofp_port = po->in_port;
+
+ /* Check actions like for flow mods. We pass a 'table_id' of 0 to
+ * ofproto_check_consistency(), which isn't strictly correct because these
+ * actions aren't in any table. This is OK as 'table_id' is only used to
+ * check instructions (e.g., goto-table), which can't appear on the action
+ * list of a packet-out. */
+ error = ofpacts_check_consistency(po->ofpacts, po->ofpacts_len,
+ flow, u16_to_ofp(p->max_ports),
+ 0, p->n_tables,
+ ofconn_get_protocol(ofconn));
+ if (!error) {
+ error = ofproto_check_ofpacts(p, po->ofpacts, po->ofpacts_len);
+ }
+
+exit:
+ return error;
+}
+
/* Checks that the 'ofpacts_len' bytes of action in 'ofpacts' are appropriate
* for 'ofproto':
*
@@ -3436,52 +3504,15 @@ handle_packet_out(struct ofconn *ofconn, const struct
ofp_header *oh)
goto exit;
}
- /* Decode message. */
- ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
- error = ofputil_decode_packet_out(&po, oh, &ofpacts);
- if (error) {
- goto exit_free_ofpacts;
- }
- if (ofp_to_u16(po.in_port) >= p->max_ports
- && ofp_to_u16(po.in_port) < ofp_to_u16(OFPP_MAX)) {
- error = OFPERR_OFPBRC_BAD_PORT;
- goto exit_free_ofpacts;
- }
-
- /* Get payload. */
- if (po.buffer_id != UINT32_MAX) {
- error = ofconn_pktbuf_retrieve(ofconn, po.buffer_id, &payload, NULL);
- if (error) {
- goto exit_free_ofpacts;
- }
- } else {
- /* Ensure that the L3 header is 32-bit aligned. */
- payload = dp_packet_clone_data_with_headroom(po.packet, po.packet_len,
2);
- }
+ error = ofproto_extract_packet_out(p, ofconn, oh, ofpacts_stub, &ofpacts,
+ &po, &payload, &flow);
- /* Verify actions against packet, then send packet if successful. */
- flow_extract(payload, &flow);
- flow.in_port.ofp_port = po.in_port;
-
- /* Check actions like for flow mods. We pass a 'table_id' of 0 to
- * ofproto_check_consistency(), which isn't strictly correct because these
- * actions aren't in any table. This is OK as 'table_id' is only used to
- * check instructions (e.g., goto-table), which can't appear on the action
- * list of a packet-out. */
- error = ofpacts_check_consistency(po.ofpacts, po.ofpacts_len,
- &flow, u16_to_ofp(p->max_ports),
- 0, p->n_tables,
- ofconn_get_protocol(ofconn));
if (!error) {
- error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len);
- if (!error) {
- error = p->ofproto_class->packet_out(p, payload, &flow,
- po.ofpacts, po.ofpacts_len);
- }
+ error = p->ofproto_class->packet_out(p, payload, &flow,
+ po.ofpacts, po.ofpacts_len);
}
dp_packet_delete(payload);
-exit_free_ofpacts:
ofpbuf_uninit(&ofpacts);
exit:
return error;
@@ -7255,54 +7286,9 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
uint64_t ofpacts_stub[1024 / 8];
struct ofpbuf ofpacts;
- /* Decode message. */
- ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
- error = ofputil_decode_packet_out(&(bmsg->opo.po), badd.msg, &ofpacts);
- /* Same validation steps as in handle_packet_out */
- if (error) {
- goto exit;
- }
-
- /* Move actions to heap. */
- bmsg->opo.po.ofpacts = ofpbuf_steal_data(&ofpacts);
-
- if (ofp_to_u16(bmsg->opo.po.in_port) >= ofproto->max_ports
- && ofp_to_u16(bmsg->opo.po.in_port) < ofp_to_u16(OFPP_MAX)) {
- error = OFPERR_OFPBRC_BAD_PORT;
- goto exit;
- }
-
- /* Get payload. */
- if (bmsg->opo.po.buffer_id != UINT32_MAX) {
- error = ofconn_pktbuf_retrieve(ofconn, bmsg->opo.po.buffer_id,
- &(bmsg->opo.payload), NULL);
- if (error) {
- goto exit;
- }
- } else {
- /* Ensure that the L3 header is 32-bit aligned. */
- bmsg->opo.payload = dp_packet_clone_data_with_headroom(
- bmsg->opo.po.packet,
-
bmsg->opo.po.packet_len,
- 2);
- }
-
- /* Verify actions against packet */
- flow_extract(bmsg->opo.payload, &(bmsg->opo.flow));
- bmsg->opo.flow.in_port.ofp_port = bmsg->opo.po.in_port;
- /* Check actions like for flow mods. */
- error = ofpacts_check_consistency(bmsg->opo.po.ofpacts,
- bmsg->opo.po.ofpacts_len,
- &(bmsg->opo.flow),
- u16_to_ofp(ofproto->max_ports),
- 0, ofproto->n_tables,
- ofconn_get_protocol(ofconn));
- if (error) {
- goto exit;
- }
-
- error = ofproto_check_ofpacts(ofproto, bmsg->opo.po.ofpacts,
- bmsg->opo.po.ofpacts_len);
+ ofproto_extract_packet_out(ofproto, ofconn, badd.msg, ofpacts_stub,
+ &ofpacts, &bmsg->opo.po,
+ &bmsg->opo.payload, &bmsg->opo.flow);
} else {
OVS_NOT_REACHED();
}
@@ -7311,7 +7297,7 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
bmsg);
}
-exit:
+
if (error) {
ofp_bundle_entry_free(bmsg);
}
--
2.5.0
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
