[ovs-dev] [PATCH 2/3] ofp-actions: plumb of12 actions support

Simon Horman horms at verge.net.au
Thu Sep 13 12:45:11 UTC 2012


From: Isaku Yamahata <yamahata at valinux.co.jp>

Signed-off-by: Isaku Yamahata <yamahata at valinux.co.jp>
Signed-off-by: Simon Horman <horms at verge.net.au>

---

v6
* Add limited testss
  - Previously part of another patch which has otherwise been dropped

v5 [Simon Horman]
* No change

v4 [Simon Horman]
* Update call to ofpacts_pull_openflow11_instructions()
  in ofctl_parse_ofp11_instructions() to include new ofp_version argument
* Add a special case for OFPAT12_SET_FIELD to decode_openflow12_action()
  as it is a variable-width feild while all other fields are
  fixed-width.

v3 [Simon Horman]
* Manual Rebase
* Make use of enum ofp_version

v2 [Isaku Yamahata]
* First post
---
 lib/ofp-actions.c     |   89 +++++++++++++++++++++++++++++++++++++++++++++++--
 lib/ofp-actions.h     |    3 +-
 lib/ofp-parse.c       |    1 +
 lib/ofp-util.c        |    7 ++--
 lib/ofp-util.def      |    2 +-
 tests/ofp-print.at    |   64 +++++++++++++++++++++++++++++++++++
 utilities/ovs-ofctl.c |    3 +-
 7 files changed, 162 insertions(+), 7 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 210f4ce..9686d5d 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -731,6 +731,84 @@ ofpacts_from_openflow11(const union ofp_action *in, size_t n_in,
     return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow11);
 }
 
+static enum ofperr
+decode_openflow12_action(const union ofp_action *a,
+                         enum ofputil_action_code *code)
+{
+    /* set_field has variable length.
+     * This just checks if struct is available. The more check will be done
+     * by set_field_from_openflow()
+     */
+    if (a->type == CONSTANT_HTONS(OFPAT12_SET_FIELD)) {
+        if (ntohs(a->header.len) >= sizeof(struct ofp12_action_set_field)) {
+            *code = OFPUTIL_OFPAT12_SET_FIELD;
+            return 0;
+        }
+        return OFPERR_OFPBAC_BAD_LEN;
+    }
+
+    switch (a->type) {
+    case CONSTANT_HTONS(OFPAT12_EXPERIMENTER):
+        return decode_nxast_action(a, code);
+
+#define OFPAT12_ACTION(ENUM, STRUCT, NAME)                          \
+        case CONSTANT_HTONS(ENUM):                                  \
+            if (a->header.len == htons(sizeof(struct STRUCT))) {    \
+                *code = OFPUTIL_##ENUM;                             \
+                return 0;                                           \
+            } else {                                                \
+                return OFPERR_OFPBAC_BAD_LEN;                       \
+            }                                                       \
+            break;
+#include "ofp-util.def"
+
+    default:
+        return OFPERR_OFPBAC_BAD_TYPE;
+    }
+}
+
+static enum ofperr
+ofpact_from_openflow12(const union ofp_action *a, struct ofpbuf *out)
+{
+    /* XXX */
+    enum ofputil_action_code code;
+    enum ofperr error;
+
+    error = decode_openflow12_action(a, &code);
+    if (error) {
+        return error;
+    }
+
+    switch (code) {
+    case OFPUTIL_ACTION_INVALID:
+#define OFPAT10_ACTION(ENUM, STRUCT, NAME) case OFPUTIL_##ENUM:
+#define OFPAT11_ACTION(ENUM, STRUCT, NAME) case OFPUTIL_##ENUM:
+#include "ofp-util.def"
+        NOT_REACHED();
+
+    case OFPUTIL_OFPAT12_OUTPUT:
+        return output_from_openflow11((const struct ofp11_action_output *) a,
+                                      out);
+
+    case OFPUTIL_OFPAT12_SET_FIELD:
+        return nxm_reg_load_from_openflow12_set_field(
+            (const struct ofp12_action_set_field *)a, out);
+
+#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) case OFPUTIL_##ENUM:
+#include "ofp-util.def"
+        return ofpact_from_nxast(a, code, out);
+    }
+
+    return error;
+}
+
+static enum ofperr
+ofpacts_from_openflow12(const union ofp_action *in, size_t n_in,
+                        struct ofpbuf *out)
+{
+    return ofpacts_from_openflow(in, n_in, out, ofpact_from_openflow12);
+}
+
 /* OpenFlow 1.1 instructions. */
 
 #define DEFINE_INST(ENUM, STRUCT, EXTENSIBLE, NAME)             \
