Re: [ovs-dev] [PATCH ovn v5 1/2] northd: Make `vxlan_mode` a global variable.

Wed, 05 Jun 2024 13:14:00 -0700 

On 6/5/24 08:51, Vladislav Odintsov wrote:

Hi Mark,

Thanks for the review!
Please, see below.

regards,
Vladislav Odintsov 

-----Original Message-----
From: Mark Michelson <[email protected]>
Date: Tuesday, 4 June 2024 at 03:45
To: Vladislav Odintsov <[email protected]>, <[email protected]>
Subject: Re: [ovs-dev] [PATCH ovn v5 1/2] northd: Make `vxlan_mode` a global 
variable.

     Hi Vladislav,

     Generally speaking, I agree with this change. However, I think that
     setting a global variable from an incremental processing engine node
     runner feels wrong.

The init_vxlan_mode() is called inside the en_global_config_run() only to
initialize global value, which is then read by get_ovn_max_dp_key_local() to
fill the "max_tunid" variable inside incremental processing engine node.

Which drawbacks do you see of such variable initialization?


The biggest drawbacks are:
* Reasoning about "ownership" of the vxlan_mode global variable
* Maintenance of the en_global_config I-P engine node.
On the first point, since vxlan_mode is a global variable in northd.c, it's not obvious that the owner of this data is the en_global_config engine node. It's an easy mistake for someone to reference the variable before it has been initialized, for instance. However, if the boolean is on the ed_type_global_config struct, then it's clear to see that this data is scoped to the en_global_config engine node.
On the second point, if someone were to overhaul the en_global_config engine node, it would be an easy mistake to make to not notice that vxlan_mode is being set by the engine node. I could see a developer splitting the node into separate nodes, for instance. In doing so, the developer could easily miss that the global vxlan_mode is being set by the engine node, since it's hidden behind an init_ function call. However, placing vxlan_mode on the ed_type_global_config makes it more clear that the en_global_config engine node is responsible for setting the value. 



     I think that instead, the "vxlan_mode" variable you have introduced
     should be a field on struct ed_type_global_config. This way, the engine
     node is only modifying data local to itself.

I guess, that moving this to the struct ed_type_global_config will make the code
a bit more complex: we have to pass this variable through all function calls to
be able to read vxlan_mode value inside ovn_datapath_assign_requested_tnl_id(),
ovn_port_assign_requested_tnl_id() and ovn_port_allocate_key().
I think dependency injection makes the code easier to read, understand, and maintain rather than making it more complex. It's clearer that the data from the en_global_config engine node is needed in all of the functions you listed if those functions require an ed_type_global_config argument. 



Apart of this, the "vxlan_mode" variable has the same "global" meaning as
"use_ct_inv_match", "check_lsp_is_up", "use_common_zone" and other global
variables, which configure the global OVN behaviour. The difference is that it
is required to read its value inside the en_global_config_run() to reflect the
max_tunid back to NB_Global.


Personally, I don't like those global variables either :)
But those globals are also set within northd.c, and are initialized at the begining of a DB run. From the perspective of northd processing, they are truly "global" in their scope. The engine nodes form a dependency tree, and so it's important that data that is scoped to a particular node is housed in that engine node's data. This way, when nodes are being created, it's clear to know which other engine nodes they depend on. If engine nodes are setting global variables, then it becomes harder to understand the dependencies. 



If the global variable setting is totally not acceptable from engine node, I
can propose another approach here. Revert init_vxlan_mode() back to
`bool is_vxlan_mode()` and assign global variable outside of global engine node:


diff --git a/northd/en-global-config.c b/northd/en-global-config.c
index 873649a89..df0f8e58c 100644
--- a/northd/en-global-config.c
+++ b/northd/en-global-config.c
@@ -115,8 +115,9 @@ en_global_config_run(struct engine_node *node , void *data)
                       config_data->svc_monitor_mac);
      }
