From: Owen Hilyard <ohilyard@iol.unh.edu> Rules in a classify table were not freed if the table had a delete function. Fixes: be41ac2a3 ("flow_classify: introduce flow classify library") Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- 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; } } -- 2.30.2
On Wed, Jun 16, 2021 at 9:57 PM <ohilyard@iol.unh.edu> wrote: > > From: Owen Hilyard <ohilyard@iol.unh.edu> > > Rules in a classify table were not freed if the table > had a delete function. > > Fixes: be41ac2a3 ("flow_classify: introduce flow classify library") Cc: stable@dpdk.org > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> > --- > 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. The same is done out of the loop which means this function returns -EINVAL and frees the rule in this case too. Bernard, Ferruh, can you review please? Thanks! -- David Marchand
Hi David, Owen, > -----Original Message----- > From: David Marchand <david.marchand@redhat.com> > Sent: Tuesday, June 22, 2021 8:24 AM > To: Owen Hilyard <ohilyard@iol.unh.edu>; Iremonger, Bernard > <bernard.iremonger@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> > Cc: dev <dev@dpdk.org> > Subject: Re: [PATCH] lib/flow_classify: fix leaking rules on delete > > On Wed, Jun 16, 2021 at 9:57 PM <ohilyard@iol.unh.edu> wrote: > > > > From: Owen Hilyard <ohilyard@iol.unh.edu> > > > > Rules in a classify table were not freed if the table had a delete > > function. > > > > Fixes: be41ac2a3 ("flow_classify: introduce flow classify library") > Cc: stable@dpdk.org > > > > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> > > --- > > 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.
From: Owen Hilyard <ohilyard@iol.unh.edu> Rules in a classify table were not freed if the table had a delete function. Fixes: be41ac2a3 ("flow_classify: introduce flow classify library") Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- lib/flow_classify/rte_flow_classify.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/flow_classify/rte_flow_classify.c b/lib/flow_classify/rte_flow_classify.c index f125267e8..d3ba2ed22 100644 --- a/lib/flow_classify/rte_flow_classify.c +++ b/lib/flow_classify/rte_flow_classify.c @@ -579,12 +579,12 @@ rte_flow_classify_table_entry_delete(struct rte_flow_classifier *cls, &rule->u.key.key_del, &rule->key_found, &rule->entry); - + if (ret == 0) + free(rule); return ret; } } } - free(rule); return ret; } -- 2.30.2
Hi Owen,
> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Wednesday, June 23, 2021 6:07 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; david.marchand@redhat.com; Owen Hilyard
> <ohilyard@iol.unh.edu>
> Subject: [PATCH v2] flow_classify: fix leaking rules on delete
>
> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> Rules in a classify table were not freed if the table had a delete function.
>
> Fixes: be41ac2a3 ("flow_classify: introduce flow classify library")
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
> lib/flow_classify/rte_flow_classify.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/flow_classify/rte_flow_classify.c
> b/lib/flow_classify/rte_flow_classify.c
> index f125267e8..d3ba2ed22 100644
> --- a/lib/flow_classify/rte_flow_classify.c
> +++ b/lib/flow_classify/rte_flow_classify.c
> @@ -579,12 +579,12 @@ rte_flow_classify_table_entry_delete(struct
> rte_flow_classifier *cls,
> &rule->u.key.key_del,
> &rule->key_found,
> &rule->entry);
> -
> + if (ret == 0)
> + free(rule);
> return ret;
> }
> }
> }
> - free(rule);
> return ret;
> }
>
> --
> 2.30.2
This patch should be backported.
Please add the following line after the Fixes line:
Cc: stable@dpdk.org
Otherwise
Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
On Thu, Jun 24, 2021 at 10:43 AM Iremonger, Bernard
<bernard.iremonger@intel.com> wrote:
> > -----Original Message-----
> > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> > Sent: Wednesday, June 23, 2021 6:07 PM
> > To: Iremonger, Bernard <bernard.iremonger@intel.com>
> > Cc: dev@dpdk.org; david.marchand@redhat.com; Owen Hilyard
> > <ohilyard@iol.unh.edu>
> > Subject: [PATCH v2] flow_classify: fix leaking rules on delete
> >
> > From: Owen Hilyard <ohilyard@iol.unh.edu>
> >
> > Rules in a classify table were not freed if the table had a delete function.
> >
> > Fixes: be41ac2a3 ("flow_classify: introduce flow classify library")
> >
> > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> This patch should be backported.
> Please add the following line after the Fixes line:
> Cc: stable@dpdk.org
I'll add it when applying, no need for a v3.
Thanks.
--
David Marchand
On Wed, Jun 23, 2021 at 7:07 PM <ohilyard@iol.unh.edu> wrote: > > From: Owen Hilyard <ohilyard@iol.unh.edu> > > Rules in a classify table were not freed if the table > had a delete function. > > Fixes: be41ac2a3 ("flow_classify: introduce flow classify library") Updated sha1 to be 12 characters. Cc: stable@dpdk.org > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com> Applied, thanks. -- David Marchand