| Message ID | 20210204132508.9958.1650.stgit@dceara.remote.csb |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ovn-controller: Make lflow cache size configurable. | expand |
On Thu, Feb 4, 2021 at 6:55 PM Dumitru Ceara <dceara@redhat.com> wrote: > > It was (and still is) the responsibility of the caller of > convert_match_to_expr() to explicitly free any 'prereqs' expression that > was passed as argument if the expression parsing of the 'lflow' match failed. > > However, convert_match_to_expr() now updates the value of '*prereqs' setting > it to NULL in the successful case. This makes it easier for callers of the > function because 'expr_destroy(prereqs)' can now be called unconditionally. > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> Numan > --- > Note: This patch doesn't change the callers of convert_match_to_expr() to > take advantage of the new semantic but following patches do. > --- > controller/lflow.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/controller/lflow.c b/controller/lflow.c > index 04ba0d1..495653f 100644 > --- a/controller/lflow.c > +++ b/controller/lflow.c > @@ -758,11 +758,12 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, > /* Converts the match and returns the simplified expr tree. > * > * The caller should evaluate the conditions and normalize the expr tree. > + * If parsing is successful, '*prereqs' is also consumed. > */ > static struct expr * > convert_match_to_expr(const struct sbrec_logical_flow *lflow, > const struct sbrec_datapath_binding *dp, > - struct expr *prereqs, > + struct expr **prereqs, > const struct shash *addr_sets, > const struct shash *port_groups, > struct lflow_resource_ref *lfrr, > @@ -795,8 +796,9 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, > sset_destroy(&port_groups_ref); > > if (!error) { > - if (prereqs) { > - e = expr_combine(EXPR_T_AND, e, prereqs); > + if (*prereqs) { > + e = expr_combine(EXPR_T_AND, e, *prereqs); > + *prereqs = NULL; > } > e = expr_annotate(e, &symtab, &error); > } > @@ -854,7 +856,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, > .n_tables = LOG_PIPELINE_LEN, > .cur_ltable = lflow->table_id, > }; > - struct expr *prereqs; > + struct expr *prereqs = NULL; > char *error; > > error = ovnacts_parse_string(lflow->actions, &pp, &ovnacts, &prereqs); > @@ -885,7 +887,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, > struct expr *expr = NULL; > if (!l_ctx_out->lflow_cache_map) { > /* Caching is disabled. */ > - expr = convert_match_to_expr(lflow, dp, prereqs, l_ctx_in->addr_sets, > + expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, > l_ctx_in->port_groups, l_ctx_out->lfrr, > NULL); > if (!expr) { > @@ -949,7 +951,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, > > bool pg_addr_set_ref = false; > if (!expr) { > - expr = convert_match_to_expr(lflow, dp, prereqs, l_ctx_in->addr_sets, > + expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, > l_ctx_in->port_groups, l_ctx_out->lfrr, > &pg_addr_set_ref); > if (!expr) { > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/lflow.c b/controller/lflow.c index 04ba0d1..495653f 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -758,11 +758,12 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, /* Converts the match and returns the simplified expr tree. * * The caller should evaluate the conditions and normalize the expr tree. + * If parsing is successful, '*prereqs' is also consumed. */ static struct expr * convert_match_to_expr(const struct sbrec_logical_flow *lflow, const struct sbrec_datapath_binding *dp, - struct expr *prereqs, + struct expr **prereqs, const struct shash *addr_sets, const struct shash *port_groups, struct lflow_resource_ref *lfrr, @@ -795,8 +796,9 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, sset_destroy(&port_groups_ref); if (!error) { - if (prereqs) { - e = expr_combine(EXPR_T_AND, e, prereqs); + if (*prereqs) { + e = expr_combine(EXPR_T_AND, e, *prereqs); + *prereqs = NULL; } e = expr_annotate(e, &symtab, &error); } @@ -854,7 +856,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, .n_tables = LOG_PIPELINE_LEN, .cur_ltable = lflow->table_id, }; - struct expr *prereqs; + struct expr *prereqs = NULL; char *error; error = ovnacts_parse_string(lflow->actions, &pp, &ovnacts, &prereqs); @@ -885,7 +887,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, struct expr *expr = NULL; if (!l_ctx_out->lflow_cache_map) { /* Caching is disabled. */ - expr = convert_match_to_expr(lflow, dp, prereqs, l_ctx_in->addr_sets, + expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, l_ctx_in->port_groups, l_ctx_out->lfrr, NULL); if (!expr) { @@ -949,7 +951,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, bool pg_addr_set_ref = false; if (!expr) { - expr = convert_match_to_expr(lflow, dp, prereqs, l_ctx_in->addr_sets, + expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets, l_ctx_in->port_groups, l_ctx_out->lfrr, &pg_addr_set_ref); if (!expr) {
It was (and still is) the responsibility of the caller of convert_match_to_expr() to explicitly free any 'prereqs' expression that was passed as argument if the expression parsing of the 'lflow' match failed. However, convert_match_to_expr() now updates the value of '*prereqs' setting it to NULL in the successful case. This makes it easier for callers of the function because 'expr_destroy(prereqs)' can now be called unconditionally. Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- Note: This patch doesn't change the callers of convert_match_to_expr() to take advantage of the new semantic but following patches do. --- controller/lflow.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)