[ovs-dev] [PATCH RFC 3/5] ofproto/bundles: Use staging area for port_mod messages
Alexandru Copot
alex.mihai.c at gmail.com
Tue May 6 18:28:29 UTC 2014
Allow port_mod messages in a bundle and apply them to copies
of struct ofport. The final state is commited to the original
data structure and notifications are sent.
Signed-off-by: Alexandru Copot <alex.mihai.c at gmail.com>
Cc: Daniel Baluta <dbaluta at ixiacom.com>
---
ofproto/bundles.c | 189 ++++++++++++++++++++++++++++++++++++++++++++-
ofproto/bundles.h | 1 -
ofproto/ofproto-provider.h | 4 +
ofproto/ofproto.c | 1 +
4 files changed, 191 insertions(+), 4 deletions(-)
diff --git a/ofproto/bundles.c b/ofproto/bundles.c
index 9904465..1e55eb1 100644
--- a/ofproto/bundles.c
+++ b/ofproto/bundles.c
@@ -41,6 +41,19 @@
VLOG_DEFINE_THIS_MODULE(bundles);
+/* OpenFlow 1.4 Bundles Implementation (Section 6.8, Page 42, OpenFlow 1.4 Spec)
+ *
+ * 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
@@ -54,11 +67,36 @@ struct ofp_bundle {
/* List of 'struct bundle_message's */
struct list msg_list;
+
+ /* 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 uint32_t
@@ -67,6 +105,37 @@ bundle_hash(uint32_t id)
return hash_int(id, 0);
}
+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, bundle_hash(port_no),
+ &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,
+ bundle_hash(port->ofp_port));
+}
+
static struct ofp_bundle *
ofp_bundle_find(struct hmap *bundles, uint32_t id)
{
@@ -92,6 +161,7 @@ ofp_bundle_create(uint32_t id, uint16_t flags)
bundle->flags = flags;
list_init(&bundle->msg_list);
+ hmap_init(&bundle->modified_ports);
return bundle;
}
@@ -127,6 +197,42 @@ ofp_bundle_remove_all(struct ofconn *ofconn)
}
}
+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)
{
@@ -181,11 +287,64 @@ 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;
+
+ 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_get_bundles(ofconn);
bundle = ofp_bundle_find(bundles, id);
@@ -199,8 +358,11 @@ ofp_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
}
/* TODO: actual commit */
+ error = do_commit(ofconn, bundle);
- return OFPERR_OFPBFC_MSG_UNSUP;
+ ofp_bundle_remove(ofconn, bundle);
+
+ return error;
}
enum ofperr
@@ -227,6 +389,9 @@ ofp_bundle_add_message(struct ofconn *ofconn, struct ofputil_bundle_add_msg *bad
struct hmap *bundles;
struct ofp_bundle *bundle;
struct bundle_message *bmsg;
+ enum ofperr error;
+ enum ofpraw raw;
+ struct ofpbuf b;
bundles = ofconn_get_bundles(ofconn);
bundle = ofp_bundle_find(bundles, badd->bundle_id);
@@ -246,6 +411,24 @@ ofp_bundle_add_message(struct ofconn *ofconn, struct ofputil_bundle_add_msg *bad
bmsg = xmalloc(sizeof *bmsg);
bmsg->msg = xmemdup(badd->msg, ntohs(badd->msg->length));
+
+ /* Check for unsupported messages */
+ ofpbuf_use_const(&b, bmsg->msg, ntohs(bmsg->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 */
+ error = check_message(bundle, bmsg, ofconn, bmsg->msg);
+ if (error) {
+ ofconn_send_error(ofconn, bmsg->msg, error);
+ free(bmsg);
+ return OFPERR_OFPBFC_MSG_FAILED;
+ }
+
list_push_back(&bundle->msg_list, &bmsg->node);
return 0;
}
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 9a6dfa5..46741bf 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -28,7 +28,6 @@
extern "C" {
#endif
-
enum ofperr ofp_bundle_open(struct ofconn *ofconn, uint32_t id, uint16_t flags);
enum ofperr ofp_bundle_close(struct ofconn *ofconn, uint32_t id, uint16_t flags);
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 141adec..b132e13 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -54,6 +54,7 @@ struct match;
struct ofputil_flow_mod;
struct bfd_cfg;
struct meter;
+struct ofp_bundle;
extern struct ovs_mutex ofproto_mutex;
@@ -171,6 +172,9 @@ struct ofport {
uint64_t change_seq;
long long int created; /* Time created, in msec. */
int mtu;
+
+ /* The bundle that has messages modifying this port or NULL. */
+ struct ofp_bundle *bundle;
};
void ofproto_port_set_state(struct ofport *, enum ofputil_port_state);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 6b25b9b..8f5ed85 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2225,6 +2225,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,
--
1.9.2
More information about the dev
mailing list