On Wed, Jul 27, 2022 at 12:09 AM Ihar Hrachyshka <ihrac...@redhat.com> wrote:
>
> Acked-By: Ihar Hrachyshka <ihrac...@redhat.com>
>
> On Wed, Jul 20, 2022 at 4:56 AM Ales Musil <amu...@redhat.com> wrote:
> >
> > Because the transaction is limited, in terms of how
> > many operations it can do, we should not allow
> > more than 15 MAC bindings to be removed at once every
> > 10 ms. This has a downside that in theory we could
> > never finish the removal, however in practice it is
> > unlikely considering that not all routers will have aging
> > enabled and the enabled will be with reasonable threshold.
> >
> > Reported-at: https://bugzilla.redhat.com/2084668
> > Signed-off-by: Ales Musil <amu...@redhat.com>
> > ---
Based on the offline discussion we had, I think it's better to make
this removal limit configurable.
I think NB_Global.options seems to be the right place.
Also, since IDL or ovsdb-server doesn't impose any limit on the
number of txn items in each txn, I'd suggest to have the default
value to 0 (i,e don't limit the bulk removal).
CMS can set it to a desired value if required.
Other than this, the series looks fine to me.
Thanks
Numan
> > v3: Rebase on top of current main.
> > Update according to the final conclusion.
> > ---
> > northd/mac-binding-aging.c | 19 +++++++++++++++++--
> > 1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/northd/mac-binding-aging.c b/northd/mac-binding-aging.c
> > index 8af477ff1..32beef38b 100644
> > --- a/northd/mac-binding-aging.c
> > +++ b/northd/mac-binding-aging.c
> > @@ -28,6 +28,9 @@
> >
> > VLOG_DEFINE_THIS_MODULE(mac_binding_aging);
> >
> > +#define MAC_BINDING_BULK_REMOVAL_LIMIT 15
> > +#define MAC_BINDING_BULK_REMOVAL_DELAY_MSEC 10
> > +
> > struct mac_binding_waker {
> > bool should_schedule;
> > long long next_wake_msec;
> > @@ -39,7 +42,8 @@ static void
> > mac_binding_aging_run_for_datapath(const struct sbrec_datapath_binding *dp,
> > const struct nbrec_logical_router *nbr,
> > struct ovsdb_idl_index *mb_by_datapath,
> > - int64_t now, int64_t *wake_delay)
> > + int64_t now, int64_t *wake_delay,
> > + uint8_t *removed_n)
> > {
> > uint64_t threshold = smap_get_uint(&nbr->options,
> > "mac_binding_age_threshold",
> > @@ -60,6 +64,10 @@ mac_binding_aging_run_for_datapath(const struct
> > sbrec_datapath_binding *dp,
> > return;
> > } else if (elapsed >= threshold) {
> > sbrec_mac_binding_delete(mb);
> > + (*removed_n)++;
> > + if (*removed_n == MAC_BINDING_BULK_REMOVAL_LIMIT) {
> > + break;
> > + }
> > } else {
> > *wake_delay = MIN(*wake_delay, threshold - elapsed);
> > }
> > @@ -78,6 +86,7 @@ en_mac_binding_aging_run(struct engine_node *node, void
> > *data OVS_UNUSED)
> >
> > int64_t next_expire_msec = INT64_MAX;
> > int64_t now = time_wall_msec();
> > + uint8_t removed_n = 0;
> > struct northd_data *northd_data = engine_get_input_data("northd",
> > node);
> > struct ovsdb_idl_index *sbrec_mac_binding_by_datapath =
> > engine_ovsdb_node_get_index(engine_get_input("SB_mac_binding",
> > node),
> > @@ -88,7 +97,13 @@ en_mac_binding_aging_run(struct engine_node *node, void
> > *data OVS_UNUSED)
> > if (od->sb && od->nbr) {
> > mac_binding_aging_run_for_datapath(od->sb, od->nbr,
> >
> > sbrec_mac_binding_by_datapath,
> > - now, &next_expire_msec);
> > + now, &next_expire_msec,
> > + &removed_n);
> > + if (removed_n == MAC_BINDING_BULK_REMOVAL_LIMIT) {
> > + /* Schedule the next run after specified delay. */
> > + next_expire_msec = MAC_BINDING_BULK_REMOVAL_DELAY_MSEC;
> > + break;
> > + }
> > }
> > }
> >
> > --
> > 2.35.3
> >
> > _______________________________________________
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
