[ovs-dev] [of1.1 v2 08/12] ofp-actions: Adjust encoding OpenFlow 1.1 actions.

Ben Pfaff blp at nicira.com
Fri Jun 15 06:35:26 UTC 2012


This mainly changes ofpact_to_openflow11() to use the
ofpact_to_nxast() function that I introduced.  I think this makes
ofpact_to_openflow11() more readable; certainly shorter.

I also broke ofpacts_to_openflow() into two functions
ofpacts_to_openflow10() and ofpacts_to_openflow11().  I might be
wrong, but my feeling is that the messages and circumstances in
which actions are present in OpenFlow 1.0 and 1.1 are different
enough that there won't be any caller that would want to blindly
pass along a protocol value.

We don't have any functions for encoding instructions yet.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-actions.c |  106 ++++++++++++-----------------------------------------
 lib/ofp-actions.h |    4 +-
 lib/ofp-util.c    |    8 ++--
 3 files changed, 30 insertions(+), 88 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 131b3f5..17c8142 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1257,6 +1257,19 @@ ofpact_to_openflow10(const struct ofpact *a, struct ofpbuf *out)
     }
 }
 
+/* Converts the ofpacts in 'ofpacts' (terminated by OFPACT_END) into OpenFlow
+ * 1.0 actions in 'openflow', appending the actions to any existing data in
+ * 'openflow'. */
+void
+ofpacts_to_openflow10(const struct ofpact ofpacts[], struct ofpbuf *openflow)
+{
+    const struct ofpact *a;
+
+    OFPACT_FOR_EACH (a, ofpacts) {
+        ofpact_to_openflow10(a, openflow);
+    }
+}
+
 /* Converting ofpacts to OpenFlow 1.1. */
 
 static void
@@ -1271,38 +1284,17 @@ ofpact_output_to_openflow11(const struct ofpact_output *output,
 }
 
 static void
-ofpact_controller_to_openflow11(const struct ofpact_controller *oc,
-                                struct ofpbuf *out)
-{
-    struct ofp11_action_output *oao;
-
-    oao = ofputil_put_OFPAT11_OUTPUT(out);
-    oao->port = htonl(OFPP_CONTROLLER);
-    oao->max_len = htons(oc->max_len);
-}
-
-static void
 ofpact_to_openflow11(const struct ofpact *a, struct ofpbuf *out)
 {
     switch (a->type) {
     case OFPACT_END:
-    case OFPACT_ENQUEUE:
         NOT_REACHED();
 
     case OFPACT_OUTPUT:
         return ofpact_output_to_openflow11(ofpact_get_OUTPUT(a), out);
 
-    case OFPACT_CONTROLLER:
-        ofpact_controller_to_openflow11(ofpact_get_CONTROLLER(a), out);
-        break;
-
-    case OFPACT_OUTPUT_REG:
-        /* FIXME: Is this generic or is an openflow11 version required ? */
-        ofpact_output_reg_to_openflow10(ofpact_get_OUTPUT_REG(a), out);
-        break;
-
-    case OFPACT_BUNDLE:
-        bundle_to_openflow(ofpact_get_BUNDLE(a), out);
+    case OFPACT_ENQUEUE:
+        /* XXX */
         break;
 
     case OFPACT_SET_VLAN_VID:
@@ -1316,8 +1308,7 @@ ofpact_to_openflow11(const struct ofpact *a, struct ofpbuf *out)
         break;
 
     case OFPACT_STRIP_VLAN:
-        /* FIXME: Is this generic or is an OFPAT11 version required ? */
-        ofputil_put_OFPAT10_STRIP_VLAN(out);
+        /* XXX */
         break;
 
     case OFPACT_SET_ETH_SRC:
@@ -1355,86 +1346,37 @@ ofpact_to_openflow11(const struct ofpact *a, struct ofpbuf *out)
             = htons(ofpact_get_SET_L4_DST_PORT(a)->port);
         break;
 
+    case OFPACT_CONTROLLER:
+    case OFPACT_OUTPUT_REG:
+    case OFPACT_BUNDLE:
     case OFPACT_REG_MOVE:
-        nxm_reg_move_to_openflow(ofpact_get_REG_MOVE(a), out);
-        break;
-
     case OFPACT_REG_LOAD:
-        nxm_reg_load_to_openflow(ofpact_get_REG_LOAD(a), out);
-        break;
-
     case OFPACT_DEC_TTL:
-        ofputil_put_NXAST_DEC_TTL(out);
-        break;
-
     case OFPACT_SET_TUNNEL:
-        /* FIXME: Is this generic or is an openflow11 version required ? */
-        ofpact_set_tunnel_to_openflow10(ofpact_get_SET_TUNNEL(a), out);
-        break;
-
     case OFPACT_SET_QUEUE:
-        ofputil_put_NXAST_SET_QUEUE(out)->queue_id
-            = htonl(ofpact_get_SET_QUEUE(a)->queue_id);
-        break;
-
     case OFPACT_POP_QUEUE:
-        ofputil_put_NXAST_POP_QUEUE(out);
-        break;
-
+    case OFPACT_FIN_TIMEOUT:
     case OFPACT_RESUBMIT:
-        /* FIXME: Is this generic or is an openflow11 version required ? */
-        ofpact_resubmit_to_openflow10(ofpact_get_RESUBMIT(a), out);
-        break;
-
     case OFPACT_LEARN:
-        learn_to_openflow(ofpact_get_LEARN(a), out);
-        break;
-
     case OFPACT_MULTIPATH:
-        multipath_to_openflow(ofpact_get_MULTIPATH(a), out);
-        break;
-
     case OFPACT_AUTOPATH:
-        autopath_to_openflow(ofpact_get_AUTOPATH(a), out);
-        break;
-
     case OFPACT_NOTE:
-        /* FIXME: Is this generic or is an openflow11 version required ? */
-        ofpact_note_to_openflow10(ofpact_get_NOTE(a), out);
-        break;
-
     case OFPACT_EXIT:
-        ofputil_put_NXAST_EXIT(out);
-        break;
-
-    case OFPACT_FIN_TIMEOUT:
-        /* FIXME: Is this generic or is an openflow11 version required ? */
-        ofpact_fin_timeout_to_openflow10(ofpact_get_FIN_TIMEOUT(a), out);
+        ofpact_to_nxast(a, out);
         break;
     }
 }
 
 /* Converts the ofpacts in 'ofpacts' (terminated by OFPACT_END) into OpenFlow
- * actions in 'openflow', appending the actions to any existing data in
+ * 1.1 actions in 'openflow', appending the actions to any existing data in
  * 'openflow'. */
 void