@@ -906,7 +984,8 @@ ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,
 }
 
 enum ofperr
-ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
+ofpacts_pull_openflow11_instructions(enum ofp_version ofp_version,
+                                     struct ofpbuf *openflow,
                                      unsigned int instructions_len,
                                      struct ofpbuf *ofpacts)
 {
@@ -947,7 +1026,13 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
 
         get_actions_from_instruction(insts[OVSINST_OFPIT11_APPLY_ACTIONS],
                                      &actions, &n_actions);
-        error = ofpacts_from_openflow11(actions, n_actions, ofpacts);
+        if (ofp_version == OFP12_VERSION) {
+            error = ofpacts_from_openflow12(actions, n_actions, ofpacts);
+        } else if (ofp_version == OFP11_VERSION){
+            error = ofpacts_from_openflow11(actions, n_actions, ofpacts);
+        } else {
+            NOT_REACHED();
+        }
         if (error) {
             goto exit;
         }
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 75ccc26..082df7b 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -399,7 +399,8 @@ enum ofperr ofpacts_pull_openflow10(struct ofpbuf *openflow,
 enum ofperr ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,
                                             unsigned int actions_len,
                                             struct ofpbuf *ofpacts);
-enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
+enum ofperr ofpacts_pull_openflow11_instructions(enum ofp_version ofp_version,
+                                                 struct ofpbuf *openflow,
                                                  unsigned int instructions_len,
                                                  struct ofpbuf *ofpacts);
 enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 165a36e..622f022 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -329,6 +329,7 @@ parse_named_action(enum ofputil_action_code code, const struct flow *flow,
 
     case OFPUTIL_OFPAT10_OUTPUT:
     case OFPUTIL_OFPAT11_OUTPUT:
+    case OFPUTIL_OFPAT12_OUTPUT:
         parse_output(arg, ofpacts);
         break;
 
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1976b94..b0458a4 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1144,7 +1144,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
             return error;
         }
 
-        error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);
+        error = ofpacts_pull_openflow11_instructions(oh->version,
+                                                     &b, b.size, ofpacts);
         if (error) {
             return error;
         }
@@ -1612,6 +1613,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
         const struct ofp11_flow_stats *ofs;
         size_t length;
         uint16_t padded_match_len;
+        struct ofp_header *oh = msg->l2;
 
         ofs = ofpbuf_try_pull(msg, sizeof *ofs);
         if (!ofs) {
@@ -1632,7 +1634,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
             return EINVAL;
         }
 
