On Fri, Jan 3, 2025 at 9:37 AM Lorenzo Bianconi <[email protected]> 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). > > Reported-at: https://issues.redhat.com/browse/FDP-1023 > Signed-off-by: Lorenzo Bianconi <[email protected]>
Thanks for the patch. I think we need to document the reduction of max datapaths from 4095 to 1023 local datapaths with vxlan mode. Can you please document in the appropriate place ? We can definitely add in NEWS. Please see if it can be documented in ovn-nb.xml. With that addressed: Acked-by: Numan Siddique <[email protected]> Thanks Numan > --- > ic/ovn-ic.c | 23 +++++++++++++++++------ > lib/ovn-util.h | 5 +++-- > northd/northd.c | 2 +- > tests/ovn-northd.at | 14 +++++++------- > tests/ovn.at | 2 +- > 5 files changed, 29 insertions(+), 17 deletions(-) > > 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, > + 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 b01e40ecd..c0fb22c49 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; > } > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 507cc302f..3e1681ed6 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2901,7 +2901,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]) > > @@ -2925,19 +2925,19 @@ 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]) > > @@ -12724,7 +12724,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 de01a649f..e59bae26b 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -24359,7 +24359,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 > -- > 2.47.1 > > _______________________________________________ > 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
