Hi David, Owen, > -----Original Message----- > From: David Marchand <[email protected]> > Sent: Tuesday, June 22, 2021 8:24 AM > To: Owen Hilyard <[email protected]>; Iremonger, Bernard > <[email protected]>; Yigit, Ferruh <[email protected]> > Cc: dev <[email protected]> > Subject: Re: [PATCH] lib/flow_classify: fix leaking rules on delete > > On Wed, Jun 16, 2021 at 9:57 PM <[email protected]> wrote: > > > > From: Owen Hilyard <[email protected]> > > > > Rules in a classify table were not freed if the table had a delete > > function. > > > > Fixes: be41ac2a3 ("flow_classify: introduce flow classify library") > Cc: [email protected] > > > > > Signed-off-by: Owen Hilyard <[email protected]> > > --- > > lib/flow_classify/rte_flow_classify.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/flow_classify/rte_flow_classify.c > > b/lib/flow_classify/rte_flow_classify.c > > index f125267e8..06aed3b70 100644 > > --- a/lib/flow_classify/rte_flow_classify.c > > +++ b/lib/flow_classify/rte_flow_classify.c > > @@ -579,7 +579,7 @@ rte_flow_classify_table_entry_delete(struct > rte_flow_classifier *cls, > > &rule->u.key.key_del, > > &rule->key_found, > > &rule->entry); > > - > > + free(rule); > > return ret; > > } > > } > > I find it strange to free the rule regardless of the result of the > f_delete() op.
I agree the result of the f_delete() op should be checked before freeing the rule. > The same is done out of the loop which means this function returns -EINVAL > and frees the rule in this case too. The free() outside the loop at line 587 does not make sense to me now and should be removed. > > Bernard, Ferruh, can you review please? > > Thanks! > > > -- > David Marchand Regards, Bernard.
