Re: [ovs-dev] [PATCH ovn v3] northd: Add support for disabling vxlan mode.
On Sat, Apr 27, 2024 at 2:14 AM Vladislav Odintsov
wrote:
> Hi Dumitru, Ihar,
>
> Thanks for the review. PSB.
>
> regards,
> Vladislav Odintsov
>
> On 26 Apr 2024, at 23:13, Ihar Hrachyshka wrote:
>
>
> Hi Vladislav,
>
> This looks good to me. I think documentation should be adjusted to not
> explain in detail ID allocations for VXLAN in two separate places. Assuming
> the documentation is de-duplicated and typos Dumitru spotted are addressed,
>
> Acked-by: Ihar Hrachyshka
>
> On Thu, Apr 4, 2024 at 2:41 PM Vladislav Odintsov
> wrote:
>
>> Commit [1] introduced a "vxlan mode" concept. It brought a limitation
>> for available tunnel IDs because of lack of space in VXLAN VNI.
>> In vxlan mode OVN is limited by 4095 datapaths (LRs or non-transit LSs)
>> and 2047 logical switch ports per datapath.
>>
>> Prior to this patch vxlan mode was enabled automatically if at least one
>> chassis had encap of vxlan type. In scenarios where one want to use VXLAN
>> only for HW VTEP (RAMP) switch, such limitation makes no sence.
>>
>> This patch adds support for explicit disabling of vxlan mode via
>> Northbound database.
>>
>> 1: https://github.com/ovn-org/ovn/commit/b07f1bc3d068
>>
>> CC: Ihar Hrachyshka
>> Fixes: b07f1bc3d068 ("Add VXLAN support for non-VTEP datapath bindings")
>> Signed-off-by: Vladislav Odintsov
>> ---
>> NEWS | 3 ++
>> northd/en-global-config.c | 9 +++-
>> northd/northd.c | 90 ++-
>> northd/northd.h | 6 ++-
>> ovn-nb.xml| 12 ++
>> tests/ovn-northd.at | 29 +
>> 6 files changed, 97 insertions(+), 52 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 141f1831c..fe95391cd 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -13,6 +13,9 @@ Post v24.03.0
>> "lflow-stage-to-oftable STAGE_NAME" that converts stage name into
>> OpenFlow
>> table id.
>>- Rename the ovs-sandbox script to ovn-sandbox.
>> + - Added new global config option NB_Global:options:disable_vxlan_mode
>> to
>> +extend available tunnel IDs space for datapaths from 4095 to
>> 16711680.
>> +For more details see man ovn-nb for mentioned option.
>>
>
> Nit: it's customary to refer to man pages with the section number
> included, e.g. `ovn-nb(5)`.
>
>
> Ack, will be addressed in v4.
>
>
>
>
>>
>> OVN v24.03.0 - 01 Mar 2024
>> --
>> diff --git a/northd/en-global-config.c b/northd/en-global-config.c
>> index 34e393b33..9310c4575 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(>options, sbrec_chassis_table);
>> +char *max_tunid = xasprintf("%d", get_ovn_max_dp_key_local());
>> smap_replace(options, "max_tunid", max_tunid);
>> free(max_tunid);
>>
>> @@ -523,6 +523,11 @@ check_nb_options_out_of_sync(const struct
>> nbrec_nb_global *nb,
>> return true;
>> }
>>
>> +if (config_out_of_sync(>options, _data->nb_options,
>> + "disable_vxlan_mode", false)) {
>> +return true;
>> +}
>> +
>> return false;
>> }
>>
>> diff --git a/northd/northd.c b/northd/northd.c
>> index c568f6360..859b233e8 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
>>
>>
>> @@ -875,24 +879,31 @@ 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 smap *nb_options,
>> +const struct sbrec_chassis_table *sbrec_chassis_table)
>> {
>> +if (smap_get_bool(nb_options, "disable_vxlan_mode", false)) {
>> +vxlan_mode = false;
>> +return;
>> +}
>> +
>> 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;
>> }
>>
>
> (No action required) AFAIU most of the changes in this patch are due to
> this change to is_vxlan_mode that is not strictly needed for the feature to
> work.
>
> It may make sense to read the option only once per
Re: [ovs-dev] [PATCH ovn v3] northd: Add support for disabling vxlan mode.
Hi Vladislav,
This looks good to me. I think documentation should be adjusted to not
explain in detail ID allocations for VXLAN in two separate places. Assuming
the documentation is de-duplicated and typos Dumitru spotted are addressed,
Acked-by: Ihar Hrachyshka
On Thu, Apr 4, 2024 at 2:41 PM Vladislav Odintsov wrote:
> Commit [1] introduced a "vxlan mode" concept. It brought a limitation
> for available tunnel IDs because of lack of space in VXLAN VNI.
> In vxlan mode OVN is limited by 4095 datapaths (LRs or non-transit LSs)
> and 2047 logical switch ports per datapath.
>
> Prior to this patch vxlan mode was enabled automatically if at least one
> chassis had encap of vxlan type. In scenarios where one want to use VXLAN
> only for HW VTEP (RAMP) switch, such limitation makes no sence.
>
> This patch adds support for explicit disabling of vxlan mode via
> Northbound database.
>
> 1: https://github.com/ovn-org/ovn/commit/b07f1bc3d068
>
> CC: Ihar Hrachyshka
> Fixes: b07f1bc3d068 ("Add VXLAN support for non-VTEP datapath bindings")
> Signed-off-by: Vladislav Odintsov
> ---
> NEWS | 3 ++
> northd/en-global-config.c | 9 +++-
> northd/northd.c | 90 ++-
> northd/northd.h | 6 ++-
> ovn-nb.xml| 12 ++
> tests/ovn-northd.at | 29 +
> 6 files changed, 97 insertions(+), 52 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 141f1831c..fe95391cd 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -13,6 +13,9 @@ Post v24.03.0
> "lflow-stage-to-oftable STAGE_NAME" that converts stage name into
> OpenFlow
> table id.
>- Rename the ovs-sandbox script to ovn-sandbox.
> + - Added new global config option NB_Global:options:disable_vxlan_mode to
> +extend available tunnel IDs space for datapaths from 4095 to 16711680.
> +For more details see man ovn-nb for mentioned option.
>
Nit: it's customary to refer to man pages with the section number included,
e.g. `ovn-nb(5)`.
>
> OVN v24.03.0 - 01 Mar 2024
> --
> diff --git a/northd/en-global-config.c b/northd/en-global-config.c
> index 34e393b33..9310c4575 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(>options, sbrec_chassis_table);
> +char *max_tunid = xasprintf("%d", get_ovn_max_dp_key_local());
> smap_replace(options, "max_tunid", max_tunid);
> free(max_tunid);
>
> @@ -523,6 +523,11 @@ check_nb_options_out_of_sync(const struct
> nbrec_nb_global *nb,
> return true;
> }
>
> +if (config_out_of_sync(>options, _data->nb_options,
> + "disable_vxlan_mode", false)) {
> +return true;
> +}
> +
> return false;
> }
>
> diff --git a/northd/northd.c b/northd/northd.c
> index c568f6360..859b233e8 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
>
>
> @@ -875,24 +879,31 @@ 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 smap *nb_options,
> +const struct sbrec_chassis_table *sbrec_chassis_table)
> {
> +if (smap_get_bool(nb_options, "disable_vxlan_mode", false)) {
> +vxlan_mode = false;
> +return;
> +}
> +
> 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;
> }
>
(No action required) AFAIU most of the changes in this patch are due to
this change to is_vxlan_mode that is not strictly needed for the feature to
work.
It may make sense to read the option only once per Global_Config update /
en_northd_run execution; but this could be judged independently from the
new flag too. But I don't insist that this patch is split. Just note that
it will clash with this series:
https://patchwork.ozlabs.org/project/ovn/list/?series=404623 (whichever
goes in first).
>
> uint32_t
> -get_ovn_max_dp_key_local(const struct sbrec_chassis_table
> *sbrec_chassis_table)
>
Re: [ovs-dev] [PATCH ovn v3] northd: Add support for disabling vxlan mode.
On 4/4/24 20:41, Vladislav Odintsov wrote:
> Commit [1] introduced a "vxlan mode" concept. It brought a limitation
> for available tunnel IDs because of lack of space in VXLAN VNI.
> In vxlan mode OVN is limited by 4095 datapaths (LRs or non-transit LSs)
> and 2047 logical switch ports per datapath.
>
> Prior to this patch vxlan mode was enabled automatically if at least one
> chassis had encap of vxlan type. In scenarios where one want to use VXLAN
> only for HW VTEP (RAMP) switch, such limitation makes no sence.
>
> This patch adds support for explicit disabling of vxlan mode via
> Northbound database.
>
> 1: https://github.com/ovn-org/ovn/commit/b07f1bc3d068
>
> CC: Ihar Hrachyshka
> Fixes: b07f1bc3d068 ("Add VXLAN support for non-VTEP datapath bindings")
> Signed-off-by: Vladislav Odintsov
> ---
Thanks for the v3, Vladislav!
[...]
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index b652046a7..9b2cb355e 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -381,6 +381,18 @@
> of SB changes would be very noticeable.
>
>
> +
> +Be default if at least one chassis in OVN cluster has VXLAN encap,
Typo: "Be default".
> +northd will run in a `vxlan mode`, which means it splits 24-bit
> +VXLAN VNI for datapath and port tunnel IDs allocation evenly.
> Maximum
> +datapaths count in this mode is 4095 and maximum ports per datapath
> is
> +2047. Rest of IDs are used for multicast groups.
Maybe "The rest of the IDs are used for.."?
> +In case VXLAN encaps are needed on chassis only to support HW VTEP
> +functionality, and main encap type is GENEVE or STT, set this option
> to
> +`false` to use defaults -- 16-bit space for datapath tunnel IDS and
> 15
> +bits for port tunnel IDs.
> +
> +
>
>
>These options control how routes are advertised between OVN
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index fc2c972a4..6edb1129e 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2819,6 +2819,35 @@ AT_CHECK(
> get_tunnel_keys
> AT_CHECK([test $lsp02 = 3 && test $ls1 = 123])
>
> +AT_CLEANUP
> +])
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([check vxlan mode disabling])
> +ovn_start
> +
> +# Create a fake chassis with vxlan encap to implicitly enable vxlan mode.
> +ovn-sbctl \
> +--id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \
> +-- --id=@c create chassis name=hv1 encaps=@e
> +
> +cmd="ovn-nbctl --wait=sb"
> +for i in {1..4097..1}; do
> +cmd="${cmd} -- ls-add lsw-${i}"
> +done
> +
> +eval $cmd
Couldn't this be just "check $cmd"?
> +
> +check_row_count nb:Logical_Switch 4097
> +wait_row_count sb:Datapath_Binding 4095
> +
> +OVS_WAIT_UNTIL([grep "all datapath tunnel ids exhausted"
> northd/ovn-northd.log])
> +
> +# Explicitly disable vxlan mode and check that two remaining datapaths were
> created.
> +check ovn-nbctl set NB_Global . options:disable_vxlan_mode=true
> +
> +check_row_count nb:Logical_Switch 4097
> +wait_row_count sb:Datapath_Binding 4097
> +
> AT_CLEANUP
> ])
>
Aside for the minor things above (which I can address when applying the
patch so no need for v4) the rest looks good to me. Ihar, what do you
think?
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v3] northd: Add support for disabling vxlan mode.
Commit [1] introduced a "vxlan mode" concept. It brought a limitation
for available tunnel IDs because of lack of space in VXLAN VNI.
In vxlan mode OVN is limited by 4095 datapaths (LRs or non-transit LSs)
and 2047 logical switch ports per datapath.
Prior to this patch vxlan mode was enabled automatically if at least one
chassis had encap of vxlan type. In scenarios where one want to use VXLAN
only for HW VTEP (RAMP) switch, such limitation makes no sence.
This patch adds support for explicit disabling of vxlan mode via
Northbound database.
1: https://github.com/ovn-org/ovn/commit/b07f1bc3d068
CC: Ihar Hrachyshka
Fixes: b07f1bc3d068 ("Add VXLAN support for non-VTEP datapath bindings")
Signed-off-by: Vladislav Odintsov
---
NEWS | 3 ++
northd/en-global-config.c | 9 +++-
northd/northd.c | 90 ++-
northd/northd.h | 6 ++-
ovn-nb.xml| 12 ++
tests/ovn-northd.at | 29 +
6 files changed, 97 insertions(+), 52 deletions(-)
diff --git a/NEWS b/NEWS
index 141f1831c..fe95391cd 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,9 @@ Post v24.03.0
"lflow-stage-to-oftable STAGE_NAME" that converts stage name into OpenFlow
table id.
- Rename the ovs-sandbox script to ovn-sandbox.
+ - Added new global config option NB_Global:options:disable_vxlan_mode to
+extend available tunnel IDs space for datapaths from 4095 to 16711680.
+For more details see man ovn-nb for mentioned option.
OVN v24.03.0 - 01 Mar 2024
--
diff --git a/northd/en-global-config.c b/northd/en-global-config.c
index 34e393b33..9310c4575 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(>options, sbrec_chassis_table);
+char *max_tunid = xasprintf("%d", get_ovn_max_dp_key_local());
smap_replace(options, "max_tunid", max_tunid);
free(max_tunid);
@@ -523,6 +523,11 @@ check_nb_options_out_of_sync(const struct nbrec_nb_global
*nb,
return true;
}
+if (config_out_of_sync(>options, _data->nb_options,
+ "disable_vxlan_mode", false)) {
+return true;
+}
+
return false;
}
diff --git a/northd/northd.c b/northd/northd.c
index c568f6360..859b233e8 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
@@ -875,24 +879,31 @@ 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 smap *nb_options,
+const struct sbrec_chassis_table *sbrec_chassis_table)
{
+if (smap_get_bool(nb_options, "disable_vxlan_mode", false)) {
+vxlan_mode = false;
+return;
+}
+
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;
}
@@ -900,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
