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 E7A8DA0471 for ; Tue, 13 Aug 2019 12:26:24 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 181241B13C; Tue, 13 Aug 2019 12:26:24 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 287AF1252 for ; Tue, 13 Aug 2019 12:26:21 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Aug 2019 03:26:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,381,1559545200"; d="scan'208";a="200436388" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga004.fm.intel.com with ESMTP; 13 Aug 2019 03:26:20 -0700 Received: from fmsmsx162.amr.corp.intel.com (10.18.125.71) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 13 Aug 2019 03:26:20 -0700 Received: from bgsmsx152.gar.corp.intel.com (10.224.48.50) by fmsmsx162.amr.corp.intel.com (10.18.125.71) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 13 Aug 2019 03:26:04 -0700 Received: from bgsmsx106.gar.corp.intel.com ([169.254.1.143]) by BGSMSX152.gar.corp.intel.com ([169.254.6.190]) with mapi id 14.03.0439.000; Tue, 13 Aug 2019 15:56:03 +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: AQHVQJHx56cHRNvUZku3cG/xMmRcqab4/z7A Date: Tue, 13 Aug 2019 10:26:03 +0000 Message-ID: <3B7166B077BF7E42BC16647C002DC50A09DBCA05@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> In-Reply-To: <3B7166B077BF7E42BC16647C002DC50A09DA038C@BGSMSX153.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: 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 li= brary > APIs >=20 > Hi >=20 > > -----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 > > >=20 > >=20 > > > +/* > > > + * 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 ? >=20 > 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. >=20 > The behavior is different for different set of values based on the availa= bility of > free mbufs in the cache or from the ring. >=20 > >=20 > > > +/* > > > + * 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 dat= a. > > > + * - 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 attac= hed to the > cloned mbuf or not. >=20 > > 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? >=20 > > > > Or just: > > clone =3D rte_pktmbuf_clone(m, pktmbuf_pool); > > > Can you please provide more clarity in the above suggestion. >=20 > > > > > > > + if (clone->ol_flags !=3D EXT_ATTACHED_MBUF) > > > + GOTO_FAIL("%s: External buffer is not attached to mbuf\n", > > > + __func__); > > > + >=20 > >=20 > Regards > Lavanya G 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/ Could you please review the above patchset. Thanks Lavanya G