[ovs-dev] [PATCH v2 19/47] ofp-util: Use enum ofp14_flow_update_event in struct ofputil_flow_update

Simon Horman horms at verge.net.au
Tue Jun 10 10:27:23 UTC 2014


Switch to using enum ofp14_flow_update_event instead of
enum nx_flow_update_event in struct ofputil_flow_update as
the former can encode all values defined for the latter but
the reverse is not true.

Also use switch rather than if when testing event values
to give a little bit of extra type checking.

This is in preparation for supporting OpenFlow 1.4 flow monitor replies.

Signed-off-by: Simon Horman <horms at verge.net.au>

---
v2
* No change
---
 lib/ofp-print.c   |  14 +++++--
 lib/ofp-util.c    | 107 +++++++++++++++++++++++++++++++++++++++++++++++-------
 lib/ofp-util.h    |   2 +-
 ofproto/connmgr.c |  19 ++++++----
 ofproto/connmgr.h |   3 +-
 ofproto/ofproto.c |  17 ++++++---
 6 files changed, 129 insertions(+), 33 deletions(-)

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 40eb16d..3d3fa0b 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2304,24 +2304,30 @@ ofp_print_nxst_flow_monitor_reply(struct ds *string,
 
         ds_put_cstr(string, "\n event=");
         switch (update.event) {
-        case NXFME_ADDED:
+        case OFPFME14_ADDED:
             ds_put_cstr(string, "ADDED");
             break;
 
-        case NXFME_DELETED:
+        case OFPFME14_REMOVED:
             ds_put_format(string, "DELETED reason=%s",
                           ofp_flow_removed_reason_to_string(update.reason,
                                                             reasonbuf,
                                                             sizeof reasonbuf));
             break;
 
-        case NXFME_MODIFIED:
+        case OFPFME14_MODIFIED:
             ds_put_cstr(string, "MODIFIED");
             break;
 
-        case NXFME_ABBREV:
+        case OFPFME14_ABBREV:
             ds_put_format(string, "ABBREV xid=0x%"PRIx32, ntohl(update.xid));
             continue;
+
+        case OFPFME14_INITIAL:
+        case OFPFME14_PAUSED:
+        case OFPFME14_RESUMED:
+        default:
+            OVS_NOT_REACHED();
         }
 
         ds_put_format(string, " table=%"PRIu8, update.table_id);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index e147fbb..fed93df 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -5428,6 +5428,57 @@ ofputil_append_flow_monitor_request(
     }
 }
 
