From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id AB4F344C7 for ; Thu, 31 May 2018 15:08:04 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 May 2018 06:08:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,463,1520924400"; d="scan'208";a="43638872" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by fmsmga007.fm.intel.com with ESMTP; 31 May 2018 06:08:01 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.126]) by IRSMSX101.ger.corp.intel.com ([169.254.1.148]) with mapi id 14.03.0319.002; Thu, 31 May 2018 14:08:01 +0100 From: "Ananyev, Konstantin" To: Alex Kiselev , "dev@dpdk.org" , "Burakov, Anatoly" Thread-Topic: [dpdk-dev] [PATCH 1/2] librte_ip_frag: add function to delete expired entries Thread-Index: AQHT7QWtLTR6SZ1lvEaqjXcDgcPB2KRJ4zag Date: Thu, 31 May 2018 13:08:00 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258C0C36A84@irsmsx105.ger.corp.intel.com> References: <20180516110420.DCA641B21F@dpdk.org> In-Reply-To: <20180516110420.DCA641B21F@dpdk.org> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTgyMDMxNDItMjZhOS00NmY2LThmNjItYTI2MzA2ODQ0OWU0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoieFNZQmhqVkRDQUtBTlwvSVVjTityRVMwY1dTdlRDblB2NmR2WjN0bHNDVmtoZFQxa05BcndwM1NWbXFabEl6YXkifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 1/2] librte_ip_frag: add function to delete expired entries 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: Thu, 31 May 2018 13:08:05 -0000 Hi Alex, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Alex Kiselev > Sent: Wednesday, May 16, 2018 12:04 PM > To: dev@dpdk.org; Burakov, Anatoly > Subject: [dpdk-dev] [PATCH 1/2] librte_ip_frag: add function to delete ex= pired entries >=20 > add new function rte_frag_table_del_expired_entries() > that scans the list of recently used packets and delete the expired ones. >=20 > A fragmented packets is supposed to live no longer than max_cycles, > but the lib deletes an expired packet only occasionally when it scans > a bucket to find an empty slot while adding a new packet. > Therefore a fragment might sit in the table forever. >=20 > Signed-off-by: Alex Kiselev > --- > lib/librte_ip_frag/ip_frag_common.h | 18 ++++++++++++ > lib/librte_ip_frag/ip_frag_internal.c | 18 ------------ > lib/librte_ip_frag/rte_ip_frag.h | 19 +++++++++++- > lib/librte_ip_frag/rte_ip_frag_common.c | 46 ++++++++++++++++++++++++= ++++++ > lib/librte_ip_frag/rte_ip_frag_version.map | 6 ++++ > 5 files changed, 88 insertions(+), 19 deletions(-) >=20 > diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_= frag_common.h > index 197acf8d8..0fdcc7d0f 100644 > --- a/lib/librte_ip_frag/ip_frag_common.h > +++ b/lib/librte_ip_frag/ip_frag_common.h > @@ -25,6 +25,12 @@ > #define IPv6_KEY_BYTES_FMT \ > "%08" PRIx64 "%08" PRIx64 "%08" PRIx64 "%08" PRIx64 >=20 > +#ifdef RTE_LIBRTE_IP_FRAG_TBL_STAT > +#define IP_FRAG_TBL_STAT_UPDATE(s, f, v) ((s)->f +=3D (v)) > +#else > +#define IP_FRAG_TBL_STAT_UPDATE(s, f, v) do {} while (0) > +#endif /* IP_FRAG_TBL_STAT */ > + > /* internal functions declarations */ > struct rte_mbuf * ip_frag_process(struct ip_frag_pkt *fp, > struct rte_ip_frag_death_row *dr, struct rte_mbuf *mb, > @@ -149,4 +155,16 @@ ip_frag_reset(struct ip_frag_pkt *fp, uint64_t tms) > fp->frags[IP_FIRST_FRAG_IDX] =3D zero_frag; > } >=20 > +/* local frag table helper functions */ > +static inline void > +ip_frag_tbl_del(struct rte_ip_frag_tbl *tbl, struct rte_ip_frag_death_ro= w *dr, > + struct ip_frag_pkt *fp) > +{ > + ip_frag_free(fp, dr); > + ip_frag_key_invalidate(&fp->key); > + TAILQ_REMOVE(&tbl->lru, fp, lru); > + tbl->use_entries--; > + IP_FRAG_TBL_STAT_UPDATE(&tbl->stat, del_num, 1); > +} > + > #endif /* _IP_FRAG_COMMON_H_ */ > diff --git a/lib/librte_ip_frag/ip_frag_internal.c b/lib/librte_ip_frag/i= p_frag_internal.c > index 2560c7713..97470a872 100644 > --- a/lib/librte_ip_frag/ip_frag_internal.c > +++ b/lib/librte_ip_frag/ip_frag_internal.c > @@ -14,24 +14,6 @@ > #define IP_FRAG_TBL_POS(tbl, sig) \ > ((tbl)->pkt + ((sig) & (tbl)->entry_mask)) >=20 > -#ifdef RTE_LIBRTE_IP_FRAG_TBL_STAT > -#define IP_FRAG_TBL_STAT_UPDATE(s, f, v) ((s)->f +=3D (v)) > -#else > -#define IP_FRAG_TBL_STAT_UPDATE(s, f, v) do {} while (0) > -#endif /* IP_FRAG_TBL_STAT */ > - > -/* local frag table helper functions */ > -static inline void > -ip_frag_tbl_del(struct rte_ip_frag_tbl *tbl, struct rte_ip_frag_death_ro= w *dr, > - struct ip_frag_pkt *fp) > -{ > - ip_frag_free(fp, dr); > - ip_frag_key_invalidate(&fp->key); > - TAILQ_REMOVE(&tbl->lru, fp, lru); > - tbl->use_entries--; > - IP_FRAG_TBL_STAT_UPDATE(&tbl->stat, del_num, 1); > -} > - > static inline void > ip_frag_tbl_add(struct rte_ip_frag_tbl *tbl, struct ip_frag_pkt *fp, > const struct ip_frag_key *key, uint64_t tms) > diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip= _frag.h > index b3f3f78df..3c694df92 100644 > --- a/lib/librte_ip_frag/rte_ip_frag.h > +++ b/lib/librte_ip_frag/rte_ip_frag.h > @@ -65,10 +65,13 @@ struct ip_frag_pkt { >=20 > #define IP_FRAG_DEATH_ROW_LEN 32 /**< death row size (in packets) */ >=20 > +/* death row size in mbufs */ > +#define IP_FRAG_DEATH_ROW_MBUF_LEN (IP_FRAG_DEATH_ROW_LEN * (IP_MAX_FRAG= _NUM + 1)) > + > /** mbuf death row (packets to be freed) */ > struct rte_ip_frag_death_row { > uint32_t cnt; /**< number of mbufs currently on death row */ > - struct rte_mbuf *row[IP_FRAG_DEATH_ROW_LEN * (IP_MAX_FRAG_NUM + 1)]; > + struct rte_mbuf *row[IP_FRAG_DEATH_ROW_MBUF_LEN]; > /**< mbufs to be freed */ > }; >=20 > @@ -325,6 +328,20 @@ void rte_ip_frag_free_death_row(struct rte_ip_frag_d= eath_row *dr, > void > rte_ip_frag_table_statistics_dump(FILE * f, const struct rte_ip_frag_tbl= *tbl); >=20 > +/** > + * Delete expired fragments > + * > + * @param tbl > + * Table to delete expired fragments from > + * @param dr > + * Death row to free buffers to > + * @param tms > + * Current timestamp > + */ > +void __rte_experimental > +rte_frag_table_del_expired_entries(struct rte_ip_frag_tbl *tbl, > + struct rte_ip_frag_death_row *dr, uint64_t tms); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/librte_ip_frag/rte_ip_frag_common.c b/lib/librte_ip_frag= /rte_ip_frag_common.c > index 659a17951..f62b5d169 100644 > --- a/lib/librte_ip_frag/rte_ip_frag_common.c > +++ b/lib/librte_ip_frag/rte_ip_frag_common.c > @@ -121,3 +121,49 @@ rte_ip_frag_table_statistics_dump(FILE *f, const str= uct rte_ip_frag_tbl *tbl) > fail_nospace, > fail_total - fail_nospace); > } > + > +/* Delete expired fragments */ > +void __rte_experimental > +rte_frag_table_del_expired_entries(struct rte_ip_frag_tbl *tbl, > + struct rte_ip_frag_death_row *dr, uint64_t tms) > +{ > + uint64_t max_cycles; > + struct ip_frag_pkt *fp; > + > + max_cycles =3D tbl->max_cycles; > + > + TAILQ_FOREACH(fp, &tbl->lru, lru) > + if (max_cycles + fp->start < tms) { > + /* check that death row has enough space */ > + if (IP_FRAG_DEATH_ROW_MBUF_LEN - dr->cnt >=3D fp->last_idx) > + ip_frag_tbl_del(tbl, dr, fp); > + else > + return; > + } > + else > + return; > +} > + > +#ifdef RTE_IP_FRAG_DEBUG I don't think you need that #ifdef at all. It's up to the user to call or not the function below. > + > +void > +rte_frag_table_print(struct rte_ip_frag_tbl *tbl) Looks like it is a public function, but I don't see any declaration for it. Also there exists rte_ip_frag_table_statistics_dump() - is it not enough? If so, then probably name it - rte_ip_frag_table_entries_dump() and And some more info for the table entry - key, etc? BTW, that function has nothing to do with rte_frag_table_del_expired_entrie= s() - So probably a separate patch or move into patch # 2. > +{ > + uint32_t i, cnt; > + printf("entries in use: %u, mbuf holded %u\n", tbl->use_entries, I think better to add extra argument: FILE *f to it, and use fprintf() here= . > + tbl->nb_mbufs); struct rte_ip_frag_tbl doesn't have nb_mbuf field. > + struct ip_frag_pkt *fp; > + TAILQ_FOREACH(fp, &tbl->lru, lru) > + if (!ip_frag_key_is_empty(&fp->key)) { > + > + /* cnt mbufs in the packet */ > + cnt =3D 0; > + for (i=3D0; i!=3Dfp->last_idx; i++) > + if (fp->frags[i].mb !=3D NULL) > + cnt++; > + > + printf("start %"PRIu64", mbuf cnt %u\n", fp->start, cnt); > + } > +} > + > +#endif /* RTE_IP_FRAG_DEBUG */ > diff --git a/lib/librte_ip_frag/rte_ip_frag_version.map b/lib/librte_ip_f= rag/rte_ip_frag_version.map > index d1acf07cb..d40d5515f 100644 > --- a/lib/librte_ip_frag/rte_ip_frag_version.map > +++ b/lib/librte_ip_frag/rte_ip_frag_version.map > @@ -18,3 +18,9 @@ DPDK_17.08 { > rte_ip_frag_table_destroy; >=20 > } DPDK_2.0; > + > +EXPERIMENTAL { > + global: > + > + rte_frag_table_del_expired_entries; > +}; > -- There are few checkpatch warnings, please fix them for new iteration. Konstantin