On Mon, Aug 21, 2023 at 8:00 PM Mark Michelson <mmich...@redhat.com> wrote:
> Hi Ales,
>
> I agree with this change, and for the code itself,
>
> Acked-by: Mark Michelson <mmich...@redhat.com>
>
> However, I'm curious why this is grouped with the other changes in this
> series since it doesn't appear to be related to fixing tests.
>
Hi Mark,
it actually fixes the "overflow the nb_cfg value across the tables" test.
> On 8/15/23 07:04, Ales Musil wrote:
> > We are correctly wrapping nb_cfg over when the value is LLONG_MAX.
> > However, the sync code is expecting the value to be always raising,
> > so it would exit early as the current >= next is true for the
> > wrap back to 0. To detect that use delta between those two values.
> > The delta threshold is set to INT32_MAX as we don't expect the nb_cfg
> > to be updated more than that during the sync call.
> >
> > Signed-off-by: Ales Musil <amu...@redhat.com>
> > ---
> > northd/ovn-northd.c | 4 +++-
> > utilities/ovn-nbctl.c | 4 +++-
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 4fa1b039e..68d58b337 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -518,7 +518,9 @@ update_sequence_numbers(int64_t loop_start_time,
> > chassis_priv->name);
> > }
> >
> > - if (chassis_priv->nb_cfg < hv_cfg) {
> > + /* Detect if overflows happened within the cfg update. */
> > + int64_t delta = chassis_priv->nb_cfg - hv_cfg;
> > + if (chassis_priv->nb_cfg < hv_cfg || delta > INT32_MAX) {
> > hv_cfg = chassis_priv->nb_cfg;
> > hv_cfg_ts = chassis_priv->nb_cfg_timestamp;
> > } else if (chassis_priv->nb_cfg == hv_cfg &&
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index 7a4f6b1b3..444fbd2fe 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -123,7 +123,9 @@ nbctl_post_execute(struct ovsdb_idl *idl, struct
> ovsdb_idl_txn *txn,
> > int64_t cur_cfg = (wait_type == NBCTL_WAIT_SB
> > ? nb->sb_cfg
> > : MIN(nb->sb_cfg, nb->hv_cfg));
> > - if (cur_cfg >= next_cfg) {
> > + /* Detect if overflows happened within the cfg update. */
> > + int64_t delta = cur_cfg - next_cfg;
> > + if (cur_cfg >= next_cfg && delta < INT32_MAX) {
> > if (print_wait_time) {
> > printf("Time spent on processing nb_cfg
> %"PRId64":\n",
> > next_cfg);
>
>
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
