[firefly] [PATCH RFC 4/7] ofproto/bundles: first stage of commit for port_mod
Alexandru Copot
alex.mihai.c at gmail.com
Thu Apr 10 20:20:57 EEST 2014
Multiple port-mod messages may be combined and executed as
part of a bundle. Validity checks are done when messages
are added and the commit is atomic.
"ofport" data structures can only be part of a single bundle at a time.
Signed-off-by: Alexandru Copot <alex.mihai.c at gmail.com>
Cc: Daniel Baluta <dbaluta at ixiacom.com>
---
lib/ofp-msgs.h | 2 +-
lib/ofp-print.c | 2 +-
lib/ofp-util.c | 5 +-
lib/ofp-util.h | 3 +-
ofproto/bundles.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++---
ofproto/bundles.h | 5 +-
ofproto/ofproto.c | 4 +-
tests/ofp-print.at | 3 +-
8 files changed, 210 insertions(+), 19 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index bfc94b2..de2953e 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -236,7 +236,7 @@ enum ofpraw {
/* OFPT 1.4+ (33): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */
OFPRAW_OFPT14_BUNDLE_CONTROL,
- /* OFPT 1.4+ (34): struct ofp14_bundle_add_msg, uint8_t[16][]. */
+ /* OFPT 1.4+ (34): struct ofp14_bundle_add_msg, uint32_t, uint16_t, uint16_t, uint8_t[8][]. */
OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE,
/* Standard statistics. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index e4e4ab1..46ce8bc 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2738,7 +2738,7 @@ ofp_print_bundle_add(struct ds *s, const struct ofp_header *oh, int verbosity)
ofp_print_bit_names(s, badd.flags, bundle_flags_to_name, ' ');
ds_put_char(s, '\n');
- msg = ofp_to_string((const void*)&badd.msg, ntohs(badd.msg.length), verbosity);
+ msg = ofp_to_string((const void*)badd.msg, badd.length, verbosity);
if (msg) {
ds_put_cstr(s, msg);
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d7d9ac6..bfb6052 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -7103,7 +7103,10 @@ ofputil_decode_bundle_add(const struct ofp_header *oh,
m = b.l3;
msg->bundle_id = ntohl(m->bundle_id);
msg->flags = ntohs(m->flags);
- memcpy(&msg->msg, &m->message, sizeof(msg->msg));
+ msg->length = ntohs(m->message.length);
+
+ msg->msg = xmalloc(msg->length);
+ memcpy(msg->msg, &m->message, msg->length);
return 0;
}
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index c9afece..a1c09c8 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -1137,7 +1137,8 @@ struct ofputil_bundle_ctrl_msg {
struct ofputil_bundle_add_msg {
uint32_t bundle_id;
uint16_t flags;
- struct ofp_header msg;
+ struct ofp_header *msg;
+ uint16_t length;
};
enum ofperr ofputil_decode_bundle_ctrl(const struct ofp_header *,
diff --git a/ofproto/bundles.c b/ofproto/bundles.c
index 41efa12..0e9d54b 100644
--- a/ofproto/bundles.c
+++ b/ofproto/bundles.c
@@ -42,6 +42,19 @@
VLOG_DEFINE_THIS_MODULE(bundles);
+/* OpenFlow 1.4 Bundles Implementation
+ *
+ * Each OpenFlow bundles is specified by a "struct ofp_bundle" structure.
+ * Messages added to a bundle are pre-checked and stored with their decoded
+ * version in a list. Currently, we support only messages of type "port_mod".
+ *
+ * For "port_mod" messages, a bundle maintains a hash table with copies
+ * of the "ofport" that are modified by each message. When a bundle is
+ * commited, modifications are done to these copies, without affecting
+ * the switch state. Notifications and netdev changes are done only
+ * for the final port state which is copied over the original.
+ */
+
enum bundle_state {
BS_OPEN,
BS_CLOSED
@@ -56,13 +69,69 @@ struct ofp_bundle {
/* List of 'struct bundle_message's */
struct list msg_list;
struct ovs_mutex list_mutex;
+
+ /* Copies of "struct ofport" being modified by this bundle. */
+ struct hmap modified_ports;
+};
+
+enum message_type {
+ MT_PORT_MOD,
+ MT_FLOW_MOD,
};
struct bundle_message {
- struct ofp_header msg;
- struct list node; /* Element in 'struct ofp_bundles's msg_list */
+ struct ofp_header *msg;
+
+ enum message_type type;
+
+ /* Decoded message */
+ struct ofputil_port_mod pm;
+
+ struct list node; /* Element in 'struct ofp_bundles's msg_list */
};
+struct modified_port {
+ struct ofport port;
+
+
+ struct ofport *original_port;
+
+
+ struct bundle_message *bmsg;
+ struct hmap_node node;
+};
+
+static struct modified_port *
+bundle_find_port(struct ofp_bundle *bundle, ofp_port_t port_no)
+{
+ struct modified_port *port;
+
+ HMAP_FOR_EACH_IN_BUCKET(port, node, hash_int(port_no, 0),
+ &bundle->modified_ports) {
+ if (port->port.ofp_port == port_no)
+ return port;
+ }
+
+ return NULL;
+}
+
+static void
+bundle_add_port_copy(struct ofp_bundle *bundle, struct bundle_message *bmsg, struct ofport *port)
+{
+ struct modified_port *copy;
+
+ copy = bundle_find_port(bundle, port->ofp_port);
+ if (copy)
+ return;
+
+ copy = xmalloc(sizeof(*copy));
+ memcpy(©->port, port, sizeof(*port));
+ copy->original_port = port;
+ copy->bmsg = bmsg;
+ hmap_insert(&bundle->modified_ports, ©->node,
+ hash_int(port->ofp_port, 0));
+}
+
static uint32_t
ofp_bundle_hash(uint32_t id)
{
@@ -93,6 +162,7 @@ ofp_bundle_create(uint32_t id, uint16_t flags)
bundle->flags = flags;
list_init(&bundle->msg_list);
+ hmap_init(&bundle->modified_ports);
ovs_mutex_init(&bundle->list_mutex);
return bundle;
@@ -106,6 +176,7 @@ ofp_bundle_remove(struct ofconn *ofconn, struct ofp_bundle *item)
LIST_FOR_EACH_SAFE (msg, next, node, &item->msg_list) {
list_remove(&msg->node);
+ free(msg->msg);
free(msg);
}
@@ -116,6 +187,42 @@ ofp_bundle_remove(struct ofconn *ofconn, struct ofp_bundle *item)
free(item);
}
+static void
+apply_port_mod(struct ofport *port, struct ofputil_port_mod pm)
+{
+ enum ofputil_port_config toggle = (pm.config ^ port->pp.config) & pm.mask;
+
+ if (toggle) {
+ port->pp.config ^= toggle;
+ }
+
+ port->pp.advertised = pm.advertise;
+}
+
+static enum ofperr
+check_message(struct ofp_bundle *bundle, struct bundle_message *bmsg,
+ struct ofconn *ofconn, struct ofp_header *oh)
+{
+ enum ofperr error;
+ struct ofport *port;
+
+ error = check_port_mod(ofconn, oh, &bmsg->pm, &port);
+ if (!error) {
+ /* Reject port already part of another bundle */
+ if (port->bundle && port->bundle != bundle) {
+ VLOG_INFO("Port %x is already part of a bundle.\n", port->ofp_port);
+ return OFPERR_OFPBFC_MSG_CONFLICT;
+ }
+ port->bundle = bundle;
+ bmsg->type = MT_PORT_MOD;
+ bundle_add_port_copy(bundle, bmsg, port);
+ }
+
+ /* TODO: check if it's a flow_mod */
+
+ return error;
+}
+
enum ofperr
ofp_bundle_open(struct ofconn *ofconn, uint32_t id, uint16_t flags)
{
@@ -173,11 +280,66 @@ ofp_bundle_close(struct ofconn *ofconn, uint32_t id, uint16_t flags)
return 0;
}
+static enum ofperr
+do_commit_port_mod(struct ofconn *ofconn, struct modified_port *mport)
+{
+ struct ofputil_port_mod pm;
+ enum ofperr error;
+
+ pm.config = mport->port.pp.config;
+ pm.mask = 0xffffffff;
+ pm.advertise = mport->port.pp.advertised;
+
+ error = 0;
+ /* Check the port mod again ? */
+
+ if (!error) {
+ update_port_config(ofconn, mport->original_port, pm.config, pm.mask);
+ if (pm.advertise) {
+ netdev_set_advertisements(mport->original_port->netdev, pm.advertise);
+ }
+ }
+
+ return error;
+}
+
+static enum ofperr
+do_commit(struct ofconn *ofconn, struct ofp_bundle *bundle)
+{
+ struct bundle_message *msg;
+ struct modified_port *mport;
+
+ LIST_FOR_EACH(msg, node, &bundle->msg_list) {
+ if (msg->type == MT_PORT_MOD) {
+ mport = bundle_find_port(bundle, msg->pm.port_no);
+
+ if (!mport)
+ continue;
+
+ atomic_store(&mport->original_port->is_updating, true);
+
+ apply_port_mod(&mport->port, msg->pm);
+ }
+ }
+
+ HMAP_FOR_EACH(mport, node, &bundle->modified_ports) {
+ do_commit_port_mod(ofconn, mport);
+ }
+
+ /* Remove the ports from the bundle */
+ HMAP_FOR_EACH(mport, node, &bundle->modified_ports) {
+ mport->original_port->bundle = NULL;
+ }
+
+ return 0;
+}
+
enum ofperr
ofp_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
{
struct hmap *bundles;
struct ofp_bundle *bundle;
+ enum ofperr error;
bundles = ofconn_lock_bundles(ofconn);
bundle = ofp_bundle_find(bundles, id);
@@ -192,8 +354,11 @@ ofp_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
}
/* TODO: actual commit */
+ error = do_commit(ofconn, bundle);
- return 0;
+ ofp_bundle_remove(ofconn, bundle);
+
+ return error;
}
enum ofperr
@@ -216,23 +381,26 @@ ofp_bundle_discard(struct ofconn *ofconn, uint32_t id)
}
enum ofperr
-ofp_bundle_add_message(struct ofconn *ofconn, uint32_t id, uint16_t flags,
- struct ofp_header *msg)
+ofp_bundle_add_message(struct ofconn *ofconn, struct ofputil_bundle_add_msg *badd)
{
+ struct ofp_header *msg;
struct hmap *bundles;
struct ofp_bundle *bundle;
struct bundle_message *bmsg;
+ enum ofperr error;
+ enum ofpraw raw;
+ struct ofpbuf b;
bundles = ofconn_lock_bundles(ofconn);
- bundle = ofp_bundle_find(bundles, id);
+ bundle = ofp_bundle_find(bundles, badd->bundle_id);
ofconn_unlock_bundles(ofconn);
if (!bundle) {
- bundle = ofp_bundle_create(id, flags);
+ bundle = ofp_bundle_create(badd->bundle_id, badd->flags);
bundle->state = BS_OPEN;
bundles = ofconn_lock_bundles(ofconn);
- hmap_insert(bundles, &bundle->node, hash_int(id, 10));
+ hmap_insert(bundles, &bundle->node, hash_int(badd->bundle_id, 10));
ofconn_unlock_bundles(ofconn);
}
@@ -241,8 +409,27 @@ ofp_bundle_add_message(struct ofconn *ofconn, uint32_t id, uint16_t flags,
return OFPERR_OFPBFC_BUNDLE_CLOSED;
}
+ msg = badd->msg;
+
+ /* Check for unsupported messages */
+ ofpbuf_use_const(&b, msg, ntohs(msg->length));
+ raw = ofpraw_pull_assert(&b);
+ if (raw == OFPRAW_OFPT_HELLO || raw == OFPRAW_OFPT_ECHO_REQUEST ||
+ raw == OFPRAW_OFPT_ECHO_REPLY || raw == OFPRAW_OFPT14_BUNDLE_CONTROL ||
+ raw == OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE) {
+ return OFPERR_OFPBFC_MSG_UNSUP;
+ }
+
+ /* Check known messages */
bmsg = xmalloc(sizeof(*bmsg));
- bmsg->msg = *msg;
+ error = check_message(bundle, bmsg, ofconn, msg);
+ if (error) {
+ ofconn_send_error(ofconn, msg, error);
+ free(bmsg);
+ return OFPERR_OFPBFC_MSG_FAILED;
+ }
+
+ bmsg->msg = msg;
ovs_mutex_lock(&bundle->list_mutex);
list_push_back(&bundle->msg_list, &bmsg->node);
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 396d0cb..8baa147 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -23,12 +23,12 @@
#include "ofp-msgs.h"
#include "connmgr.h"
+#include "ofp-util.h"
#ifdef __cplusplus
extern "C" {
#endif
-
enum ofperr
ofp_bundle_open(struct ofconn *ofconn, uint32_t id, uint16_t flags);
@@ -43,8 +43,7 @@ enum ofperr
ofp_bundle_discard(struct ofconn *ofconn, uint32_t id);
enum ofperr
-ofp_bundle_add_message(struct ofconn *ofconn, uint32_t id, uint16_t flags,
- struct ofp_header *msg);
+ofp_bundle_add_message(struct ofconn *ofconn, struct ofputil_bundle_add_msg *badd);
#ifdef __cplusplus
}
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 03a6713..ace1077 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2195,6 +2195,7 @@ ofport_install(struct ofproto *p,
ofport->pp = *pp;
ofport->ofp_port = pp->port_no;
ofport->created = time_msec();
+ ofport->bundle = NULL;
/* Add port to 'p'. */
hmap_insert(&p->ports, &ofport->hmap_node,
@@ -5913,8 +5914,7 @@ handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
return error;
}
- error = ofp_bundle_add_message(ofconn, badd.bundle_id,
- badd.flags, &badd.msg);
+ error = ofp_bundle_add_message(ofconn, &badd);
return error;
}
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 08acf54..ba1abf1 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -2922,8 +2922,9 @@ AT_CLEANUP
AT_SETUP([OFPT_BUNDLE_ADD_MESSAGE - OFPT_HELLO])
AT_KEYWORDS([ofp-print])
AT_CHECK([ovs-ofctl ofp-print "\
-05 22 00 18 00 00 00 00 \
+05 22 00 20 00 00 00 00 \
00 00 00 01 00 01 00 01 02 00 00 08 00 00 00 00 \
+00 00 00 00 00 00 00 00 \
"], [0], [dnl
OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x0):
bundle_id=0x1 flags=atomic
--
1.9.1
More information about the firefly
mailing list