[ovs-dev] [learning v2 05/19] ofp-util: Add type-safe functions for serializing actions.

Ben Pfaff blp at nicira.com
Fri Aug 19 22:28:20 UTC 2011


---
 lib/autopath.c  |    6 +--
 lib/bundle.c    |    7 +--
 lib/multipath.c |    6 +--
 lib/nx-match.c  |   10 +----
 lib/ofp-parse.c |  124 ++++++++++++++++---------------------------------------
 lib/ofp-util.c  |   59 ++++++++++++++++++++++++--
 lib/ofp-util.h  |   24 +++++++++++
 7 files changed, 122 insertions(+), 114 deletions(-)

diff --git a/lib/autopath.c b/lib/autopath.c
index b42826c..9a39c6a 100644
--- a/lib/autopath.c
+++ b/lib/autopath.c
@@ -67,11 +67,7 @@ autopath_parse(struct nx_action_autopath *ap, const char *s_)
                   "less than required 65536", s_, n_bits, 1u << n_bits);
     }
 
-    memset(ap, 0, sizeof *ap);
-    ap->type = htons(OFPAT_VENDOR);
-    ap->len = htons(sizeof *ap);
-    ap->vendor = htonl(NX_VENDOR_ID);
-    ap->subtype = htons(NXAST_AUTOPATH);
+    ofputil_init_NXAST_AUTOPATH(ap);
     ap->id = htonl(id_int);
     ap->ofs_nbits = nxm_encode_ofs_nbits(ofs, n_bits);
     ap->dst = htonl(reg);
diff --git a/lib/bundle.c b/lib/bundle.c
index 86762f9..ac0bade 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -193,6 +193,7 @@ bundle_parse__(struct ofpbuf *b, const char *s, char **save_ptr,
                const char *slave_type, const char *dst,
                const char *slave_delim)
 {
+    enum ofputil_action_code code;
     struct nx_action_bundle *nab;
     uint16_t n_slaves;
 
@@ -205,7 +206,8 @@ bundle_parse__(struct ofpbuf *b, const char *s, char **save_ptr,
                    s, slave_delim);
     }
 
-    b->l2 = ofpbuf_put_zeros(b, sizeof *nab);
+    code = dst ? OFPUTIL_NXAST_BUNDLE_LOAD : OFPUTIL_NXAST_BUNDLE;
+    b->l2 = ofputil_put_action(code, b);
 
     n_slaves = 0;
     for (;;) {
@@ -229,10 +231,7 @@ bundle_parse__(struct ofpbuf *b, const char *s, char **save_ptr,
     }
 
     nab = b->l2;
-    nab->type = htons(OFPAT_VENDOR);
     nab->len = htons(b->size - ((char *) b->l2 - (char *) b->data));
-    nab->vendor = htonl(NX_VENDOR_ID);
-    nab->subtype = htons(dst ? NXAST_BUNDLE_LOAD: NXAST_BUNDLE);
     nab->n_slaves = htons(n_slaves);
     nab->basis = htons(atoi(basis));
 
diff --git a/lib/multipath.c b/lib/multipath.c
index e85829a..f68dafd 100644
--- a/lib/multipath.c
+++ b/lib/multipath.c
@@ -180,11 +180,7 @@ multipath_parse(struct nx_action_multipath *mp, const char *s_)
         ovs_fatal(0, "%s: not enough arguments to multipath action", s_);
     }
 
-    memset(mp, 0, sizeof *mp);
-    mp->type = htons(OFPAT_VENDOR);
-    mp->len = htons(sizeof *mp);
-    mp->vendor = htonl(NX_VENDOR_ID);
-    mp->subtype = htons(NXAST_MULTIPATH);
+    ofputil_init_NXAST_MULTIPATH(mp);
     if (!strcasecmp(fields, "eth_src")) {
         mp->fields = htons(NX_HASH_FIELDS_ETH_SRC);
     } else if (!strcasecmp(fields, "symmetric_l4")) {
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 2654dde..422ce76 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1093,10 +1093,7 @@ nxm_parse_reg_move(struct nx_action_reg_move *move, const char *s)
                   "%d bits wide", full_s, src_n_bits, dst_n_bits);
     }
 
-    move->type = htons(OFPAT_VENDOR);
-    move->len = htons(sizeof *move);
-    move->vendor = htonl(NX_VENDOR_ID);
-    move->subtype = htons(NXAST_REG_MOVE);
+    ofputil_init_NXAST_REG_MOVE(move);
     move->n_bits = htons(src_n_bits);
     move->src_ofs = htons(src_ofs);
     move->dst_ofs = htons(dst_ofs);
