On 7/31/20 7:48 PM, Han Zhou wrote:
> nb_cfg as a mechanism to "ping" OVN control plane is very useful
> in many ways. However, the current implementation will trigger
> update notifications flooding in the whole control plane. Each
> HV updates to SB the nb_cfg number and all these updates are
> notified to all the other HVs, which is O(n^2). Although updates
> are batched in fewers notifications than n^2, it still generates
> significant load on SB DB and ovn-controllers.
> 
> To solve this problem and make the mechanism more useful in large
> scale producation deployment, this patch separates the per HV
> *private* data (write only by the owning chassis and not
> interesting to any other HVs) from the Chassis table to a separate
> table, so that each HV can conditionally monitor and get updates
> only for its own record.
> 
> Test result shows great improvement:
> In a test environment with 1200 sandbox HVs, and 12K ports created
> on 80 lswitches and 1 lrouter, do the sync test when the system
> is idle, with command:
> 
>     time ovn-nbctl --wait=hv sync
> 
> Original result:
> real    0m13.724s
> user    0m0.295s
> sys     0m0.012s
> 
> With this patch:
> real    0m3.255s
> user    0m0.248s
> sys     0m0.020s
> 
> Also, regarding backwards compatibility note that the nb_cfg from the
> Chassis table is no longer updated. If any system is relying on this
> mechanism they should start using the nb_cfg from the Chassis_Private
> table from now on.
> 
> Co-authored-by: Lucas Alvares Gomes <[email protected]>
> Signed-off-by: Lucas Alvares Gomes <[email protected]>
> Signed-off-by: Han Zhou <[email protected]>

Hi Han, Lucas,

This version of the patch looks good to me overall. I only have a couple
of comments below.

