Looks good,
I would be inclined to s/false/0 and s/true/1 in ofp-util.def. That
would allow us to align it against NXAST_RESUBMIT_TABLE without going
over 79 characters. Doesn't really matter though.
Ethan
On Tue, Aug 16, 2011 at 16:29, Ben Pfaff <b...@nicira.com> wrote:
> This commit primarily moves the OFPAT_ACTION and NXAST_ACTION invocations
> into a new file ofp-util.def. This allows multiple places in the source to
> use them.
>
> This commit also adds a new function ofputil_action_code_from_name().
> The following commit will add the first user.
> ---
> lib/automake.mk | 1 +
> lib/ofp-util.c | 76
> +++++++++++++++++++++++++++---------------------------
> lib/ofp-util.def | 37 ++++++++++++++++++++++++++
> lib/ofp-util.h | 76 +++++++++++++++++++++++++++++++++--------------------
> 4 files changed, 123 insertions(+), 67 deletions(-)
> create mode 100644 lib/ofp-util.def
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index d55465b..1fa2b46 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -95,6 +95,7 @@ lib_libopenvswitch_a_SOURCES = \
> lib/ofp-print.c \
> lib/ofp-print.h \
> lib/ofp-util.c \
> + lib/ofp-util.def \
> lib/ofp-util.h \
> lib/ofpbuf.c \
> lib/ofpbuf.h \
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index d6c0602..6bee2da 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -2187,26 +2187,16 @@ ofputil_decode_ofpat_action(const union ofp_action *a)
> enum ofp_action_type type = ntohs(a->type);
>
> switch (type) {
> -#define OFPAT_ACTION(ENUM, TYPE) \
> +#define OFPAT_ACTION(ENUM, STRUCT, NAME) \
> case ENUM: { \
> static const struct ofputil_action action = { \
> - OFPUTIL_##ENUM, sizeof(TYPE), sizeof(TYPE) \
> + OFPUTIL_##ENUM, \
> + sizeof(struct STRUCT), \
> + sizeof(struct STRUCT) \
> }; \
> return &action; \
> }
> - OFPAT_ACTION(OFPAT_OUTPUT, struct ofp_action_output);
> - OFPAT_ACTION(OFPAT_SET_VLAN_VID, struct ofp_action_vlan_vid);
> - OFPAT_ACTION(OFPAT_SET_VLAN_PCP, struct ofp_action_vlan_pcp);
> - OFPAT_ACTION(OFPAT_STRIP_VLAN, struct ofp_action_header);
> - OFPAT_ACTION(OFPAT_SET_DL_SRC, struct ofp_action_dl_addr);
> - OFPAT_ACTION(OFPAT_SET_DL_DST, struct ofp_action_dl_addr);
> - OFPAT_ACTION(OFPAT_SET_NW_SRC, struct ofp_action_nw_addr);
> - OFPAT_ACTION(OFPAT_SET_NW_DST, struct ofp_action_nw_addr);
> - OFPAT_ACTION(OFPAT_SET_NW_TOS, struct ofp_action_nw_tos);
> - OFPAT_ACTION(OFPAT_SET_TP_SRC, struct ofp_action_tp_port);
> - OFPAT_ACTION(OFPAT_SET_TP_DST, struct ofp_action_tp_port);
> - OFPAT_ACTION(OFPAT_ENQUEUE, struct ofp_action_enqueue);
> -#undef OFPAT_ACTION
> +#include "ofp-util.def"
>
> case OFPAT_VENDOR:
> default:
> @@ -2221,30 +2211,16 @@ ofputil_decode_nxast_action(const union ofp_action *a)
> enum nx_action_subtype subtype = ntohs(nah->subtype);
>
> switch (subtype) {
> -#define NXAST_ACTION(ENUM, TYPE, EXTENSIBLE) \
> - case ENUM: { \
> - static const struct ofputil_action action = { \
> - OFPUTIL_##ENUM, \
> - sizeof(TYPE), \
> - EXTENSIBLE ? UINT_MAX : sizeof(TYPE) \
> - }; \
> - return &action; \
> +#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) \
> + case ENUM: { \
> + static const struct ofputil_action action = { \
> + OFPUTIL_##ENUM, \
> + sizeof(struct STRUCT), \
> + EXTENSIBLE ? UINT_MAX : sizeof(struct STRUCT) \
> + }; \
> + return &action; \
> }
> - NXAST_ACTION(NXAST_RESUBMIT, struct nx_action_resubmit,
> false);
> - NXAST_ACTION(NXAST_SET_TUNNEL, struct nx_action_set_tunnel,
> false);
> - NXAST_ACTION(NXAST_SET_QUEUE, struct nx_action_set_queue,
> false);
> - NXAST_ACTION(NXAST_POP_QUEUE, struct nx_action_pop_queue,
> false);
> - NXAST_ACTION(NXAST_REG_MOVE, struct nx_action_reg_move,
> false);
> - NXAST_ACTION(NXAST_REG_LOAD, struct nx_action_reg_load,
> false);
> - NXAST_ACTION(NXAST_NOTE, struct nx_action_note,
> true);
> - NXAST_ACTION(NXAST_SET_TUNNEL64, struct nx_action_set_tunnel64,
> false);
> - NXAST_ACTION(NXAST_MULTIPATH, struct nx_action_multipath,
> false);
> - NXAST_ACTION(NXAST_AUTOPATH, struct nx_action_autopath,
> false);
> - NXAST_ACTION(NXAST_BUNDLE, struct nx_action_bundle,
> true);
> - NXAST_ACTION(NXAST_BUNDLE_LOAD, struct nx_action_bundle,
> true);
> - NXAST_ACTION(NXAST_RESUBMIT_TABLE, struct nx_action_resubmit,
> false);
> - NXAST_ACTION(NXAST_OUTPUT_REG, struct nx_action_output_reg,
> false);
> -#undef NXAST_ACTION
> +#include "ofp-util.def"
>
> case NXAST_SNAT__OBSOLETE:
> case NXAST_DROP_SPOOFED_ARP__OBSOLETE:
> @@ -2308,6 +2284,30 @@ ofputil_decode_action_unsafe(const union ofp_action *a)
> return action->code;
> }
>
> +/* Returns the 'enum ofputil_action_code' corresponding to 'name' (e.g. if
> + * 'name' is "output" then the return value is OFPUTIL_OFPAT_OUTPUT), or -1
> if
> + * 'name' is not the name of any action.
> + *
> + * ofp-util.def lists the mapping from names to action. */
> +int
> +ofputil_action_code_from_name(const char *name)
> +{
> + static const char *names[OFPUTIL_N_ACTIONS] = {
> +#define OFPAT_ACTION(ENUM, STRUCT, NAME) NAME,
> +#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) NAME,
> +#include "ofp-util.def"
> + };
> +
> + const char **p;
> +
> + for (p = names; p < &names[ARRAY_SIZE(names)]; p++) {
> + if (*p && !strcasecmp(name, *p)) {
> + return p - names;
> + }
> + }
> + return -1;
> +}
> +
> /* 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.def b/lib/ofp-util.def
> new file mode 100644
> index 0000000..5f726d5
> --- /dev/null
> +++ b/lib/ofp-util.def
> @@ -0,0 +1,37 @@
> +/* -*- c -*- */
> +
> +#ifndef OFPAT_ACTION
> +#define OFPAT_ACTION(ENUM, STRUCT, NAME)
> +#endif
> +OFPAT_ACTION(OFPAT_OUTPUT, ofp_action_output, "output")
> +OFPAT_ACTION(OFPAT_SET_VLAN_VID, ofp_action_vlan_vid, "mod_vlan_vid")
> +OFPAT_ACTION(OFPAT_SET_VLAN_PCP, ofp_action_vlan_pcp, "mod_vlan_pcp")
> +OFPAT_ACTION(OFPAT_STRIP_VLAN, ofp_action_header, "strip_vlan")
> +OFPAT_ACTION(OFPAT_SET_DL_SRC, ofp_action_dl_addr, "mod_dl_src")
> +OFPAT_ACTION(OFPAT_SET_DL_DST, ofp_action_dl_addr, "mod_dl_dst")
> +OFPAT_ACTION(OFPAT_SET_NW_SRC, ofp_action_nw_addr, "mod_nw_src")
> +OFPAT_ACTION(OFPAT_SET_NW_DST, ofp_action_nw_addr, "mod_nw_dst")
> +OFPAT_ACTION(OFPAT_SET_NW_TOS, ofp_action_nw_tos, "mod_nw_tos")
> +OFPAT_ACTION(OFPAT_SET_TP_SRC, ofp_action_tp_port, "mod_tp_src")
> +OFPAT_ACTION(OFPAT_SET_TP_DST, ofp_action_tp_port, "mod_tp_dst")
> +OFPAT_ACTION(OFPAT_ENQUEUE, ofp_action_enqueue, "enqueue")
> +#undef OFPAT_ACTION
> +
> +#ifndef NXAST_ACTION
> +#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME)
> +#endif
> +NXAST_ACTION(NXAST_RESUBMIT, nx_action_resubmit, false, "resubmit")
> +NXAST_ACTION(NXAST_SET_TUNNEL, nx_action_set_tunnel, false, "set_tunnel")
> +NXAST_ACTION(NXAST_SET_QUEUE, nx_action_set_queue, false, "set_queue")
> +NXAST_ACTION(NXAST_POP_QUEUE, nx_action_pop_queue, false, "pop_queue")
> +NXAST_ACTION(NXAST_REG_MOVE, nx_action_reg_move, false, "move")
> +NXAST_ACTION(NXAST_REG_LOAD, nx_action_reg_load, false, "load")
> +NXAST_ACTION(NXAST_NOTE, nx_action_note, true, "note")
> +NXAST_ACTION(NXAST_SET_TUNNEL64, nx_action_set_tunnel64, false,
> "set_tunnel64")
> +NXAST_ACTION(NXAST_MULTIPATH, nx_action_multipath, false, "multipath")
> +NXAST_ACTION(NXAST_AUTOPATH, nx_action_autopath, false, "autopath")
> +NXAST_ACTION(NXAST_BUNDLE, nx_action_bundle, true, "bundle")
> +NXAST_ACTION(NXAST_BUNDLE_LOAD, nx_action_bundle, true,
> "bundle_load")
> +NXAST_ACTION(NXAST_RESUBMIT_TABLE, nx_action_resubmit, false, NULL)
> +NXAST_ACTION(NXAST_OUTPUT_REG, nx_action_output_reg, false, NULL)
> +#undef NXAST_ACTION
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index e3640db..e70d963 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -280,42 +280,60 @@ struct ofpbuf *make_echo_reply(const struct ofp_header
> *rq);
>
> /* Actions. */
>
> +/* The type of an action.
> + *
> + * For each implemented OFPAT_* and NXAST_* action type, there is a
> + * corresponding constant prefixed with OFPUTIL_, e.g.:
> + *
> + * OFPUTIL_OFPAT_OUTPUT
> + * OFPUTIL_OFPAT_SET_VLAN_VID
> + * OFPUTIL_OFPAT_SET_VLAN_PCP
> + * OFPUTIL_OFPAT_STRIP_VLAN
> + * OFPUTIL_OFPAT_SET_DL_SRC
> + * OFPUTIL_OFPAT_SET_DL_DST
> + * OFPUTIL_OFPAT_SET_NW_SRC
> + * OFPUTIL_OFPAT_SET_NW_DST
> + * OFPUTIL_OFPAT_SET_NW_TOS
> + * OFPUTIL_OFPAT_SET_TP_SRC
> + * OFPUTIL_OFPAT_SET_TP_DST
> + * OFPUTIL_OFPAT_ENQUEUE
> + * OFPUTIL_NXAST_RESUBMIT
> + * OFPUTIL_NXAST_SET_TUNNEL
> + * OFPUTIL_NXAST_SET_QUEUE
> + * OFPUTIL_NXAST_POP_QUEUE
> + * OFPUTIL_NXAST_REG_MOVE
> + * OFPUTIL_NXAST_REG_LOAD
> + * OFPUTIL_NXAST_NOTE
> + * OFPUTIL_NXAST_SET_TUNNEL64
> + * OFPUTIL_NXAST_MULTIPATH
> + * OFPUTIL_NXAST_AUTOPATH
> + * OFPUTIL_NXAST_BUNDLE
> + * OFPUTIL_NXAST_BUNDLE_LOAD
> + * OFPUTIL_NXAST_RESUBMIT_TABLE
> + * OFPUTIL_NXAST_OUTPUT_REG
> + *
> + * (The above list helps developers who want to "grep" for these
> definitions.)
> + */
> enum ofputil_action_code {
> - /* OFPAT_* actions. */
> - OFPUTIL_OFPAT_OUTPUT,
> - OFPUTIL_OFPAT_SET_VLAN_VID,
> - OFPUTIL_OFPAT_SET_VLAN_PCP,
> - OFPUTIL_OFPAT_STRIP_VLAN,
> - OFPUTIL_OFPAT_SET_DL_SRC,
> - OFPUTIL_OFPAT_SET_DL_DST,
> - OFPUTIL_OFPAT_SET_NW_SRC,
> - OFPUTIL_OFPAT_SET_NW_DST,
> - OFPUTIL_OFPAT_SET_NW_TOS,
> - OFPUTIL_OFPAT_SET_TP_SRC,
> - OFPUTIL_OFPAT_SET_TP_DST,
> - OFPUTIL_OFPAT_ENQUEUE,
> -
> - /* NXAST_* actions. */
> - OFPUTIL_NXAST_RESUBMIT,
> - OFPUTIL_NXAST_SET_TUNNEL,
> - OFPUTIL_NXAST_SET_QUEUE,
> - OFPUTIL_NXAST_POP_QUEUE,
> - OFPUTIL_NXAST_REG_MOVE,
> - OFPUTIL_NXAST_REG_LOAD,
> - OFPUTIL_NXAST_NOTE,
> - OFPUTIL_NXAST_SET_TUNNEL64,
> - OFPUTIL_NXAST_MULTIPATH,
> - OFPUTIL_NXAST_AUTOPATH,
> - OFPUTIL_NXAST_BUNDLE,
> - OFPUTIL_NXAST_BUNDLE_LOAD,
> - OFPUTIL_NXAST_RESUBMIT_TABLE,
> - OFPUTIL_NXAST_OUTPUT_REG
> +#define OFPAT_ACTION(ENUM, STRUCT, NAME) OFPUTIL_##ENUM,
> +#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) OFPUTIL_##ENUM,
> +#include "ofp-util.def"
> +};
> +
> +/* The number of values of "enum ofputil_action_code". */
> +enum {
> +#define OFPAT_ACTION(ENUM, STRUCT, NAME) + 1
> +#define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) + 1
> + OFPUTIL_N_ACTIONS = 0
> +#include "ofp-util.def"
> };
>
> int ofputil_decode_action(const union ofp_action *);
> enum ofputil_action_code ofputil_decode_action_unsafe(
> const union ofp_action *);
>
> +int ofputil_action_code_from_name(const char *);
> +
> #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
