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 C0ABBA04AB; Wed, 6 Nov 2019 16:00:42 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 712FD1C23B; Wed, 6 Nov 2019 16:00:30 +0100 (CET) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by dpdk.org (Postfix) with ESMTP id 5E7641C206 for ; Wed, 6 Nov 2019 16:00:28 +0100 (CET) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id xA6EuNQH001327; Wed, 6 Nov 2019 07:00:27 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : subject : date : message-id : references : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pfpt0818; bh=hLFvxPxRIYwh422q8o9RMQ1TKEz3Qnb5/L7ztgiRQM4=; b=gikMcSi2ViQUBblW4GPYK6ZKKrcoXSDtEZmRAMpJ/FdIYGG4g0pak2CUjyxWOnbfeu6J 3Bs+U5Lf5r89+XScoctmfkhMxPwQxckqKzd2JHk7DCErQTRyOgobW5sjhA4E08JnJiBp UAOW4bcMNE3TMovBZscg0Vd9RbUtL00iFgNVwnSAcmREw9c0Zihm/3y5TLmHfmZkFMAd GlW9Zkedy2vTkqqLAw4duGL03HBx3st5VCQCE2CP0bJB6YxIKJqedGCDx4aECGS2kUMS Hngnuk8di3xBN8tZA83JCcbPjlItBUhMgddk3qUdirquSESCRvzkPdkwM/b6TeramMnZ +A== Received: from sc-exch03.marvell.com ([199.233.58.183]) by mx0a-0016f401.pphosted.com with ESMTP id 2w3eud4dmw-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 06 Nov 2019 07:00:27 -0800 Received: from SC-EXCH03.marvell.com (10.93.176.83) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Wed, 6 Nov 2019 07:00:26 -0800 Received: from NAM01-BN3-obe.outbound.protection.outlook.com (104.47.33.51) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1367.3 via Frontend Transport; Wed, 6 Nov 2019 07:00:26 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nK8uQoowYt/tPwPlPqqQRsykdhH+6Po9h+UDGJl36jOrphGIHAqoAIORVsj4HqtSdYpJmo0uYCdEXsiimbG2YWyAzxHw1a8I7mNFl9qvaU5kJpM1r7RxS8KgCJ5WKi+w6y3XWB6UIuQzG5Yae/Jhn8sTw81+e3NJ+iWGDOF+irijvq22uriDCsolIU0DAowiOhPKGASjR7p4LZDZMyYjHKy3nKzmopTDwlHbv/pKmDlDy3w9bb2jvt/1y+JBbtAM/YfZNXZCo25eQFCzeLTsSN2115HM/jbq0FxjzuKmRp6n7LpCJL+wCpY6zL4qkw7mwoL8CCF6G0/FOLFhzlvH6Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=hLFvxPxRIYwh422q8o9RMQ1TKEz3Qnb5/L7ztgiRQM4=; b=ilCszbtndMVJKDqPfRUNlHmCRp8jC+anFcSNv+jiZ6ThTPWirgMVn2GRGs4vPzI3uazmnq0WaE0OCHQ5jhxiRzNKNAgAxPika1ValebZEPgcqNWr0fgNq8N85+gQz0Tky3V+o4sXwJfgdUH5qbw9ZNDfe5eeTQuxogu2cfzPIWCFLsDNEJxaZqnFCDsGk85ftySaHxfHV8t8GtTd/YxL8YEZq+HmK6bC57XWCmd3NdOd/ymVxhR99XymlJCLLBKgtm2WKbnMypBqyLxw0uxUsCm/VdusDAPpi4xF/j8QIK3gQU4HQW3nsfNSl5BHMY5V4jaCPeqIiuEVTs1ZzDV55A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector2-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=hLFvxPxRIYwh422q8o9RMQ1TKEz3Qnb5/L7ztgiRQM4=; b=Up9brIdmuRtW769HRRzsIPTfhsFLyLcli7VUjc9rxF7EwcndkcRyy9gBhjjqwoOO2b166OC1e1Ed8dBVhqgrfvwRoRo/DPgSUSqVsCjvfMxewis1h6XcykBXPfIUMEY9WTX1eVGdVp5bNUNIb5NZsQLpiWSgGQOHqnmOhIVY6kQ= Received: from BN8PR18MB2867.namprd18.prod.outlook.com (20.179.72.89) by BN8PR18MB2948.namprd18.prod.outlook.com (20.179.75.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2430.22; Wed, 6 Nov 2019 15:00:22 +0000 Received: from BN8PR18MB2867.namprd18.prod.outlook.com ([fe80::44da:d61b:7fc3:4353]) by BN8PR18MB2867.namprd18.prod.outlook.com ([fe80::44da:d61b:7fc3:4353%7]) with mapi id 15.20.2430.020; Wed, 6 Nov 2019 15:00:22 +0000 From: Shally Verma To: "Trybula, ArturX" , "dev@dpdk.org" , "Trahe, Fiona" , "Dybkowski, AdamX" , "akhil.goyal@nxp.com" , Ashish Gupta Thread-Topic: [EXT] [PATCH v3 1/1] test/compress: unit tests refactoring Thread-Index: AQHVikwCFNfkVivdu0aZlwUMoGELQ6d1EEPAgAXPQQCAAEJmsIAC9XKAgAA3aLA= Date: Wed, 6 Nov 2019 15:00:21 +0000 Message-ID: References: <20190912143431.26917-1-arturx.trybula@intel.com> <20191024091616.31224-1-arturx.trybula@intel.com> <20191024091616.31224-2-arturx.trybula@intel.com> <5B6D1C77E9D7034C93E97BD83D1D9F572F231F97@hasmsx107.ger.corp.intel.com> <5B6D1C77E9D7034C93E97BD83D1D9F572F23270C@hasmsx107.ger.corp.intel.com> In-Reply-To: <5B6D1C77E9D7034C93E97BD83D1D9F572F23270C@hasmsx107.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [223.230.111.80] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: b420d30b-94d9-4083-ab1b-08d762ca0c2a x-ms-traffictypediagnostic: BN8PR18MB2948: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 02135EB356 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(366004)(346002)(396003)(39860400002)(376002)(136003)(199004)(189003)(13464003)(52084003)(110136005)(5660300002)(66066001)(74316002)(26005)(66446008)(478600001)(66476007)(66556008)(64756008)(33656002)(76116006)(8676002)(186003)(52536014)(66946007)(8936002)(81166006)(7736002)(81156014)(6506007)(305945005)(53546011)(11346002)(25786009)(476003)(486006)(446003)(102836004)(256004)(14444005)(3846002)(6116002)(76176011)(7696005)(86362001)(2906002)(2501003)(316002)(6436002)(55016002)(14454004)(6636002)(71190400001)(71200400001)(229853002)(6246003)(99286004)(9686003); DIR:OUT; SFP:1101; SCL:1; SRVR:BN8PR18MB2948; H:BN8PR18MB2867.namprd18.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: marvell.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Ca9Dc8TXsLJSXCtGqJuByb5SvUYi0Zowgx1HMoRokpL/urxPyOk+GmVlLHnJpvVvm4jjEdv7UAAd/ANgm1+wuNaokWf1B/HwhfmEoc0YoK53h0YHOpu1Pfe8c227zZOlRi4uS5c6oD+2RMuf1tfPxx4t2SYz3Nb2aoGK2FKdNMAA50XPazw2VcSeCh0QvhskRGeJYepUXzTA52tjQGDR6Nfvy8MydIWSc3V0be/cxWle4YlcipVKbiGgfTlLBE5xbyFYWJiUfkqO7vPpvHawCU1ovzzNdwnjb2tZAPfO33eWn4lJTzD+7zXNgOStO07NcV6rtxIzIAsPbK/o5x2gdjEFgA+MYeypkQPIQr2kQ8Epdlq9IBnb2S8SmkF1fLiDGHZFY9vd6K+u7jZbAhUJQeEWFmo3PLBL37q99VqX5XvIrUfyqMkBFOcn43zqJhmg Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: b420d30b-94d9-4083-ab1b-08d762ca0c2a X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Nov 2019 15:00:22.1304 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: bOOesbcda3EA933HzX4bnRXGC/n0/4+Q2uJOeNOafrTQzyzsIMVDbgsEAbWuQ6ke0/KLuJU6Ef/9jwf3kzRxyQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR18MB2948 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,18.0.572 definitions=2019-11-06_04:2019-11-06,2019-11-06 signatures=0 Subject: Re: [dpdk-dev] [EXT] [PATCH v3 1/1] test/compress: unit tests refactoring 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" > -----Original Message----- > From: Trybula, ArturX > Sent: Wednesday, November 6, 2019 5:03 PM > To: Shally Verma ; dev@dpdk.org; Trahe, Fiona > ; Dybkowski, AdamX > ; akhil.goyal@nxp.com; Ashish Gupta > > Subject: RE: [EXT] [PATCH v3 1/1] test/compress: unit tests refactoring >=20 >=20 > > -----Original Message----- > > From: Trybula, ArturX > > Sent: Monday, November 4, 2019 3:55 PM > > To: Shally Verma ; dev@dpdk.org; Trahe, Fiona > > ; Dybkowski, AdamX > ; > > akhil.goyal@nxp.com > > Subject: RE: [EXT] [PATCH v3 1/1] test/compress: unit tests > > refactoring > > > > Hi Shally, > > > > Please find my comments below. > > > > > -------------------------------------------------------------------- > > > -- Core engine refactoring (test_deflate_comp_decomp function). > > > Smaller specialized functions created. > > > > > > Signed-off-by: Artur Trybula > > > --- > > > app/test/test_compressdev.c | 1118 +++++++++++++++++-----= -- > > > doc/guides/rel_notes/release_19_11.rst | 5 + > > > 2 files changed, 826 insertions(+), 297 deletions(-) > > > > ..... ... > > ... > > > + * Developer is requested to provide input params > > > + * according to the following rule: > > > + * if ops_processed =3D=3D NULL -> compression > > > + * if ops_processed !=3D NULL -> decompression > [Shally] we are trying to overload its purpose here. Can it be make simp= ler . > we can use interim test param to check if op type is > compression/decompression and then use op_processed[] on need basis > This will help in code readability else function looks complex to underst= and > where as purpose is very simple. > [Artur] In some way it is not a perfect solution, but from my point of vi= ew it's > very clear. > There is no info in the interim test param (I suppose you meant > interim_data_params) that could be useful in this case. There are pointer= s to > xforms, but which one should I check? There must be some switch to choose > the right flow. I can add another input param to the function, to indicat= e if it's > a compress or decompress operation. But it seems to me, that it would be > too much for this one case. Finally I can add another input param if you = like > this idea? >=20 [Shally] Believe you agreed below that you'll add an param op_type? ... > > > + else { > > > + if (ops_processed =3D=3D NULL) { > > > + > > > + not_zlib_compr =3D (test_data->zlib_dir =3D=3D > > > ZLIB_DECOMPRESS > > > + || test_data->zlib_dir =3D=3D ZLIB_NONE); > > > + > > > + ratio =3D (not_zlib_compr && > > > + (overflow =3D=3D OVERFLOW_ENABLED)) ? > > > + COMPRESS_BUF_SIZE_RATIO_OVERFLOW : > > > + COMPRESS_BUF_SIZE_RATIO; > [Shally] Why this condition is not true in case of ZLIB compression? For = zlib > compression too it can overflow right in case of deflate uncompressed > output?! > [Artur] I discussed this question with Fiona. The test was design for QAT= and > other tools like QAT. In this case Zlib is used just for a verification i= f the > compression using QAT was correct. This condition should be preserved. >=20 [Shally] ohh ok I noticed not_zlib_compr means zlib will decompress. I thou= ght it otherwise. So ignore my feedback. > > > + > > > + data_size =3D strlen(test_bufs[i]) * ratio; > > > + > > > + } else { > > > + priv_data =3D (struct priv_op_data *) > > > + (ops_processed[i] + 1); > > > + data_size =3D strlen(test_bufs[priv_data->orig_idx]) + > > > 1; > > > + } > > > + } > > > + > > > + return data_size; > [Shally] On the other hand, I don't see reason why it should return 0 unl= ess > object priv data is corrupted or not updated with test buf size, which id= eally > should not be the case. > Given that, it can be just updated that func returns expected output buf= fer > size. > [Artur] I don't see any reason why priv data could be corrupted. It would= be a > bigger problem if it happened. > All the cases are covered and the return value must be correct. If > ops_processed is not NULL than priv_data has to be correct or we have a > problem with QAT PMD. Verification if the filed orig_idx is correct is ev= en > more complicated. From my perspective mentioned verification shouldn't be > considered as a part of this function. It's too late. [Shally] I think we are on same page here. I meant same thing that I don't = see reason why func should return 0. It will always return some non-zero va= lue. So my point was value 0 seems redundant here. > Another input param discussed below: op_type? >=20 > > > +} > > > + > > > + > > > +/** > > > + * Memory buffers preparation (for both compression and > > decompression). > > > + * > > > + * Memory allocation for comp/decomp buffers from mempool, > > depending > > > on > [Shally] can be reworded " function allocate output buffer depending on > op_type : compression / decompression [Artur] Ok >=20 > > > + * ops_processed value. Developer is requested to provide input > > > + params > > > + * according to the following rule: > > > + * if ops_processed =3D=3D NULL -> current_bufs =3D comp_bufs[] > > > + * if ops_processed !=3D NULL -> current_bufs =3D decomp_bufs[] > > > + * -1 returned if function fail, without modifying the mbuf. > > > + * > [Shally] Same feedback here. This can be made simpler by adding another > op_type in arg or getting this info from test args. > [Artur] Similar issue was discussed above. So let's do both cases in the = same > way. I'm going to define another enum. It will be an extra input param of > these functions: > 1. test_mbufs_calculate_data_size > 2. test_mbufs_destination_preparation > Is it ok for you? >=20 [Shally] Ok >=20 ... > > > + * @return > > > + * - 0: On success. > > > + * - -1: On error. > > > + */ > > > +static int > > > +test_mbufs_destination_preparation( > > > + struct rte_comp_op *ops_processed[], /*can be equal to > > > NULL*/ > > > + struct rte_mbuf *current_bufs[], > > > + unsigned int out_of_space_and_zlib, > > > + const struct interim_data_params *int_data, > > > + const struct test_data_params *test_data, > > > + struct rte_mbuf_ext_shared_info *current_extbuf_info, > > > + const struct rte_memzone *current_memzone) { >=20 > [Shally] I would still recommend to make it part of priv array and keep > prototype simpler to read [Artur] What object do you mean precisely? >=20 My suggestion was to move extbuf_info, out_of_space_zlib too to priv_data .= . but its just suggestion... you can ignore If it complicate the purpose > ... ... > > > 2.17.1