| Message ID | 20180112205722.25360-3-blp@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | support table names in CLI | expand |
Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> On Fri, Jan 12, 2018 at 12:57 PM, Ben Pfaff <blp@ovn.org> wrote: > This shares the infrastructure for mapping port names and numbers. It will > be used in an upcoming commit. > > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > include/openvswitch/ofp-util.h | 33 +++++++-- > lib/ofp-util.c | 150 ++++++++++++++++++++++++++++++ > ----------- > 2 files changed, 138 insertions(+), 45 deletions(-) > > diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp- > util.h > index 296078a2fe8b..d9780dd44582 100644 > --- a/include/openvswitch/ofp-util.h > +++ b/include/openvswitch/ofp-util.h > @@ -43,15 +43,24 @@ union ofp_action; > struct ofpact_set_field; > struct vl_mff_map; > > -/* Mapping between port numbers and names. */ > -struct ofputil_port_map { > +/* Name-number mapping. > + * > + * This is not exported directly but only through specializations for port > + * name-number and table name-number mappings. */ > +struct ofputil_name_map { > struct hmap by_name; > struct hmap by_number; > }; > - > -#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ > +#define OFPUTIL_NAME_MAP_INITIALIZER(MAP) \ > { HMAP_INITIALIZER(&(MAP)->by_name), HMAP_INITIALIZER(&(MAP)->by_number) > } > > +/* Mapping between port numbers and names. */ > +struct ofputil_port_map { > + struct ofputil_name_map map; > +}; > +#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ > + { OFPUTIL_NAME_MAP_INITIALIZER(&(MAP)->map) } > + > void ofputil_port_map_init(struct ofputil_port_map *); > const char *ofputil_port_map_get_name(const struct ofputil_port_map *, > ofp_port_t); > @@ -791,6 +800,22 @@ struct ofputil_table_mod_prop_vacancy { > uint8_t vacancy; /* Current vacancy (%). */ > }; > > +/* Mapping between table numbers and names. */ > +struct ofputil_table_map { > + struct ofputil_name_map map; > +}; > +#define OFPUTIL_TABLE_MAP_INITIALIZER(MAP) \ > + { OFPUTIL_NAME_MAP_INITIALIZER((MAP).map) } > + > +void ofputil_table_map_init(struct ofputil_table_map *); > +const char *ofputil_table_map_get_name(const struct ofputil_table_map *, > + uint8_t); > +uint8_t ofputil_table_map_get_number(const struct ofputil_table_map *, > + const char *name); > +void ofputil_table_map_put(struct ofputil_table_map *, > + uint8_t, const char *name); > +void ofputil_table_map_destroy(struct ofputil_table_map *); > + > /* Abstract ofp_table_mod. */ > struct ofputil_table_mod { > uint8_t table_id; /* ID of the table, 0xff indicates all > tables. */ > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index 597112e4f84e..f3b2e3f6108c 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -7454,12 +7454,14 @@ ofputil_port_to_string(ofp_port_t port, > snprintf(namebuf, bufsize, "%"PRIu32, port); > } > > -/* ofputil_port_map. */ > -struct ofputil_port_map_node { > +/* ofputil_name_map. */ > + > +struct ofputil_name_map_node { > struct hmap_node name_node; > struct hmap_node number_node; > - ofp_port_t ofp_port; /* Port number. */ > - char *name; /* Port name. */ > + > + uint32_t number; > + char *name; > > /* OpenFlow doesn't require port names to be unique, although that's > the > * only sensible way. However, even in Open vSwitch it's possible > for two > @@ -7469,22 +7471,25 @@ struct ofputil_port_map_node { > * corner case. > * > * OpenFlow does require port numbers to be unique. We check for > duplicate > - * ports numbers just in case a switch has a bug. */ > + * ports numbers just in case a switch has a bug. > + * > + * OpenFlow doesn't require table names to be unique and Open vSwitch > + * doesn't try to make them unique. */ > bool duplicate; > }; > > -void > -ofputil_port_map_init(struct ofputil_port_map *map) > +static void > +ofputil_name_map_init(struct ofputil_name_map *map) > { > hmap_init(&map->by_name); > hmap_init(&map->by_number); > } > > -static struct ofputil_port_map_node * > -ofputil_port_map_find_by_name(const struct ofputil_port_map *map, > +static struct ofputil_name_map_node * > +ofputil_name_map_find_by_name(const struct ofputil_name_map *map, > const char *name) > { > - struct ofputil_port_map_node *node; > + struct ofputil_name_map_node *node; > > HMAP_FOR_EACH_WITH_HASH (node, name_node, hash_string(name, 0), > &map->by_name) { > @@ -7495,38 +7500,38 @@ ofputil_port_map_find_by_name(const struct > ofputil_port_map *map, > return NULL; > } > > -static struct ofputil_port_map_node * > -ofputil_port_map_find_by_number(const struct ofputil_port_map *map, > - ofp_port_t ofp_port) > +static struct ofputil_name_map_node * > +ofputil_name_map_find_by_number(const struct ofputil_name_map *map, > + uint32_t number) > { > - struct ofputil_port_map_node *node; > + struct ofputil_name_map_node *node; > > - HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_ofp_port(ofp_port), > + HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_int(number, 0), > &map->by_number) { > - if (node->ofp_port == ofp_port) { > + if (node->number == number) { > return node; > } > } > return NULL; > } > > -void > -ofputil_port_map_put(struct ofputil_port_map *map, > - ofp_port_t ofp_port, const char *name) > +static void > +ofputil_name_map_put(struct ofputil_name_map *map, > + uint32_t number, const char *name) > { > - struct ofputil_port_map_node *node; > + struct ofputil_name_map_node *node; > > /* Look for duplicate name. */ > - node = ofputil_port_map_find_by_name(map, name); > + node = ofputil_name_map_find_by_name(map, name); > if (node) { > - if (node->ofp_port != ofp_port) { > + if (node->number != number) { > node->duplicate = true; > } > return; > } > > /* Look for duplicate number. */ > - node = ofputil_port_map_find_by_number(map, ofp_port); > + node = ofputil_name_map_find_by_number(map, number); > if (node) { > node->duplicate = true; > return; > @@ -7534,19 +7539,53 @@ ofputil_port_map_put(struct ofputil_port_map *map, > > /* Add new node. */ > node = xmalloc(sizeof *node); > - hmap_insert(&map->by_number, &node->number_node, > hash_ofp_port(ofp_port)); > + hmap_insert(&map->by_number, &node->number_node, hash_int(number, 0)); > hmap_insert(&map->by_name, &node->name_node, hash_string(name, 0)); > - node->ofp_port = ofp_port; > + node->number = number; > node->name = xstrdup(name); > node->duplicate = false; > } > > +static void > +ofputil_name_map_destroy(struct ofputil_name_map *map) > +{ > + if (map) { > + struct ofputil_name_map_node *node, *next; > + > + HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { > + hmap_remove(&map->by_name, &node->name_node); > + hmap_remove(&map->by_number, &node->number_node); > + free(node->name); > + free(node); > + } > + hmap_destroy(&map->by_name); > + hmap_destroy(&map->by_number); > + } > +} > + > +/* ofputil_port_map. */ > + > +void > +ofputil_port_map_init(struct ofputil_port_map *map) > +{ > + ofputil_name_map_init(&map->map); > +} > + > +void > +ofputil_port_map_put(struct ofputil_port_map *map, > + ofp_port_t ofp_port, const char *name) > +{ > + ofputil_name_map_put(&map->map, ofp_to_u16(ofp_port), name); > +} > + > const char * > ofputil_port_map_get_name(const struct ofputil_port_map *map, > ofp_port_t ofp_port) > { > - struct ofputil_port_map_node *node > - = map ? ofputil_port_map_find_by_number(map, ofp_port) : NULL; > + struct ofputil_name_map_node *node > + = (map > + ? ofputil_name_map_find_by_number(&map->map, > ofp_to_u16(ofp_port)) > + : NULL); > return node && !node->duplicate ? node->name : NULL; > } > > @@ -7554,26 +7593,55 @@ ofp_port_t > ofputil_port_map_get_number(const struct ofputil_port_map *map, > const char *name) > { > - struct ofputil_port_map_node *node > - = map ? ofputil_port_map_find_by_name(map, name) : NULL; > - return node && !node->duplicate ? node->ofp_port : OFPP_NONE; > + struct ofputil_name_map_node *node > + = map ? ofputil_name_map_find_by_name(&map->map, name) : NULL; > + return node && !node->duplicate ? u16_to_ofp(node->number) : > OFPP_NONE; > } > > void > ofputil_port_map_destroy(struct ofputil_port_map *map) > { > - if (map) { > - struct ofputil_port_map_node *node, *next; > + ofputil_name_map_destroy(&map->map); > +} > + > + > +/* ofputil_table_map. */ > > - HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { > - hmap_remove(&map->by_name, &node->name_node); > - hmap_remove(&map->by_number, &node->number_node); > - free(node->name); > - free(node); > - } > - hmap_destroy(&map->by_name); > - hmap_destroy(&map->by_number); > - } > +void > +ofputil_table_map_init(struct ofputil_table_map *map) > +{ > + ofputil_name_map_init(&map->map); > +} > + > +void > +ofputil_table_map_put(struct ofputil_table_map *map, > + uint8_t table_id, const char *name) > +{ > + ofputil_name_map_put(&map->map, table_id, name); > +} > + > +const char * > +ofputil_table_map_get_name(const struct ofputil_table_map *map, > + uint8_t table_id) > +{ > + struct ofputil_name_map_node *node > + = map ? ofputil_name_map_find_by_number(&map->map, table_id) : > NULL; > + return node && !node->duplicate ? node->name : NULL; > +} > + > +uint8_t > +ofputil_table_map_get_number(const struct ofputil_table_map *map, > + const char *name) > +{ > + struct ofputil_name_map_node *node > + = map ? ofputil_name_map_find_by_name(&map->map, name) : NULL; > + return node && !node->duplicate ? node->number : UINT8_MAX; > +} > + > +void > +ofputil_table_map_destroy(struct ofputil_table_map *map) > +{ > + ofputil_name_map_destroy(&map->map); > } > > /* Stores the group id represented by 's' into '*group_idp'. 's' may be > an > -- > 2.10.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thanks for the reviews of patches 1 and 2. Do you plan to review patch 3 as well? I rebased and sent out v3 just now. On Tue, Jan 30, 2018 at 01:29:56PM -0800, Yifeng Sun wrote: > Looks good to me, thanks. > > Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> > > > On Fri, Jan 12, 2018 at 12:57 PM, Ben Pfaff <blp@ovn.org> wrote: > > > This shares the infrastructure for mapping port names and numbers. It will > > be used in an upcoming commit. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > include/openvswitch/ofp-util.h | 33 +++++++-- > > lib/ofp-util.c | 150 ++++++++++++++++++++++++++++++ > > ----------- > > 2 files changed, 138 insertions(+), 45 deletions(-) > > > > diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp- > > util.h > > index 296078a2fe8b..d9780dd44582 100644 > > --- a/include/openvswitch/ofp-util.h > > +++ b/include/openvswitch/ofp-util.h > > @@ -43,15 +43,24 @@ union ofp_action; > > struct ofpact_set_field; > > struct vl_mff_map; > > > > -/* Mapping between port numbers and names. */ > > -struct ofputil_port_map { > > +/* Name-number mapping. > > + * > > + * This is not exported directly but only through specializations for port > > + * name-number and table name-number mappings. */ > > +struct ofputil_name_map { > > struct hmap by_name; > > struct hmap by_number; > > }; > > - > > -#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ > > +#define OFPUTIL_NAME_MAP_INITIALIZER(MAP) \ > > { HMAP_INITIALIZER(&(MAP)->by_name), HMAP_INITIALIZER(&(MAP)->by_number) > > } > > > > +/* Mapping between port numbers and names. */ > > +struct ofputil_port_map { > > + struct ofputil_name_map map; > > +}; > > +#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ > > + { OFPUTIL_NAME_MAP_INITIALIZER(&(MAP)->map) } > > + > > void ofputil_port_map_init(struct ofputil_port_map *); > > const char *ofputil_port_map_get_name(const struct ofputil_port_map *, > > ofp_port_t); > > @@ -791,6 +800,22 @@ struct ofputil_table_mod_prop_vacancy { > > uint8_t vacancy; /* Current vacancy (%). */ > > }; > > > > +/* Mapping between table numbers and names. */ > > +struct ofputil_table_map { > > + struct ofputil_name_map map; > > +}; > > +#define OFPUTIL_TABLE_MAP_INITIALIZER(MAP) \ > > + { OFPUTIL_NAME_MAP_INITIALIZER((MAP).map) } > > + > > +void ofputil_table_map_init(struct ofputil_table_map *); > > +const char *ofputil_table_map_get_name(const struct ofputil_table_map *, > > + uint8_t); > > +uint8_t ofputil_table_map_get_number(const struct ofputil_table_map *, > > + const char *name); > > +void ofputil_table_map_put(struct ofputil_table_map *, > > + uint8_t, const char *name); > > +void ofputil_table_map_destroy(struct ofputil_table_map *); > > + > > /* Abstract ofp_table_mod. */ > > struct ofputil_table_mod { > > uint8_t table_id; /* ID of the table, 0xff indicates all > > tables. */ > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > > index 597112e4f84e..f3b2e3f6108c 100644 > > --- a/lib/ofp-util.c > > +++ b/lib/ofp-util.c > > @@ -7454,12 +7454,14 @@ ofputil_port_to_string(ofp_port_t port, > > snprintf(namebuf, bufsize, "%"PRIu32, port); > > } > > > > -/* ofputil_port_map. */ > > -struct ofputil_port_map_node { > > +/* ofputil_name_map. */ > > + > > +struct ofputil_name_map_node { > > struct hmap_node name_node; > > struct hmap_node number_node; > > - ofp_port_t ofp_port; /* Port number. */ > > - char *name; /* Port name. */ > > + > > + uint32_t number; > > + char *name; > > > > /* OpenFlow doesn't require port names to be unique, although that's > > the > > * only sensible way. However, even in Open vSwitch it's possible > > for two > > @@ -7469,22 +7471,25 @@ struct ofputil_port_map_node { > > * corner case. > > * > > * OpenFlow does require port numbers to be unique. We check for > > duplicate > > - * ports numbers just in case a switch has a bug. */ > > + * ports numbers just in case a switch has a bug. > > + * > > + * OpenFlow doesn't require table names to be unique and Open vSwitch > > + * doesn't try to make them unique. */ > > bool duplicate; > > }; > > > > -void > > -ofputil_port_map_init(struct ofputil_port_map *map) > > +static void > > +ofputil_name_map_init(struct ofputil_name_map *map) > > { > > hmap_init(&map->by_name); > > hmap_init(&map->by_number); > > } > > > > -static struct ofputil_port_map_node * > > -ofputil_port_map_find_by_name(const struct ofputil_port_map *map, > > +static struct ofputil_name_map_node * > > +ofputil_name_map_find_by_name(const struct ofputil_name_map *map, > > const char *name) > > { > > - struct ofputil_port_map_node *node; > > + struct ofputil_name_map_node *node; > > > > HMAP_FOR_EACH_WITH_HASH (node, name_node, hash_string(name, 0), > > &map->by_name) { > > @@ -7495,38 +7500,38 @@ ofputil_port_map_find_by_name(const struct > > ofputil_port_map *map, > > return NULL; > > } > > > > -static struct ofputil_port_map_node * > > -ofputil_port_map_find_by_number(const struct ofputil_port_map *map, > > - ofp_port_t ofp_port) > > +static struct ofputil_name_map_node * > > +ofputil_name_map_find_by_number(const struct ofputil_name_map *map, > > + uint32_t number) > > { > > - struct ofputil_port_map_node *node; > > + struct ofputil_name_map_node *node; > > > > - HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_ofp_port(ofp_port), > > + HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_int(number, 0), > > &map->by_number) { > > - if (node->ofp_port == ofp_port) { > > + if (node->number == number) { > > return node; > > } > > } > > return NULL; > > } > > > > -void > > -ofputil_port_map_put(struct ofputil_port_map *map, > > - ofp_port_t ofp_port, const char *name) > > +static void > > +ofputil_name_map_put(struct ofputil_name_map *map, > > + uint32_t number, const char *name) > > { > > - struct ofputil_port_map_node *node; > > + struct ofputil_name_map_node *node; > > > > /* Look for duplicate name. */ > > - node = ofputil_port_map_find_by_name(map, name); > > + node = ofputil_name_map_find_by_name(map, name); > > if (node) { > > - if (node->ofp_port != ofp_port) { > > + if (node->number != number) { > > node->duplicate = true; > > } > > return; > > } > > > > /* Look for duplicate number. */ > > - node = ofputil_port_map_find_by_number(map, ofp_port); > > + node = ofputil_name_map_find_by_number(map, number); > > if (node) { > > node->duplicate = true; > > return; > > @@ -7534,19 +7539,53 @@ ofputil_port_map_put(struct ofputil_port_map *map, > > > > /* Add new node. */ > > node = xmalloc(sizeof *node); > > - hmap_insert(&map->by_number, &node->number_node, > > hash_ofp_port(ofp_port)); > > + hmap_insert(&map->by_number, &node->number_node, hash_int(number, 0)); > > hmap_insert(&map->by_name, &node->name_node, hash_string(name, 0)); > > - node->ofp_port = ofp_port; > > + node->number = number; > > node->name = xstrdup(name); > > node->duplicate = false; > > } > > > > +static void > > +ofputil_name_map_destroy(struct ofputil_name_map *map) > > +{ > > + if (map) { > > + struct ofputil_name_map_node *node, *next; > > + > > + HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { > > + hmap_remove(&map->by_name, &node->name_node); > > + hmap_remove(&map->by_number, &node->number_node); > > + free(node->name); > > + free(node); > > + } > > + hmap_destroy(&map->by_name); > > + hmap_destroy(&map->by_number); > > + } > > +} > > + > > +/* ofputil_port_map. */ > > + > > +void > > +ofputil_port_map_init(struct ofputil_port_map *map) > > +{ > > + ofputil_name_map_init(&map->map); > > +} > > + > > +void > > +ofputil_port_map_put(struct ofputil_port_map *map, > > + ofp_port_t ofp_port, const char *name) > > +{ > > + ofputil_name_map_put(&map->map, ofp_to_u16(ofp_port), name); > > +} > > + > > const char * > > ofputil_port_map_get_name(const struct ofputil_port_map *map, > > ofp_port_t ofp_port) > > { > > - struct ofputil_port_map_node *node > > - = map ? ofputil_port_map_find_by_number(map, ofp_port) : NULL; > > + struct ofputil_name_map_node *node > > + = (map > > + ? ofputil_name_map_find_by_number(&map->map, > > ofp_to_u16(ofp_port)) > > + : NULL); > > return node && !node->duplicate ? node->name : NULL; > > } > > > > @@ -7554,26 +7593,55 @@ ofp_port_t > > ofputil_port_map_get_number(const struct ofputil_port_map *map, > > const char *name) > > { > > - struct ofputil_port_map_node *node > > - = map ? ofputil_port_map_find_by_name(map, name) : NULL; > > - return node && !node->duplicate ? node->ofp_port : OFPP_NONE; > > + struct ofputil_name_map_node *node > > + = map ? ofputil_name_map_find_by_name(&map->map, name) : NULL; > > + return node && !node->duplicate ? u16_to_ofp(node->number) : > > OFPP_NONE; > > } > > > > void > > ofputil_port_map_destroy(struct ofputil_port_map *map) > > { > > - if (map) { > > - struct ofputil_port_map_node *node, *next; > > + ofputil_name_map_destroy(&map->map); > > +} > > + > > + > > +/* ofputil_table_map. */ > > > > - HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { > > - hmap_remove(&map->by_name, &node->name_node); > > - hmap_remove(&map->by_number, &node->number_node); > > - free(node->name); > > - free(node); > > - } > > - hmap_destroy(&map->by_name); > > - hmap_destroy(&map->by_number); > > - } > > +void > > +ofputil_table_map_init(struct ofputil_table_map *map) > > +{ > > + ofputil_name_map_init(&map->map); > > +} > > + > > +void > > +ofputil_table_map_put(struct ofputil_table_map *map, > > + uint8_t table_id, const char *name) > > +{ > > + ofputil_name_map_put(&map->map, table_id, name); > > +} > > + > > +const char * > > +ofputil_table_map_get_name(const struct ofputil_table_map *map, > > + uint8_t table_id) > > +{ > > + struct ofputil_name_map_node *node > > + = map ? ofputil_name_map_find_by_number(&map->map, table_id) : > > NULL; > > + return node && !node->duplicate ? node->name : NULL; > > +} > > + > > +uint8_t > > +ofputil_table_map_get_number(const struct ofputil_table_map *map, > > + const char *name) > > +{ > > + struct ofputil_name_map_node *node > > + = map ? ofputil_name_map_find_by_name(&map->map, name) : NULL; > > + return node && !node->duplicate ? node->number : UINT8_MAX; > > +} > > + > > +void > > +ofputil_table_map_destroy(struct ofputil_table_map *map) > > +{ > > + ofputil_name_map_destroy(&map->map); > > } > > > > /* Stores the group id represented by 's' into '*group_idp'. 's' may be > > an > > -- > > 2.10.2 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
Sure, I just reviewed the 3rd patch. On Wed, Jan 31, 2018 at 11:36 AM, Ben Pfaff <blp@ovn.org> wrote: > Thanks for the reviews of patches 1 and 2. Do you plan to review patch > 3 as well? I rebased and sent out v3 just now. > > On Tue, Jan 30, 2018 at 01:29:56PM -0800, Yifeng Sun wrote: > > Looks good to me, thanks. > > > > Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> > > > > > > On Fri, Jan 12, 2018 at 12:57 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > > This shares the infrastructure for mapping port names and numbers. It > will > > > be used in an upcoming commit. > > > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > > --- > > > include/openvswitch/ofp-util.h | 33 +++++++-- > > > lib/ofp-util.c | 150 ++++++++++++++++++++++++++++++ > > > ----------- > > > 2 files changed, 138 insertions(+), 45 deletions(-) > > > > > > diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp- > > > util.h > > > index 296078a2fe8b..d9780dd44582 100644 > > > --- a/include/openvswitch/ofp-util.h > > > +++ b/include/openvswitch/ofp-util.h > > > @@ -43,15 +43,24 @@ union ofp_action; > > > struct ofpact_set_field; > > > struct vl_mff_map; > > > > > > -/* Mapping between port numbers and names. */ > > > -struct ofputil_port_map { > > > +/* Name-number mapping. > > > + * > > > + * This is not exported directly but only through specializations for > port > > > + * name-number and table name-number mappings. */ > > > +struct ofputil_name_map { > > > struct hmap by_name; > > > struct hmap by_number; > > > }; > > > - > > > -#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ > > > +#define OFPUTIL_NAME_MAP_INITIALIZER(MAP) \ > > > { HMAP_INITIALIZER(&(MAP)->by_name), HMAP_INITIALIZER(&(MAP)->by_ > number) > > > } > > > > > > +/* Mapping between port numbers and names. */ > > > +struct ofputil_port_map { > > > + struct ofputil_name_map map; > > > +}; > > > +#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ > > > + { OFPUTIL_NAME_MAP_INITIALIZER(&(MAP)->map) } > > > + > > > void ofputil_port_map_init(struct ofputil_port_map *); > > > const char *ofputil_port_map_get_name(const struct ofputil_port_map > *, > > > ofp_port_t); > > > @@ -791,6 +800,22 @@ struct ofputil_table_mod_prop_vacancy { > > > uint8_t vacancy; /* Current vacancy (%). */ > > > }; > > > > > > +/* Mapping between table numbers and names. */ > > > +struct ofputil_table_map { > > > + struct ofputil_name_map map; > > > +}; > > > +#define OFPUTIL_TABLE_MAP_INITIALIZER(MAP) \ > > > + { OFPUTIL_NAME_MAP_INITIALIZER((MAP).map) } > > > + > > > +void ofputil_table_map_init(struct ofputil_table_map *); > > > +const char *ofputil_table_map_get_name(const struct > ofputil_table_map *, > > > + uint8_t); > > > +uint8_t ofputil_table_map_get_number(const struct ofputil_table_map > *, > > > + const char *name); > > > +void ofputil_table_map_put(struct ofputil_table_map *, > > > + uint8_t, const char *name); > > > +void ofputil_table_map_destroy(struct ofputil_table_map *); > > > + > > > /* Abstract ofp_table_mod. */ > > > struct ofputil_table_mod { > > > uint8_t table_id; /* ID of the table, 0xff indicates all > > > tables. */ > > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > > > index 597112e4f84e..f3b2e3f6108c 100644 > > > --- a/lib/ofp-util.c > > > +++ b/lib/ofp-util.c > > > @@ -7454,12 +7454,14 @@ ofputil_port_to_string(ofp_port_t port, > > > snprintf(namebuf, bufsize, "%"PRIu32, port); > > > } > > > > > > -/* ofputil_port_map. */ > > > -struct ofputil_port_map_node { > > > +/* ofputil_name_map. */ > > > + > > > +struct ofputil_name_map_node { > > > struct hmap_node name_node; > > > struct hmap_node number_node; > > > - ofp_port_t ofp_port; /* Port number. */ > > > - char *name; /* Port name. */ > > > + > > > + uint32_t number; > > > + char *name; > > > > > > /* OpenFlow doesn't require port names to be unique, although > that's > > > the > > > * only sensible way. However, even in Open vSwitch it's possible > > > for two > > > @@ -7469,22 +7471,25 @@ struct ofputil_port_map_node { > > > * corner case. > > > * > > > * OpenFlow does require port numbers to be unique. We check for > > > duplicate > > > - * ports numbers just in case a switch has a bug. */ > > > + * ports numbers just in case a switch has a bug. > > > + * > > > + * OpenFlow doesn't require table names to be unique and Open > vSwitch > > > + * doesn't try to make them unique. */ > > > bool duplicate; > > > }; > > > > > > -void > > > -ofputil_port_map_init(struct ofputil_port_map *map) > > > +static void > > > +ofputil_name_map_init(struct ofputil_name_map *map) > > > { > > > hmap_init(&map->by_name); > > > hmap_init(&map->by_number); > > > } > > > > > > -static struct ofputil_port_map_node * > > > -ofputil_port_map_find_by_name(const struct ofputil_port_map *map, > > > +static struct ofputil_name_map_node * > > > +ofputil_name_map_find_by_name(const struct ofputil_name_map *map, > > > const char *name) > > > { > > > - struct ofputil_port_map_node *node; > > > + struct ofputil_name_map_node *node; > > > > > > HMAP_FOR_EACH_WITH_HASH (node, name_node, hash_string(name, 0), > > > &map->by_name) { > > > @@ -7495,38 +7500,38 @@ ofputil_port_map_find_by_name(const struct > > > ofputil_port_map *map, > > > return NULL; > > > } > > > > > > -static struct ofputil_port_map_node * > > > -ofputil_port_map_find_by_number(const struct ofputil_port_map *map, > > > - ofp_port_t ofp_port) > > > +static struct ofputil_name_map_node * > > > +ofputil_name_map_find_by_number(const struct ofputil_name_map *map, > > > + uint32_t number) > > > { > > > - struct ofputil_port_map_node *node; > > > + struct ofputil_name_map_node *node; > > > > > > - HMAP_FOR_EACH_IN_BUCKET (node, number_node, > hash_ofp_port(ofp_port), > > > + HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_int(number, 0), > > > &map->by_number) { > > > - if (node->ofp_port == ofp_port) { > > > + if (node->number == number) { > > > return node; > > > } > > > } > > > return NULL; > > > } > > > > > > -void > > > -ofputil_port_map_put(struct ofputil_port_map *map, > > > - ofp_port_t ofp_port, const char *name) > > > +static void > > > +ofputil_name_map_put(struct ofputil_name_map *map, > > > + uint32_t number, const char *name) > > > { > > > - struct ofputil_port_map_node *node; > > > + struct ofputil_name_map_node *node; > > > > > > /* Look for duplicate name. */ > > > - node = ofputil_port_map_find_by_name(map, name); > > > + node = ofputil_name_map_find_by_name(map, name); > > > if (node) { > > > - if (node->ofp_port != ofp_port) { > > > + if (node->number != number) { > > > node->duplicate = true; > > > } > > > return; > > > } > > > > > > /* Look for duplicate number. */ > > > - node = ofputil_port_map_find_by_number(map, ofp_port); > > > + node = ofputil_name_map_find_by_number(map, number); > > > if (node) { > > > node->duplicate = true; > > > return; > > > @@ -7534,19 +7539,53 @@ ofputil_port_map_put(struct ofputil_port_map > *map, > > > > > > /* Add new node. */ > > > node = xmalloc(sizeof *node); > > > - hmap_insert(&map->by_number, &node->number_node, > > > hash_ofp_port(ofp_port)); > > > + hmap_insert(&map->by_number, &node->number_node, hash_int(number, > 0)); > > > hmap_insert(&map->by_name, &node->name_node, hash_string(name, > 0)); > > > - node->ofp_port = ofp_port; > > > + node->number = number; > > > node->name = xstrdup(name); > > > node->duplicate = false; > > > } > > > > > > +static void > > > +ofputil_name_map_destroy(struct ofputil_name_map *map) > > > +{ > > > + if (map) { > > > + struct ofputil_name_map_node *node, *next; > > > + > > > + HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { > > > + hmap_remove(&map->by_name, &node->name_node); > > > + hmap_remove(&map->by_number, &node->number_node); > > > + free(node->name); > > > + free(node); > > > + } > > > + hmap_destroy(&map->by_name); > > > + hmap_destroy(&map->by_number); > > > + } > > > +} > > > + > > > +/* ofputil_port_map. */ > > > + > > > +void > > > +ofputil_port_map_init(struct ofputil_port_map *map) > > > +{ > > > + ofputil_name_map_init(&map->map); > > > +} > > > + > > > +void > > > +ofputil_port_map_put(struct ofputil_port_map *map, > > > + ofp_port_t ofp_port, const char *name) > > > +{ > > > + ofputil_name_map_put(&map->map, ofp_to_u16(ofp_port), name); > > > +} > > > + > > > const char * > > > ofputil_port_map_get_name(const struct ofputil_port_map *map, > > > ofp_port_t ofp_port) > > > { > > > - struct ofputil_port_map_node *node > > > - = map ? ofputil_port_map_find_by_number(map, ofp_port) : > NULL; > > > + struct ofputil_name_map_node *node > > > + = (map > > > + ? ofputil_name_map_find_by_number(&map->map, > > > ofp_to_u16(ofp_port)) > > > + : NULL); > > > return node && !node->duplicate ? node->name : NULL; > > > } > > > > > > @@ -7554,26 +7593,55 @@ ofp_port_t > > > ofputil_port_map_get_number(const struct ofputil_port_map *map, > > > const char *name) > > > { > > > - struct ofputil_port_map_node *node > > > - = map ? ofputil_port_map_find_by_name(map, name) : NULL; > > > - return node && !node->duplicate ? node->ofp_port : OFPP_NONE; > > > + struct ofputil_name_map_node *node > > > + = map ? ofputil_name_map_find_by_name(&map->map, name) : > NULL; > > > + return node && !node->duplicate ? u16_to_ofp(node->number) : > > > OFPP_NONE; > > > } > > > > > > void > > > ofputil_port_map_destroy(struct ofputil_port_map *map) > > > { > > > - if (map) { > > > - struct ofputil_port_map_node *node, *next; > > > + ofputil_name_map_destroy(&map->map); > > > +} > > > + > > > + > > > +/* ofputil_table_map. */ > > > > > > - HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { > > > - hmap_remove(&map->by_name, &node->name_node); > > > - hmap_remove(&map->by_number, &node->number_node); > > > - free(node->name); > > > - free(node); > > > - } > > > - hmap_destroy(&map->by_name); > > > - hmap_destroy(&map->by_number); > > > - } > > > +void > > > +ofputil_table_map_init(struct ofputil_table_map *map) > > > +{ > > > + ofputil_name_map_init(&map->map); > > > +} > > > + > > > +void > > > +ofputil_table_map_put(struct ofputil_table_map *map, > > > + uint8_t table_id, const char *name) > > > +{ > > > + ofputil_name_map_put(&map->map, table_id, name); > > > +} > > > + > > > +const char * > > > +ofputil_table_map_get_name(const struct ofputil_table_map *map, > > > + uint8_t table_id) > > > +{ > > > + struct ofputil_name_map_node *node > > > + = map ? ofputil_name_map_find_by_number(&map->map, table_id) > : > > > NULL; > > > + return node && !node->duplicate ? node->name : NULL; > > > +} > > > + > > > +uint8_t > > > +ofputil_table_map_get_number(const struct ofputil_table_map *map, > > > + const char *name) > > > +{ > > > + struct ofputil_name_map_node *node > > > + = map ? ofputil_name_map_find_by_name(&map->map, name) : > NULL; > > > + return node && !node->duplicate ? node->number : UINT8_MAX; > > > +} > > > + > > > +void > > > +ofputil_table_map_destroy(struct ofputil_table_map *map) > > > +{ > > > + ofputil_name_map_destroy(&map->map); > > > } > > > > > > /* Stores the group id represented by 's' into '*group_idp'. 's' may > be > > > an > > > -- > > > 2.10.2 > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > >
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h index 296078a2fe8b..d9780dd44582 100644 --- a/include/openvswitch/ofp-util.h +++ b/include/openvswitch/ofp-util.h @@ -43,15 +43,24 @@ union ofp_action; struct ofpact_set_field; struct vl_mff_map; -/* Mapping between port numbers and names. */ -struct ofputil_port_map { +/* Name-number mapping. + * + * This is not exported directly but only through specializations for port + * name-number and table name-number mappings. */ +struct ofputil_name_map { struct hmap by_name; struct hmap by_number; }; - -#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ +#define OFPUTIL_NAME_MAP_INITIALIZER(MAP) \ { HMAP_INITIALIZER(&(MAP)->by_name), HMAP_INITIALIZER(&(MAP)->by_number) } +/* Mapping between port numbers and names. */ +struct ofputil_port_map { + struct ofputil_name_map map; +}; +#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \ + { OFPUTIL_NAME_MAP_INITIALIZER(&(MAP)->map) } + void ofputil_port_map_init(struct ofputil_port_map *); const char *ofputil_port_map_get_name(const struct ofputil_port_map *, ofp_port_t); @@ -791,6 +800,22 @@ struct ofputil_table_mod_prop_vacancy { uint8_t vacancy; /* Current vacancy (%). */ }; +/* Mapping between table numbers and names. */ +struct ofputil_table_map { + struct ofputil_name_map map; +}; +#define OFPUTIL_TABLE_MAP_INITIALIZER(MAP) \ + { OFPUTIL_NAME_MAP_INITIALIZER((MAP).map) } + +void ofputil_table_map_init(struct ofputil_table_map *); +const char *ofputil_table_map_get_name(const struct ofputil_table_map *, + uint8_t); +uint8_t ofputil_table_map_get_number(const struct ofputil_table_map *, + const char *name); +void ofputil_table_map_put(struct ofputil_table_map *, + uint8_t, const char *name); +void ofputil_table_map_destroy(struct ofputil_table_map *); + /* Abstract ofp_table_mod. */ struct ofputil_table_mod { uint8_t table_id; /* ID of the table, 0xff indicates all tables. */ diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 597112e4f84e..f3b2e3f6108c 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -7454,12 +7454,14 @@ ofputil_port_to_string(ofp_port_t port, snprintf(namebuf, bufsize, "%"PRIu32, port); } -/* ofputil_port_map. */ -struct ofputil_port_map_node { +/* ofputil_name_map. */ + +struct ofputil_name_map_node { struct hmap_node name_node; struct hmap_node number_node; - ofp_port_t ofp_port; /* Port number. */ - char *name; /* Port name. */ + + uint32_t number; + char *name; /* OpenFlow doesn't require port names to be unique, although that's the * only sensible way. However, even in Open vSwitch it's possible for two @@ -7469,22 +7471,25 @@ struct ofputil_port_map_node { * corner case. * * OpenFlow does require port numbers to be unique. We check for duplicate - * ports numbers just in case a switch has a bug. */ + * ports numbers just in case a switch has a bug. + * + * OpenFlow doesn't require table names to be unique and Open vSwitch + * doesn't try to make them unique. */ bool duplicate; }; -void -ofputil_port_map_init(struct ofputil_port_map *map) +static void +ofputil_name_map_init(struct ofputil_name_map *map) { hmap_init(&map->by_name); hmap_init(&map->by_number); } -static struct ofputil_port_map_node * -ofputil_port_map_find_by_name(const struct ofputil_port_map *map, +static struct ofputil_name_map_node * +ofputil_name_map_find_by_name(const struct ofputil_name_map *map, const char *name) { - struct ofputil_port_map_node *node; + struct ofputil_name_map_node *node; HMAP_FOR_EACH_WITH_HASH (node, name_node, hash_string(name, 0), &map->by_name) { @@ -7495,38 +7500,38 @@ ofputil_port_map_find_by_name(const struct ofputil_port_map *map, return NULL; } -static struct ofputil_port_map_node * -ofputil_port_map_find_by_number(const struct ofputil_port_map *map, - ofp_port_t ofp_port) +static struct ofputil_name_map_node * +ofputil_name_map_find_by_number(const struct ofputil_name_map *map, + uint32_t number) { - struct ofputil_port_map_node *node; + struct ofputil_name_map_node *node; - HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_ofp_port(ofp_port), + HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_int(number, 0), &map->by_number) { - if (node->ofp_port == ofp_port) { + if (node->number == number) { return node; } } return NULL; } -void -ofputil_port_map_put(struct ofputil_port_map *map, - ofp_port_t ofp_port, const char *name) +static void +ofputil_name_map_put(struct ofputil_name_map *map, + uint32_t number, const char *name) { - struct ofputil_port_map_node *node; + struct ofputil_name_map_node *node; /* Look for duplicate name. */ - node = ofputil_port_map_find_by_name(map, name); + node = ofputil_name_map_find_by_name(map, name); if (node) { - if (node->ofp_port != ofp_port) { + if (node->number != number) { node->duplicate = true; } return; } /* Look for duplicate number. */ - node = ofputil_port_map_find_by_number(map, ofp_port); + node = ofputil_name_map_find_by_number(map, number); if (node) { node->duplicate = true; return; @@ -7534,19 +7539,53 @@ ofputil_port_map_put(struct ofputil_port_map *map, /* Add new node. */ node = xmalloc(sizeof *node); - hmap_insert(&map->by_number, &node->number_node, hash_ofp_port(ofp_port)); + hmap_insert(&map->by_number, &node->number_node, hash_int(number, 0)); hmap_insert(&map->by_name, &node->name_node, hash_string(name, 0)); - node->ofp_port = ofp_port; + node->number = number; node->name = xstrdup(name); node->duplicate = false; } +static void +ofputil_name_map_destroy(struct ofputil_name_map *map) +{ + if (map) { + struct ofputil_name_map_node *node, *next; + + HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { + hmap_remove(&map->by_name, &node->name_node); + hmap_remove(&map->by_number, &node->number_node); + free(node->name); + free(node); + } + hmap_destroy(&map->by_name); + hmap_destroy(&map->by_number); + } +} + +/* ofputil_port_map. */ + +void +ofputil_port_map_init(struct ofputil_port_map *map) +{ + ofputil_name_map_init(&map->map); +} + +void +ofputil_port_map_put(struct ofputil_port_map *map, + ofp_port_t ofp_port, const char *name) +{ + ofputil_name_map_put(&map->map, ofp_to_u16(ofp_port), name); +} + const char * ofputil_port_map_get_name(const struct ofputil_port_map *map, ofp_port_t ofp_port) { - struct ofputil_port_map_node *node - = map ? ofputil_port_map_find_by_number(map, ofp_port) : NULL; + struct ofputil_name_map_node *node + = (map + ? ofputil_name_map_find_by_number(&map->map, ofp_to_u16(ofp_port)) + : NULL); return node && !node->duplicate ? node->name : NULL; } @@ -7554,26 +7593,55 @@ ofp_port_t ofputil_port_map_get_number(const struct ofputil_port_map *map, const char *name) { - struct ofputil_port_map_node *node - = map ? ofputil_port_map_find_by_name(map, name) : NULL; - return node && !node->duplicate ? node->ofp_port : OFPP_NONE; + struct ofputil_name_map_node *node + = map ? ofputil_name_map_find_by_name(&map->map, name) : NULL; + return node && !node->duplicate ? u16_to_ofp(node->number) : OFPP_NONE; } void ofputil_port_map_destroy(struct ofputil_port_map *map) { - if (map) { - struct ofputil_port_map_node *node, *next; + ofputil_name_map_destroy(&map->map); +} + + +/* ofputil_table_map. */ - HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) { - hmap_remove(&map->by_name, &node->name_node); - hmap_remove(&map->by_number, &node->number_node); - free(node->name); - free(node); - } - hmap_destroy(&map->by_name); - hmap_destroy(&map->by_number); - } +void +ofputil_table_map_init(struct ofputil_table_map *map) +{ + ofputil_name_map_init(&map->map); +} + +void +ofputil_table_map_put(struct ofputil_table_map *map, + uint8_t table_id, const char *name) +{ + ofputil_name_map_put(&map->map, table_id, name); +} + +const char * +ofputil_table_map_get_name(const struct ofputil_table_map *map, + uint8_t table_id) +{ + struct ofputil_name_map_node *node + = map ? ofputil_name_map_find_by_number(&map->map, table_id) : NULL; + return node && !node->duplicate ? node->name : NULL; +} + +uint8_t +ofputil_table_map_get_number(const struct ofputil_table_map *map, + const char *name) +{ + struct ofputil_name_map_node *node + = map ? ofputil_name_map_find_by_name(&map->map, name) : NULL; + return node && !node->duplicate ? node->number : UINT8_MAX; +} + +void +ofputil_table_map_destroy(struct ofputil_table_map *map) +{ + ofputil_name_map_destroy(&map->map); } /* Stores the group id represented by 's' into '*group_idp'. 's' may be an
This shares the infrastructure for mapping port names and numbers. It will be used in an upcoming commit. Signed-off-by: Ben Pfaff <blp@ovn.org> --- include/openvswitch/ofp-util.h | 33 +++++++-- lib/ofp-util.c | 150 ++++++++++++++++++++++++++++++----------- 2 files changed, 138 insertions(+), 45 deletions(-)