Message ID | 20210821171429.938565-1-mheib@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,v2] ovn-nbctl: Monitor nb_cfg to detect and handle potential overflows | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Sat, Aug 21, 2021 at 1:15 PM <mheib@redhat.com> wrote: > > From: Mohammad Heib <mheib@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@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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