From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B9B7EA046B for ; Thu, 22 Aug 2019 13:22:11 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6AD431BF72; Thu, 22 Aug 2019 13:22:10 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 9F62A1BF71 for ; Thu, 22 Aug 2019 13:22:08 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Aug 2019 04:22:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,416,1559545200"; d="scan'208";a="186540247" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 22 Aug 2019 04:22:07 -0700 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 22 Aug 2019 04:22:06 -0700 Received: from BGSMSX107.gar.corp.intel.com (10.223.4.191) by fmsmsx117.amr.corp.intel.com (10.18.116.17) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 22 Aug 2019 04:22:06 -0700 Received: from bgsmsx106.gar.corp.intel.com ([169.254.1.143]) by BGSMSX107.gar.corp.intel.com ([169.254.9.251]) with mapi id 14.03.0439.000; Thu, 22 Aug 2019 16:51:57 +0530 From: "Govindarajan, LavanyaX" To: Olivier Matz CC: "dev@dpdk.org" , "Pattan, Reshma" , "Richardson, Bruce" Thread-Topic: [dpdk-dev] [PATCH] app/test: add unit test cases for mbuf library APIs Thread-Index: AQHVUcGa41ipMpJyKEeJCjkdhwerAKcHE4Ig Date: Thu, 22 Aug 2019 11:21:57 +0000 Message-ID: <3B7166B077BF7E42BC16647C002DC50A09DBD558@BGSMSX106.gar.corp.intel.com> References: <1555332015-31506-1-git-send-email-lavanyax.govindarajan@intel.com> <20190603083936.my6rdbsmbj2dtazk@platinum> <3B7166B077BF7E42BC16647C002DC50A09DA038C@BGSMSX153.gar.corp.intel.com> <3B7166B077BF7E42BC16647C002DC50A09DBCA05@BGSMSX106.gar.corp.intel.com> In-Reply-To: <3B7166B077BF7E42BC16647C002DC50A09DBCA05@BGSMSX106.gar.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.223.10.10] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] app/test: add unit test cases for mbuf library APIs 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Govindarajan, > LavanyaX > Sent: Tuesday, August 13, 2019 3:56 PM > To: Olivier Matz > Cc: dev@dpdk.org; Pattan, Reshma ; Richardson, > Bruce > Subject: Re: [dpdk-dev] [PATCH] app/test: add unit test cases for mbuf li= brary > APIs >=20 > Hi, >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Govindarajan, > > LavanyaX > > Sent: Monday, July 22, 2019 7:02 PM > > To: Olivier Matz > > Cc: dev@dpdk.org; Pattan, Reshma ; > > Richardson, Bruce > > Subject: Re: [dpdk-dev] [PATCH] app/test: add unit test cases for mbuf > > library APIs > > > > Hi > > > > > -----Original Message----- > > > From: Olivier Matz [mailto:olivier.matz@6wind.com] > > > Sent: Monday, June 3, 2019 2:10 PM > > > To: Govindarajan, LavanyaX > > > Cc: dev@dpdk.org; Pattan, Reshma ; > > > Richardson, Bruce > > > Subject: Re: [PATCH] app/test: add unit test cases for mbuf library > > > APIs > > > > > > Hi Lavanya, > > > > > > Please find some comments inline. > > > > > > On Mon, Apr 15, 2019 at 01:40:15PM +0100, Lavanya Govindarajan wrote: > > > > added new unit test cases for > > > > rte_validate_tx_offload, > > > > rte_pktmbuf_alloc_bulk, > > > > rte_pktmbuf_read, > > > > rte_pktmbuf_ext_shinfo_init_helper, > > > > rte_pktmbuf_attach_extbuf, > > > > rte_mbuf_ext_refcnt_read, > > > > rte_mbuf_ext_refcnt_update, > > > > rte_mbuf_ext_refcnt_set, > > > > rte_pktmbuf_detach_extbuf > > > > > > > > Signed-off-by: Lavanya Govindarajan > > > > > > > > --- > > > > app/test/test_mbuf.c | 820 > > > > ++++++++++++++++++++++++++++++++++++++++++- > > > > 1 file changed, 817 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index > > > > 030385ec5..74259b313 100644 > > > > --- a/app/test/test_mbuf.c > > > > +++ b/app/test/test_mbuf.c > > > > > > > > > > > > > +/* > > > > + * Test for allocating a bulk of mbufs > > > > + * define an array with positive sizes for mbufs allocations. > > > > + */ > > > > +static int > > > > +test_rte_pktmbuf_alloc_bulk(struct rte_mempool *pktmbuf_pool) { > > > > + int ret =3D 0; > > > > + unsigned int idx, loop; > > > > + unsigned int alloc_counts[] =3D { > > > > + 0, > > > > + MEMPOOL_CACHE_SIZE - 1, > > > > + MEMPOOL_CACHE_SIZE, > > > > + MEMPOOL_CACHE_SIZE + 1, > > > > + MEMPOOL_CACHE_SIZE * 1.5, > > > > + MEMPOOL_CACHE_SIZE * 2, > > > > + MEMPOOL_CACHE_SIZE * 2 - 1, > > > > + MEMPOOL_CACHE_SIZE * 2 + 1, > > > > + 89, /* random number */ > > > > + MEMPOOL_CACHE_SIZE /* repeat cache size */ > > > > + }; > > > > > > instead of testing these particular values, why not testing every > > > values between > > > 0 and NB_MBUF ? > > > > Testing every value from 0, 1, 2.. NB_MBUF(128 here) dilutes the > > purpose of testing bulk allocation of mbufs from the same pool. > > Boundary conditions and some random values are targeted which will > > cover major cases. > > > > The behavior is different for different set of values based on the > > availability of free mbufs in the cache or from the ring. > > > > > > > > > > +/* > > > > + * Test to initialize shared data in external buffer before > > > > +attaching to mbuf > > > > + * - Allocate mbuf with no data. > > > > + * - Allocate external buffer with size should be large enough > > > > +to > > > accommodate > > > > + * rte_mbuf_ext_shared_info. > > > > + * - Invoke pktmbuf_ext_shinfo_init_helper to initialize shared d= ata. > > > > + * - Invoke rte_pktmbuf_attach_extbuf to attach external buffer > > > > + to the > > mbuf. > > > > + * - Clone another mbuf and attach the same external buffer to it= . > > > > + * - Invoke rte_pktmbuf_detach_extbuf to detach the external > > > > + buffer from > > > mbuf. > > > > + */ > > > > +static int > > > > +test_pktmbuf_ext_shinfo_init_helper(struct rte_mempool > > > > +*pktmbuf_pool) { > > > > + struct rte_mbuf *m =3D NULL; > > > > + struct rte_mbuf *clone =3D NULL; > > > > + struct rte_mbuf_ext_shared_info *ret_shinfo =3D NULL; > > > > + rte_iova_t buf_iova; > > > > + void *ext_buf_addr =3D NULL; > > > > + uint16_t buf_len =3D EXT_BUF_TEST_DATA_LEN + > > > > + sizeof(struct rte_mbuf_ext_shared_info); > > > > + > > > > + /* alloc a mbuf */ > > > > + m =3D rte_pktmbuf_alloc(pktmbuf_pool); > > > > + if (m =3D=3D NULL) > > > > + GOTO_FAIL("%s: mbuf allocation failed!\n", __func__); > > > > + if (rte_pktmbuf_pkt_len(m) !=3D 0) > > > > + GOTO_FAIL("%s: Bad packet length\n", __func__); > > > > + rte_mbuf_sanity_check(m, 0); > > > > + > > > > + ext_buf_addr =3D rte_malloc("External buffer", buf_len, > > > > + RTE_CACHE_LINE_SIZE); > > > > + if (ext_buf_addr =3D=3D NULL) > > > > + GOTO_FAIL("%s: External buffer allocation failed\n", __func__); > > > > + > > > > + ret_shinfo =3D rte_pktmbuf_ext_shinfo_init_helper(ext_buf_addr, > > > &buf_len, > > > > + ext_buf_free_callback_fn, ext_buf_addr); > > > > + if (ret_shinfo =3D=3D NULL) > > > > + GOTO_FAIL("%s: Shared info initialization failed!\n", > > > > +__func__); > > > > + > > > > + if (rte_mbuf_ext_refcnt_read(ret_shinfo) !=3D 1) > > > > + GOTO_FAIL("%s: External refcount is not 1\n", __func__); > > > > + > > > > + if (rte_mbuf_refcnt_read(m) !=3D 1) > > > > + GOTO_FAIL("%s: Invalid refcnt in mbuf\n", __func__); > > > > + > > > > + buf_iova =3D rte_mempool_virt2iova(ext_buf_addr); > > > > + rte_pktmbuf_attach_extbuf(m, ext_buf_addr, buf_iova, buf_len, > > > > + ret_shinfo); > > > > + if (m->ol_flags !=3D EXT_ATTACHED_MBUF) > > > > + GOTO_FAIL("%s: External buffer is not attached to mbuf\n", > > > > + __func__); > > > > + > > > > + /* allocate one more mbuf */ > > > > + clone =3D rte_pktmbuf_clone(m, pktmbuf_pool); > > > > + if (clone =3D=3D NULL) > > > > + GOTO_FAIL("%s: mbuf clone allocation failed!\n", __func__); > > > > + if (rte_pktmbuf_pkt_len(clone) !=3D 0) > > > > + GOTO_FAIL("%s: Bad packet length\n", __func__); > > > > + > > > > + /* attach the same external buffer to the cloned mbuf */ > > > > + rte_pktmbuf_attach_extbuf(clone, ext_buf_addr, buf_iova, buf_len, > > > > + ret_shinfo); > > > > > > Instead of: > > > > > > clone =3D rte_pktmbuf_clone(m, pktmbuf_pool); > > > rte_pktmbuf_attach_extbuf(clone, ext_buf_addr, buf_iova, buf_len, > > > ret_shinfo); /* << useless */ > > > > > The purpose of the above lines is to test if external buffer can be > > attached to the cloned mbuf or not. > > > > > I'd prefer: > > > m2 =3D rte_pktmbuf_alloc(pktmbuf_pool); > > > rte_pktmbuf_attach_extbuf(clone, ext_buf_addr, buf_iova, buf_len, > > > ret_shinfo); > > Do you mean, m2 in the above line? > > > > > > > > Or just: > > > clone =3D rte_pktmbuf_clone(m, pktmbuf_pool); > > > > > Can you please provide more clarity in the above suggestion. > > > > > > > > > > > > + if (clone->ol_flags !=3D EXT_ATTACHED_MBUF) > > > > + GOTO_FAIL("%s: External buffer is not attached to mbuf\n", > > > > + __func__); > > > > + > > > > > > > > Regards > > Lavanya G >=20 > New patchset for mbuf UT has been raised. > 1/2 - Addresses the review comments from Olivier > http://patches.dpdk.org/patch/57595/ > 2/2 - Added UT cases for rte_mbuf.h > http://patches.dpdk.org/patch/57596/ >=20 > Could you please review the above patchset. >=20 > Thanks > Lavanya G Kindly review the above patchset. Thanks in advance. Lavanya G