On Thu, Jun 6, 2024 at 1:27 AM Vladislav Odintsov <[email protected]> wrote:
> 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. > > Happy we are doing it. Mark is more eloquent than me. :) > 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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
