[ovs-dev] [PATCH v2 2/3] ofp-util: New data structure for mapping between table names and numbers.
Yifeng Sun
pkusunyifeng at gmail.com
Thu Feb 1 17:57:56 UTC 2018
Sure, I just reviewed the 3rd patch.
On Wed, Jan 31, 2018 at 11:36 AM, Ben Pfaff <blp at 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 at gmail.com>
> >
> >
> > On Fri, Jan 12, 2018 at 12:57 PM, Ben Pfaff <blp at 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 at 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 at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
>
More information about the dev
mailing list