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