From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id B7F077CDF for ; Sun, 4 Jun 2017 18:21:01 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jun 2017 09:21:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,296,1493708400"; d="scan'208";a="110260369" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by fmsmga005.fm.intel.com with ESMTP; 04 Jun 2017 09:20:59 -0700 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.250]) by IRSMSX102.ger.corp.intel.com ([169.254.2.87]) with mapi id 14.03.0319.002; Sun, 4 Jun 2017 17:20:57 +0100 From: "Ananyev, Konstantin" To: "Legacy, Allain (Wind River)" CC: "dev@dpdk.org" , "Osman, Dahir (Wind River)" Thread-Topic: [PATCH] ip_frag: free mbufs on reassembly table destroy Thread-Index: AQHSveY26vDZP7+eXEu6uE/0PIX3tqIVHwlg Date: Sun, 4 Jun 2017 16:20:56 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583FB058B4@IRSMSX109.ger.corp.intel.com> References: <20170425170450.173221-1-allain.legacy@windriver.com> In-Reply-To: <20170425170450.173221-1-allain.legacy@windriver.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] ip_frag: free mbufs on reassembly table destroy X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Jun 2017 16:21:02 -0000 Hu Allain, > -----Original Message----- > From: Allain Legacy [mailto:allain.legacy@windriver.com] > Sent: Tuesday, April 25, 2017 6:05 PM > To: Ananyev, Konstantin > Cc: dev@dpdk.org; Osman, Dahir (Wind River) > Subject: [PATCH] ip_frag: free mbufs on reassembly table destroy >=20 > From: Dahir Osman >=20 > The rte_ip_frag_table_destroy procedure simply releases the memory for th= e > table without freeing the packet buffers that may be referenced in the ha= sh > table for in-flight or incomplete packet reassembly operations. To preve= nt > leaked mbufs go through the list of fragments and free each one > individually. >=20 > Reported-by: Matt Peters > Signed-off-by: Allain Legacy > --- > lib/librte_ip_frag/ip_frag_common.h | 20 ++++++++++++++++++++ > lib/librte_ip_frag/rte_ip_frag.h | 7 ++----- > lib/librte_ip_frag/rte_ip_frag_common.c | 12 ++++++++++++ > lib/librte_ip_frag/rte_ipfrag_version.map | 7 +++++++ > 4 files changed, 41 insertions(+), 5 deletions(-) >=20 > diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_= frag_common.h > index 835e4f93f..9f5619651 100644 > --- a/lib/librte_ip_frag/ip_frag_common.h > +++ b/lib/librte_ip_frag/ip_frag_common.h > @@ -130,6 +130,26 @@ ip_frag_free(struct ip_frag_pkt *fp, struct rte_ip_f= rag_death_row *dr) > dr->cnt =3D k; > } >=20 > +/* delete fragment's mbufs immediately instead of using death row */ > +static inline void > +ip_frag_free_immediate(struct ip_frag_pkt *fp) > +{ > + uint32_t i; > + > + for (i =3D 0; i < fp->last_idx; i++) { > + if (fp->frags[i].mb !=3D NULL) { > + IP_FRAG_LOG(DEBUG, "%s:%d\n" > + "mbuf: %p, tms: %" PRIu64", key: <%" PRIx64 ", %#x>\n", > + __func__, __LINE__, fp->frags[i].mb, fp->start, > + fp->key.src_dst[0], fp->key.id); > + rte_pktmbuf_free(fp->frags[i].mb); > + fp->frags[i].mb =3D NULL; > + } > + } > + > + fp->last_idx =3D 0; > +} > + > /* if key is empty, mark key as in use */ > static inline void > ip_frag_inuse(struct rte_ip_frag_tbl *tbl, const struct ip_frag_pkt *fp= ) > diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip= _frag.h > index 6708906d3..ff16f4c52 100644 > --- a/lib/librte_ip_frag/rte_ip_frag.h > +++ b/lib/librte_ip_frag/rte_ip_frag.h > @@ -180,11 +180,8 @@ struct rte_ip_frag_tbl * rte_ip_frag_table_create(ui= nt32_t bucket_num, > * @param tbl > * Fragmentation table to free. > */ > -static inline void > -rte_ip_frag_table_destroy(struct rte_ip_frag_tbl *tbl) > -{ > - rte_free(tbl); > -} > +void > +rte_ip_frag_table_destroy(struct rte_ip_frag_tbl *tbl); >=20 > /** > * This function implements the fragmentation of IPv6 packets. > diff --git a/lib/librte_ip_frag/rte_ip_frag_common.c b/lib/librte_ip_frag= /rte_ip_frag_common.c > index 6176ff4e0..70964101c 100644 > --- a/lib/librte_ip_frag/rte_ip_frag_common.c > +++ b/lib/librte_ip_frag/rte_ip_frag_common.c > @@ -109,6 +109,18 @@ rte_ip_frag_table_create(uint32_t bucket_num, uint32= _t bucket_entries, > return tbl; > } >=20 > +/* delete fragmentation table */ > +void > +rte_ip_frag_table_destroy(struct rte_ip_frag_tbl *tbl) > +{ > + uint32_t i; > + > + for (i =3D 0; i < tbl->nb_entries; i++) > + ip_frag_free_immediate(&tbl->pkt[i]); Looks ok, just one thought: wouldn't it be better(faster) in most cases to = iterate over lru list? I suppose usually we wouldn't have nearly all entries filled in. Konstantin > + > + rte_free(tbl); > +} > + > /* dump frag table statistics to file */ > void > rte_ip_frag_table_statistics_dump(FILE *f, const struct rte_ip_frag_tbl = *tbl) > diff --git a/lib/librte_ip_frag/rte_ipfrag_version.map b/lib/librte_ip_fr= ag/rte_ipfrag_version.map > index 354fa0822..29e2cfea5 100644 > --- a/lib/librte_ip_frag/rte_ipfrag_version.map > +++ b/lib/librte_ip_frag/rte_ipfrag_version.map > @@ -11,3 +11,10 @@ DPDK_2.0 { >=20 > local: *; > }; > + > +DPDK_17.05 { > + global: > + > + rte_ip_frag_table_destroy; > + > +} DPDK_2.0; > -- > 2.12.1