[ovs-dev] [PATCH ovn v2] ovn-nbctl: Monitor nb_cfg to detect and handle potential overflows

Numan Siddique numans at ovn.org
Tue Aug 24 23:51:26 UTC 2021


On Sat, Aug 21, 2021 at 1:15 PM <mheib at redhat.com> wrote:
>
> From: Mohammad Heib <mheib at redhat.com>
>
> ovn-nbctl sync command will increase nb_cfg value each time it is executed
> with a specific wait_type, this increment will be handled without testing
> the current nb_cfg value because it is not monitored and that could lead
> to an overflow issue if nb_cfg == LLONG_MAX.
>
> To avoid such potential overflow cases we must monitor the real value
> of nb_cfg each time sync is executed and if there is any overflow issue
> it will be handled by function nbctl_pre_execute later on the execution.
>
> Fixes: be3a60f8e6a3 ("ovn-nbctl: Deal with nb_cfg overflows.")
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1979774
> Signed-off-by: Mohammad Heib <mheib at redhat.com>

Thanks for adding the test cases.

I applied this patch to the main branch with the below changes in the test code.
I also added your name in the AUTHORS file.

-------------------------------------------------------------------
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 2fe5ac561..8c80b00c9 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -403,11 +403,10 @@ as hv
 ovs-vsctl add-br br-phys
 ovn_attach n1 br-phys 192.168.0.1

-OVS_WAIT_UNTIL([test xhv = x`ovn-sbctl --columns name --bare find chassis`])
+check ovn-nbctl --wait=hv sync

 # overflow the NB_Global nb_cfg value
-nb_global_id=$(ovn-nbctl --columns _uuid --bare find nb_global)
-ovn-nbctl set NB_Global ${nb_global_id} nb_cfg=9223372036854775806
+check ovn-nbctl set NB_Global . nb_cfg=9223372036854775806

 # nb_cfg must be set to zero if it exceed the value of LLONG_MAX
 # the command below will try incress the value of nb_cfg to be
greater than LLONG_MAX and
@@ -415,12 +414,10 @@ ovn-nbctl set NB_Global ${nb_global_id}
nb_cfg=9223372036854775806
-OVS_WAIT_UNTIL([test x0 = x`ovn-nbctl --wait=hv sync && ovn-nbctl
--wait=hv sync && echo $?`])
+check ovn-nbctl --wait=hv sync
+check ovn-nbctl --wait=hv sync

 # nb_cfg should be set to 1 in the chassis_private/nb_global/sb_global table
-OVS_WAIT_UNTIL([test x1 = x`ovn-sbctl --columns nb_cfg --bare find
chassis_private`])
-OVS_WAIT_UNTIL([test x1 = x`ovn-sbctl --columns nb_cfg --bare find sb_global`])
-OVS_WAIT_UNTIL([test x1 = x`ovn-nbctl --columns nb_cfg --bare find nb_global`])
-
-# Assert that the the nb_cfg from the Chassis table was not incremented
-OVS_WAIT_UNTIL([test x0 = x`ovn-sbctl --columns nb_cfg --bare find chassis`])
+check_column 1 chassis_private nb_cfg
+check_column 1 sb_global nb_cfg
+check_column 1 nb:nb_global nb_cfg
+check_column 0 chassis nb_cfg

 OVN_CLEANUP([hv])
 AT_CLEANUP
----------------------------------------------------------------------------

Thanks
Numan



> ---
>  tests/ovn-controller.at | 33 +++++++++++++++++++++++++++++++++
>  utilities/ovn-nbctl.c   |  2 ++
>  2 files changed, 35 insertions(+)
>
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index e8550a5dc39a..2fe5ac56169c 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -392,6 +392,39 @@ OVN_CLEANUP([hv])
>  AT_CLEANUP
>  ])
>
> +# check that nb_cfg overflow cases handled properly
> +AT_SETUP([ovn-controller - overflow the nb_cfg value across the tables])
> +AT_KEYWORDS([ovn])
> +ovn_start
> +
> +net_add n1
> +sim_add hv
> +as hv
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +OVS_WAIT_UNTIL([test xhv = x`ovn-sbctl --columns name --bare find chassis`])
> +
> +# overflow the NB_Global nb_cfg value
> +nb_global_id=$(ovn-nbctl --columns _uuid --bare find nb_global)
> +ovn-nbctl set NB_Global ${nb_global_id} nb_cfg=9223372036854775806
> +
> +# nb_cfg must be set to zero if it exceed the value of LLONG_MAX
> +# the command below will try incress the value of nb_cfg to be greater than LLONG_MAX and
> +# expect zero as a return value
> +OVS_WAIT_UNTIL([test x0 = x`ovn-nbctl --wait=hv sync && ovn-nbctl --wait=hv sync && echo $?`])
> +
> +# nb_cfg should be set to 1 in the chassis_private/nb_global/sb_global table
> +OVS_WAIT_UNTIL([test x1 = x`ovn-sbctl --columns nb_cfg --bare find chassis_private`])
> +OVS_WAIT_UNTIL([test x1 = x`ovn-sbctl --columns nb_cfg --bare find sb_global`])
> +OVS_WAIT_UNTIL([test x1 = x`ovn-nbctl --columns nb_cfg --bare find nb_global`])
> +
> +# Assert that the the nb_cfg from the Chassis table was not incremented
> +OVS_WAIT_UNTIL([test x0 = x`ovn-sbctl --columns nb_cfg --bare find chassis`])
> +
> +OVN_CLEANUP([hv])
> +AT_CLEANUP
> +
>  # Test unix command: debug/delay-nb-cfg-report
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([ovn-controller - debug/delay-nb-cfg-report])
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index ada53b662d3c..69be775bc567 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -830,6 +830,8 @@ static void
>  nbctl_pre_sync(struct ctl_context *base OVS_UNUSED)
>  {
>      force_wait = true;
> +    /* Monitor nb_cfg to detect and handle potential overflows. */
> +    ovsdb_idl_add_column(base->idl, &nbrec_nb_global_col_nb_cfg);
>  }
>
>  static void
> --
> 2.27.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list