[dpdk-dev] [PATCH] lpm/lpm6: fix missing free of rules_tbl and lpm
Bruce Richardson
bruce.richardson at intel.com
Fri Mar 11 15:00:11 CET 2016
On Tue, Mar 08, 2016 at 05:38:28PM +0100, Christian Ehrhardt wrote:
> Hi,
> Stephen acked the other LPM patch I had last week (thanks).
> There was no feedback to this one so far and none of the two patches is
> committed yet.
>
> So I wanted to give this another "ping" for feedback or acceptance.
>
> Thanks in advance,
> Christian
>
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>
Hi Christian,
sorry for the delayed comments. This fix looks ok to me, but the patch appears
to be missing your signoff.
Two minor comments
* This is probably better as two patches, one for lpm6, other for lpm
* Coding standards state that we don't use "{}" for blocks with only
a single line.
Otherwise:
Acked-by: Bruce Richardson <bruce.richardson at intel.com>
Regards,
/Bruce
> On Fri, Mar 4, 2016 at 2:29 PM, Christian Ehrhardt <
> christian.ehrhardt at canonical.com> wrote:
>
> > lpm6 autotests failed with the default alloc of 512M Memory.
> > While >=2500M was a workaround it became clear while debugging that it
> > had a leak.
> > One could see a lot of output like:
> > LPM Test tests6[i]: FAIL
> > LPM: LPM memory allocation failed
> >
> > It turned out that in rte_lpm6_free
> > - lpm might not be freed if it didn't find a te (early return)
> > - lpm->rules_tbl was not freed ever
> >
> > The first of the two also applies to lpm.
> > ---
> > lib/librte_lpm/rte_lpm.c | 7 ++-----
> > lib/librte_lpm/rte_lpm6.c | 9 ++++-----
> > 2 files changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
> > index cfdf959..941720d 100644
> > --- a/lib/librte_lpm/rte_lpm.c
> > +++ b/lib/librte_lpm/rte_lpm.c
> > @@ -236,13 +236,10 @@ rte_lpm_free(struct rte_lpm *lpm)
> > if (te->data == (void *) lpm)
> > break;
> > }
> > - if (te == NULL) {
> > - rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
> > - return;
> > + if (te != NULL) {
> > + TAILQ_REMOVE(lpm_list, te, next);
> > }
> >
> > - TAILQ_REMOVE(lpm_list, te, next);
> > -
> > rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
> >
> > rte_free(lpm);
> > diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
> > index 48931cc..5abfc78 100644
> > --- a/lib/librte_lpm/rte_lpm6.c
> > +++ b/lib/librte_lpm/rte_lpm6.c
> > @@ -278,15 +278,14 @@ rte_lpm6_free(struct rte_lpm6 *lpm)
> > if (te->data == (void *) lpm)
> > break;
> > }
> > - if (te == NULL) {
> > - rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
> > - return;
> > - }
> >
> > - TAILQ_REMOVE(lpm_list, te, next);
> > + if (te != NULL) {
> > + TAILQ_REMOVE(lpm_list, te, next);
> > + }
> >
> > rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
> >
> > + rte_free(lpm->rules_tbl);
> > rte_free(lpm);
> > rte_free(te);
> > }
> > --
> > 2.7.0
> >
> >
More information about the dev
mailing list