> ---
> v3 -> v4:
> - Change the column "chassis_name" back to "name", as requested by Lucas.
> 
> v2 -> v3:
> - Rebase on master.
> - Fixed the RBAC problem found by Numan in v2.
> - Took care of "is-remote" check.
> - Renamed column "name" in Chassis_Private table to "chassis_name" to
>   be more clear that it is a reference to the Chassis table.
> - Removed TODO about ovsdb monitor_cond change since there is no plan
>   to do that.
> - Updated test result in commit message based on latest scale test,
>   which is quite different from the old one because I-P has been added.
> 
>  NEWS                        |  5 +++++
>  controller/chassis.c        | 20 +++++++++++++++--
>  controller/chassis.h        |  8 +++++--
>  controller/ovn-controller.c | 37 ++++++++++++++++++++++++++------
>  lib/chassis-index.c         | 26 +++++++++++++++++++++++
>  lib/chassis-index.h         |  6 ++++++
>  northd/ovn-northd.c         | 52 
> +++++++++++++++++++++++++++++++++++++--------
>  ovn-sb.ovsschema            | 13 ++++++++++--
>  ovn-sb.xml                  | 37 ++++++++++++++++++++++++++++----
>  tests/ovn-controller.at     | 26 +++++++++++++++++++++++
>  10 files changed, 205 insertions(+), 25 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index f89a93a..a1ce4e8 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -6,6 +6,11 @@ Post-v20.06.0
>       OVN DHCPv4 responder.
>     - Added support for DHCP domain search option (119) in native
>       OVN DHCPv4 responder.
> +   - The nb_cfg column from the Chassis table in the OVN Southbound
> +     database has been deprecated and is no longer updated. A new table
> +     called Chassis_Private now contains the nb_cfg column which is updated
> +     by incrementing the value in the NB_Global table, CMSes relying on
> +     this mechanism should update their code to use this new table.
>  
>  OVN v20.06.0
>  --------------------------
> diff --git a/controller/chassis.c b/controller/chassis.c
> index eec270e..e289f5f 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -604,14 +604,18 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
>  const struct sbrec_chassis *
>  chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>              struct ovsdb_idl_index *sbrec_chassis_by_name,
> +            struct ovsdb_idl_index *sbrec_chassis_private_by_name,
>              const struct ovsrec_open_vswitch_table *ovs_table,
>              const struct sbrec_chassis_table *chassis_table,
>              const char *chassis_id,
>              const struct ovsrec_bridge *br_int,
> -            const struct sset *transport_zones)
> +            const struct sset *transport_zones,
> +            const struct sbrec_chassis_private **chassis_private)
>  {
>      struct ovs_chassis_cfg ovs_cfg;
>  
> +    *chassis_private = NULL;
> +
>      /* Get the chassis config from the ovs table. */
>      ovs_chassis_cfg_init(&ovs_cfg);
>      if (!chassis_parse_ovs_config(ovs_table, br_int, &ovs_cfg)) {
> @@ -638,6 +642,16 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                                        !existed ? "registering" : "updating",
>                                        chassis_id);
>          }
> +
> +        const struct sbrec_chassis_private *chassis_private_rec =
> +            chassis_private_lookup_by_name(sbrec_chassis_private_by_name,
> +                                           chassis_id);
> +        if (!chassis_private_rec && ovnsb_idl_txn) {
> +            chassis_private_rec = 
> sbrec_chassis_private_insert(ovnsb_idl_txn);
> +            sbrec_chassis_private_set_name(chassis_private_rec,
> +                                           chassis_id);
> +        }
> +        *chassis_private = chassis_private_rec;
>      }
>  
>      ovs_chassis_cfg_destroy(&ovs_cfg);
> @@ -693,7 +707,8 @@ chassis_get_mac(const struct sbrec_chassis *chassis_rec,
>   * required. */
>  bool
>  chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
> -                const struct sbrec_chassis *chassis_rec)
> +                const struct sbrec_chassis *chassis_rec,
> +                const struct sbrec_chassis_private *chassis_private_rec)
>  {
>      if (!chassis_rec) {
>          return true;
> @@ -703,6 +718,7 @@ chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
>                                    "ovn-controller: unregistering chassis 
> '%s'",
>                                    chassis_rec->name);
>          sbrec_chassis_delete(chassis_rec);
> +        sbrec_chassis_private_delete(chassis_private_rec);

I don't think it should happen in real runs but wouldn't it be better to
check for chassis_private_rec != NULL? Just in case we end up in a
scenario where chassis_rec != NULL && chassis_private_rec == NULL.

>      }
>      return false;
>  }
> diff --git a/controller/chassis.h b/controller/chassis.h
> index 178d295..81055b4 100644
> --- a/controller/chassis.h
> +++ b/controller/chassis.h
> @@ -17,6 +17,7 @@
>  #define OVN_CHASSIS_H 1
>  
>  #include <stdbool.h>
> +#include "lib/ovn-sb-idl.h"
>  
>  struct ovsdb_idl;
>  struct ovsdb_idl_index;
> @@ -33,12 +34,15 @@ void chassis_register_ovs_idl(struct ovsdb_idl *);
>  const struct sbrec_chassis *chassis_run(
>      struct ovsdb_idl_txn *ovnsb_idl_txn,
>      struct ovsdb_idl_index *sbrec_chassis_by_name,
> +    struct ovsdb_idl_index *sbrec_chassis_private_by_name,
>      const struct ovsrec_open_vswitch_table *,
>      const struct sbrec_chassis_table *,
>      const char *chassis_id, const struct ovsrec_bridge *br_int,
> -    const struct sset *transport_zones);
> +    const struct sset *transport_zones,
> +    const struct sbrec_chassis_private **chassis_private);
>  bool chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
> -                     const struct sbrec_chassis *);
> +                     const struct sbrec_chassis *,
> +                     const struct sbrec_chassis_private *);
>  bool chassis_get_mac(const struct sbrec_chassis *chassis,
>                       const char *bridge_mapping,
>                       struct eth_addr *chassis_mac);
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 5ca32ac..52c1d86 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -155,6 +155,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>      struct ovsdb_idl_condition ce =  OVSDB_IDL_CONDITION_INIT(&ce);
>      struct ovsdb_idl_condition ip_mcast = 
> OVSDB_IDL_CONDITION_INIT(&ip_mcast);
>      struct ovsdb_idl_condition igmp = OVSDB_IDL_CONDITION_INIT(&igmp);
> +    struct ovsdb_idl_condition chprv = OVSDB_IDL_CONDITION_INIT(&chprv);
>  
>      if (monitor_all) {
>          ovsdb_idl_condition_add_clause_true(&pb);
> @@ -165,6 +166,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>          ovsdb_idl_condition_add_clause_true(&ce);
>          ovsdb_idl_condition_add_clause_true(&ip_mcast);
>          ovsdb_idl_condition_add_clause_true(&igmp);
> +        ovsdb_idl_condition_add_clause_true(&chprv);
>          goto out;
>      }
>  
> @@ -196,6 +198,10 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
>                                                    &chassis->header_.uuid);
>          sbrec_igmp_group_add_clause_chassis(&igmp, OVSDB_F_EQ,
>                                              &chassis->header_.uuid);
> +
> +        /* Monitors Chassis_Private record for current chassis only */
> +        sbrec_chassis_private_add_clause_name(&chprv, OVSDB_F_EQ,
> +                                              chassis->name);
>      }
>      if (local_ifaces) {
>          const char *name;
> @@ -229,6 +235,7 @@ out:
>      sbrec_controller_event_set_condition(ovnsb_idl, &ce);
>      sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast);
>      sbrec_igmp_group_set_condition(ovnsb_idl, &igmp);
> +    sbrec_chassis_private_set_condition(ovnsb_idl, &chprv);
>      ovsdb_idl_condition_destroy(&pb);
>      ovsdb_idl_condition_destroy(&lf);
>      ovsdb_idl_condition_destroy(&mb);
> @@ -237,6 +244,7 @@ out:
>      ovsdb_idl_condition_destroy(&ce);
>      ovsdb_idl_condition_destroy(&ip_mcast);
>      ovsdb_idl_condition_destroy(&igmp);
> +    ovsdb_idl_condition_destroy(&chprv);
>  }
>  
>  static const char *
> @@ -2113,6 +2121,8 @@ main(int argc, char *argv[])
>  
>      struct ovsdb_idl_index *sbrec_chassis_by_name
>          = chassis_index_create(ovnsb_idl_loop.idl);
> +    struct ovsdb_idl_index *sbrec_chassis_private_by_name
> +        = chassis_private_index_create(ovnsb_idl_loop.idl);
>      struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath
>          = mcast_group_index_create(ovnsb_idl_loop.idl);
>      struct ovsdb_idl_index *sbrec_logical_flow_by_logical_datapath
> @@ -2141,7 +2151,8 @@ main(int argc, char *argv[])
>          = igmp_group_index_create(ovnsb_idl_loop.idl);
>  
>      ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
> -    ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
> +    ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> +                         &sbrec_chassis_private_col_nb_cfg);
>  
>      /* Omit the external_ids column of all the tables except for -
>       *  - DNS. pinctrl.c uses the external_ids column of DNS,
> @@ -2178,6 +2189,10 @@ main(int argc, char *argv[])
>       * other_config column so we no longer need to monitor it */
>      ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, 
> &sbrec_chassis_col_external_ids);
>  
> +    /* Do not monitor Chassis_Private external_ids */
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl,
> +                   &sbrec_chassis_private_col_external_ids);
> +
>      update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, false);
>  
>      stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);
> @@ -2384,10 +2399,13 @@ main(int argc, char *argv[])
>                  process_br_int(ovs_idl_txn, bridge_table, ovs_table);
>              const char *chassis_id = get_ovs_chassis_id(ovs_table);
>              const struct sbrec_chassis *chassis = NULL;
> +            const struct sbrec_chassis_private *chassis_private = NULL;
>              if (chassis_id) {
>                  chassis = chassis_run(ovnsb_idl_txn, sbrec_chassis_by_name,
> +                                      sbrec_chassis_private_by_name,
>                                        ovs_table, chassis_table, chassis_id,
> -                                      br_int, &transport_zones);
> +                                      br_int, &transport_zones,
> +                                      &chassis_private);
>              }
>  
>              if (br_int) {
> @@ -2512,10 +2530,10 @@ main(int argc, char *argv[])
>                  engine_set_force_recompute(false);
>              }
>  
> -            if (ovnsb_idl_txn && chassis) {
> +            if (ovnsb_idl_txn && chassis_private) {
>                  int64_t cur_cfg = ofctrl_get_cur_cfg();
> -                if (cur_cfg && cur_cfg != chassis->nb_cfg) {
> -                    sbrec_chassis_set_nb_cfg(chassis, cur_cfg);
> +                if (cur_cfg && cur_cfg != chassis_private->nb_cfg) {
> +                    sbrec_chassis_private_set_nb_cfg(chassis_private, 
> cur_cfg);
>                  }
>              }
>  
> @@ -2618,10 +2636,17 @@ main(int argc, char *argv[])
>                     ? chassis_lookup_by_name(sbrec_chassis_by_name, 
> chassis_id)
>                     : NULL);
>  
> +            const struct sbrec_chassis_private *chassis_private
> +                = (chassis_id
> +                   ? chassis_private_lookup_by_name(
> +                         sbrec_chassis_private_by_name, chassis_id)
> +                   : NULL);
> +
>              /* Run all of the cleanup functions, even if one of them returns
>               * false. We're done if all of them return true. */
>              done = binding_cleanup(ovnsb_idl_txn, port_binding_table, 
> chassis);
> -            done = chassis_cleanup(ovnsb_idl_txn, chassis) && done;
> +            done = chassis_cleanup(ovnsb_idl_txn,
> +                                   chassis, chassis_private) && done;
>              done = encaps_cleanup(ovs_idl_txn, br_int) && done;
>              done = igmp_group_cleanup(ovnsb_idl_txn, sbrec_igmp_group) && 
> done;
>              if (done) {
> diff --git a/lib/chassis-index.c b/lib/chassis-index.c
> index 39066f4..13120fe 100644
> --- a/lib/chassis-index.c
> +++ b/lib/chassis-index.c
> @@ -41,6 +41,32 @@ chassis_lookup_by_name(struct ovsdb_idl_index 
> *sbrec_chassis_by_name,
>  }
>  
>  struct ovsdb_idl_index *
> +chassis_private_index_create(struct ovsdb_idl *idl)
> +{
> +    return ovsdb_idl_index_create1(idl,
> +                                   &sbrec_chassis_private_col_name);
> +}
> +
> +/* Finds and returns the chassis with the given 'name', or NULL if no such
> + * chassis exists. */
> +const struct sbrec_chassis_private *
> +chassis_private_lookup_by_name(
> +    struct ovsdb_idl_index *sbrec_chassis_private_by_name,
> +    const char *name)
> +{
> +    struct sbrec_chassis_private *target =
> +        sbrec_chassis_private_index_init_row(sbrec_chassis_private_by_name);
> +    sbrec_chassis_private_index_set_name(target, name);
> +
> +    struct sbrec_chassis_private *retval = sbrec_chassis_private_index_find(
> +        sbrec_chassis_private_by_name, target);
> +
> +    sbrec_chassis_private_index_destroy_row(target);
> +
> +    return retval;
> +}
> +
> +struct ovsdb_idl_index *
>  ha_chassis_group_index_create(struct ovsdb_idl *idl)
>  {
>      return ovsdb_idl_index_create1(idl, &sbrec_ha_chassis_group_col_name);
> diff --git a/lib/chassis-index.h b/lib/chassis-index.h
> index 302e5f0..b9b331f 100644
> --- a/lib/chassis-index.h
> +++ b/lib/chassis-index.h
> @@ -23,6 +23,12 @@ struct ovsdb_idl_index *chassis_index_create(struct 
> ovsdb_idl *);
>  const struct sbrec_chassis *chassis_lookup_by_name(
>      struct ovsdb_idl_index *sbrec_chassis_by_name, const char *name);
>  
> +struct ovsdb_idl_index *chassis_private_index_create(struct ovsdb_idl *);
> +
> +const struct sbrec_chassis_private *
> +chassis_private_lookup_by_name(
> +    struct ovsdb_idl_index *sbrec_chassis_private_by_name, const char *name);
> +
>  struct ovsdb_idl_index *ha_chassis_group_index_create(struct ovsdb_idl *idl);
>  const struct sbrec_ha_chassis_group *ha_chassis_group_lookup_by_name(
>      struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name, const char *name);
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 573a88c..ec10120 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11892,6 +11892,11 @@ static const char *rbac_chassis_update[] =
>      {"nb_cfg", "external_ids", "encaps", "vtep_logical_switches",
>       "other_config"};
>  
> +static const char *rbac_chassis_private_auth[] =
> +    {"name"};
> +static const char *rbac_chassis_private_update[] =
> +    {"nb_cfg"};
> +
>  static const char *rbac_encap_auth[] =
>      {"chassis_name"};
>  static const char *rbac_encap_update[] =
> @@ -11930,6 +11935,14 @@ static struct rbac_perm_cfg {
>          .n_update = ARRAY_SIZE(rbac_chassis_update),
>          .row = NULL
>      },{
> +        .table = "Chassis_Private",
> +        .auth = rbac_chassis_private_auth,
> +        .n_auth = ARRAY_SIZE(rbac_chassis_private_auth),
> +        .insdel = true,
> +        .update = rbac_chassis_private_update,
> +        .n_update = ARRAY_SIZE(rbac_chassis_private_update),
> +        .row = NULL
> +    },{
>          .table = "Encap",
>          .auth = rbac_encap_auth,
>          .n_auth = ARRAY_SIZE(rbac_encap_auth),
> @@ -12086,6 +12099,7 @@ check_and_update_rbac(struct northd_context *ctx)
>  /* Updates the sb_cfg and hv_cfg columns in the northbound NB_Global table. 
> */
>  static void
>  update_northbound_cfg(struct northd_context *ctx,
> +                      struct ovsdb_idl_index *sbrec_chassis_by_name,
>                        struct ovsdb_idl_loop *sb_loop)
>  {
>      /* Update northbound sb_cfg if appropriate. */
> @@ -12098,12 +12112,25 @@ update_northbound_cfg(struct northd_context *ctx,
>      /* Update northbound hv_cfg if appropriate. */
>      if (nbg) {
>          /* Find minimum nb_cfg among all chassis. */
> -        const struct sbrec_chassis *chassis;
> +        const struct sbrec_chassis_private *chassis_priv;
>          int64_t hv_cfg = nbg->nb_cfg;
> -        SBREC_CHASSIS_FOR_EACH (chassis, ctx->ovnsb_idl) {
> -            if (!smap_get_bool(&chassis->other_config, "is-remote", false) &&
> -                chassis->nb_cfg < hv_cfg) {
> -                hv_cfg = chassis->nb_cfg;
> +        SBREC_CHASSIS_PRIVATE_FOR_EACH (chassis_priv, ctx->ovnsb_idl) {
> +            const struct sbrec_chassis *chassis =
> +                chassis_lookup_by_name(sbrec_chassis_by_name,
> +                                       chassis_priv->name);

To avoid this O(log(n)) lookup would it be an option to store a
reference from Chassis_Private to Chassis in the SB schema?

Thanks,
Dumitru

> +            if (chassis) {
> +                if (smap_get_bool(&chassis->other_config,
> +                                  "is-remote", false)) {
> +                    /* Skip remote chassises. */
> +                    continue;
> +                }
> +            } else {
> +                VLOG_WARN("Chassis not found for Chassis_Private record, "
> +                          "name: %s", chassis_priv->name);
> +            }
> +
> +            if (chassis_priv->nb_cfg < hv_cfg) {
> +                hv_cfg = chassis_priv->nb_cfg;
>              }
>          }
>  
> @@ -12116,7 +12143,9 @@ update_northbound_cfg(struct northd_context *ctx,
>  
>  /* Handle a fairly small set of changes in the southbound database. */
>  static void
> -ovnsb_db_run(struct northd_context *ctx, struct ovsdb_idl_loop *sb_loop,
> +ovnsb_db_run(struct northd_context *ctx,
> +             struct ovsdb_idl_index *sbrec_chassis_by_name,
> +             struct ovsdb_idl_loop *sb_loop,
>               struct hmap *ports)
>  {
>      if (!ctx->ovnnb_txn || !ovsdb_idl_has_ever_connected(ctx->ovnsb_idl)) {
> @@ -12125,7 +12154,7 @@ ovnsb_db_run(struct northd_context *ctx, struct 
> ovsdb_idl_loop *sb_loop,
>  
>      struct shash ha_ref_chassis_map = SHASH_INITIALIZER(&ha_ref_chassis_map);
>      handle_port_binding_changes(ctx, ports, &ha_ref_chassis_map);
> -    update_northbound_cfg(ctx, sb_loop);
> +    update_northbound_cfg(ctx, sbrec_chassis_by_name, sb_loop);
>      if (ctx->ovnsb_txn) {
>          update_sb_ha_group_ref_chassis(&ha_ref_chassis_map);
>      }
> @@ -12144,7 +12173,7 @@ ovn_db_run(struct northd_context *ctx,
>      hmap_init(&ports);
>      ovnnb_db_run(ctx, sbrec_chassis_by_name, ovnsb_idl_loop,
>                   &datapaths, &ports, &lr_list);
> -    ovnsb_db_run(ctx, ovnsb_idl_loop, &ports);
> +    ovnsb_db_run(ctx, sbrec_chassis_by_name, ovnsb_idl_loop, &ports);
>      destroy_datapaths_and_ports(&datapaths, &ports, &lr_list);
>  }
>  
> @@ -12397,10 +12426,15 @@ main(int argc, char *argv[])
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, 
> &sbrec_meter_band_col_burst_size);
>  
>      ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_chassis);
> -    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_chassis_col_name);
>      ovsdb_idl_add_column(ovnsb_idl_loop.idl, 
> &sbrec_chassis_col_other_config);
>  
> +    ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_chassis_private);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> +                         &sbrec_chassis_private_col_name);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> +                         &sbrec_chassis_private_col_nb_cfg);
> +
>      ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_ha_chassis);
>      add_column_noalert(ovnsb_idl_loop.idl,
>                         &sbrec_ha_chassis_col_chassis);
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 99c5de8..4098f19 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Southbound",
> -    "version": "2.8.2",
> -    "cksum": "464326363 21916",
> +    "version": "2.9.0",
> +    "cksum": "599935918 22295",
>      "tables": {
>          "SB_Global": {
>              "columns": {
> @@ -46,6 +46,15 @@
>                                                "max": "unlimited"}}},
>              "isRoot": true,
>              "indexes": [["name"]]},
> +        "Chassis_Private": {
> +            "columns": {
> +                "name": {"type": "string"},
> +                "nb_cfg": {"type": {"key": "integer"}},
> +                "external_ids": {
> +                    "type": {"key": "string", "value": "string",
> +                             "min": 0, "max": "unlimited"}}},
> +            "isRoot": true,
> +            "indexes": [["name"]]},
>          "Encap": {
>              "columns": {
>                  "type": {"type": {"key": {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 32281eb..fa56366 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -256,10 +256,8 @@
>      </column>
>  
>      <column name="nb_cfg">
> -      Sequence number for the configuration.  When 
> <code>ovn-controller</code>
> -      updates the configuration of a chassis from the contents of the
> -      southbound database, it copies <ref table="SB_Global" column="nb_cfg"/>
> -      from the <ref table="SB_Global"/> table into this column.
> +      Deprecated. This column is replaced by the <ref table="Chassis_Private"
> +      column="nb_cfg"/> column of the <ref table="Chassis_Private"/> table.
>      </column>
>  
>      <column name="other_config" key="ovn-bridge-mappings">
> @@ -366,6 +364,37 @@
>      </group>
>    </table>
>  
> +  <table name="Chassis_Private" title="Chassis Private">
> +    <p>
> +      Each row in this table maintains per chassis private data that are
> +      accessed only by the owning chassis (write only) and ovn-northd, not by
> +      any other chassis.  These data are stored in this separate table 
> instead
> +      of the <ref table="Chassis"/> table for performance considerations:
> +      the rows in this table can be conditionally monitored by chassises so
> +      that each chassis only get update notifications for its own row, to 
> avoid
> +      unnecessary chassis private data update flooding in a large scale
> +      deployment.
> +    </p>
> +
> +    <column name="name">
> +      The name of the chassis that owns these chassis-private data.
> +    </column>
> +
> +    <column name="nb_cfg">
> +      Sequence number for the configuration.  When 
> <code>ovn-controller</code>
> +      updates the configuration of a chassis from the contents of the
> +      southbound database, it copies <ref table="SB_Global" column="nb_cfg"/>
> +      from the <ref table="SB_Global"/> table into this column.
> +    </column>
> +
> +    <group title="Common Columns">
> +      The overall purpose of these columns is described under <code>Common
> +      Columns</code> at the beginning of this document.
> +
> +      <column name="external_ids"/>
> +    </group>
> +  </table>
> +
>    <table name="Encap" title="Encapsulation Types">
>      <p>
>        The <ref column="encaps" table="Chassis"/> column in the <ref
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 77936c7..1b96934 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -321,3 +321,29 @@ as ovn-sb
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>  
>  AT_CLEANUP
> +
> +# Checks that ovn-controller increments the nb_cfg value in the 
> Chassis_Private table
> +AT_SETUP([ovn-controller - Bump Chassis_Private nb_cfg value])
> +AT_KEYWORDS([ovn])
> +ovn_start
> +
> +net_add n1
> +sim_add hv
> +as hv
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +OVS_WAIT_UNTIL([test xhv = x`ovn-sbctl --columns name --bare find chassis`])
> +
> +# Bump the NB_Global nb_cfg value
> +nb_global_id=$(ovn-nbctl --columns _uuid --bare find nb_global)
> +ovn-nbctl set NB_Global ${nb_global_id} nb_cfg=999
> +
> +# ovn-controller should bump the nb_cfg in the chassis_private table
> +OVS_WAIT_UNTIL([test x999 = x`ovn-sbctl --columns nb_cfg --bare find 
> chassis_private`])
> +
> +# Assert that the the nb_cfg from the Chassis table was not incremented
> +OVS_WAIT_UNTIL([test x0 = x`ovn-sbctl --columns nb_cfg --bare find chassis`])
> +
> +OVN_CLEANUP([hv])
> +AT_CLEANUP
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to