Looks good,

Ethan

On Tue, Aug 16, 2011 at 16:29, Ben Pfaff <b...@nicira.com> wrote:
> These functions were previously used only in ofp-parse.c and ofp-print.c,
> but they are more generally useful and future commits will add more users.
> ---
>  lib/ofp-parse.c |   39 ++-------------------------------
>  lib/ofp-print.c |   57 ++++++++-----------------------------------------
>  lib/ofp-util.c  |   62 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/ofp-util.h  |    4 +++
>  4 files changed, 79 insertions(+), 83 deletions(-)
>
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index e352bd4..b1afd4b 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -285,37 +285,6 @@ put_dl_addr_action(struct ofpbuf *b, uint16_t type, 
> const char *addr)
>     str_to_mac(addr, oada->dl_addr);
>  }
>
> -static bool
> -parse_port_name(const char *name, uint16_t *port)
> -{
> -    struct pair {
> -        const char *name;
> -        uint16_t value;
> -    };
> -    static const struct pair pairs[] = {
> -#define DEF_PAIR(NAME) {#NAME, OFPP_##NAME}
> -        DEF_PAIR(IN_PORT),
> -        DEF_PAIR(TABLE),
> -        DEF_PAIR(NORMAL),
> -        DEF_PAIR(FLOOD),
> -        DEF_PAIR(ALL),
> -        DEF_PAIR(CONTROLLER),
> -        DEF_PAIR(LOCAL),
> -        DEF_PAIR(NONE),
> -#undef DEF_PAIR
> -    };
> -    static const int n_pairs = ARRAY_SIZE(pairs);
> -    size_t i;
> -
> -    for (i = 0; i < n_pairs; i++) {
> -        if (!strcasecmp(name, pairs[i].name)) {
> -            *port = pairs[i].value;
> -            return true;
> -        }
> -    }
> -    return false;
> -}
> -
>  static void
>  parse_output(struct ofpbuf *b, char *arg)
>  {
> @@ -346,7 +315,7 @@ parse_resubmit(struct nx_action_resubmit *nar, char *arg)
>
>     in_port_s = strsep(&arg, ",");
>     if (in_port_s && in_port_s[0]) {
> -        if (!parse_port_name(in_port_s, &in_port)) {
> +        if (!ofputil_port_from_string(in_port_s, &in_port)) {
>             in_port = str_to_u32(in_port_s);
>         }
>     } else {
> @@ -590,10 +559,8 @@ str_to_action(char *str, struct ofpbuf *b)
>             } else {
>                 oao->max_len = htons(UINT16_MAX);
>             }
> -        } else if (parse_port_name(act, &port)) {
> +        } else if (ofputil_port_from_string(act, &port)) {
>             put_output_action(b, port);
> -        } else if (strspn(act, "0123456789") == strlen(act)) {
> -            put_output_action(b, str_to_u32(act));
>         } else {
>             ovs_fatal(0, "Unknown action: %s", act);
>         }
> @@ -724,7 +691,7 @@ parse_field_value(struct cls_rule *rule, enum field_index 
> index,
>         break;
>
>     case F_IN_PORT:
> -        if (!parse_port_name(value, &port_no)) {
> +        if (!ofputil_port_from_string(value, &port_no)) {
>             port_no = atoi(value);
>         }
>         cls_rule_set_in_port(rule, port_no);
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index d7804fe..3b9c582 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -43,7 +43,6 @@
>  #include "unaligned.h"
>  #include "util.h"
>
> -static void ofp_print_port_name(struct ds *string, uint16_t port);
>  static void ofp_print_queue_name(struct ds *string, uint32_t port);
>  static void ofp_print_error(struct ds *, int error);
>
> @@ -115,7 +114,7 @@ ofp_print_packet_in(struct ds *string, const struct 
> ofp_packet_in *op,
>
>     ds_put_format(string, " total_len=%"PRIu16" in_port=",
>                   ntohs(op->total_len));
> -    ofp_print_port_name(string, ntohs(op->in_port));
> +    ofputil_format_port(ntohs(op->in_port), string);
>
>     if (op->reason == OFPR_ACTION)
>         ds_put_cstr(string, " (via action)");
> @@ -152,42 +151,6 @@ ofp_print_packet_in(struct ds *string, const struct 
> ofp_packet_in *op,
>     }
>  }
>
> -static void ofp_print_port_name(struct ds *string, uint16_t port)
> -{
> -    const char *name;
> -    switch (port) {
> -    case OFPP_IN_PORT:
> -        name = "IN_PORT";
> -        break;
> -    case OFPP_TABLE:
> -        name = "TABLE";
> -        break;
> -    case OFPP_NORMAL:
> -        name = "NORMAL";
> -        break;
> -    case OFPP_FLOOD:
> -        name = "FLOOD";
> -        break;
> -    case OFPP_ALL:
> -        name = "ALL";
> -        break;
> -    case OFPP_CONTROLLER:
> -        name = "CONTROLLER";
> -        break;
> -    case OFPP_LOCAL:
> -        name = "LOCAL";
> -        break;
> -    case OFPP_NONE:
> -        name = "NONE";
> -        break;
> -    default:
> -        ds_put_format(string, "%"PRIu16, port);
> -        return;
> -    }
> -    ds_put_cstr(string, name);
> -}
> -
> -
>  static void
>  print_note(struct ds *string, const struct nx_action_note *nan)
>  {
> @@ -227,7 +190,7 @@ ofp_print_action(struct ds *s, const union ofp_action *a,
>         if (port < OFPP_MAX) {
>             ds_put_format(s, "output:%"PRIu16, port);
>         } else {
> -            ofp_print_port_name(s, port);
> +            ofputil_format_port(port, s);
>             if (port == OFPP_CONTROLLER) {
>                 if (a->output.max_len != htons(0)) {
>                     ds_put_format(s, ":%"PRIu16, ntohs(a->output.max_len));
> @@ -241,7 +204,7 @@ ofp_print_action(struct ds *s, const union ofp_action *a,
>     case OFPUTIL_OFPAT_ENQUEUE:
>         oae = (const struct ofp_action_enqueue *) a;
>         ds_put_format(s, "enqueue:");
> -        ofp_print_port_name(s, ntohs(oae->port));
> +        ofputil_format_port(ntohs(oae->port), s);
>         ds_put_format(s, "q%"PRIu32, ntohl(oae->queue_id));
>         break;
>
> @@ -293,14 +256,14 @@ ofp_print_action(struct ds *s, const union ofp_action 
> *a,
>     case OFPUTIL_NXAST_RESUBMIT:
>         nar = (struct nx_action_resubmit *)a;
>         ds_put_format(s, "resubmit:");
> -        ofp_print_port_name(s, ntohs(nar->in_port));
> +        ofputil_format_port(ntohs(nar->in_port), s);
>         break;
>
>     case OFPUTIL_NXAST_RESUBMIT_TABLE:
>         nar = (struct nx_action_resubmit *)a;
>         ds_put_format(s, "resubmit(");
>         if (nar->in_port != htons(OFPP_IN_PORT)) {
> -            ofp_print_port_name(s, ntohs(nar->in_port));
> +            ofputil_format_port(ntohs(nar->in_port), s);
>         }
>         ds_put_char(s, ',');
>         if (nar->table != 255) {
> @@ -412,7 +375,7 @@ ofp_print_packet_out(struct ds *string, const struct 
> ofp_packet_out *opo,
>     size_t actions_len = ntohs(opo->actions_len);
>
>     ds_put_cstr(string, " in_port=");
> -    ofp_print_port_name(string, ntohs(opo->in_port));
> +    ofputil_format_port(ntohs(opo->in_port), string);
>
>     ds_put_format(string, " actions_len=%zu ", actions_len);
>     if (actions_len > (ntohs(opo->header.length) - sizeof *opo)) {
> @@ -575,7 +538,7 @@ ofp_print_phy_port(struct ds *string, const struct 
> ofp_phy_port *port)
>     name[j] = '\0';
>
>     ds_put_char(string, ' ');
> -    ofp_print_port_name(string, ntohs(port->port_no));
> +    ofputil_format_port(ntohs(port->port_no), string);
>     ds_put_format(string, "(%s): addr:"ETH_ADDR_FMT"\n",
>                   name, ETH_ADDR_ARGS(port->hw_addr));
>
> @@ -1040,7 +1003,7 @@ ofp_print_flow_stats_request(struct ds *string,
>
>     if (fsr.out_port != OFPP_NONE) {
>         ds_put_cstr(string, " out_port=");
> -        ofp_print_port_name(string, fsr.out_port);
> +        ofputil_format_port(fsr.out_port, string);
>     }
>
>     /* A flow stats request doesn't include a priority, but cls_rule_format()
> @@ -1209,7 +1172,7 @@ ofp_print_ofpst_queue_request(struct ds *string,
>                               const struct ofp_queue_stats_request *qsr)
>  {
>     ds_put_cstr(string, "port=");
> -    ofp_print_port_name(string, ntohs(qsr->port_no));
> +    ofputil_format_port(ntohs(qsr->port_no), string);
>
>     ds_put_cstr(string, " queue=");
>     ofp_print_queue_name(string, ntohl(qsr->queue_id));
> @@ -1228,7 +1191,7 @@ ofp_print_ofpst_queue_reply(struct ds *string, const 
> struct ofp_header *oh,
>
>     for (; n--; qs++) {
>         ds_put_cstr(string, "  port ");
> -        ofp_print_port_name(string, ntohs(qs->port_no));
> +        ofputil_format_port(ntohs(qs->port_no), string);
>         ds_put_cstr(string, " queue ");
>         ofp_print_queue_name(string, ntohl(qs->queue_id));
>         ds_put_cstr(string, ": ");
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index cdb12e6..d6c0602 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -1960,6 +1960,68 @@ ofputil_check_output_port(uint16_t port, int max_ports)
>     }
>  }
>
> +#define OFPUTIL_NAMED_PORTS                     \
> +        OFPUTIL_NAMED_PORT(IN_PORT)             \
> +        OFPUTIL_NAMED_PORT(TABLE)               \
> +        OFPUTIL_NAMED_PORT(NORMAL)              \
> +        OFPUTIL_NAMED_PORT(FLOOD)               \
> +        OFPUTIL_NAMED_PORT(ALL)                 \
> +        OFPUTIL_NAMED_PORT(CONTROLLER)          \
> +        OFPUTIL_NAMED_PORT(LOCAL)               \
> +        OFPUTIL_NAMED_PORT(NONE)
> +
> +/* Checks whether 's' is the string representation of an OpenFlow port 
> number,
> + * either as an integer or a string name (e.g. "LOCAL").  If it is, stores 
> the
> + * number in '*port' and returns true.  Otherwise, returns false. */
> +bool
> +ofputil_port_from_string(const char *name, uint16_t *port)
> +{
> +    struct pair {
> +        const char *name;
> +        uint16_t value;
> +    };
> +    static const struct pair pairs[] = {
> +#define OFPUTIL_NAMED_PORT(NAME) {#NAME, OFPP_##NAME},
> +        OFPUTIL_NAMED_PORTS
> +#undef OFPUTIL_NAMED_PORT
> +    };
> +    static const int n_pairs = ARRAY_SIZE(pairs);
> +    int i;
> +
> +    if (str_to_int(name, 0, &i) && i >= 0 && i < UINT16_MAX) {
> +        *port = i;
> +        return true;
> +    }
> +
> +    for (i = 0; i < n_pairs; i++) {
> +        if (!strcasecmp(name, pairs[i].name)) {
> +            *port = pairs[i].value;
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +/* Appends to 's' a string representation of the OpenFlow port number 'port'.
> + * Most ports' string representation is just the port number, but for special
> + * ports, e.g. OFPP_LOCAL, it is the name, e.g. "LOCAL". */
> +void
> +ofputil_format_port(uint16_t port, struct ds *s)
> +{
> +    const char *name;
> +
> +    switch (port) {
> +#define OFPUTIL_NAMED_PORT(NAME) case OFPP_##NAME: name = #NAME; break;
> +        OFPUTIL_NAMED_PORTS
> +#undef OFPUTIL_NAMED_PORT
> +
> +    default:
> +        ds_put_format(s, "%"PRIu16, port);
> +        return;
> +    }
> +    ds_put_cstr(s, name);
> +}
> +
>  static int
>  check_resubmit_table(const struct nx_action_resubmit *nar)
>  {
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index b110d71..e3640db 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -92,7 +92,11 @@ int ofputil_decode_msg_type(const struct ofp_header *,
>                             const struct ofputil_msg_type **);
>  enum ofputil_msg_code ofputil_msg_type_code(const struct ofputil_msg_type *);
>  const char *ofputil_msg_type_name(const struct ofputil_msg_type *);
> +
> +/* Port numbers. */
>  int ofputil_check_output_port(uint16_t ofp_port, int max_ports);
> +bool ofputil_port_from_string(const char *, uint16_t *port);
> +void ofputil_format_port(uint16_t port, struct ds *);
>
>  /* Converting OFPFW_NW_SRC_MASK and OFPFW_NW_DST_MASK wildcard bit counts to
>  * and from IP bitmasks. */
> --
> 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

Reply via email to