| Message ID | 20240502095104.169103-2-odivlad@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add support to disable vxlan mode. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Thu, May 2, 2024 at 5:51 AM Vladislav Odintsov <odivlad@gmail.com> wrote: > This simplifies code and subsequent commit to explicitely disable vxlan > I personally find it debatable that moving from explicit dependency through a function argument to implicit dependency through a global variable is a simplification. But I will leave others to chime in. > mode is based on these changes. > > Also `vxlan mode` term is introduced in ovn-architecture man page. > Should the mode name keep VXLAN capitalized? > > Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> > --- > northd/en-global-config.c | 4 +- > northd/northd.c | 94 ++++++++++++++++----------------------- > northd/northd.h | 5 ++- > ovn-architecture.7.xml | 11 +++-- > 4 files changed, 50 insertions(+), 64 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 5e12fd1e8..b54219a85 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,24 +885,25 @@ 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)) { > + if (vxlan_mode) { > /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for vxlan mode. */ > return OVN_MAX_DP_VXLAN_KEY; > } > @@ -906,15 +911,14 @@ get_ovn_max_dp_key_local(const struct > sbrec_chassis_table *sbrec_chassis_table) > } > > 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,17 +4078,10 @@ 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)) { > - sbrec_port_binding_delete(op->sb); > - ovs_list_remove(&op->list); > - ovn_port_destroy(ports, op); > - } > + ovn_port_allocate_key(op); > Shouldn't you still check the result of `ovn_port_allocate_key`? (Below too.) > } > LIST_FOR_EACH_SAFE (op, list, &nb_only) { > - if (!ovn_port_allocate_key(sbrec_chassis_table, op)) { > - ovs_list_remove(&op->list); > - ovn_port_destroy(ports, op); > - } > + ovn_port_allocate_key(op); > } > > /* For logical ports that are in both databases, update the southbound > @@ -4303,14 +4290,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 +4306,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. */ > @@ -4333,6 +4319,10 @@ ls_port_init(struct ovn_port *op, struct > ovsdb_idl_txn *ovnsb_txn, > op->sb = sbrec_port_binding_insert(ovnsb_txn); > sbrec_port_binding_set_logical_port(op->sb, op->key); > } > + /* Assign new tunnel ids where needed. */ > + if (!ovn_port_allocate_key(op)) { > + return false; > + } > Was this ^ intended? I believe recent patches structured the code here so that any potential failures happen before a new binding is created. > ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name, > sbrec_chassis_by_hostname, NULL, > sbrec_mirror_table, > op, NULL, NULL); > @@ -4344,15 +4334,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 +4355,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 +4362,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 +4507,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 +4538,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 +17593,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 +17606,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..7abb1fa83 100644 > --- a/ovn-architecture.7.xml > +++ b/ovn-architecture.7.xml > @@ -2809,13 +2809,12 @@ > </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). Such mode is a <code>vxlan mode</code>. To accommodate for > The text is slightly ambiguous (one could interpret it as: vxlan mode is the mode where the metadata space is large). Consider: ``` + 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> > <li> > 12-bit logical datapath identifier, derived from the > -- > 2.44.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
Hi Ihar, thanks for your review! > On 2 May 2024, at 18:11, Ihar Hrachyshka <ihrachys@redhat.com> wrote: > > On Thu, May 2, 2024 at 5:51 AM Vladislav Odintsov <odivlad@gmail.com <mailto:odivlad@gmail.com>> wrote: > >> This simplifies code and subsequent commit to explicitely disable vxlan >> > > I personally find it debatable that moving from explicit dependency through > a function argument to implicit dependency through a global variable is a > simplification. But I will leave others to chime in. > Here I wanted to mention that in many pieces of code argument which was passed just to find VXLAN encaps was removed and with less code/arguments it looks more simple. > >> mode is based on these changes. >> >> Also `vxlan mode` term is introduced in ovn-architecture man page. >> > > Should the mode name keep VXLAN capitalized? > Dunno. This was inspired by writing in initial commit [1]. I’m fine with both writings. > >> >> Signed-off-by: Vladislav Odintsov <odivlad@gmail.com> >> --- >> northd/en-global-config.c | 4 +- >> northd/northd.c | 94 ++++++++++++++++----------------------- >> northd/northd.h | 5 ++- >> ovn-architecture.7.xml | 11 +++-- >> 4 files changed, 50 insertions(+), 64 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 5e12fd1e8..b54219a85 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,24 +885,25 @@ 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)) { >> + if (vxlan_mode) { >> /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for vxlan mode. */ >> return OVN_MAX_DP_VXLAN_KEY; >> } >> @@ -906,15 +911,14 @@ get_ovn_max_dp_key_local(const struct >> sbrec_chassis_table *sbrec_chassis_table) >> } >> >> 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,17 +4078,10 @@ 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)) { >> - sbrec_port_binding_delete(op->sb); >> - ovs_list_remove(&op->list); >> - ovn_port_destroy(ports, op); >> - } >> + ovn_port_allocate_key(op); >> > > Shouldn't you still check the result of `ovn_port_allocate_key`? (Below > too.) > Oops. This is a result of a wrong rebase, sorry. Will be fixed in v5. > >> } >> LIST_FOR_EACH_SAFE (op, list, &nb_only) { >> - if (!ovn_port_allocate_key(sbrec_chassis_table, op)) { >> - ovs_list_remove(&op->list); >> - ovn_port_destroy(ports, op); >> - } >> + ovn_port_allocate_key(op); >> } >> >> /* For logical ports that are in both databases, update the southbound >> @@ -4303,14 +4290,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 +4306,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. */ >> @@ -4333,6 +4319,10 @@ ls_port_init(struct ovn_port *op, struct >> ovsdb_idl_txn *ovnsb_txn, >> op->sb = sbrec_port_binding_insert(ovnsb_txn); >> sbrec_port_binding_set_logical_port(op->sb, op->key); >> } >> + /* Assign new tunnel ids where needed. */ >> + if (!ovn_port_allocate_key(op)) { >> + return false; >> + } >> > > Was this ^ intended? I believe recent patches structured the code here so > that any potential failures happen before a new binding is created. Also, it’s caused by mistakes in rebase. Will be fixed. > > >> ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name, >> sbrec_chassis_by_hostname, NULL, >> sbrec_mirror_table, >> op, NULL, NULL); >> @@ -4344,15 +4334,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 +4355,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 +4362,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 +4507,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 +4538,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 +17593,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 +17606,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..7abb1fa83 100644 >> --- a/ovn-architecture.7.xml >> +++ b/ovn-architecture.7.xml >> @@ -2809,13 +2809,12 @@ >> </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). Such mode is a <code>vxlan mode</code>. To accommodate for >> > > The text is slightly ambiguous (one could interpret it as: vxlan mode is > the mode where the metadata space is large). > > Consider: > > ``` > + 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 > ``` > Ok, this completely removes all possible wrong readings. > >> + VXLAN, 24 bits available are split as follows: >> </p> >> - >> <ul> >> <li> >> 12-bit logical datapath identifier, derived from the >> -- >> 2.44.0 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev 1: https://github.com/ovn-org/ovn/commit/b07f1bc3d068#diff-935a3771045d93c92d2ffc5adb052cc0ed59bfd965aaf064c7c3c257f2ff332aR1211 Regards, Vladislav Odintsov
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 5e12fd1e8..b54219a85 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,24 +885,25 @@ 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)) { + if (vxlan_mode) { /* OVN_MAX_DP_GLOBAL_NUM doesn't apply for vxlan mode. */ return OVN_MAX_DP_VXLAN_KEY; } @@ -906,15 +911,14 @@ get_ovn_max_dp_key_local(const struct sbrec_chassis_table *sbrec_chassis_table) } 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,17 +4078,10 @@ 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)) { - sbrec_port_binding_delete(op->sb); - ovs_list_remove(&op->list); - ovn_port_destroy(ports, op); - } + ovn_port_allocate_key(op); } LIST_FOR_EACH_SAFE (op, list, &nb_only) { - if (!ovn_port_allocate_key(sbrec_chassis_table, op)) { - ovs_list_remove(&op->list); - ovn_port_destroy(ports, op); - } + ovn_port_allocate_key(op); } /* For logical ports that are in both databases, update the southbound @@ -4303,14 +4290,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 +4306,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. */ @@ -4333,6 +4319,10 @@ ls_port_init(struct ovn_port *op, struct ovsdb_idl_txn *ovnsb_txn, op->sb = sbrec_port_binding_insert(ovnsb_txn); sbrec_port_binding_set_logical_port(op->sb, op->key); } + /* Assign new tunnel ids where needed. */ + if (!ovn_port_allocate_key(op)) { + return false; + } ovn_port_update_sbrec(ovnsb_txn, sbrec_chassis_by_name, sbrec_chassis_by_hostname, NULL, sbrec_mirror_table, op, NULL, NULL); @@ -4344,15 +4334,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 +4355,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 +4362,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 +4507,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 +4538,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 +17593,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 +17606,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..7abb1fa83 100644 --- a/ovn-architecture.7.xml +++ b/ovn-architecture.7.xml @@ -2809,13 +2809,12 @@ </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). Such mode is a <code>vxlan mode</code>. To accommodate for + VXLAN, 24 bits available are split as follows: </p> - <ul> <li> 12-bit logical datapath identifier, derived from the
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 <odivlad@gmail.com> --- northd/en-global-config.c | 4 +- northd/northd.c | 94 ++++++++++++++++----------------------- northd/northd.h | 5 ++- ovn-architecture.7.xml | 11 +++-- 4 files changed, 50 insertions(+), 64 deletions(-)