[ovs-dev] [PATCH 1/3] ovn-controller: Add extend_table instead of group_table to expand meter.

Miguel Angel Ajo Pelayo majopela at redhat.com
Tue Jan 30 10:42:33 UTC 2018


Acked-By: Miguel Angel Ajo <majopela at redhat.com>

The refactor looks good to me.

On Wed, Jan 24, 2018 at 2:40 AM Ben Pfaff <blp at ovn.org> wrote:

> From: Guoshuai Li <ligs at dtdream.com>
>
> The structure and function of the group table and meter table are similar,
> refactoring code is used to extend for add the meter table.
> The following function as lib: table init/destroy/clear,
> install contents from desired, remove contents from existing,
> Move the contents of desired to existing.
>
> Signed-off-by: Guoshuai Li <ligs at dtdream.com>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  include/ovn/actions.h           |  21 +----
>  ovn/controller/lflow.c          |   8 +-
>  ovn/controller/lflow.h          |   4 +-
>  ovn/controller/ofctrl.c         | 180 +++++++++++++-----------------------
>  ovn/controller/ofctrl.h         |   7 +-
>  ovn/controller/ovn-controller.c |  20 +---
>  ovn/lib/actions.c               |  58 ++----------
>  ovn/lib/automake.mk             |   2 +
>  ovn/lib/extend-table.c          | 198
> ++++++++++++++++++++++++++++++++++++++++
>  ovn/lib/extend-table.h          |  69 ++++++++++++++
>  tests/test-ovn.c                |   8 +-
>  11 files changed, 356 insertions(+), 219 deletions(-)
>  create mode 100644 ovn/lib/extend-table.c
>  create mode 100644 ovn/lib/extend-table.h
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 85a484ffac20..ea90dbb2a69a 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -31,6 +31,7 @@ struct lexer;
>  struct ofpbuf;
>  struct shash;
>  struct simap;
> +struct ovn_extend_table;
>
>  /* List of OVN logical actions.
>   *
> @@ -338,24 +339,6 @@ void *ovnact_put(struct ofpbuf *, enum ovnact_type,
> size_t len);
>  OVNACTS
>  #undef OVNACT
>
> -#define MAX_OVN_GROUPS 65535
> -
> -struct group_table {
> -    unsigned long *group_ids;  /* Used as a bitmap with value set
> -                                * for allocated group ids in either
> -                                * desired_groups or existing_groups. */
> -    struct hmap desired_groups;
> -    struct hmap existing_groups;
> -};
> -
> -struct group_info {
> -    struct hmap_node hmap_node;
> -    struct ds group;
> -    uint32_t group_id;
> -    bool new_group_id;  /* 'True' if 'group_id' was reserved from
> -                         * group_table's 'group_ids' bitmap. */
> -};
> -
>  enum action_opcode {
>      /* "arp { ...actions... }".
>       *
> @@ -505,7 +488,7 @@ struct ovnact_encode_params {
>      bool is_gateway_router;
>
>      /* A struct to figure out the group_id for group actions. */
> -    struct group_table *group_table;
> +    struct ovn_extend_table *group_table;
>
>      /* OVN maps each logical flow table (ltable), one-to-one, onto a
> physical
>       * OpenFlow flow table (ptable).  A number of parameters describe this
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index a62ec6ebe09f..3d990c49c195 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -61,7 +61,7 @@ static void consider_logical_flow(struct controller_ctx
> *ctx,
>                                    const struct chassis_index
> *chassis_index,
>                                    const struct sbrec_logical_flow *lflow,
>                                    const struct hmap *local_datapaths,
> -                                  struct group_table *group_table,
> +                                  struct ovn_extend_table *group_table,
>                                    const struct sbrec_chassis *chassis,
>                                    struct hmap *dhcp_opts,
>                                    struct hmap *dhcpv6_opts,
> @@ -143,7 +143,7 @@ static void
>  add_logical_flows(struct controller_ctx *ctx,
>                    const struct chassis_index *chassis_index,
>                    const struct hmap *local_datapaths,
> -                  struct group_table *group_table,
> +                  struct ovn_extend_table *group_table,
>                    const struct sbrec_chassis *chassis,
>                    const struct shash *addr_sets,
>                    struct hmap *flow_table,
> @@ -190,7 +190,7 @@ consider_logical_flow(struct controller_ctx *ctx,
>                        const struct chassis_index *chassis_index,
>                        const struct sbrec_logical_flow *lflow,
>                        const struct hmap *local_datapaths,
> -                      struct group_table *group_table,
> +                      struct ovn_extend_table *group_table,
>                        const struct sbrec_chassis *chassis,
>                        struct hmap *dhcp_opts,
>                        struct hmap *dhcpv6_opts,
> @@ -434,7 +434,7 @@ lflow_run(struct controller_ctx *ctx,
>            const struct sbrec_chassis *chassis,
>            const struct chassis_index *chassis_index,
>            const struct hmap *local_datapaths,
> -          struct group_table *group_table,
> +          struct ovn_extend_table *group_table,
>            const struct shash *addr_sets,
>            struct hmap *flow_table,
>            struct sset *active_tunnels,
> diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
> index bfb7415e2b13..087b0ed8da35 100644
> --- a/ovn/controller/lflow.h
> +++ b/ovn/controller/lflow.h
> @@ -37,7 +37,7 @@
>
>  struct chassis_index;
>  struct controller_ctx;
> -struct group_table;
> +struct ovn_extend_table;
>  struct hmap;
>  struct sbrec_chassis;
>  struct simap;
> @@ -66,7 +66,7 @@ void lflow_run(struct controller_ctx *,
>                 const struct sbrec_chassis *chassis,
>                 const struct chassis_index *,
>                 const struct hmap *local_datapaths,
> -               struct group_table *group_table,
> +               struct ovn_extend_table *group_table,
>                 const struct shash *addr_sets,
>                 struct hmap *flow_table,
>                 struct sset *active_tunnels,
> diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
> index 2fa980ebdae1..94ee69c1c3ed 100644
> --- a/ovn/controller/ofctrl.c
> +++ b/ovn/controller/ofctrl.c
> @@ -36,6 +36,7 @@
>  #include "openvswitch/vlog.h"
>  #include "ovn-controller.h"
>  #include "ovn/actions.h"
> +#include "ovn/lib/extend-table.h"
>  #include "openvswitch/poll-loop.h"
>  #include "physical.h"
>  #include "openvswitch/rconn.h"
> @@ -68,6 +69,9 @@ static char *ovn_flow_to_string(const struct ovn_flow *);
>  static void ovn_flow_log(const struct ovn_flow *, const char *action);
>  static void ovn_flow_destroy(struct ovn_flow *);
>
> +static void add_group(uint32_t table_id, struct ds *ds, struct ovs_list
> *msgs);
> +static void delete_group(uint32_t table_id, struct ovs_list *msgs);
> +
>  /* OpenFlow connection to the switch. */
>  static struct rconn *swconn;
>
> @@ -131,7 +135,7 @@ static struct rconn_packet_counter *tx_counter;
>  static struct hmap installed_flows;
>
>  /* A reference to the group_table. */
> -static struct group_table *groups;
> +static struct ovn_extend_table *groups;
>
>  /* MFF_* field ID for our Geneve option.  In S_TLV_TABLE_MOD_SENT, this is
>   * the option we requested (we don't know whether we obtained it yet).  In
> @@ -150,7 +154,7 @@ static void ovn_flow_table_destroy(struct hmap
> *flow_table);
>  static void ofctrl_recv(const struct ofp_header *, enum ofptype);
>
>  void
> -ofctrl_init(struct group_table *group_table)
> +ofctrl_init(struct ovn_extend_table *group_table)
>  {
>      swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP13_VERSION);
>      tx_counter = rconn_packet_counter_create();
> @@ -158,6 +162,8 @@ ofctrl_init(struct group_table *group_table)
>      ovs_list_init(&flow_updates);
>      ovn_init_symtab(&symtab);
>      groups = group_table;
> +    groups->create = add_group;
> +    groups->remove = delete_group;
>  }
>
>  /* S_NEW, for a new connection.
> @@ -385,7 +391,7 @@ run_S_CLEAR_FLOWS(void)
>
>      /* Clear existing groups, to match the state of the switch. */
>      if (groups) {
> -        ovn_group_table_clear(groups, true);
> +        ovn_extend_table_clear(groups, true);
>      }
>
>      /* All flow updates are irrelevant now. */
> @@ -747,44 +753,6 @@ add_flow_mod(struct ofputil_flow_mod *fm, struct
> ovs_list *msgs)
>
>  /* group_table. */
>
> -/* Finds and returns a group_info in 'existing_groups' whose key is
> identical
> - * to 'target''s key, or NULL if there is none. */
> -static struct group_info *
> -ovn_group_lookup(struct hmap *exisiting_groups,
> -                 const struct group_info *target)
> -{
> -    struct group_info *e;
> -
> -    HMAP_FOR_EACH_WITH_HASH(e, hmap_node, target->hmap_node.hash,
> -                            exisiting_groups) {
> -        if (e->group_id == target->group_id) {
> -            return e;
> -        }
> -   }
> -    return NULL;
> -}
> -
> -/* Clear either desired_groups or existing_groups in group_table. */
> -void
> -ovn_group_table_clear(struct group_table *group_table, bool existing)
> -{
> -    struct group_info *g, *next;
> -    struct hmap *target_group = existing
> -                                ? &group_table->existing_groups
> -                                : &group_table->desired_groups;
> -
> -    HMAP_FOR_EACH_SAFE (g, next, hmap_node, target_group) {
> -        hmap_remove(target_group, &g->hmap_node);
> -        /* Don't unset bitmap for desired group_info if the group_id
> -         * was not freshly reserved. */
> -        if (existing || g->new_group_id) {
> -            bitmap_set0(group_table->group_ids, g->group_id);
> -        }
> -        ds_destroy(&g->group);
> -        free(g);
> -    }
> -}
> -
>  static struct ofpbuf *
>  encode_group_mod(const struct ofputil_group_mod *gm)
>  {
> @@ -797,6 +765,54 @@ add_group_mod(const struct ofputil_group_mod *gm,
> struct ovs_list *msgs)
>      struct ofpbuf *msg = encode_group_mod(gm);
>      ovs_list_push_back(msgs, &msg->list_node);
>  }
> +
> +static void
> +add_group(uint32_t table_id, struct ds *ds, struct ovs_list *msgs) {
> +    /* Create and install new group. */
> +    struct ofputil_group_mod gm;
> +    enum ofputil_protocol usable_protocols;
> +    char *error;
> +    struct ds group_string = DS_EMPTY_INITIALIZER;
> +    ds_put_format(&group_string, "group_id=%"PRIu32",%s",
> +                  table_id, ds_cstr(ds));
> +
> +    error = parse_ofp_group_mod_str(&gm, OFPGC11_ADD,
> ds_cstr(&group_string),
> +                                    NULL, &usable_protocols);
> +    if (!error) {
> +        add_group_mod(&gm, msgs);
> +    } else {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        VLOG_ERR_RL(&rl, "new group %s %s", error,
> +                    ds_cstr(&group_string));
> +        free(error);
> +    }
> +    ds_destroy(&group_string);
> +    ofputil_uninit_group_mod(&gm);
> +}
> +
> +static void
> +delete_group(uint32_t table_id, struct ovs_list *msgs) {
> +    /* Delete the group. */
> +    struct ofputil_group_mod gm;
> +    enum ofputil_protocol usable_protocols;
> +    char *error;
> +    struct ds group_string = DS_EMPTY_INITIALIZER;
> +    ds_put_format(&group_string, "group_id=%"PRIu32"", table_id);
> +
> +    error = parse_ofp_group_mod_str(&gm, OFPGC11_DELETE,
> +                                    ds_cstr(&group_string), NULL,
> +                                    &usable_protocols);
> +    if (!error) {
> +        add_group_mod(&gm, msgs);
> +    } else {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +        VLOG_ERR_RL(&rl, "Error deleting group %"PRIu32": %s",
> +                    table_id, error);
> +        free(error);
> +    }
> +    ds_destroy(&group_string);
> +    ofputil_uninit_group_mod(&gm);
> +}
>
>  static void
>  add_ct_flush_zone(uint16_t zone_id, struct ovs_list *msgs)
> @@ -844,7 +860,7 @@ ofctrl_put(struct hmap *flow_table, struct shash
> *pending_ct_zones,
>  {
>      if (!ofctrl_can_put()) {
>          ovn_flow_table_clear(flow_table);
> -        ovn_group_table_clear(groups, false);
> +        ovn_extend_table_clear(groups, false);
>          return;
>      }
>
> @@ -862,34 +878,7 @@ ofctrl_put(struct hmap *flow_table, struct shash
> *pending_ct_zones,
>          }
>      }
>
> -    /* Iterate through all the desired groups. If there are new ones,
> -     * add them to the switch. */
> -    struct group_info *desired;
> -    HMAP_FOR_EACH(desired, hmap_node, &groups->desired_groups) {
> -        if (!ovn_group_lookup(&groups->existing_groups, desired)) {
> -            /* Create and install new group. */
> -            struct ofputil_group_mod gm;
> -            enum ofputil_protocol usable_protocols;
> -            char *error;
> -            struct ds group_string = DS_EMPTY_INITIALIZER;
> -            ds_put_format(&group_string, "group_id=%u,%s",
> -                          desired->group_id, ds_cstr(&desired->group));
> -
> -            error = parse_ofp_group_mod_str(&gm, OFPGC11_ADD,
> -                                            ds_cstr(&group_string), NULL,
> -                                            &usable_protocols);
> -            if (!error) {
> -                add_group_mod(&gm, &msgs);
> -            } else {
> -                static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 1);
> -                VLOG_ERR_RL(&rl, "new group %s %s", error,
> -                         ds_cstr(&group_string));
> -                free(error);
> -            }
> -            ds_destroy(&group_string);
> -            ofputil_uninit_group_mod(&gm);
> -        }
> -    }
> +    ovn_extend_table_install_desired(groups, &msgs);
>
>      /* Iterate through all of the installed flows.  If any of them are no
>       * longer desired, delete them; if any of them should have different
> @@ -962,55 +951,10 @@ ofctrl_put(struct hmap *flow_table, struct shash
> *pending_ct_zones,
>          hmap_insert(&installed_flows, &d->hmap_node, d->hmap_node.hash);
>      }
>
> -    /* Iterate through the installed groups from previous runs. If they
> -     * are not needed delete them. */
> -    struct group_info *installed, *next_group;
> -    HMAP_FOR_EACH_SAFE(installed, next_group, hmap_node,
> -                       &groups->existing_groups) {
> -        if (!ovn_group_lookup(&groups->desired_groups, installed)) {
> -            /* Delete the group. */
> -            struct ofputil_group_mod gm;
> -            enum ofputil_protocol usable_protocols;
> -            char *error;
> -            struct ds group_string = DS_EMPTY_INITIALIZER;
> -            ds_put_format(&group_string, "group_id=%u",
> installed->group_id);
> -
> -            error = parse_ofp_group_mod_str(&gm, OFPGC11_DELETE,
> -                                            ds_cstr(&group_string), NULL,
> -                                            &usable_protocols);
> -            if (!error) {
> -                add_group_mod(&gm, &msgs);
> -            } else {
> -                static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 1);
> -                VLOG_ERR_RL(&rl, "Error deleting group %d: %s",
> -                         installed->group_id, error);
> -                free(error);
> -            }
> -            ds_destroy(&group_string);
> -            ofputil_uninit_group_mod(&gm);
> -
> -            /* Remove 'installed' from 'groups->existing_groups' */
> -            hmap_remove(&groups->existing_groups, &installed->hmap_node);
> -            ds_destroy(&installed->group);
> -
> -            /* Dealloc group_id. */
> -            bitmap_set0(groups->group_ids, installed->group_id);
> -            free(installed);
> -        }
> -    }
> +    ovn_extend_table_remove_existing(groups, &msgs);
>
> -    /* Move the contents of desired_groups to existing_groups. */
> -    HMAP_FOR_EACH_SAFE(desired, next_group, hmap_node,
> -                       &groups->desired_groups) {
> -        hmap_remove(&groups->desired_groups, &desired->hmap_node);
> -        if (!ovn_group_lookup(&groups->existing_groups, desired)) {
> -            hmap_insert(&groups->existing_groups, &desired->hmap_node,
> -                        desired->hmap_node.hash);
> -        } else {
> -           ds_destroy(&desired->group);
> -           free(desired);
> -        }
> -    }
> +    /* Move the contents of groups->desired to groups->existing. */
> +    ovn_extend_table_move(groups);
>
>      if (!ovs_list_is_empty(&msgs)) {
>          /* Add a barrier to the list of messages. */
> diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
> index d83f6aec42c1..9b5eab1f43ab 100644
> --- a/ovn/controller/ofctrl.h
> +++ b/ovn/controller/ofctrl.h
> @@ -23,7 +23,7 @@
>  #include "ovsdb-idl.h"
>
>  struct controller_ctx;
> -struct group_table;
> +struct ovn_extend_table;
>  struct hmap;
>  struct match;
>  struct ofpbuf;
> @@ -31,7 +31,7 @@ struct ovsrec_bridge;
>  struct shash;
>
>  /* Interface for OVN main loop. */
> -void ofctrl_init(struct group_table *group_table);
> +void ofctrl_init(struct ovn_extend_table *group_table);
>  enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int,
>                              struct shash *pending_ct_zones);
>  bool ofctrl_can_put(void);
> @@ -55,7 +55,4 @@ void ofctrl_add_flow(struct hmap *desired_flows, uint8_t
> table_id,
>
>  void ofctrl_flow_table_clear(void);
>
> -void ovn_group_table_clear(struct group_table *group_table,
> -                           bool existing);
> -
>  #endif /* ovn/ofctrl.h */
> diff --git a/ovn/controller/ovn-controller.c
> b/ovn/controller/ovn-controller.c
> index c286ccbcaf8d..c486887a5274 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -42,6 +42,7 @@
>  #include "openvswitch/vlog.h"
>  #include "ovn/actions.h"
>  #include "ovn/lib/chassis-index.h"
> +#include "ovn/lib/extend-table.h"
>  #include "ovn/lib/ovn-sb-idl.h"
>  #include "ovn/lib/ovn-util.h"
>  #include "patch.h"
> @@ -593,11 +594,8 @@ main(int argc, char *argv[])
>      unixctl_command_register("exit", "", 0, 0, ovn_controller_exit,
> &exiting);
>
>      /* Initialize group ids for loadbalancing. */
> -    struct group_table group_table;
> -    group_table.group_ids = bitmap_allocate(MAX_OVN_GROUPS);
> -    bitmap_set1(group_table.group_ids, 0); /* Group id 0 is invalid. */
> -    hmap_init(&group_table.desired_groups);
> -    hmap_init(&group_table.existing_groups);
> +    struct ovn_extend_table group_table;
> +    ovn_extend_table_init(&group_table);
>
>      daemonize_complete();
>
> @@ -845,17 +843,7 @@ main(int argc, char *argv[])
>      simap_destroy(&ct_zones);
>      shash_destroy(&pending_ct_zones);
>
> -    bitmap_free(group_table.group_ids);
> -    hmap_destroy(&group_table.desired_groups);
> -
> -    struct group_info *installed, *next_group;
> -    HMAP_FOR_EACH_SAFE(installed, next_group, hmap_node,
> -                       &group_table.existing_groups) {
> -        hmap_remove(&group_table.existing_groups, &installed->hmap_node);
> -        ds_destroy(&installed->group);
> -        free(installed);
> -    }
> -    hmap_destroy(&group_table.existing_groups);
> +    ovn_extend_table_destroy(&group_table);
>
>      ovsdb_idl_loop_destroy(&ovs_idl_loop);
>      ovsdb_idl_loop_destroy(&ovnsb_idl_loop);
> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
> index 69a2172a80a1..8a7fe4f59e02 100644
> --- a/ovn/lib/actions.c
> +++ b/ovn/lib/actions.c
> @@ -35,6 +35,7 @@
>  #include "ovn/expr.h"
>  #include "ovn/lex.h"
>  #include "ovn/lib/acl-log.h"
> +#include "ovn/lib/extend-table.h"
>  #include "packets.h"
>  #include "openvswitch/shash.h"
>  #include "simap.h"
> @@ -1026,8 +1027,7 @@ encode_CT_LB(const struct ovnact_ct_lb *cl,
>          return;
>      }
>
> -    uint32_t group_id = 0, hash;
> -    struct group_info *group_info;
> +    uint32_t table_id = 0;
>      struct ofpact_group *og;
>      uint32_t zone_reg = ep->is_switch ? MFF_LOG_CT_ZONE - MFF_REG0
>                              : MFF_LOG_DNAT_ZONE - MFF_REG0;
> @@ -1059,59 +1059,17 @@ encode_CT_LB(const struct ovnact_ct_lb *cl,
>                        recirc_table, zone_reg);
>      }
>
> -    hash = hash_string(ds_cstr(&ds), 0);
> -
> -    /* Check whether we have non installed but allocated group_id. */
> -    HMAP_FOR_EACH_WITH_HASH (group_info, hmap_node, hash,
> -                             &ep->group_table->desired_groups) {
> -        if (!strcmp(ds_cstr(&group_info->group), ds_cstr(&ds))) {
> -            group_id = group_info->group_id;
> -            break;
> -        }
> -    }
> -
> -    if (!group_id) {
> -        /* Check whether we already have an installed entry for this
> -         * combination. */
> -        HMAP_FOR_EACH_WITH_HASH (group_info, hmap_node, hash,
> -                                 &ep->group_table->existing_groups) {
> -            if (!strcmp(ds_cstr(&group_info->group), ds_cstr(&ds))) {
> -                group_id = group_info->group_id;
> -            }
> -        }
> -
> -        bool new_group_id = false;
> -        if (!group_id) {
> -            /* Reserve a new group_id. */
> -            group_id = bitmap_scan(ep->group_table->group_ids, 0, 1,
> -                                   MAX_OVN_GROUPS + 1);
> -            new_group_id = true;
> -        }
> -
> -        if (group_id == MAX_OVN_GROUPS + 1) {
> -            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> -            VLOG_ERR_RL(&rl, "out of group ids");
> -
> -            ds_destroy(&ds);
> -            return;
> -        }
> -        bitmap_set1(ep->group_table->group_ids, group_id);
> -
> -        group_info = xmalloc(sizeof *group_info);
> -        group_info->group = ds;
> -        group_info->group_id = group_id;
> -        group_info->hmap_node.hash = hash;
> -        group_info->new_group_id = new_group_id;
> -
> -        hmap_insert(&ep->group_table->desired_groups,
> -                    &group_info->hmap_node, group_info->hmap_node.hash);
> -    } else {
> +    table_id = ovn_extend_table_assign_id(ep->group_table, &ds);
> +    if (table_id == EXT_TABLE_ID_INVALID) {
>          ds_destroy(&ds);
> +        return;
>      }
>
> +    ds_destroy(&ds);
> +
>      /* Create an action to set the group. */
>      og = ofpact_put_GROUP(ofpacts);
> -    og->group_id = group_id;
> +    og->group_id = table_id;
>  }
>
>  static void
> diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk
> index dae06a5c158c..6178fc2d5aa4 100644
> --- a/ovn/lib/automake.mk
> +++ b/ovn/lib/automake.mk
> @@ -10,6 +10,8 @@ ovn_lib_libovn_la_SOURCES = \
>         ovn/lib/chassis-index.c \
>         ovn/lib/chassis-index.h \
>         ovn/lib/expr.c \
> +       ovn/lib/extend-table.h \
> +       ovn/lib/extend-table.c \
>         ovn/lib/lex.c \
>         ovn/lib/ovn-l7.h \
>         ovn/lib/ovn-util.c \
> diff --git a/ovn/lib/extend-table.c b/ovn/lib/extend-table.c
> new file mode 100644
> index 000000000000..dfd1a9413cfc
> --- /dev/null
> +++ b/ovn/lib/extend-table.c
> @@ -0,0 +1,198 @@
> +/*
> + * Copyright (c) 2017 DtDream Technology Co.,Ltd.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "bitmap.h"
> +#include "hash.h"
> +#include "openvswitch/vlog.h"
> +#include "ovn/lib/extend-table.h"
> +
> +VLOG_DEFINE_THIS_MODULE(extend_table);
> +
> +void ovn_extend_table_init(struct ovn_extend_table *table)
> +{
> +    table->table_ids = bitmap_allocate(MAX_EXT_TABLE_ID);
> +    bitmap_set1(table->table_ids, 0); /* table id 0 is invalid. */
> +    hmap_init(&table->desired);
> +    hmap_init(&table->existing);
> +}
> +
> +void ovn_extend_table_destroy(struct ovn_extend_table *table)
> +{
> +    bitmap_free(table->table_ids);
> +
> +    struct ovn_extend_table_info *desired, *desired_next;
> +    HMAP_FOR_EACH_SAFE (desired, desired_next, hmap_node,
> &table->existing) {
> +        hmap_remove(&table->existing, &desired->hmap_node);
> +        ds_destroy(&desired->info);
> +        free(desired);
> +    }
> +    hmap_destroy(&table->desired);
> +
> +    struct ovn_extend_table_info *existing, *existing_next;
> +    HMAP_FOR_EACH_SAFE (existing, existing_next, hmap_node,
> &table->existing) {
> +        hmap_remove(&table->existing, &existing->hmap_node);
> +        ds_destroy(&existing->info);
> +        free(existing);
> +    }
> +    hmap_destroy(&table->existing);
> +}
> +
> +/* Finds and returns a group_info in 'existing' whose key is identical
> + * to 'target''s key, or NULL if there is none. */
> +static struct ovn_extend_table_info *
> +ovn_extend_table_lookup(struct hmap *exisiting,
> +                        const struct ovn_extend_table_info *target)
> +{
> +    struct ovn_extend_table_info *e;
> +
> +    HMAP_FOR_EACH_WITH_HASH (e, hmap_node, target->hmap_node.hash,
> +                             exisiting) {
> +        if (e->table_id == target->table_id) {
> +            return e;
> +        }
> +   }
> +    return NULL;
> +}
> +
> +/* Clear either desired or existing in ovn_extend_table. */
> +void
> +ovn_extend_table_clear(struct ovn_extend_table *table, bool existing)
> +{
> +    struct ovn_extend_table_info *g, *next;
> +    struct hmap *target = existing ? &table->existing : &table->desired;
> +
> +    HMAP_FOR_EACH_SAFE (g, next, hmap_node, target) {
> +        hmap_remove(target, &g->hmap_node);
> +        /* Don't unset bitmap for desired group_info if the group_id
> +         * was not freshly reserved. */
> +        if (existing || g->new_table_id) {
> +            bitmap_set0(table->table_ids, g->table_id);
> +        }
> +        ds_destroy(&g->info);
> +        free(g);
> +    }
> +}
> +
> +void
> +ovn_extend_table_install_desired(struct ovn_extend_table *table,
> +                                 struct ovs_list *msgs)
> +{
> +    struct ovn_extend_table_info *desired;
> +
> +    /* Iterate through all the desired tables. If there are new ones,
> +     * add them to the switch. */
> +    HMAP_FOR_EACH (desired, hmap_node, &table->desired) {
> +        if (!ovn_extend_table_lookup(&table->existing, desired)) {
> +            /* Create and install new group. */
> +            table->create(desired->table_id, &desired->info, msgs);
> +        }
> +    }
> +}
> +
> +void
> +ovn_extend_table_remove_existing(struct ovn_extend_table *table,
> +                                 struct ovs_list *msgs)
> +{
> +    struct ovn_extend_table_info *existing, *next;
> +
> +    /* Iterate through the installed tables from previous runs. If they
> +     * are not needed delete them. */
> +    HMAP_FOR_EACH_SAFE (existing, next, hmap_node, &table->existing) {
> +        if (!ovn_extend_table_lookup(&table->desired, existing)) {
> +
> +            table->remove(existing->table_id, msgs);
> +
> +            /* Remove 'installed' from 'groups->existing' */
> +            hmap_remove(&table->existing, &existing->hmap_node);
> +            ds_destroy(&existing->info);
> +
> +            /* Dealloc group_id. */
> +            bitmap_set0(table->table_ids, existing->table_id);
> +            free(existing);
> +        }
> +    }
> +}
> +
> +void
> +ovn_extend_table_move(struct ovn_extend_table *table)
> +{
> +    struct ovn_extend_table_info *desired, *next;
> +
> +    /* Move the contents of desired to existing. */
> +    HMAP_FOR_EACH_SAFE (desired, next, hmap_node, &table->desired) {
> +        hmap_remove(&table->desired, &desired->hmap_node);
> +
> +        if (!ovn_extend_table_lookup(&table->existing, desired)) {
> +            hmap_insert(&table->existing, &desired->hmap_node,
> +                        desired->hmap_node.hash);
> +        } else {
> +           ds_destroy(&desired->info);
> +           free(desired);
> +        }
> +    }
> +}
> +
> +/* Assign a new table ID for the table information from the bitmap.
> + * If it already exists, return the old ID. */
> +uint32_t
> +ovn_extend_table_assign_id(struct ovn_extend_table *table, struct ds *ds)
> +{
> +    uint32_t table_id = 0, hash;
> +    struct ovn_extend_table_info *table_info;
> +
> +    hash = hash_string(ds_cstr(ds), 0);
> +
> +    /* Check whether we have non installed but allocated group_id. */
> +    HMAP_FOR_EACH_WITH_HASH (table_info, hmap_node, hash,
> &table->desired) {
> +        if (!strcmp(ds_cstr(&table_info->info), ds_cstr(ds))) {
> +            return table_info->table_id;
> +        }
> +    }
> +
> +    /* Check whether we already have an installed entry for this
> +     * combination. */
> +    HMAP_FOR_EACH_WITH_HASH (table_info, hmap_node, hash,
> &table->existing) {
> +        if (!strcmp(ds_cstr(&table_info->info), ds_cstr(ds))) {
> +            table_id = table_info->table_id;
> +        }
> +    }
> +
> +    bool new_table_id = false;
> +    if (!table_id) {
> +        /* Reserve a new group_id. */
> +        table_id = bitmap_scan(table->table_ids, 0, 1, MAX_EXT_TABLE_ID +
> 1);
> +        new_table_id = true;
> +    }
> +
> +    if (table_id == MAX_EXT_TABLE_ID + 1) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> +        VLOG_ERR_RL(&rl, "%"PRIu32" out of table ids.", table_id);
> +        return EXT_TABLE_ID_INVALID;
> +    }
> +    bitmap_set1(table->table_ids, table_id);
> +
> +    table_info = xmalloc(sizeof *table_info);
> +    ds_clone(&table_info->info, ds);
> +    table_info->table_id = table_id;
> +    table_info->hmap_node.hash = hash;
> +    table_info->new_table_id = new_table_id;
> +
> +    hmap_insert(&table->desired,
> +                &table_info->hmap_node, table_info->hmap_node.hash);
> +
> +    return table_id;
> +}
> diff --git a/ovn/lib/extend-table.h b/ovn/lib/extend-table.h
> new file mode 100644
> index 000000000000..ededba59dc01
> --- /dev/null
> +++ b/ovn/lib/extend-table.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (c) 2017 DtDream Technology Co.,Ltd.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef EXTEND_TABLE_H
> +#define EXTEND_TABLE_H 1
> +
> +#define MAX_EXT_TABLE_ID 65535
> +#define EXT_TABLE_ID_INVALID 0
> +
> +#include "openvswitch/dynamic-string.h"
> +#include "openvswitch/hmap.h"
> +#include "openvswitch/list.h"
> +
> +/* Used to manage expansion tables associated with Flow table,
> + * such as the Group Table or Meter Table. */
> +struct ovn_extend_table {
> +    unsigned long *table_ids;  /* Used as a bitmap with value set
> +                                * for allocated group ids in either
> +                                * desired or existing. */
> +    struct hmap desired;
> +    struct hmap existing;
> +
> +    /* Used to create a form entity through the openflow channel. */
> +    void (*create)(uint32_t, struct ds *, struct ovs_list *);
> +
> +    /* Used to remove a form entity through the openflow channel. */
> +    void (*remove)(uint32_t, struct ovs_list *);
> +};
> +
> +struct ovn_extend_table_info {
> +    struct hmap_node hmap_node;
> +    struct ds info;     /* Details string for the table entity. */
> +    uint32_t table_id;
> +    bool new_table_id;  /* 'True' if 'table_id' was reserved from
> +                         * ovn_extend_table's 'table_ids' bitmap. */
> +};
> +
> +void ovn_extend_table_init(struct ovn_extend_table *table);
> +
> +void ovn_extend_table_destroy(struct ovn_extend_table *table);
> +
> +void ovn_extend_table_install_desired(struct ovn_extend_table *table,
> +                                      struct ovs_list *msgs);
> +
> +void ovn_extend_table_remove_existing(struct ovn_extend_table *table,
> +                                      struct ovs_list *msgs);
> +
> +/* Move the contents of desired to existing. */
> +void ovn_extend_table_move(struct ovn_extend_table *table);
> +
> +void ovn_extend_table_clear(struct ovn_extend_table *table, bool
> existing);
> +
> +uint32_t ovn_extend_table_assign_id(struct ovn_extend_table *table,
> +                                    struct ds *ds);
> +
> +#endif /* ovn/lib/extend-table.h */
> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
> index f9a5085f7185..4f65ee9d1004 100644
> --- a/tests/test-ovn.c
> +++ b/tests/test-ovn.c
> @@ -33,6 +33,7 @@
>  #include "ovn/lex.h"
>  #include "ovn/lib/logical-fields.h"
>  #include "ovn/lib/ovn-l7.h"
> +#include "ovn/lib/extend-table.h"
>  #include "ovs-thread.h"
>  #include "ovstest.h"
>  #include "openvswitch/shash.h"
> @@ -1207,11 +1208,8 @@ test_parse_actions(struct ovs_cmdl_context *ctx
> OVS_UNUSED)
>      create_gen_opts(&dhcp_opts, &dhcpv6_opts, &nd_ra_opts);
>
>      /* Initialize group ids. */
> -    struct group_table group_table;
> -    group_table.group_ids = bitmap_allocate(MAX_OVN_GROUPS);
> -    bitmap_set1(group_table.group_ids, 0); /* Group id 0 is invalid. */
> -    hmap_init(&group_table.desired_groups);
> -    hmap_init(&group_table.existing_groups);
> +    struct ovn_extend_table group_table;
> +    ovn_extend_table_init(&group_table);
>
>      simap_init(&ports);
>      simap_put(&ports, "eth0", 5);
> --
> 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