- init_vxlan_mode(sbrec_chassis_table); 
-    char *max_tunid = xasprintf("%d", get_ovn_max_dp_key_local());
+    char *max_tunid = xasprintf("%d",
+                                get_ovn_max_dp_key_local(
+                                    is_vxlan_mode(sbrec_chassis_table)));
      smap_replace(options, "max_tunid", max_tunid);
      free(max_tunid);
diff --git a/northd/northd.c b/northd/northd.c 
index 0e0ae24db..9ac608f03 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -885,25 +885,24 @@ join_datapaths(const struct nbrec_logical_switch_table 
*nbrec_ls_table,
      }
  }
-void 
-init_vxlan_mode(const struct sbrec_chassis_table *sbrec_chassis_table)
+bool
+is_vxlan_mode(const struct sbrec_chassis_table *sbrec_chassis_table)
  {
      const struct sbrec_chassis *chassis;
      SBREC_CHASSIS_TABLE_FOR_EACH (chassis, sbrec_chassis_table) {
          for (int i = 0; i < chassis->n_encaps; i++) {
              if (!strcmp(chassis->encaps[i]->type, "vxlan")) {
-                vxlan_mode = true;
-                return;
+                return true;
              }
          }
      }
-    vxlan_mode = false;
+    return false;
  }
