On 5/28/20 1:04 PM, [email protected] wrote:
> From: Numan Siddique <[email protected]>
>
> With this patch, an engine node which is an input to another engine node
> can provide the tracked data. The parent of this engine node can handle
> this tracked data incrementally.
>
> At the end of the engine_run(), the tracked data of the nodes are
> cleared.
>
> Acked-by: Mark Michelson <[email protected]>
> Signed-off-by: Numan Siddique <[email protected]>
> ---
> lib/inc-proc-eng.c | 30 +++++++++++++++++++++++++++++-
> lib/inc-proc-eng.h | 20 ++++++++++++++++++++
> 2 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
> index 9b1479a1c..8d63feac7 100644
> --- a/lib/inc-proc-eng.c
> +++ b/lib/inc-proc-eng.c
> @@ -125,6 +125,11 @@ engine_cleanup(void)
> engine_nodes[i]->cleanup(engine_nodes[i]->data);
> }
> free(engine_nodes[i]->data);
> +
> + if (engine_nodes[i]->clear_tracked_data) {
> +
> engine_nodes[i]->clear_tracked_data(engine_nodes[i]->tracked_data);
> + }
> + free(engine_nodes[i]->tracked_data);
> }
> free(engine_nodes);
> engine_nodes = NULL;
> @@ -222,6 +227,23 @@ engine_node_changed(struct engine_node *node)
> return node->state == EN_UPDATED;
> }
>
> +void *
> +engine_get_tracked_data(struct engine_node *node)
> +{
> + if (engine_node_valid(node)) {
> + return node->tracked_data;
> + }
> +
> + return NULL;
> +}
> +
> +void *
> +engine_get_input_tracked_data(const char *input_name, struct engine_node
> *node)
> +{
> + struct engine_node *input_node = engine_get_input(input_name, node);
> + return engine_get_tracked_data(input_node);
> +}
> +
> bool
> engine_has_run(void)
> {
> @@ -370,7 +392,13 @@ engine_run(bool recompute_allowed)
>
> if (engine_nodes[i]->state == EN_ABORTED) {
> engine_run_aborted = true;
> - return;
> + break;
> + }
> + }
> +
> + for (size_t i = 0; i < engine_n_nodes; i++) {
> + if (engine_nodes[i]->clear_tracked_data) {
> +
> engine_nodes[i]->clear_tracked_data(engine_nodes[i]->tracked_data);
> }
> }
> }
> diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
> index 780c3cd22..4b5e69edb 100644
> --- a/lib/inc-proc-eng.h
> +++ b/lib/inc-proc-eng.h
> @@ -123,6 +123,15 @@ struct engine_node {
> */
> void *data;
>
> + /* A pointer to node internal tracked data. The tracke data can be
> + * used by en engine node to provide the changed data during the
> + * engine run if its an input to other engine node. This data can
> + * be accessed during the engine run using the function
> + * engine_get_tracked_data(). This data can be cleared by
> + * calling the clean_tracked_data() engine node function.
> + */
> + void *tracked_data;
> +
> /* State of the node after the last engine run. */
> enum engine_node_state state;
>
> @@ -149,6 +158,9 @@ struct engine_node {
> * doesn't store pointers to DB records it's still safe to use).
> */
> bool (*is_valid)(struct engine_node *);
> +
> + /* Method to clear up tracked data if any. It may be NULL. */
> + void (*clear_tracked_data)(void *tracked_data);
Hi Numan,
As discussed during the IRC meeting, this patch is OK but I found it a
bit confusing that the only difference between "data" and "tracked_data"
is the fact that tracked_data can be cleared after an engine run. It's
not a big deal but it forces readers to look at how the tracked_data
APIs are used to understand what the tracked_data can be. I think we
could just rename "clear_tracked_data" to "clear_data" and let users
store whatever tracked data they need in "data" and provide callbacks to
clear what needs to be cleared after every run.
What do you think?
Thanks,
Dumitru
> };
>
> /* Initialize the data for the engine nodes. It calls each node's
> @@ -183,6 +195,12 @@ struct engine_node * engine_get_input(const char
> *input_name,
> /* Get the data from the input node with <name> for <node> */
> void *engine_get_input_data(const char *input_name, struct engine_node *);
>
> +void *engine_get_tracked_data(struct engine_node *);
> +
> +/* Get the tracked data from the input node with <name> for <node> */
> +void *engine_get_input_tracked_data(const char *input_name,
> + struct engine_node *);
> +
> /* Add an input (dependency) for <node>, with corresponding change_handler,
> * which can be NULL. If the change_handler is NULL, the engine will not
> * be able to process the change incrementally, and will fall back to call
> @@ -270,11 +288,13 @@ void engine_ovsdb_node_add_index(struct engine_node *,
> const char *name,
> struct engine_node en_##NAME = { \
> .name = NAME_STR, \
> .data = NULL, \
> + .tracked_data = NULL, \
> .state = EN_STALE, \
> .init = en_##NAME##_init, \
> .run = en_##NAME##_run, \
> .cleanup = en_##NAME##_cleanup, \
> .is_valid = en_##NAME##_is_valid, \
> + .clear_tracked_data = NULL, \
> };
>
> #define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