+static enum ofperr
+nx_to_ofp14_flow_update_event(enum nx_flow_update_event nx_event,
+                              enum ofp14_flow_update_event *ofp_event)
+{
+    switch (nx_event) {
+    case NXFME_ADDED:
+        *ofp_event = OFPFME14_ADDED;
+        break;
+    case NXFME_DELETED:
+        *ofp_event = OFPFME14_REMOVED;
+        break;
+    case NXFME_MODIFIED:
+        *ofp_event = OFPFME14_MODIFIED;
+        break;
+    case NXFME_ABBREV:
+        *ofp_event = OFPFME14_ABBREV;
+        break;
+    default:
+        return OFPERR_NXBRC_FM_BAD_EVENT;
+    }
+
+    return 0;
+}
+
+static enum ofperr
+nx_from_ofp14_flow_update_event(enum ofp14_flow_update_event ofp_event,
+                                enum nx_flow_update_event *nx_event)
+{
+    switch (ofp_event) {
+    case OFPFME14_INITIAL:
+    case OFPFME14_ADDED:
+        *nx_event = NXFME_ADDED;
+        break;
+    case OFPFME14_REMOVED:
+        *nx_event = NXFME_DELETED;
+        break;
+    case OFPFME14_MODIFIED:
+        *nx_event = NXFME_MODIFIED;
+        break;
+    case OFPFME14_ABBREV:
+        *nx_event = NXFME_ABBREV;
+        break;
+    case OFPFME14_PAUSED:
+    case OFPFME14_RESUMED:
+    default:
+        return OFPERR_NXBRC_FM_BAD_EVENT;
+    }
+
+    return 0;
+}
+
 /* Converts an NXST_FLOW_MONITOR reply (also known as a flow update) in 'msg'
  * into an abstract ofputil_flow_update in 'update'.  The caller must have
  * initialized update->match to point to space allocated for a match.
@@ -5449,8 +5500,10 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update,
                            struct ofpbuf *msg, struct ofpbuf *ofpacts)
 {
     struct nx_flow_update_header *nfuh;
+    enum nx_flow_update_event nevent;
     unsigned int length;
     struct ofp_header *oh;
+    enum ofperr error;
 
     if (!msg->frame) {
         ofpraw_pull_assert(msg);
@@ -5467,13 +5520,22 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update,
     oh = msg->frame;
 
     nfuh = ofpbuf_data(msg);
-    update->event = ntohs(nfuh->event);
+    nevent = ntohs(nfuh->event);
     length = ntohs(nfuh->length);
     if (length > ofpbuf_size(msg) || length % 8) {
         goto bad_len;
     }
 
-    if (update->event == NXFME_ABBREV) {
+    error = nx_to_ofp14_flow_update_event(nevent, &update->event);
+    if (error) {
+        VLOG_WARN_RL(&bad_ofmsg_rl,
+                     "NXST_FLOW_MONITOR reply has bad event %"PRIu16,
+                     ntohs(nfuh->event));
+        return error;
+    }
+
+    switch (update->event) {
+    case OFPFME14_ABBREV: {
         struct nx_flow_update_abbrev *nfua;
 
         if (length != sizeof *nfua) {
@@ -5483,9 +5545,10 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update,
         nfua = ofpbuf_pull(msg, sizeof *nfua);
         update->xid = nfua->xid;
         return 0;
-    } else if (update->event == NXFME_ADDED
-               || update->event == NXFME_DELETED
-               || update->event == NXFME_MODIFIED) {
+    }
+    case OFPFME14_ADDED:
+    case OFPFME14_REMOVED:
+    case OFPFME14_MODIFIED: {
         struct nx_flow_update_full *nfuf;
         unsigned int actions_len;
         unsigned int match_len;
@@ -5523,11 +5586,12 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update,
         update->ofpacts = ofpbuf_data(ofpacts);
         update->ofpacts_len = ofpbuf_size(ofpacts);
         return 0;
-    } else {
-        VLOG_WARN_RL(&bad_ofmsg_rl,
-                     "NXST_FLOW_MONITOR reply has bad event %"PRIu16,
-                     ntohs(nfuh->event));
-        return OFPERR_NXBRC_FM_BAD_EVENT;
+    }
+    case OFPFME14_INITIAL:
+    case OFPFME14_PAUSED:
+    case OFPFME14_RESUMED:
+    default:
+        OVS_NOT_REACHED();
     }
 
 bad_len:
@@ -5574,18 +5638,25 @@ ofputil_append_flow_update(const struct ofputil_flow_update *update,
 {
     enum ofp_version version = ofpmp_version(replies);
     struct nx_flow_update_header *nfuh;
+    enum nx_flow_update_event nevent;
     struct ofpbuf *msg;
     size_t start_ofs;
 
     msg = ofpbuf_from_list(list_back(replies));
     start_ofs = ofpbuf_size(msg);
 
-    if (update->event == NXFME_ABBREV) {
+    switch(update->event) {
+    case OFPFME14_ABBREV: {
         struct nx_flow_update_abbrev *nfua;
 
         nfua = ofpbuf_put_zeros(msg, sizeof *nfua);
         nfua->xid = update->xid;
-    } else {
+        break;
+    }
+    case OFPFME14_INITIAL:
+    case OFPFME14_ADDED:
+    case OFPFME14_REMOVED:
+    case OFPFME14_MODIFIED: {
         struct nx_flow_update_full *nfuf;
         int match_len;
 
@@ -5601,11 +5672,21 @@ ofputil_append_flow_update(const struct ofputil_flow_update *update,
         nfuf->match_len = htons(match_len);
         nfuf->table_id = update->table_id;
         nfuf->cookie = update->cookie;
+        break;
+    }
+    case OFPFME14_PAUSED:
+    case OFPFME14_RESUMED:
+    default:
+       OVS_NOT_REACHED();
     }
 
     nfuh = ofpbuf_at_assert(msg, start_ofs, sizeof *nfuh);
     nfuh->length = htons(ofpbuf_size(msg) - start_ofs);
-    nfuh->event = htons(update->event);
+
+    if (nx_from_ofp14_flow_update_event(update->event, &nevent)) {
+        OVS_NOT_REACHED();
+    }
+    nfuh->event = htons(nevent);
 
     ofpmp_postappend(replies, start_ofs);
 }
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index c41d8c9..2a6e81d 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -855,7 +855,7 @@ void ofputil_append_flow_monitor_request(
 
 /* Abstract nx_flow_update. */
 struct ofputil_flow_update {
-    enum nx_flow_update_event event;
+    enum ofp14_flow_update_event event;
 
     /* Used only for NXFME_ADDED, NXFME_DELETED, NXFME_MODIFIED. */
     enum ofp_flow_removed_reason reason;
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 896a0a1..e468d08 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -2148,7 +2148,7 @@ ofmonitor_destroy(struct ofmonitor *m)
 
 void
 ofmonitor_report(struct connmgr *mgr, struct rule *rule,
-                 enum nx_flow_update_event event,
+                 enum ofp14_flow_update_event event,
                  enum ofp_flow_removed_reason reason,
                  const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid)
     OVS_REQUIRES(ofproto_mutex)
@@ -2157,22 +2157,25 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
     struct ofconn *ofconn;
 
     switch (event) {
-    case NXFME_ADDED:
+    case OFPFME14_ADDED:
         update = OFPFMF14_ADD;
         rule->add_seqno = rule->modify_seqno = monitor_seqno++;
         break;
 
-    case NXFME_DELETED:
+    case OFPFME14_REMOVED:
         update = OFPFMF14_REMOVED;
         break;
 
-    case NXFME_MODIFIED:
+    case OFPFME14_MODIFIED:
         update = OFPFMF14_MODIFY;
         rule->modify_seqno = monitor_seqno++;
         break;
 
     default:
-    case NXFME_ABBREV:
+    case OFPFME14_INITIAL:
+    case OFPFME14_ABBREV:
+    case OFPFME14_PAUSED:
+    case OFPFME14_RESUMED:
         OVS_NOT_REACHED();
     }
 
@@ -2183,7 +2186,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
         if (ofconn->monitor_paused) {
             /* Only send NXFME_DELETED notifications for flows that were added
              * before we paused. */
-            if (event != NXFME_DELETED
+            if (event != OFPFME14_REMOVED
                 || rule->add_seqno > ofconn->monitor_paused) {
                 continue;
             }
@@ -2212,7 +2215,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
                 struct match match;
 
                 fu.event = event;
-                fu.reason = event == NXFME_DELETED ? reason : 0;
+                fu.reason = event == OFPFME14_REMOVED ? reason : 0;
                 fu.table_id = rule->table_id;
                 fu.cookie = rule->flow_cookie;
                 minimatch_expand(&rule->cr.match, &match);
@@ -2236,7 +2239,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
             } else if (!ofconn->sent_abbrev_update) {
                 struct ofputil_flow_update fu;
 
-                fu.event = NXFME_ABBREV;
+                fu.event = OFPFME14_ABBREV;
                 fu.xid = abbrev_xid;
                 ofputil_append_flow_update(&fu, &ofconn->updates);
 
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index 87e4d3a..5abeb99 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -225,7 +225,8 @@ void ofmonitor_destroy(struct ofmonitor *)
     OVS_REQUIRES(ofproto_mutex);
 
 void ofmonitor_report(struct connmgr *, struct rule *,
-                      enum nx_flow_update_event, enum ofp_flow_removed_reason,
+                      enum ofp14_flow_update_event,
+                      enum ofp_flow_removed_reason,
                       const struct ofconn *abbrev_ofconn, ovs_be32 abbrev_xid)
     OVS_REQUIRES(ofproto_mutex);
 void ofmonitor_flush(struct connmgr *) OVS_REQUIRES(ofproto_mutex);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 4ac4fa9..eb97681 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4758,8 +4758,13 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
         return;
     }
 
-    fu.event = (flags & (OFPFMF14_INITIAL | OFPFMF14_ADD)
-                ? NXFME_ADDED : NXFME_MODIFIED);
+    if (flags & OFPFMF14_INITIAL) {
+        fu.event = OFPFME14_INITIAL;
+    } else if (flags & OFPFMF14_ADD) {
+        fu.event = OFPFME14_ADDED;
+    } else {
+        fu.event = OFPFME14_MODIFIED;
+    }
     fu.reason = 0;
     ovs_mutex_lock(&rule->mutex);
     fu.idle_timeout = rule->idle_timeout;
@@ -6273,20 +6278,20 @@ ofopgroup_complete(struct ofopgroup *group)
               || (op->type == OFOPERATION_MODIFY
                   && !op->actions
                   && rule->flow_cookie == op->flow_cookie))) {
-            enum nx_flow_update_event event_type;
+            enum ofp14_flow_update_event event_type;
 
             switch (op->type) {
             case OFOPERATION_ADD:
             case OFOPERATION_REPLACE:
-                event_type = NXFME_ADDED;
+                event_type = OFPFME14_ADDED;
                 break;
 
             case OFOPERATION_DELETE:
-                event_type = NXFME_DELETED;
+                event_type = OFPFME14_REMOVED;
                 break;
 
             case OFOPERATION_MODIFY:
-                event_type = NXFME_MODIFIED;
+                event_type = OFPFME14_MODIFIED;
                 break;
 
             default:
-- 
2.0.0.rc2




More information about the dev mailing list