uint32_t 
-get_ovn_max_dp_key_local(void)
+get_ovn_max_dp_key_local(bool _vxlan_mode)
  {
-    if (vxlan_mode) {
+    if (_vxlan_mode) {
          /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for VXLAN mode. */
          return OVN_MAX_DP_VXLAN_KEY;
      }
@@ -916,9 +915,7 @@ ovn_datapath_allocate_key(struct hmap *datapaths, struct 
hmap *dp_tnlids,
  {
      if (!od->tunnel_key) {
          od->tunnel_key = ovn_allocate_tnlid(dp_tnlids, "datapath",
-                                            OVN_MIN_DP_KEY_LOCAL,
-                                            get_ovn_max_dp_key_local(),
-                                            hint);
+            OVN_MIN_DP_KEY_LOCAL, get_ovn_max_dp_key_local(vxlan_mode), hint);
          if (!od->tunnel_key) {
              if (od->sb) {
                  sbrec_datapath_binding_delete(od->sb);
@@ -17596,7 +17593,7 @@ ovnnb_db_run(struct northd_input *input_data,
      use_common_zone = smap_get_bool(input_data->nb_options, "use_common_zone",
                                      false);
- init_vxlan_mode(input_data->sbrec_chassis_table); 
+    vxlan_mode = is_vxlan_mode(input_data->sbrec_chassis_table);
build_datapaths(ovnsb_txn, 
                      input_data->nbrec_logical_switch_table,
diff --git a/northd/northd.h b/northd/northd.h
index be480003e..c613652e9 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -791,9 +791,9 @@ lr_has_multiple_gw_ports(const struct ovn_datapath *od)
      return od->n_l3dgw_ports > 1 && !od->is_gw_router;
  }
-void 
-init_vxlan_mode(const struct sbrec_chassis_table *sbrec_chassis_table);
+bool
+is_vxlan_mode(const struct sbrec_chassis_table *sbrec_chassis_table);
-uint32_t get_ovn_max_dp_key_local(void); 
+uint32_t get_ovn_max_dp_key_local(bool);
#endif /* NORTHD_H */ 


What do you think?

Or, maybe I totally misunderstood your idea here, please correct me if yes.
I think this idea is better than what is in the current patch. It also allows for you to add the NB global data as an argument to is_vxlan_mode() in patch 2 so that you can factor that into the decision about whether vxlan mode is enabled.
The potential upside to caching a boolean is that you don't have to repeatedly walk the SB chassis table to determine if vxlan mode is enabled. I have no idea if this is actually a performance concern, though. Since northd performance tests have not yet shown this to be a bottleneck, I think your alternate proposal of keeping is_vxlan_mode() is the best approach. 




     On 5/3/24 04:13, Vladislav Odintsov wrote:
     > This simplifies code and subsequent commit to explicitely disable VXLAN
     > mode is based on these changes.
     >
     > Also "VXLAN mode" term is introduced in ovn-architecture man page.
     >
     > Signed-off-by: Vladislav Odintsov <[email protected]>
     > ---
     >   northd/en-global-config.c |  4 +-
     >   northd/northd.c           | 85 +++++++++++++++++----------------------
     >   northd/northd.h           |  5 ++-
     >   ovn-architecture.7.xml    | 10 ++---
     >   4 files changed, 47 insertions(+), 57 deletions(-)
     >
     > diff --git a/northd/en-global-config.c b/northd/en-global-config.c
     > index 28c78a12c..873649a89 100644
     > --- a/northd/en-global-config.c
     > +++ b/northd/en-global-config.c
     > @@ -115,8 +115,8 @@ en_global_config_run(struct engine_node *node , void 
*data)
     >                        config_data->svc_monitor_mac);
     >       }
     >
     > -    char *max_tunid = xasprintf("%d",
     > -        get_ovn_max_dp_key_local(sbrec_chassis_table));
     > +    init_vxlan_mode(sbrec_chassis_table);
     > +    char *max_tunid = xasprintf("%d", get_ovn_max_dp_key_local());
     >       smap_replace(options, "max_tunid", max_tunid);
     >       free(max_tunid);
     >
     > diff --git a/northd/northd.c b/northd/northd.c
     > index 133cddb69..0e0ae24db 100644
     > --- a/northd/northd.c
     > +++ b/northd/northd.c
     > @@ -90,6 +90,10 @@ static bool use_ct_inv_match = true;
     >    */
     >   static bool default_acl_drop;
     >
     > +/* If this option is 'true' northd will use limited 24-bit space for 
datapath
     > + * and ports tunnel key allocation (12 bits for each instead of default 
16). */
     > +static bool vxlan_mode;
     > +
     >   #define MAX_OVN_TAGS 4096
     >

     >
     > @@ -881,40 +885,40 @@ join_datapaths(const struct 
nbrec_logical_switch_table *nbrec_ls_table,
     >       }
     >   }
     >
     > -static bool
     > -is_vxlan_mode(const struct sbrec_chassis_table *sbrec_chassis_table)
     > +void
     > +init_vxlan_mode(const struct sbrec_chassis_table *sbrec_chassis_table)
     >   {
     >       const struct sbrec_chassis *chassis;
     >       SBREC_CHASSIS_TABLE_FOR_EACH (chassis, sbrec_chassis_table) {
     >           for (int i = 0; i < chassis->n_encaps; i++) {
     >               if (!strcmp(chassis->encaps[i]->type, "vxlan")) {
     > -                return true;
     > +                vxlan_mode = true;
     > +                return;
     >               }
     >           }
     >       }
     > -    return false;
     > +    vxlan_mode = false;
     >   }
     >
     >   uint32_t
     > -get_ovn_max_dp_key_local(const struct sbrec_chassis_table 
*sbrec_chassis_table)
     > +get_ovn_max_dp_key_local(void)
     >   {
     > -    if (is_vxlan_mode(sbrec_chassis_table)) {
     > -        /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for vxlan mode. */
     > +    if (vxlan_mode) {
     > +        /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for VXLAN mode. */
     >           return OVN_MAX_DP_VXLAN_KEY;
     >       }
     >       return OVN_MAX_DP_KEY - OVN_MAX_DP_GLOBAL_NUM;
     >   }
     >
     >   static void
     > -ovn_datapath_allocate_key(const struct sbrec_chassis_table 
*sbrec_ch_table,
     > -                          struct hmap *datapaths, struct hmap 
*dp_tnlids,
     > +ovn_datapath_allocate_key(struct hmap *datapaths, struct hmap 
*dp_tnlids,
     >                             struct ovn_datapath *od, uint32_t *hint)
     >   {
     >       if (!od->tunnel_key) {
     >           od->tunnel_key = ovn_allocate_tnlid(dp_tnlids, "datapath",
     > -                                    OVN_MIN_DP_KEY_LOCAL,
     > -                                    
get_ovn_max_dp_key_local(sbrec_ch_table),
     > -                                    hint);
     > +                                            OVN_MIN_DP_KEY_LOCAL,
     > +                                            get_ovn_max_dp_key_local(),
     > +                                            hint);
     >           if (!od->tunnel_key) {
     >               if (od->sb) {
     >                   sbrec_datapath_binding_delete(od->sb);
     > @@ -927,7 +931,6 @@ ovn_datapath_allocate_key(const struct 
sbrec_chassis_table *sbrec_ch_table,
     >
     >   static void
     >   ovn_datapath_assign_requested_tnl_id(
     > -    const struct sbrec_chassis_table *sbrec_chassis_table,
     >       struct hmap *dp_tnlids, struct ovn_datapath *od)
     >   {
     >       const struct smap *other_config = (od->nbs
     > @@ -936,8 +939,7 @@ ovn_datapath_assign_requested_tnl_id(
     >       uint32_t tunnel_key = smap_get_int(other_config, 
"requested-tnl-key", 0);
     >       if (tunnel_key) {
     >           const char *interconn_ts = smap_get(other_config, 
"interconn-ts");
     > -        if (!interconn_ts && is_vxlan_mode(sbrec_chassis_table) &&
     > -            tunnel_key >= 1 << 12) {
     > +        if (!interconn_ts && vxlan_mode && tunnel_key >= 1 << 12) {
     >               static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 
1);
     >               VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for datapath %s is 
"
     >                            "incompatible with VXLAN", tunnel_key,
     > @@ -985,7 +987,6 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
     >                   const struct nbrec_logical_switch_table 
*nbrec_ls_table,
     >                   const struct nbrec_logical_router_table 
*nbrec_lr_table,
     >                   const struct sbrec_datapath_binding_table 
*sbrec_dp_table,
     > -                const struct sbrec_chassis_table *sbrec_chassis_table,
     >                   struct ovn_datapaths *ls_datapaths,
     >                   struct ovn_datapaths *lr_datapaths,
     >                   struct ovs_list *lr_list)
     > @@ -1000,12 +1001,11 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
     >       struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids);
     >       struct ovn_datapath *od;
     >       LIST_FOR_EACH (od, list, &both) {
     > -        ovn_datapath_assign_requested_tnl_id(sbrec_chassis_table, 
&dp_tnlids,
     > -                                             od);
     > +        ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od);
     >       }
     >       LIST_FOR_EACH (od, list, &nb_only) {
     > -        ovn_datapath_assign_requested_tnl_id(sbrec_chassis_table, 
&dp_tnlids,
     > -                                             od); }
     > +        ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od);
     > +    }
     >
     >       /* Keep nonconflicting tunnel IDs that are already assigned. */
     >       LIST_FOR_EACH (od, list, &both) {
     > @@ -1017,12 +1017,10 @@ build_datapaths(struct ovsdb_idl_txn *ovnsb_txn,
     >       /* Assign new tunnel ids where needed. */
     >       uint32_t hint = 0;
     >       LIST_FOR_EACH_SAFE (od, list, &both) {
     > -        ovn_datapath_allocate_key(sbrec_chassis_table,
     > -                                  datapaths, &dp_tnlids, od, &hint);
     > +        ovn_datapath_allocate_key(datapaths, &dp_tnlids, od, &hint);
     >       }
     >       LIST_FOR_EACH_SAFE (od, list, &nb_only) {
     > -        ovn_datapath_allocate_key(sbrec_chassis_table,
     > -                                  datapaths, &dp_tnlids, od, &hint);
     > +        ovn_datapath_allocate_key(datapaths, &dp_tnlids, od, &hint);
     >       }
     >
     >       /* Sync tunnel ids from nb to sb. */
     > @@ -3982,16 +3980,14 @@ ovn_port_add_tnlid(struct ovn_port *op, uint32_t 
tunnel_key)
     >    * that the I-P engine can fallback to recompute if needed; otherwise 
return
     >    * true (even if the key is not allocated). */
     >   static bool
     > -ovn_port_assign_requested_tnl_id(
     > -    const struct sbrec_chassis_table *sbrec_chassis_table, struct 
ovn_port *op)
     > +ovn_port_assign_requested_tnl_id(struct ovn_port *op)
     >   {
     >       const struct smap *options = (op->nbsp
     >                                     ? &op->nbsp->options
     >                                     : &op->nbrp->options);
     >       uint32_t tunnel_key = smap_get_int(options, "requested-tnl-key", 
0);
     >       if (tunnel_key) {
     > -        if (is_vxlan_mode(sbrec_chassis_table) &&
     > -                tunnel_key >= OVN_VXLAN_MIN_MULTICAST) {
     > +        if (vxlan_mode && tunnel_key >= OVN_VXLAN_MIN_MULTICAST) {
     >               static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 
1);
     >               VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for port %s "
     >                            "is incompatible with VXLAN",
     > @@ -4011,11 +4007,10 @@ ovn_port_assign_requested_tnl_id(
     >   }
     >
     >   static bool
     > -ovn_port_allocate_key(const struct sbrec_chassis_table 
*sbrec_chassis_table,
     > -                      struct ovn_port *op)
     > +ovn_port_allocate_key(struct ovn_port *op)
     >   {
     >       if (!op->tunnel_key) {
     > -        uint8_t key_bits = is_vxlan_mode(sbrec_chassis_table)? 12 : 16;
     > +        uint8_t key_bits = vxlan_mode ? 12 : 16;
     >           op->tunnel_key = ovn_allocate_tnlid(&op->od->port_tnlids, 
"port",
     >                                               1, (1u << (key_bits - 1)) 
- 1,
     >                                               &op->od->port_key_hint);
     > @@ -4035,7 +4030,6 @@ ovn_port_allocate_key(const struct 
sbrec_chassis_table *sbrec_chassis_table,
     >   static void
     >   build_ports(struct ovsdb_idl_txn *ovnsb_txn,
     >       const struct sbrec_port_binding_table *sbrec_port_binding_table,
     > -    const struct sbrec_chassis_table *sbrec_chassis_table,
     >       const struct sbrec_mirror_table *sbrec_mirror_table,
     >       const struct sbrec_mac_binding_table *sbrec_mac_binding_table,
     >       const struct sbrec_ha_chassis_group_table 
*sbrec_ha_chassis_group_table,
     > @@ -4069,10 +4063,10 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
     >       /* Assign explicitly requested tunnel ids first. */
     >       struct ovn_port *op;
     >       LIST_FOR_EACH (op, list, &both) {
     > -        ovn_port_assign_requested_tnl_id(sbrec_chassis_table, op);
     > +        ovn_port_assign_requested_tnl_id(op);
     >       }
     >       LIST_FOR_EACH (op, list, &nb_only) {
     > -        ovn_port_assign_requested_tnl_id(sbrec_chassis_table, op);
     > +        ovn_port_assign_requested_tnl_id(op);
     >       }
     >
     >       /* Keep nonconflicting tunnel IDs that are already assigned. */
     > @@ -4084,14 +4078,14 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
     >
     >       /* Assign new tunnel ids where needed. */
     >       LIST_FOR_EACH_SAFE (op, list, &both) {
     > -        if (!ovn_port_allocate_key(sbrec_chassis_table, op)) {
     > +        if (!ovn_port_allocate_key(op)) {
     >               sbrec_port_binding_delete(op->sb);
     >               ovs_list_remove(&op->list);
     >               ovn_port_destroy(ports, op);
     >           }
     >       }
     >       LIST_FOR_EACH_SAFE (op, list, &nb_only) {
     > -        if (!ovn_port_allocate_key(sbrec_chassis_table, op)) {
     > +        if (!ovn_port_allocate_key(op)) {
     >               ovs_list_remove(&op->list);
     >               ovn_port_destroy(ports, op);
     >           }
     > @@ -4303,14 +4297,13 @@ ls_port_init(struct ovn_port *op, struct 
ovsdb_idl_txn *ovnsb_txn,
     >                struct ovn_datapath *od,
     >                const struct sbrec_port_binding *sb,
     >                const struct sbrec_mirror_table *sbrec_mirror_table,
     > -             const struct sbrec_chassis_table *sbrec_chassis_table,
     >                struct ovsdb_idl_index *sbrec_chassis_by_name,
     >                struct ovsdb_idl_index *sbrec_chassis_by_hostname)
     >   {
     >       op->od = od;
     >       parse_lsp_addrs(op);
     >       /* Assign explicitly requested tunnel ids first. */
     > -    if (!ovn_port_assign_requested_tnl_id(sbrec_chassis_table, op)) {
     > +    if (!ovn_port_assign_requested_tnl_id(op)) {
     >           return false;
     >       }
     >       /* Keep nonconflicting tunnel IDs that are already assigned. */
     > @@ -4320,7 +4313,7 @@ ls_port_init(struct ovn_port *op, struct 
ovsdb_idl_txn *ovnsb_txn,
     >           }
     >       }
     >       /* Assign new tunnel ids where needed. */
     > -    if (!ovn_port_allocate_key(sbrec_chassis_table, op)) {
     > +    if (!ovn_port_allocate_key(op)) {
     >           return false;
     >       }
     >       /* Create new binding, if needed. */
     > @@ -4344,15 +4337,13 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, 
struct hmap *ls_ports,
     >                  const char *key, const struct nbrec_logical_switch_port 
*nbsp,
     >                  struct ovn_datapath *od,
     >                  const struct sbrec_mirror_table *sbrec_mirror_table,
     > -               const struct sbrec_chassis_table *sbrec_chassis_table,
     >                  struct ovsdb_idl_index *sbrec_chassis_by_name,
     >                  struct ovsdb_idl_index *sbrec_chassis_by_hostname)
     >   {
     >       struct ovn_port *op = ovn_port_create(ls_ports, key, nbsp, NULL,
     >                                             NULL);
     >       hmap_insert(&od->ports, &op->dp_node, 
hmap_node_hash(&op->key_node));
     > -    if (!ls_port_init(op, ovnsb_txn, od, NULL,
     > -                      sbrec_mirror_table, sbrec_chassis_table,
     > +    if (!ls_port_init(op, ovnsb_txn, od, NULL, sbrec_mirror_table,
     >                         sbrec_chassis_by_name, 
sbrec_chassis_by_hostname)) {
     >           ovn_port_destroy(ls_ports, op);
     >           return NULL;
     > @@ -4367,7 +4358,6 @@ ls_port_reinit(struct ovn_port *op, struct 
ovsdb_idl_txn *ovnsb_txn,
     >                   struct ovn_datapath *od,
     >                   const struct sbrec_port_binding *sb,
     >                   const struct sbrec_mirror_table *sbrec_mirror_table,
     > -                const struct sbrec_chassis_table *sbrec_chassis_table,
     >                   struct ovsdb_idl_index *sbrec_chassis_by_name,
     >                   struct ovsdb_idl_index *sbrec_chassis_by_hostname)
     >   {
     > @@ -4375,8 +4365,7 @@ ls_port_reinit(struct ovn_port *op, struct 
ovsdb_idl_txn *ovnsb_txn,
     >       op->sb = sb;
     >       ovn_port_set_nb(op, nbsp, NULL);
     >       op->l3dgw_port = op->cr_port = NULL;
     > -    return ls_port_init(op, ovnsb_txn, od, sb,
     > -                        sbrec_mirror_table, sbrec_chassis_table,
     > +    return ls_port_init(op, ovnsb_txn, od, sb, sbrec_mirror_table,
     >                           sbrec_chassis_by_name, 
sbrec_chassis_by_hostname);
     >   }
     >
     > @@ -4521,7 +4510,6 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn 
*ovnsb_idl_txn,
     >               op = ls_port_create(ovnsb_idl_txn, &nd->ls_ports,
     >                                   new_nbsp->name, new_nbsp, od,
     >                                   ni->sbrec_mirror_table,
     > -                                ni->sbrec_chassis_table,
     >                                   ni->sbrec_chassis_by_name,
     >                                   ni->sbrec_chassis_by_hostname);
     >               if (!op) {
     > @@ -4553,7 +4541,6 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn 
*ovnsb_idl_txn,
     >               if (!ls_port_reinit(op, ovnsb_idl_txn,
     >                                   new_nbsp,
     >                                   od, sb, ni->sbrec_mirror_table,
     > -                                ni->sbrec_chassis_table,
     >                                   ni->sbrec_chassis_by_name,
     >                                   ni->sbrec_chassis_by_hostname)) {
     >                   if (sb) {
     > @@ -17609,11 +17596,12 @@ ovnnb_db_run(struct northd_input *input_data,
     >       use_common_zone = smap_get_bool(input_data->nb_options, 
"use_common_zone",
     >                                       false);
     >
     > +    init_vxlan_mode(input_data->sbrec_chassis_table);
     > +
     >       build_datapaths(ovnsb_txn,
     >                       input_data->nbrec_logical_switch_table,
     >                       input_data->nbrec_logical_router_table,
     >                       input_data->sbrec_datapath_binding_table,
     > -                    input_data->sbrec_chassis_table,
     >                       &data->ls_datapaths,
     >                       &data->lr_datapaths, &data->lr_list);
     >       build_lb_datapaths(input_data->lbs, input_data->lbgrps,
     > @@ -17621,7 +17609,6 @@ ovnnb_db_run(struct northd_input *input_data,
     >                          &data->lb_datapaths_map, 
&data->lb_group_datapaths_map);
     >       build_ports(ovnsb_txn,
     >                   input_data->sbrec_port_binding_table,
     > -                input_data->sbrec_chassis_table,
     >                   input_data->sbrec_mirror_table,
     >                   input_data->sbrec_mac_binding_table,
     >                   input_data->sbrec_ha_chassis_group_table,
     > diff --git a/northd/northd.h b/northd/northd.h
     > index 940926945..be480003e 100644
     > --- a/northd/northd.h
     > +++ b/northd/northd.h
     > @@ -791,6 +791,9 @@ lr_has_multiple_gw_ports(const struct ovn_datapath 
*od)
     >       return od->n_l3dgw_ports > 1 && !od->is_gw_router;
     >   }
     >
     > -uint32_t get_ovn_max_dp_key_local(const struct sbrec_chassis_table *);
     > +void
     > +init_vxlan_mode(const struct sbrec_chassis_table *sbrec_chassis_table);
     > +
     > +uint32_t get_ovn_max_dp_key_local(void);
     >
     >   #endif /* NORTHD_H */
     > diff --git a/ovn-architecture.7.xml b/ovn-architecture.7.xml
     > index bfd8680ce..3ecb58933 100644
     > --- a/ovn-architecture.7.xml
     > +++ b/ovn-architecture.7.xml
     > @@ -2809,11 +2809,11 @@
     >     </ul>
     >
     >     <p>
     > -      When VXLAN is enabled on any hypervisor in a cluster, datapath 
and egress
     > -      port identifier ranges are reduced to 12-bits. This is done 
because only
     > -      STT and Geneve provide the large space for metadata (over 32 bits 
per
     > -      packet). To accommodate for VXLAN, 24 bits available are split as
     > -      follows:
     > +    When VXLAN is enabled on any hypervisor in a cluster, datapath and 
egress
     > +    port identifier ranges are reduced to 12-bits.  This is done 
because only
     > +    STT and Geneve provide the large space for metadata (over 32 bits 
per
     > +    packet).  The mode with reduced ranges is called <code>VXLAN 
mode</code>.
     > +    To accommodate for VXLAN, 24 bits available are split as follows:
     >     </p>
     >
     >     <ul>





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

Reply via email to