From 93996add1ca50e85e4b3938133fcac15b56fbf01 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 16 Aug 2011 16:30:57 -0700 Subject: [PATCH] ofp-util: Add type-safe functions for serializing actions. --- 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 b42826c14..9a39c6a1f 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 86762f92a..ac0badef6 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 e85829a06..f68dafdc4 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 2654dde07..422ce761a 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 0fe4ac30f..b29b34a79 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,18 +275,11 @@ 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) { @@ -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 00d9df496..c8589c678 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 + * and length 'sizeof *action' and zeros the rest. For actions that + * have variable length, the length used and cleared is that of 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 e70d963d5..a658df113 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 that has a corresponding action structure + * struct , this defines two functions: + * + * void ofputil_init_(struct *action); + * + * Initializes the parts of 'action' that identify it as having type + * and length 'sizeof *action' and zeros the rest. For actions that have + * variable length, the length used and cleared is that of struct . + * + * struct *ofputil_put_(struct ofpbuf *buf); + * + * Appends a new 'action', of length 'sizeof(struct )', to 'buf', + * initializes it with ofputil_init_(), 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 * -- 2.43.0