@@ -1127,10 +1124,7 @@ nxm_parse_reg_load(struct nx_action_reg_load *load, const char *s)
                   full_s, value, n_bits);
     }
 
-    load->type = htons(OFPAT_VENDOR);
-    load->len = htons(sizeof *load);
-    load->vendor = htonl(NX_VENDOR_ID);
-    load->subtype = htons(NXAST_REG_LOAD);
+    ofputil_init_NXAST_REG_LOAD(load);
     load->ofs_nbits = nxm_encode_ofs_nbits(ofs, n_bits);
     load->dst = htonl(dst);
     load->value = htonll(value);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index eae5d58..7e38565 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -253,19 +253,12 @@ str_to_ipv6(const char *str_, struct in6_addr *addrp, struct in6_addr *maskp)
     free(str);
 }
 
-static void *
-put_action(struct ofpbuf *b, size_t size, uint16_t type)
-{
-    struct ofp_action_header *ah = ofpbuf_put_zeros(b, size);
-    ah->type = htons(type);
-    ah->len = htons(size);
-    return ah;
-}
-
 static struct ofp_action_output *
 put_output_action(struct ofpbuf *b, uint16_t port)
 {
-    struct ofp_action_output *oao = put_action(b, sizeof *oao, OFPAT_OUTPUT);
+    struct ofp_action_output *oao;
+
+    oao = ofputil_put_OFPAT_OUTPUT(b);
     oao->port = htons(port);
     return oao;
 }
@@ -282,19 +275,12 @@ parse_enqueue(struct ofpbuf *b, char *arg)
         ovs_fatal(0, "\"enqueue\" syntax is \"enqueue:PORT:QUEUE\"");
     }
 
-    oae = put_action(b, sizeof *oae, OFPAT_ENQUEUE);
+    oae = ofputil_put_OFPAT_ENQUEUE(b);
     oae->port = htons(str_to_u32(port));
     oae->queue_id = htonl(str_to_u32(queue));
 }
 
 static void
-put_dl_addr_action(struct ofpbuf *b, uint16_t type, const char *addr)
-{
-    struct ofp_action_dl_addr *oada = put_action(b, sizeof *oada, type);
-    str_to_mac(addr, oada->dl_addr);
-}
-
-static void
 parse_output(struct ofpbuf *b, char *arg)
 {
     if (strchr(arg, '[')) {
@@ -304,9 +290,7 @@ parse_output(struct ofpbuf *b, char *arg)
 
         nxm_parse_field_bits(arg, &src, &ofs, &n_bits);
 
-        naor = put_action(b, sizeof *naor, OFPAT_VENDOR);
-        naor->vendor = htonl(NX_VENDOR_ID);
-        naor->subtype = htons(NXAST_OUTPUT_REG);
+        naor = ofputil_put_NXAST_OUTPUT_REG(b);
         naor->ofs_nbits = nxm_encode_ofs_nbits(ofs, n_bits);
         naor->src = htonl(src);
         naor->max_len = htons(UINT16_MAX);
@@ -316,8 +300,9 @@ parse_output(struct ofpbuf *b, char *arg)
 }
 
 static void
-parse_resubmit(struct nx_action_resubmit *nar, char *arg)
+parse_resubmit(struct ofpbuf *b, char *arg)
 {
+    struct nx_action_resubmit *nar;
     char *in_port_s, *table_s;
     uint16_t in_port;
     uint8_t table;
@@ -339,33 +324,23 @@ parse_resubmit(struct nx_action_resubmit *nar, char *arg)
                   " on resubmit");
     }
 
-    nar->vendor = htonl(NX_VENDOR_ID);
-    nar->in_port = htons(in_port);
     if (in_port != OFPP_IN_PORT && table == 255) {
-        nar->subtype = htons(NXAST_RESUBMIT);
+        nar = ofputil_put_NXAST_RESUBMIT(b);
     } else {
-        nar->subtype = htons(NXAST_RESUBMIT_TABLE);
+        nar = ofputil_put_NXAST_RESUBMIT_TABLE(b);
         nar->table = table;
     }
+    nar->in_port = htons(in_port);
 }
 
 static void
