Sorry for forgetting to include changelogs in these submissions.

v1: initial version.
v2: cover both cases of hint = 0 and hint > max.
v3: reduce the number of ports to create in the hint > max scenario needed
to trigger the problem.

On Fri, Apr 5, 2024 at 12:10 PM Ihar Hrachyshka <ihrac...@redhat.com> wrote:

> The original version of the scenario passed with or without the fix.
> This is because all LSs were processed in one go, so the allocate
> function was never entered with *hint==0.
>
> Also, added another scenario that will check behavior when *hint is out
> of [min;max] bounds but > max (this happens in an obscure scenario where
> a vxlan chassis is added to the cluster mid-light, forcing northd to
> reduce its effective max value for tunnel ids; which may become lower
> than the current *hint for ports.)
>
> Fixes: a1f165a7b807 ("northd: fix infinite loop in ovn_allocate_tnlid()")
> Co-Authored-By: Vladislav Odintsov <odiv...@gmail.com>
> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com>
> Signed-off-by: Ihar Hrachyshka <ihrac...@redhat.com>
> ---
>  lib/ovn-util.c      | 11 ++++-------
>  tests/ovn-northd.at | 43 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 44 insertions(+), 10 deletions(-)
>
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index 9f97ae2ca..248f00b9e 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -693,17 +693,14 @@ uint32_t
>  ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min,
>                     uint32_t max, uint32_t *hint)
>  {
> -    /* Normalize hint, because it can be outside of [min, max]. */
> -    *hint = next_tnlid(*hint, min, max);
> -
> -    uint32_t tnlid = *hint;
> -    do {
> +    VLOG_ERR("IHAR starting hint = %u", hint);
> +    for (uint32_t tnlid = next_tnlid(*hint, min, max); tnlid != *hint;
> +         tnlid = next_tnlid(tnlid, min, max)) {
>          if (ovn_add_tnlid(set, tnlid)) {
>              *hint = tnlid;
>              return tnlid;
>          }
> -        tnlid = next_tnlid(tnlid, min, max);
> -    } while (tnlid != *hint);
> +    }
>
>      static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
>      VLOG_WARN_RL(&rl, "all %s tunnel ids exhausted", name);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index be006fb32..1a4e7274d 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2823,7 +2823,7 @@ AT_CLEANUP
>  ])
>
>  OVN_FOR_EACH_NORTHD_NO_HV([
> -AT_SETUP([check tunnel ids exhaustion])
> +AT_SETUP([check datapath tunnel ids exhaustion])
>  ovn_start
>
>  # Create a fake chassis with vxlan encap to lower MAX DP tunnel key to
> 2^12
> @@ -2833,13 +2833,18 @@ ovn-sbctl \
>
>  cmd="ovn-nbctl --wait=sb"
>
> -for i in {1..4097}; do
> +for i in {1..4095}; do
>      cmd="${cmd} -- ls-add lsw-${i}"
>  done
>
>  eval $cmd
>
> -check_row_count nb:Logical_Switch 4097
> +check_row_count nb:Logical_Switch 4095
> +wait_row_count sb:Datapath_Binding 4095
> +
> +ovn-nbctl ls-add lsw-exhausted
> +
> +check_row_count nb:Logical_Switch 4096
>  wait_row_count sb:Datapath_Binding 4095
>
>  OVS_WAIT_UNTIL([grep "all datapath tunnel ids exhausted"
> northd/ovn-northd.log])
> @@ -2847,6 +2852,38 @@ OVS_WAIT_UNTIL([grep "all datapath tunnel ids
> exhausted" northd/ovn-northd.log])
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([check port tunnel ids exhaustion; vxlan chassis pops up
> midflight])
> +ovn_start
> +
> +cmd="ovn-nbctl --wait=sb"
> +
> +cmd="${cmd} -- ls-add lsw"
> +for i in {1..2048}; do
> +    cmd="${cmd} -- lsp-add lsw lsp-${i}"
> +done
> +
> +eval $cmd
> +
> +check_row_count nb:Logical_Switch_Port 2048
> +wait_row_count sb:Port_Binding 2048
> +
> +# Now create a fake chassis with vxlan encap to lower MAX port tunnel key
> to 2^11
> +ovn-sbctl \
> +    --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \
> +    -- --id=@c create chassis name=hv1 encaps=@e
> +
> +ovn-nbctl lsp-add lsw lsp-exhausted
> +
> +check_row_count nb:Logical_Switch_Port 2049
> +wait_row_count sb:Port_Binding 2048
> +
> +OVS_WAIT_UNTIL([grep "all port tunnel ids exhausted"
> northd/ovn-northd.log])
> +
> +AT_CLEANUP
> +])
> +
> +
>
>  OVN_FOR_EACH_NORTHD_NO_HV([
>  AT_SETUP([Logical Flow Datapath Groups])
> --
> 2.41.0
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to