[ovs-dev] [prelearning 3/5] ofp-util: Further abstract definitions of action properties.

Ethan Jackson ethan at nicira.com
Wed Aug 17 01:13:59 UTC 2011


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 <blp at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list