-ofpacts_to_openflow(const struct ofpact ofpacts[], struct ofpbuf *openflow,
-                    enum ofputil_protocol protocol)
+ofpacts_to_openflow11(const struct ofpact ofpacts[], struct ofpbuf *openflow)
 {
     const struct ofpact *a;
-    uint8_t ofp_version = ofputil_protocol_to_ofp_version(protocol);
-    static void (*f)(const struct ofpact *, struct ofpbuf *);
-
-    if (ofp_version == OFP11_VERSION || ofp_version == OFP12_VERSION) {
-        f = ofpact_to_openflow11;
-    } else if (ofp_version == OFP10_VERSION) {
-        f = ofpact_to_openflow10;
-    } else {
-        NOT_REACHED();
-    }
 
     OFPACT_FOR_EACH (a, ofpacts) {
-        f(a, openflow);
+        ofpact_to_openflow11(a, openflow);
     }
 }
 
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 915c147..49c2c27 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -389,8 +389,8 @@ enum ofperr ofpacts_check(const struct ofpact[],
                           const struct flow *, int max_ports);
 
 /* Converting ofpacts to OpenFlow. */
-void ofpacts_to_openflow(const struct ofpact[], struct ofpbuf *openflow,
-                         enum ofputil_protocol protocol);
+void ofpacts_to_openflow10(const struct ofpact[], struct ofpbuf *openflow);
+void ofpacts_to_openflow11(const struct ofpact[], struct ofpbuf *openflow);
 
 /* Working with ofpacts. */
 bool ofpacts_output_to_port(const struct ofpact[], uint16_t port);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 75f9b01..190b36c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1808,7 +1808,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
     }
 
     if (fm->ofpacts) {
-        ofpacts_to_openflow(fm->ofpacts, msg, protocol);
+        ofpacts_to_openflow10(fm->ofpacts, msg);
     }
     update_openflow_length(msg);
     return msg;
@@ -2150,7 +2150,7 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs,
                            htonll(unknown_to_zero(fs->packet_count)));
         put_32aligned_be64(&ofs->byte_count,
                            htonll(unknown_to_zero(fs->byte_count)));
-        ofpacts_to_openflow(fs->ofpacts, reply, OFPUTIL_P_OF10);
+        ofpacts_to_openflow10(fs->ofpacts, reply);
 
         ofs = ofpbuf_at_assert(reply, start_ofs, sizeof *ofs);
         ofs->length = htons(reply->size - start_ofs);
@@ -2175,7 +2175,7 @@ ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs,
         nfs->cookie = fs->cookie;
         nfs->packet_count = htonll(fs->packet_count);
         nfs->byte_count = htonll(fs->byte_count);
-        ofpacts_to_openflow(fs->ofpacts, reply, OFPUTIL_P_NXM);
+        ofpacts_to_openflow10(fs->ofpacts, reply);
 
         nfs = ofpbuf_at_assert(reply, start_ofs, sizeof *nfs);
         nfs->length = htons(reply->size - start_ofs);
@@ -3095,7 +3095,7 @@ ofputil_encode_packet_out(const struct ofputil_packet_out *po)
 
     msg = ofpbuf_new(size);
     put_openflow(sizeof *opo, OFPT10_PACKET_OUT, msg);
-    ofpacts_to_openflow(po->ofpacts, msg, OFPUTIL_P_OF10);
+    ofpacts_to_openflow10(po->ofpacts, msg);
 
     opo = msg->data;
     opo->buffer_id = htonl(po->buffer_id);
-- 
1.7.2.5




More information about the dev mailing list