On 1/17/25 7:08 PM, Lorenzo Bianconi wrote:
> Fix ovn-ic mode when vxlan is used as encapsulation mode reducing the
> maximum local dp key to ((2<<10)-1) in order to make some room for
> OVN_MAX_DP_VXLAN_KEY_GLOBAL (vxlan tunnels export just 12 bit for
> metadata key).
>
> Acked-by: Numan Siddique <[email protected]>
> Reported-at: https://issues.redhat.com/browse/FDP-1023
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
> - Changes in v2:
> Document local datapath limitation
> ---
Hi Lorenzo,
I'm not sure this is completely correct for the non-IC case. Please see
below.
> NEWS | 3 +++
> ic/ovn-ic.c | 23 +++++++++++++++++------
> lib/ovn-util.h | 5 +++--
> northd/northd.c | 2 +-
> ovn-nb.xml | 3 +++
> tests/ovn-northd.at | 14 +++++++-------
> tests/ovn.at | 2 +-
> 7 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 72c5a6339..70bce8fdc 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -28,6 +28,9 @@ Post v24.09.0
> this option was not set). TLS ciphersuites for TLSv1.3 and later can
> be configured via --ssl-ciphersuites (--ssl-ciphers only applies to
> TLSv1.2 and earlier).
> + - Reduce the max number of local datapath to 1024 when OVN is using VXLAN
> + encapsulation type. This is needed to support OVN-interconnect in VXLAN
This reduction should only be for transit switches, right? Shouldn't we
mention that here?
> + mode.
>
> OVN v24.09.0 - 13 Sep 2024
> --------------------------
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 8320cbea5..4234397c9 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -181,12 +181,13 @@ az_run(struct ic_context *ctx)
> }
>
> static uint32_t
> -allocate_ts_dp_key(struct hmap *dp_tnlids)
> +allocate_ts_dp_key(struct hmap *dp_tnlids, bool vxlan_mode)
> {
> - static uint32_t hint = OVN_MIN_DP_KEY_GLOBAL;
> + static uint32_t hint = OVN_MIN_DP_VXLAN_KEY_GLOBAL;
> return ovn_allocate_tnlid(dp_tnlids, "transit switch datapath",
> - OVN_MIN_DP_KEY_GLOBAL, OVN_MAX_DP_KEY_GLOBAL,
> - &hint);
> + vxlan_mode ? OVN_MIN_DP_VXLAN_KEY_GLOBAL :
> OVN_MIN_DP_KEY_GLOBAL,
Nit: extra space
> + vxlan_mode ? OVN_MAX_DP_VXLAN_KEY_GLOBAL : OVN_MAX_DP_KEY_GLOBAL,
> + &hint);
> }
>
> static void
> @@ -255,12 +256,22 @@ ts_run(struct ic_context *ctx)
> * SB, to avoid uncommitted ISB datapath tunnel key to be synced back to
> * AZ. */
> if (ctx->ovnisb_txn) {
> + bool vxlan_mode = false;
> +
> + const struct icsbrec_encap *encap;
> + ICSBREC_ENCAP_FOR_EACH (encap, ctx->ovnisb_idl) {
> + if (!strcmp(encap->type, "vxlan")) {
> + vxlan_mode = true;
> + break;
> + }
> + }
> +
> /* Create ISB Datapath_Binding */
> ICNBREC_TRANSIT_SWITCH_FOR_EACH (ts, ctx->ovninb_idl) {
> isb_dp = shash_find_and_delete(&isb_dps, ts->name);
> if (!isb_dp) {
> /* Allocate tunnel key */
> - int64_t dp_key = allocate_ts_dp_key(&dp_tnlids);
> + int64_t dp_key = allocate_ts_dp_key(&dp_tnlids, vxlan_mode);
> if (!dp_key) {
> continue;
> }
> @@ -1922,8 +1933,8 @@ static void
> ovn_db_run(struct ic_context *ctx,
> const struct icsbrec_availability_zone *az)
> {
> - ts_run(ctx);
> gateway_run(ctx, az);
> + ts_run(ctx);
> port_binding_run(ctx, az);
> route_run(ctx, az);
> }
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 899bd9d12..663460d23 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -162,8 +162,9 @@ void set_idl_probe_interval(struct ovsdb_idl *idl, const
> char *remote,
> #define OVN_MIN_DP_KEY_GLOBAL (OVN_MAX_DP_KEY_LOCAL + 1)
> #define OVN_MAX_DP_KEY_GLOBAL OVN_MAX_DP_KEY
>
> -#define OVN_MAX_DP_VXLAN_KEY ((1u << 12) - 1)
> -#define OVN_MAX_DP_VXLAN_KEY_LOCAL (OVN_MAX_DP_KEY - OVN_MAX_DP_GLOBAL_NUM)
> +#define OVN_MAX_DP_VXLAN_KEY_LOCAL ((1u << 10) - 1)
> +#define OVN_MIN_DP_VXLAN_KEY_GLOBAL (OVN_MAX_DP_VXLAN_KEY_LOCAL + 1)
> +#define OVN_MAX_DP_VXLAN_KEY_GLOBAL ((1u << 12) - 1)
>
> struct hmap;
> void ovn_destroy_tnlids(struct hmap *tnlids);
> diff --git a/northd/northd.c b/northd/northd.c
> index 74014de32..a53ae485c 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -940,7 +940,7 @@ get_ovn_max_dp_key_local(bool _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_VXLAN_KEY_LOCAL;
> }
> return OVN_MAX_DP_KEY - OVN_MAX_DP_GLOBAL_NUM;
> }
This function is called for non-IC OVN+vxlan deployments too. I don't
think we want to reduce the number of datapaths in that case too. I
think we should differentiate between "regular" logical switches and
transit switches. We can do that by checking the
NB.LS.other_config:interconn-ts value.
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 24ef12f3b..3612a47ad 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -394,6 +394,9 @@
> support HW VTEP functionality and main encap type is GENEVE or STT,
> set
> this option to <code>false</code> to use default
> non-<code>VXLAN mode</code> tunnel IDs allocation logic.
> + Please consider when OVN is running in <code>VXLAN mode</code>,
> + in order to support <code>OVN-interconnect</code>, the max number of
> + local datapath is reduced to 1024.
> </column>
>
> <column name="options" key="always_tunnel"
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 93d569389..885da06af 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2904,7 +2904,7 @@ done
> check ovn-nbctl --wait=sb $cmd
>
> check_row_count nb:Logical_Switch 4097
> -wait_row_count sb:Datapath_Binding 4095
> +wait_row_count sb:Datapath_Binding 1023
>
> OVS_WAIT_UNTIL([grep "all datapath tunnel ids exhausted"
> northd/ovn-northd.log])
>
> @@ -2928,19 +2928,19 @@ check_uuid ovn-sbctl \
>
> cmd="ovn-nbctl --wait=sb"
>
> -for i in {1..4095}; do
> +for i in {1..1023}; do
> cmd="${cmd} -- ls-add lsw-${i}"
> done
>
> eval $cmd
>
> -check_row_count nb:Logical_Switch 4095
> -wait_row_count sb:Datapath_Binding 4095
> +check_row_count nb:Logical_Switch 1023
> +wait_row_count sb:Datapath_Binding 1023
>
> check ovn-nbctl ls-add lsw-exhausted
>
> -check_row_count nb:Logical_Switch 4096
> -wait_row_count sb:Datapath_Binding 4095
> +check_row_count nb:Logical_Switch 1024
> +wait_row_count sb:Datapath_Binding 1023
>
> OVS_WAIT_UNTIL([grep "all datapath tunnel ids exhausted"
> northd/ovn-northd.log])
>
> @@ -12730,7 +12730,7 @@ check_engine_stats northd recompute nocompute
> check_engine_stats lflow recompute nocompute
>
> AT_CHECK([ovn-nbctl get NB_Global . options:max_tunid | \
> -sed s/":"//g | sed s/\"//g], [0], [4095
> +sed s/":"//g | sed s/\"//g], [0], [1023
> ], [])
>
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 7a794f08d..98b4f157f 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -24429,7 +24429,7 @@ m4_define([DVR_N_S_ARP_HANDLING],
> # validate max_tunid reflects the type of encapsulation used
> max_tunid=`ovn-nbctl get NB_Global . options:max_tunid | sed s/":"//g |
> sed s/\"//g`
> if [[ $encap = vxlan ]]; then
> - max_tunid_expected=4095
> + max_tunid_expected=1023
> else
> max_tunid_expected=16711680
> fi
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
