On Sun, Jan 18, 2026 at 4:02 PM Mark Michelson via dev <
[email protected]> wrote:

> This adds a stopwatch for all engine nodes that tracks the time it takes
> to recompute. All engine nodes automatically have this stopwatch created
> and run for all recomputes.
>
> Signed-off-by: Mark Michelson <[email protected]>
> ---
>

Hi Mark,

thank you for the patch. I have one question down below.


>  lib/inc-proc-eng.c | 8 +++++++-
>  lib/inc-proc-eng.h | 5 ++++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
> index 1cc4d56e7..7986efa83 100644
> --- a/lib/inc-proc-eng.c
> +++ b/lib/inc-proc-eng.c
> @@ -33,6 +33,7 @@
>  #include "unixctl.h"
>  #include "vec.h"
>  #include "sset.h"
> +#include "lib/stopwatch.h"
>
>  VLOG_DEFINE_THIS_MODULE(inc_proc_eng);
>
> @@ -217,6 +218,7 @@ engine_init(struct engine_node *node, struct
> engine_arg *arg)
>              sorted_node->get_compute_failure_info =
>                  engine_get_compute_failure_info;
>          }
> +        stopwatch_create(sorted_node->recompute_stopwatch, SW_MS);
>      }
>
>      unixctl_command_register("inc-engine/show-stats", "", 0, 2,
> @@ -420,7 +422,11 @@ engine_init_run(void)
>  static enum engine_node_state
>  run_recompute_callback(struct engine_node *node)
>  {
> -    return node->run(node, node->data);
> +    enum engine_node_state ret;
> +    stopwatch_start(node->recompute_stopwatch, time_msec());
> +    ret = node->run(node, node->data);
> +    stopwatch_stop(node->recompute_stopwatch, time_msec());
> +    return ret;
>  }
>
>  static enum engine_input_handler_result
> diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
> index 1cb2466b2..39445a56b 100644
> --- a/lib/inc-proc-eng.h
> +++ b/lib/inc-proc-eng.h
> @@ -282,6 +282,8 @@ struct engine_node {
>
>      /* Indication if the node writes to SB DB. */
>      bool sb_write;
> +
> +    char *recompute_stopwatch;
>  };
>
>  /* Initialize the data for the engine nodes. It calls each node's
> @@ -440,7 +442,8 @@ void engine_ovsdb_node_add_index(struct engine_node *,
> const char *name,
>          .state = EN_STALE, \
>          .init = en_##NAME##_init, \
>          .run = en_##NAME##_run, \
> -        .cleanup = en_##NAME##_cleanup,
> +        .cleanup = en_##NAME##_cleanup, \
> +        .recompute_stopwatch = #NAME"_run",
>

Is there a reason why we couldn't use the node name?
I think that would be completely fine.


>
>  #define ENGINE_NODE_DEF_END };
>
> --
> 2.51.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to