On 8/9/22 13:05, Ales Musil wrote:
> Add configuration option into NB global table
> called "mac_binding_removal_limit" defaulting to 0.
> This option allows to limit number of MAC bindings
> that can be removed by the aging mechanism in a single
> transaction. The 0 means that the mechanism is disabled.
> If the limit is reached next removal will be delayed by
> 10 ms. This option when being set 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 <[email protected]>
> ---
> v3: Rebase on top of current main.
> Update according to the final conclusion.
> v4: Rebase on top of current main.
> Address comment from Numan.
> ---
> northd/inc-proc-northd.c | 1 +
> northd/mac-binding-aging.c | 33 +++++++++++++++++++++++++++++++--
> ovn-nb.xml | 7 +++++++
> 3 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 4a3699106..cdfd39cc6 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -215,6 +215,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_northd, &en_sb_load_balancer, NULL);
> engine_add_input(&en_northd, &en_sb_fdb, NULL);
> engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL);
> + engine_add_input(&en_mac_binding_aging, &en_nb_nb_global, NULL);
> engine_add_input(&en_mac_binding_aging, &en_sb_mac_binding, NULL);
> engine_add_input(&en_mac_binding_aging, &en_northd, NULL);
> engine_add_input(&en_mac_binding_aging, &en_mac_binding_aging_waker,
> NULL);
> diff --git a/northd/mac-binding-aging.c b/northd/mac-binding-aging.c
> index e8217e8bc..10e995f63 100644
> --- a/northd/mac-binding-aging.c
> +++ b/northd/mac-binding-aging.c
> @@ -28,6 +28,8 @@
>
> VLOG_DEFINE_THIS_MODULE(mac_binding_aging);
>
> +#define MAC_BINDING_BULK_REMOVAL_DELAY_MSEC 10
> +
> struct mac_binding_waker {
> bool should_schedule;
> long long next_wake_msec;
> @@ -39,7 +41,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,
> + uint32_t removal_limit, uint32_t
> *removed_n)
> {
> uint64_t threshold = smap_get_uint(&nbr->options,
> "mac_binding_age_threshold",
> @@ -60,6 +63,10 @@ mac_binding_aging_run_for_datapath(const struct
> sbrec_datapath_binding *dp,
> continue;
> } else if (elapsed >= threshold) {
> sbrec_mac_binding_delete(mb);
> + (*removed_n)++;
> + if (removal_limit && *removed_n == removal_limit) {
> + break;
> + }
> } else {
> *wake_delay = MIN(*wake_delay, threshold - elapsed);
> }
> @@ -67,6 +74,20 @@ mac_binding_aging_run_for_datapath(const struct
> sbrec_datapath_binding *dp,
> sbrec_mac_binding_index_destroy_row(mb_index_row);
> }
>
> +static uint32_t
> +get_removal_limit(struct engine_node *node)
> +{
> + const struct nbrec_nb_global_table *nb_global_table =
> + EN_OVSDB_GET(engine_get_input("NB_nb_global", node));
> + const struct nbrec_nb_global *nb =
> + nbrec_nb_global_table_first(nb_global_table);
> + if (!nb) {
> + return 0;
> + }
> +
> + return smap_get_uint(&nb->options, "mac_binding_removal_limit", 0);
> +}
> +
> void
> en_mac_binding_aging_run(struct engine_node *node, void *data OVS_UNUSED)
> {
> @@ -78,6 +99,8 @@ 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();
> + uint32_t removal_limit = get_removal_limit(node);
> + uint32_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 +111,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,
> + removal_limit, &removed_n);>
> + if (removal_limit && removed_n == removal_limit) {
> + /* Schedule the next run after specified delay. */
> + next_expire_msec = MAC_BINDING_BULK_REMOVAL_DELAY_MSEC;
> + break;
> + }
> }
> }
>
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index d3fba9bdc..83f60265f 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -162,6 +162,13 @@
> dynamically assigned, e.g. <code>00:11:22</code>
> </column>
>
> + <column name="options" key="mac_binding_removal_limit"
> + type='{"type": "integer", "minInteger": 0, "maxInteger":
> 4294967295}'>
> + MAC binding aging bulk removal limit. This limits how many rows
> + can expire in a single transaction. Default value is 0 which
> + is unlimited.
I think we should also mention the 10msec delay if the limit is hit.
Thanks,
Dumitru
> + </column>
> +
> <column name="options" key="controller_event" type='{"type":
> "boolean"}'>
> Value set by the CMS to enable/disable ovn-controller event
> reporting.
> Traffic into OVS can raise a 'controller' event that results in a
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
