Message ID | 20200528110438.1058114-1-numans@ovn.org |
---|---|
State | Superseded |
Headers | show |
Series | Incremental processing improvements. | expand |
On 5/28/20 1:04 PM, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > 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 <mmichels@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > 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) \ >
On Thu, May 28, 2020 at 11:29 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 5/28/20 1:04 PM, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > 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 <mmichels@redhat.com> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > 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 > Just to add my 2 cents. I understand the motivation of trying to abstract "tracked_data". However, since the data in I-P node are completely vague and unstructured, getting tracked data from the API doesn't make things easier than just get the tracked data from the data. By contrast, tracked changes of OVSDB IDL has unified APIs (templates) to traverse and access the data, which makes the tracking APIs useful and necessary. So I tend to agree with Dumitru that adding an API to clean tracked data after each iteration would be enough and clear, and let each node maintain its tracked data (if necessary) as part of its "data", which can keep the data interface of I-P as simple as possible. (but let's make sure the API name is not confused with cleanup() that is called before program exits. Maybe we can still name the API as clear_tracked_data()). Thanks, Han > > }; > > > > /* 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Fri, May 29, 2020 at 12:29 AM Han Zhou <zhouhan@gmail.com> wrote: > On Thu, May 28, 2020 at 11:29 AM Dumitru Ceara <dceara@redhat.com> wrote: > > > > On 5/28/20 1:04 PM, numans@ovn.org wrote: > > > From: Numan Siddique <numans@ovn.org> > > > > > > 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 <mmichels@redhat.com> > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > > --- > > > 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 > > > > Just to add my 2 cents. I understand the motivation of trying to abstract > "tracked_data". However, since the data in I-P node are completely vague > and unstructured, getting tracked data from the API doesn't make things > easier than just get the tracked data from the data. By contrast, tracked > changes of OVSDB IDL has unified APIs (templates) to traverse and access > the data, which makes the tracking APIs useful and necessary. So I tend to > agree with Dumitru that adding an API to clean tracked data after each > iteration would be enough and clear, and let each node maintain its tracked > data (if necessary) as part of its "data", which can keep the data > interface of I-P as simple as possible. (but let's make sure the API name > is not confused with cleanup() that is called before program exits. Maybe > we can still name the API as clear_tracked_data()). > > Thanks, > Han > Thanks Han and Dumitru. Sure. I can have the tracked data maintained as part of the engine data. I'll address this in v10. I'll wait for sometime to incorporate any comments if you have for other patches of the series before submitting v10. Thanks Numan > > > }; > > > > > > /* 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 > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Thu, May 28, 2020 at 12:15 PM Numan Siddique <numans@ovn.org> wrote: > > > > On Fri, May 29, 2020 at 12:29 AM Han Zhou <zhouhan@gmail.com> wrote: >> >> On Thu, May 28, 2020 at 11:29 AM Dumitru Ceara <dceara@redhat.com> wrote: >> > >> > On 5/28/20 1:04 PM, numans@ovn.org wrote: >> > > From: Numan Siddique <numans@ovn.org> >> > > >> > > 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 <mmichels@redhat.com> >> > > Signed-off-by: Numan Siddique <numans@ovn.org> >> > > --- >> > > 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 >> > >> >> Just to add my 2 cents. I understand the motivation of trying to abstract >> "tracked_data". However, since the data in I-P node are completely vague >> and unstructured, getting tracked data from the API doesn't make things >> easier than just get the tracked data from the data. By contrast, tracked >> changes of OVSDB IDL has unified APIs (templates) to traverse and access >> the data, which makes the tracking APIs useful and necessary. So I tend to >> agree with Dumitru that adding an API to clean tracked data after each >> iteration would be enough and clear, and let each node maintain its tracked >> data (if necessary) as part of its "data", which can keep the data >> interface of I-P as simple as possible. (but let's make sure the API name >> is not confused with cleanup() that is called before program exits. Maybe >> we can still name the API as clear_tracked_data()). >> >> Thanks, >> Han > > > > Thanks Han and Dumitru. > > Sure. I can have the tracked data maintained as part of the engine data. > I'll address this in v10. I'll wait for sometime to incorporate any comments if you > have for other patches of the series before submitting v10. > > Thanks > Numan > Thanks Numan. I have some more comment for this patch, assuming it will add interface for clear_tracked_data only. 1) I think it is better to pass the callback as argument for the node definition, instead of dynamically assign it in init(). Since it is optional and most node doesn't need it, it can be set to NULL in macro ENGINE_NODE and ENGINE_NODE_CUSTOM_DATA, and define a new macro ENGINE_NODE_WITH_CLEAR_TRACK to support clear data but no is_valid(). I know this won't scale if we add more optional callbacks in the future. I am not sure if there is a smarter way to do it. 2) I think it is better be called by the engine_init_run(), instead of by engine_run(). Although it doesn't make much difference today, but just in case that in the future the tracked changes needs to be accessed after the engine run, for some of the output nodes. It is safe if we always clear the tracked data before the next engine_run() (which is the engine_init_run()). What do you think? >> >> > > }; >> > > >> > > /* 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 >> > dev@openvswitch.org >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>
On Fri, May 29, 2020 at 1:31 AM Han Zhou <zhouhan@gmail.com> wrote: > On Thu, May 28, 2020 at 12:15 PM Numan Siddique <numans@ovn.org> wrote: > > > > > > > > On Fri, May 29, 2020 at 12:29 AM Han Zhou <zhouhan@gmail.com> wrote: > >> > >> On Thu, May 28, 2020 at 11:29 AM Dumitru Ceara <dceara@redhat.com> > wrote: > >> > > >> > On 5/28/20 1:04 PM, numans@ovn.org wrote: > >> > > From: Numan Siddique <numans@ovn.org> > >> > > > >> > > 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 <mmichels@redhat.com> > >> > > Signed-off-by: Numan Siddique <numans@ovn.org> > >> > > --- > >> > > 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 > >> > > >> > >> Just to add my 2 cents. I understand the motivation of trying to > abstract > >> "tracked_data". However, since the data in I-P node are completely vague > >> and unstructured, getting tracked data from the API doesn't make things > >> easier than just get the tracked data from the data. By contrast, > tracked > >> changes of OVSDB IDL has unified APIs (templates) to traverse and access > >> the data, which makes the tracking APIs useful and necessary. So I tend > to > >> agree with Dumitru that adding an API to clean tracked data after each > >> iteration would be enough and clear, and let each node maintain its > tracked > >> data (if necessary) as part of its "data", which can keep the data > >> interface of I-P as simple as possible. (but let's make sure the API > name > >> is not confused with cleanup() that is called before program exits. > Maybe > >> we can still name the API as clear_tracked_data()). > >> > >> Thanks, > >> Han > > > > > > > > Thanks Han and Dumitru. > > > > Sure. I can have the tracked data maintained as part of the engine data. > > I'll address this in v10. I'll wait for sometime to incorporate any > comments if you > > have for other patches of the series before submitting v10. > > > > Thanks > > Numan > > > > Thanks Numan. I have some more comment for this patch, assuming it will add > interface for clear_tracked_data only. > > 1) I think it is better to pass the callback as argument for the node > definition, instead of dynamically assign it in init(). Since it is > optional and most node doesn't need it, it can be set to NULL in macro > ENGINE_NODE and ENGINE_NODE_CUSTOM_DATA, and define a new macro > ENGINE_NODE_WITH_CLEAR_TRACK to support clear data but no is_valid(). I > know this won't scale if we add more optional callbacks in the future. I am > not sure if there is a smarter way to do it. > Sounds good to me. You mean an engine node which supports tracking can not support is_valid() ? > > 2) I think it is better be called by the engine_init_run(), instead of by > engine_run(). Although it doesn't make much difference today, but just in > case that in the future the tracked changes needs to be accessed after the > engine run, for some of the output nodes. It is safe if we always clear the > tracked data before the next engine_run() (which is the engine_init_run()). > > What do you think? > Ok. Since the tracked data will be part of the engine data and since the engine data can be accessed outside of the engine, I think it makes sense to clear it in engine_init_run(). Thanks Numan > >> > >> > > }; > >> > > > >> > > /* 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 > >> > dev@openvswitch.org > >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Thu, May 28, 2020 at 1:13 PM Numan Siddique <numans@ovn.org> wrote: > > > > On Fri, May 29, 2020 at 1:31 AM Han Zhou <zhouhan@gmail.com> wrote: >> >> On Thu, May 28, 2020 at 12:15 PM Numan Siddique <numans@ovn.org> wrote: >> > >> > >> > >> > On Fri, May 29, 2020 at 12:29 AM Han Zhou <zhouhan@gmail.com> wrote: >> >> >> >> On Thu, May 28, 2020 at 11:29 AM Dumitru Ceara <dceara@redhat.com> wrote: >> >> > >> >> > On 5/28/20 1:04 PM, numans@ovn.org wrote: >> >> > > From: Numan Siddique <numans@ovn.org> >> >> > > >> >> > > 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 <mmichels@redhat.com> >> >> > > Signed-off-by: Numan Siddique <numans@ovn.org> >> >> > > --- >> >> > > 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 >> >> > >> >> >> >> Just to add my 2 cents. I understand the motivation of trying to abstract >> >> "tracked_data". However, since the data in I-P node are completely vague >> >> and unstructured, getting tracked data from the API doesn't make things >> >> easier than just get the tracked data from the data. By contrast, tracked >> >> changes of OVSDB IDL has unified APIs (templates) to traverse and access >> >> the data, which makes the tracking APIs useful and necessary. So I tend >> to >> >> agree with Dumitru that adding an API to clean tracked data after each >> >> iteration would be enough and clear, and let each node maintain its >> tracked >> >> data (if necessary) as part of its "data", which can keep the data >> >> interface of I-P as simple as possible. (but let's make sure the API name >> >> is not confused with cleanup() that is called before program exits. Maybe >> >> we can still name the API as clear_tracked_data()). >> >> >> >> Thanks, >> >> Han >> > >> > >> > >> > Thanks Han and Dumitru. >> > >> > Sure. I can have the tracked data maintained as part of the engine data. >> > I'll address this in v10. I'll wait for sometime to incorporate any >> comments if you >> > have for other patches of the series before submitting v10. >> > >> > Thanks >> > Numan >> > >> >> Thanks Numan. I have some more comment for this patch, assuming it will add >> interface for clear_tracked_data only. >> >> 1) I think it is better to pass the callback as argument for the node >> definition, instead of dynamically assign it in init(). Since it is >> optional and most node doesn't need it, it can be set to NULL in macro >> ENGINE_NODE and ENGINE_NODE_CUSTOM_DATA, and define a new macro >> ENGINE_NODE_WITH_CLEAR_TRACK to support clear data but no is_valid(). I >> know this won't scale if we add more optional callbacks in the future. I am >> not sure if there is a smarter way to do it. > > > Sounds good to me. You mean an engine node which supports tracking > can not support is_valid() ? > No I didn't mean that. If a node need both clear_tracked_data() and is_valid(), it can use ENGINE_NODE_DEF. Right now I think this is ok, but if there are more optional callbacks needed in the future we will need a more clever way. > >> >> >> 2) I think it is better be called by the engine_init_run(), instead of by >> engine_run(). Although it doesn't make much difference today, but just in >> case that in the future the tracked changes needs to be accessed after the >> engine run, for some of the output nodes. It is safe if we always clear the >> tracked data before the next engine_run() (which is the engine_init_run()). >> >> What do you think? > > > Ok. Since the tracked data will be part of the engine data and since the engine data > can be accessed outside of the engine, I think it makes sense to clear it > in engine_init_run(). > > Thanks > Numan > > >> >> >> >> >> > > }; >> >> > > >> >> > > /* 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 >> >> > dev@openvswitch.org >> >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> _______________________________________________ >> >> dev mailing list >> >> dev@openvswitch.org >> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>
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); }; /* 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) \