Nifty patch. This has always been a pain point when implementing new
actions. Glad to see it all generated now.
Ethan
On Tue, Aug 16, 2011 at 16:29, Ben Pfaff <b...@nicira.com> wrote:
> ---
> 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..3c9b459 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
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