-        if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -
+        if (ofpacts_pull_openflow11_instructions(oh->version, msg,
+                                                 length - sizeof *ofs -
                                                  padded_match_len, ofpacts)) {
             VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");
             return EINVAL;
diff --git a/lib/ofp-util.def b/lib/ofp-util.def
index 6f5113e..42b399a 100644
--- a/lib/ofp-util.def
+++ b/lib/ofp-util.def
@@ -39,7 +39,7 @@ OFPAT11_ACTION(OFPAT11_SET_TP_DST,   ofp_action_tp_port,  "mod_tp_dst")
 #ifndef OFPAT12_ACTION
 #define OFPAT12_ACTION(ENUM, STRUCT, NAME)
 #endif
-//OFPAT12_ACTION(OFPAT12_OUTPUT, , "output")
+OFPAT12_ACTION(OFPAT12_OUTPUT, ofp11_action_output, "output")
 //OFPAT12_ACTION(OFPAT12_COPY_TTL_OUT, ofp_action_header, "copy_ttl_out")
 //OFPAT12_ACTION(OFPAT12_COPY_TTL_IN, ofp_action_header, "copy_ttl_in")
 //OFPAT12_ACTION(OFPAT12_SET_MPLS_TTL, , "set_mpls_ttl")
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 9844592..4a2244e 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -517,6 +517,70 @@ OFPT_FLOW_MOD (OF1.2) (xid=0x2): ADD table:255 priority=65535,arp,in_port=1,vlan
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPT_FLOW_MOD - OF1.2 - set-field ip_src])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\
+03 0e 00 58 52 33 45 02 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ff \
+ff ff ff ff ff ff ff ff ff ff ff ff 00 00 00 00 \
+00 01 00 0a 80 00 0a 02 08 00 00 00 00 00 00 00 \
+00 04 00 18 00 00 00 00 00 19 00 10 80 00 16 04 \
+c0 a8 03 5c 00 00 00 00                         \
+" 2], [0], [dnl
+OFPT_FLOW_MOD (OF1.2) (xid=0x52334502): ADD priority=255,ip actions=set_field:192.168.3.92->ip_src
+], [dnl
+])
+AT_CLEANUP
+
+AT_SETUP([OFPT_FLOW_MOD - OF1.2 - set-field ip_dst])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\
+03 0e 00 58 52 33 45 07 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ff \
+ff ff ff ff ff ff ff ff ff ff ff ff 00 00 00 00 \
+00 01 00 0a 80 00 0a 02 08 00 00 00 00 00 00 00 \
+00 04 00 18 00 00 00 00 00 19 00 10 80 00 18 04 \
+c0 a8 4a 7a 00 00 00 00                         \
+" 2], [0], [dnl
+OFPT_FLOW_MOD (OF1.2) (xid=0x52334507): ADD priority=255,ip actions=set_field:192.168.74.122->ip_dst
+], [dnl
+])
+AT_CLEANUP
+
+AT_SETUP([OFPT_FLOW reply - OF1.2 - set-field ip_src])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\
+03 13 00 68 52 33 45 04 00 01 00 00 00 00 00 00 \
+00 58 00 00 00 00 00 00 00 00 00 00 00 ff 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 01 00 0a 80 00 0a 02 08 00 00 00 00 00 00 00 \
+00 04 00 18 00 00 00 00 00 19 00 10 80 00 16 04 \
+c0 a8 03 5c 00 00 00 00                         \
+" 2], [0], [dnl
+OFPST_FLOW reply (OF1.2) (xid=0x52334504):
+ cookie=0x0, duration=0s, table=0, n_packets=0, n_bytes=0, priority=255,ip actions=set_field:192.168.3.92->ip_src
+], [dnl
+])
+AT_CLEANUP
+
+AT_SETUP([OFPT_FLOW reply - OF1.2 - set-field ip_dst])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' ofp-print "\
+03 13 00 68 52 33 45 09 00 01 00 00 00 00 00 00 \
+00 58 00 00 00 00 00 00 00 00 00 00 00 ff 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 01 00 0a 80 00 0a 02 08 00 00 00 00 00 00 00 \
+00 04 00 18 00 00 00 00 00 19 00 10 80 00 18 04 \
+c0 a8 4a 7a 00 00 00 00                         \
+" 2], [0], [dnl
+OFPST_FLOW reply (OF1.2) (xid=0x52334509):
+ cookie=0x0, duration=0s, table=0, n_packets=0, n_bytes=0, priority=255,ip actions=set_field:192.168.74.122->ip_dst
+], [dnl
+])
+AT_CLEANUP
+
 AT_SETUP([OFPT_PORT_MOD - OF1.0])
 AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print "\
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 5f61fd6..125523d 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2551,7 +2551,8 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
         /* Convert to ofpacts. */
         ofpbuf_init(&ofpacts, 0);
         size = of11_in.size;
-        error = ofpacts_pull_openflow11_instructions(&of11_in, of11_in.size,
+        error = ofpacts_pull_openflow11_instructions(OFP11_VERSION,
+                                                     &of11_in, of11_in.size,
                                                      &ofpacts);
         if (error) {
             printf("bad OF1.1 instructions: %s\n\n", ofperr_get_name(error));
-- 
1.7.10.4




More information about the dev mailing list