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 > > > >