-parse_set_tunnel(struct ofpbuf *b, enum ofputil_action_code code,
-                 const char *arg)
+parse_set_tunnel(struct ofpbuf *b, const char *arg)
 {
     uint64_t tun_id = str_to_u64(arg);
-    if (code == OFPUTIL_NXAST_SET_TUNNEL64 || tun_id > UINT32_MAX) {
-        struct nx_action_set_tunnel64 *nast64;
-        nast64 = put_action(b, sizeof *nast64, OFPAT_VENDOR);
-        nast64->vendor = htonl(NX_VENDOR_ID);
-        nast64->subtype = htons(NXAST_SET_TUNNEL64);
-        nast64->tun_id = htonll(tun_id);
+    if (tun_id > UINT32_MAX) {
+        ofputil_put_NXAST_SET_TUNNEL64(b)->tun_id = htonll(tun_id);
     } else {
-        struct nx_action_set_tunnel *nast;
-        nast = put_action(b, sizeof *nast, OFPAT_VENDOR);
-        nast->vendor = htonl(NX_VENDOR_ID);
-        nast->subtype = htons(NXAST_SET_TUNNEL);
-        nast->tun_id = htonl(tun_id);
+        ofputil_put_NXAST_SET_TUNNEL(b)->tun_id = htonl(tun_id);
     }
 }
 
@@ -377,9 +352,7 @@ parse_note(struct ofpbuf *b, const char *arg)
     int remainder;
     size_t len;
 
-    nan = put_action(b, sizeof *nan, OFPAT_VENDOR);
-    nan->vendor = htonl(NX_VENDOR_ID);
-    nan->subtype = htons(NXAST_NOTE);
+    nan = ofputil_put_NXAST_NOTE(b);
 
     b->size -= sizeof nan->note;
     while (*arg != '\0') {
@@ -414,18 +387,11 @@ parse_note(struct ofpbuf *b, const char *arg)
 static void
 parse_named_action(enum ofputil_action_code code, struct ofpbuf *b, char *arg)
 {
+    struct ofp_action_dl_addr *oada;
     struct ofp_action_vlan_pcp *oavp;
     struct ofp_action_vlan_vid *oavv;
     struct ofp_action_nw_addr *oana;
-    struct ofp_action_nw_tos *oant;
     struct ofp_action_tp_port *oata;
-    struct nx_action_header *nah;
-    struct nx_action_resubmit *nar;
-    struct nx_action_set_queue *nasq;
-    struct nx_action_reg_move *move;
-    struct nx_action_reg_load *load;
-    struct nx_action_multipath *nam;
-    struct nx_action_autopath *naa;
 
     switch (code) {
     case OFPUTIL_OFPAT_OUTPUT:
@@ -433,49 +399,38 @@ parse_named_action(enum ofputil_action_code code, struct ofpbuf *b, char *arg)
         break;
 
     case OFPUTIL_OFPAT_SET_VLAN_VID:
-        oavv = put_action(b, sizeof *oavv, OFPAT_SET_VLAN_VID);
+        oavv = ofputil_put_OFPAT_SET_VLAN_VID(b);
         oavv->vlan_vid = htons(str_to_u32(arg));
         break;
 
     case OFPUTIL_OFPAT_SET_VLAN_PCP:
-        oavp = put_action(b, sizeof *oavp, OFPAT_SET_VLAN_PCP);
+        oavp = ofputil_put_OFPAT_SET_VLAN_PCP(b);
         oavp->vlan_pcp = str_to_u32(arg);
         break;
 
     case OFPUTIL_OFPAT_STRIP_VLAN:
-        put_action(b, sizeof(struct ofp_action_header), OFPAT_STRIP_VLAN);
+        ofputil_put_OFPAT_STRIP_VLAN(b);
         break;
 
     case OFPUTIL_OFPAT_SET_DL_SRC:
-        put_dl_addr_action(b, OFPAT_SET_DL_SRC, arg);
-        break;
-
     case OFPUTIL_OFPAT_SET_DL_DST:
-        put_dl_addr_action(b, OFPAT_SET_DL_DST, arg);
+        oada = ofputil_put_action(code, b);
+        str_to_mac(arg, oada->dl_addr);
         break;
 
     case OFPUTIL_OFPAT_SET_NW_SRC:
-        oana = put_action(b, sizeof *oana, OFPAT_SET_NW_SRC);
-        str_to_ip(arg, &oana->nw_addr, NULL);
-        break;
-
     case OFPUTIL_OFPAT_SET_NW_DST:
-        oana = put_action(b, sizeof *oana, OFPAT_SET_NW_DST);
+        oana = ofputil_put_action(code, b);
         str_to_ip(arg, &oana->nw_addr, NULL);
         break;
 
     case OFPUTIL_OFPAT_SET_NW_TOS:
-        oant = put_action(b, sizeof *oant, OFPAT_SET_NW_TOS);
-        oant->nw_tos = str_to_u32(arg);
+        ofputil_put_OFPAT_SET_NW_TOS(b)->nw_tos = str_to_u32(arg);
         break;
 
     case OFPUTIL_OFPAT_SET_TP_SRC:
-        oata = put_action(b, sizeof *oata, OFPAT_SET_TP_SRC);
-        oata->tp_port = htons(str_to_u32(arg));
-        break;
-
     case OFPUTIL_OFPAT_SET_TP_DST:
-        oata = put_action(b, sizeof *oata, OFPAT_SET_TP_DST);
+        oata = ofputil_put_action(code, b);
         oata->tp_port = htons(str_to_u32(arg));
         break;
 
@@ -484,50 +439,43 @@ parse_named_action(enum ofputil_action_code code, struct ofpbuf *b, char *arg)
         break;
 
     case OFPUTIL_NXAST_RESUBMIT:
-        nar = put_action(b, sizeof *nar, OFPAT_VENDOR);
-        parse_resubmit(nar, arg);
+        parse_resubmit(b, arg);
         break;
 
     case OFPUTIL_NXAST_SET_TUNNEL:
-    case OFPUTIL_NXAST_SET_TUNNEL64:
-        parse_set_tunnel(b, code, arg);
+        parse_set_tunnel(b, arg);
         break;
 
     case OFPUTIL_NXAST_SET_QUEUE:
-        nasq = put_action(b, sizeof *nasq, OFPAT_VENDOR);
-        nasq->vendor = htonl(NX_VENDOR_ID);
-        nasq->subtype = htons(NXAST_SET_QUEUE);
-        nasq->queue_id = htonl(str_to_u32(arg));
+        ofputil_put_NXAST_SET_QUEUE(b)->queue_id = htonl(str_to_u32(arg));
         break;
 
     case OFPUTIL_NXAST_POP_QUEUE:
-        nah = put_action(b, sizeof *nah, OFPAT_VENDOR);
-        nah->vendor = htonl(NX_VENDOR_ID);
-        nah->subtype = htons(NXAST_POP_QUEUE);
+        ofputil_put_NXAST_POP_QUEUE(b);
         break;
 
     case OFPUTIL_NXAST_REG_MOVE:
-        move = ofpbuf_put_uninit(b, sizeof *move);
-        nxm_parse_reg_move(move, arg);
+        nxm_parse_reg_move(ofputil_put_NXAST_REG_MOVE(b), arg);
         break;
 
     case OFPUTIL_NXAST_REG_LOAD:
-        load = ofpbuf_put_uninit(b, sizeof *load);
-        nxm_parse_reg_load(load, arg);
+        nxm_parse_reg_load(ofputil_put_NXAST_REG_LOAD(b), arg);
         break;
 
     case OFPUTIL_NXAST_NOTE:
         parse_note(b, arg);
         break;
 
+    case OFPUTIL_NXAST_SET_TUNNEL64:
+        ofputil_put_NXAST_SET_TUNNEL64(b)->tun_id = htonll(str_to_u64(arg));
+        break;
+
     case OFPUTIL_NXAST_MULTIPATH:
-        nam = ofpbuf_put_uninit(b, sizeof *nam);
-        multipath_parse(nam, arg);
+        multipath_parse(ofputil_put_NXAST_MULTIPATH(b), arg);
         break;
 
     case OFPUTIL_NXAST_AUTOPATH:
-        naa = ofpbuf_put_uninit(b, sizeof *naa);
-        autopath_parse(naa, arg);
+        autopath_parse(ofputil_put_NXAST_AUTOPATH(b), arg);
         break;
 
     case OFPUTIL_NXAST_BUNDLE:
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6bee2da..5cc1d40 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1825,10 +1825,7 @@ make_add_simple_flow(const struct cls_rule *rule,
         struct ofpbuf *buffer;
 
         buffer = make_add_flow(rule, buffer_id, idle_timeout, sizeof *oao);
-        oao = ofpbuf_put_zeros(buffer, sizeof *oao);
-        oao->type = htons(OFPAT_OUTPUT);
-        oao->len = htons(sizeof *oao);
-        oao->port = htons(out_port);
+        ofputil_put_OFPAT_OUTPUT(buffer)->port = htons(out_port);
         return buffer;
     } else {
         return make_add_flow(rule, buffer_id, idle_timeout, 0);
@@ -2308,6 +2305,60 @@ ofputil_action_code_from_name(const char *name)
     return -1;
 }
 
+/* Appends an action of the type specified by 'code' to 'buf' and returns the
+ * action.  Initializes the parts of 'action' that identify it as having type
+ * <ENUM> and length 'sizeof *action' and zeros the rest.  For actions that
+ * have variable length, the length used and cleared is that of struct
+ * <STRUCT>.  */
+void *
+ofputil_put_action(enum ofputil_action_code code, struct ofpbuf *buf)
+{
+    switch (code) {
+#define OFPAT_ACTION(ENUM, STRUCT, NAME)                    \
+    case OFPUTIL_##ENUM: return ofputil_put_##ENUM(buf);
+#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)        \
+    case OFPUTIL_##ENUM: return ofputil_put_##ENUM(buf);
+#include "ofp-util.def"
+    }
+    NOT_REACHED();
+}
+
+#define OFPAT_ACTION(ENUM, STRUCT, NAME)                        \
+    void                                                        \
+    ofputil_init_##ENUM(struct STRUCT *s)                       \
+    {                                                           \
+        memset(s, 0, sizeof *s);                                \
+        s->type = htons(ENUM);                                  \
+        s->len = htons(sizeof *s);                              \
+    }                                                           \
+                                                                \
+    struct STRUCT *                                             \
+    ofputil_put_##ENUM(struct ofpbuf *buf)                      \
+    {                                                           \
+        struct STRUCT *s = ofpbuf_put_uninit(buf, sizeof *s);   \
+        ofputil_init_##ENUM(s);                                 \
+        return s;                                               \
+    }
+#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)            \
+    void                                                        \
+    ofputil_init_##ENUM(struct STRUCT *s)                       \
+    {                                                           \
+        memset(s, 0, sizeof *s);                                \
+        s->type = htons(OFPAT_VENDOR);                          \
+        s->len = htons(sizeof *s);                              \
+        s->vendor = htonl(NX_VENDOR_ID);                        \
+        s->subtype = htons(ENUM);                               \
+    }                                                           \
+                                                                \
+    struct STRUCT *                                             \
+    ofputil_put_##ENUM(struct ofpbuf *buf)                      \
+    {                                                           \
+        struct STRUCT *s = ofpbuf_put_uninit(buf, sizeof *s);   \
+        ofputil_init_##ENUM(s);                                 \
+        return s;                                               \
+    }
+#include "ofp-util.def"
+
 /* Returns true if 'action' outputs to 'port', false otherwise. */
 bool
 action_outputs_to_port(const union ofp_action *action, ovs_be16 port)
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index e70d963..a658df1 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -334,6 +334,30 @@ enum ofputil_action_code ofputil_decode_action_unsafe(
 
 int ofputil_action_code_from_name(const char *);
 
+void *ofputil_put_action(enum ofputil_action_code, struct ofpbuf *buf);
+
+/* For each OpenFlow action <ENUM> that has a corresponding action structure
+ * struct <STRUCT>, this defines two functions:
+ *
+ *   void ofputil_init_<ENUM>(struct <STRUCT> *action);
+ *
+ *     Initializes the parts of 'action' that identify it as having type <ENUM>
+ *     and length 'sizeof *action' and zeros the rest.  For actions that have
+ *     variable length, the length used and cleared is that of struct <STRUCT>.
+ *
+ *  struct <STRUCT> *ofputil_put_<ENUM>(struct ofpbuf *buf);
+ *
+ *     Appends a new 'action', of length 'sizeof(struct <STRUCT>)', to 'buf',
+ *     initializes it with ofputil_init_<ENUM>(), and returns it.
+ */
+#define OFPAT_ACTION(ENUM, STRUCT, NAME)                \
+    void ofputil_init_##ENUM(struct STRUCT *);          \
+    struct STRUCT *ofputil_put_##ENUM(struct ofpbuf *);
+#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)    \
+    void ofputil_init_##ENUM(struct STRUCT *);          \
+    struct STRUCT *ofputil_put_##ENUM(struct ofpbuf *);
+#include "ofp-util.def"
+
 #define OFP_ACTION_ALIGN 8      /* Alignment of ofp_actions. */
 
 static inline union ofp_action *
-- 
1.7.4.4




More information about the dev mailing list