Thanks Mark for such a detailed answer! I agree with your points, and also was thinking about them, but could not value their importance in terms of I-P logic. You helped with that.
I’d prefer to apply my proposal to revert back to “bool is_vxlan_mode()” to make the “global” a true global. Will submit v6. regards, Vladislav Odintsov > On 5 Jun 2024, at 23:13, Mark Michelson <[email protected]> wrote: > > 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. This change was an intended potential performance enhancement by not only to avoid iterations over all chassises, but also to avoid doing this multiple times from mentioned functions. Though I didn’t do any performance testing, because it was not the concern. > 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
