From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM04-BN3-obe.outbound.protection.outlook.com (mail-eopbgr680084.outbound.protection.outlook.com [40.107.68.84]) by dpdk.org (Postfix) with ESMTP id C49B414EC for ; Tue, 6 Nov 2018 16:40:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3vwxTDKDWmNRpn2B6VCkXxypGCxNxffH7f7bSfLOKAc=; b=n2dhM2Z7bghaBVesffi5Ct8tK1fdsc7ZydZEu3QVbvPjvR+PrQi2am6NA04gZgc7/Wm6UTNR1ApXiD2yq92yF7tu9Pf/DTQXbtFm0RthC+/Lw+WxXE6KxwSjSlc/InAlwE9iyyFX7O/RNqfARfoBtYJVxUh/Y/Mhxkm3N3w/2Y8= Received: from SN6PR07MB5152.namprd07.prod.outlook.com (52.135.101.33) by SN6PR07MB5054.namprd07.prod.outlook.com (52.135.121.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1294.31; Tue, 6 Nov 2018 15:39:59 +0000 Received: from SN6PR07MB5152.namprd07.prod.outlook.com ([fe80::49cb:b2a:974:2211]) by SN6PR07MB5152.namprd07.prod.outlook.com ([fe80::49cb:b2a:974:2211%4]) with mapi id 15.20.1294.034; Tue, 6 Nov 2018 15:39:59 +0000 From: "Verma, Shally" To: "Jozwiak, TomaszX" , "dev@dpdk.org" , "Trahe, Fiona" , "akhil.goyal@nxp.com" Thread-Topic: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance measurement Thread-Index: AQHUWYqjhLJt3XAtpkeXnW8WfRqeNaUVmp7AgCbXsACABJ3+AIABizAAgAACQCCAAA8JAIAAbY0g Date: Tue, 6 Nov 2018 15:39:58 +0000 Message-ID: References: <1538400427-20164-1-git-send-email-tomaszx.jozwiak@intel.com> <1538400427-20164-3-git-send-email-tomaszx.jozwiak@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Shally.Verma@cavium.com; x-originating-ip: [223.230.32.215] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; SN6PR07MB5054; 6:AYfPSJ0wK0OLtxIg1rYzLWDewrmjRrHMeUHsTvHLwtIez5yheby0O/6qjt+vB9WzaVVW2rcFYQYENjbK+hYEedAkRB0L7l+2Xr8nKAjzUHjNIVqDT2Lv8PHz34Xp7uF8yiiYVSD7pTmJx9XSlOwoZOiLwH05/J4YvLK1HCfFplmSK3Wl7lj3YyXJAQjfNTwKq2ptraH/3dAkrWqxSl7TfNB8lgx5h5yxg5vv1f/5xzT5nUwfD8uBib+XobOuJJgAB54GoD6i7deaTrvJyePPieC2BVrQCEzSOGk3KIwf70hDVNBmEzJttWJ6k/AFiecQ/vdl9YCoiMULA48iyk+3WVC1YKxp2xYn1F2RsNvhtUKZXilTvkD2m/RCw6ROmFOGPgjC42wRYolCBGAVy+oMZ40bSmtp00X4cjs8wdL8CuKzy+gTK/IqJNJZhnYPymNqnN4OVEJXxi4gRHOBFTnrgw==; 5:If3UHl4aNIRD3OJVn2oz0DhmMsJQln7A9C2ZPzSMak1mEvy8w3ig0itjibuGIHEIb9LCL24RuzUvouiewZeVosVMsT2f+XJmzXvqD0L4Exb6g9L7cxlYKyxkPZ+wT8SA4kYclsVWfPyfJLiH3YCNDLZAoNqAx+CEoo/YzdPg5c8=; 7:pHrHS5K4sfNVs1/U70IYconbHk6cnPs4b0VLjxylt5JNjsCxmzYZ6IOwFvmI2D+2yUHzsIG2GG+5vGiswwSxhq+bWpSkiJ/AbhM1MEXoz3cZCSoe6oI61CHc+/IXpgIYfGX4zNOtrxg4OkU270KJ0Q== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 9ebd63a4-a05a-447d-8ad3-08d643fe1c60 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(5600074)(711020)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:SN6PR07MB5054; x-ms-traffictypediagnostic: SN6PR07MB5054: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(185117386973197)(17755550239193)(228905959029699); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231382)(944501410)(52105095)(3002001)(10201501046)(93006095)(93001095)(148016)(149066)(150057)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123560045)(20161123564045)(20161123558120)(201708071742011)(7699051)(76991095); SRVR:SN6PR07MB5054; BCL:0; PCL:0; RULEID:; SRVR:SN6PR07MB5054; x-forefront-prvs: 0848C1A6AA x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(376002)(366004)(136003)(346002)(39860400002)(396003)(13464003)(199004)(189003)(6116002)(3846002)(26005)(6506007)(99286004)(53546011)(14454004)(68736007)(72206003)(2900100001)(186003)(81156014)(8936002)(2501003)(256004)(561944003)(102836004)(316002)(33656002)(6436002)(97736004)(6246003)(110136005)(81166006)(2906002)(7736002)(305945005)(74316002)(486006)(446003)(476003)(11346002)(5660300001)(9686003)(106356001)(229853002)(71190400001)(71200400001)(4744004)(25786009)(66066001)(93886005)(55016002)(105586002)(7696005)(478600001)(86362001)(76176011)(53936002); DIR:OUT; SFP:1101; SCL:1; SRVR:SN6PR07MB5054; H:SN6PR07MB5152.namprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: 8gbICvTa7P14DYAahaEvUEjmFDqUmPQwRAoMWL9479bViA+z4UOPNSblkJY13jbxjNQyyhNem5h3dpKFcRFWJSr3TpyuLYXro/HAVcOMy2QAih5Od/hQmlN7wUG22237D3f3ZJ7KGqQKPSh1tc6+hoTZm8VWYc+xuRwm9pODkWJreoWDRvnjtHAN3wTXC2SNL0RfEZwx28Dtx4XXulnuJxg/eM6SHIfMI1OlP4FE3hpOKyn09F1Z9cenBTD+l8H0ITkbnuBTBIzeEnAzMaiF8j0JcZXoyJrP0wzEsXgmq0qOdsU2/Wmw6tMlqeYxDCbe2DIFG70o8BpnJiVMELqV9bjFkEmjM7SnBDkc44GNFIs= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: cavium.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9ebd63a4-a05a-447d-8ad3-08d643fe1c60 X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Nov 2018 15:39:59.4896 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR07MB5054 Subject: Re: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance measurement 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: Tue, 06 Nov 2018 15:40:02 -0000 >-----Original Message----- >From: Jozwiak, TomaszX >Sent: 06 November 2018 14:36 >To: Verma, Shally ; dev@dpdk.org; Trahe, Fiona ; akhil.goyal@nxp.com >Subject: RE: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance mea= surement > >External Email > >> -----Original Message----- >> From: Verma, Shally [mailto:Shally.Verma@cavium.com] >> Sent: Tuesday, November 6, 2018 9:16 AM >> To: Jozwiak, TomaszX ; dev@dpdk.org; Trahe, >> Fiona ; akhil.goyal@nxp.com >> Subject: RE: [dpdk-dev] [PATCH 2/3] app/compress-perf: add performance >> measurement ... >> >> >> >+ >> >> >> >+ /* Window size */ >> >> >> >+ if (test_data->window_sz !=3D -1) { >> >> >> >+ if (param_range_check(test_data->window_sz, >> >> >> >+ &cap->window_size) >> >> >> What if cap->window_size is 0 i.e. implementation default? >> >> > >> >> >TJ: You probably mean cap->window_size.increment =3D 0 (because >> >> >cap->window_size is a structure). In that case we check if >> >> >test_data->window_sz >=3Dmin and test_data->window_sz <=3D max only, >> >> because increment =3D 0 means (base on compression API) we have only >> >> one value of windows_size (no range is supported). >> >> But PMD can set min and max too 0 for such case. >> > >> >TJ: I can't see any issue in that case too. Maybe I don't understand wh= at you >> mean but the logic is as follow: >> >1) if you pass '--window-sz ...' param. into command line your >> >intention is to force that value of window size during test. We check i= s this >> value is allow (by param_range_check() function). >> >2) if you plan to use default value - just don't pass '--window-sz' >> >param. in command line at all. In that case we get windows size from >> >window_size.max field, so if window_size.min=3D window_size.max=3D0 >> test_data->window_sz will be zero, as well. >> >If you mean that behavior is not good - I will be grateful for other >> suggestions. >> >> This is fine. but I am thinking of 3rd case here: >> c) user pass window sz but PMD window_sz.min =3D max =3D 0, then user >> requested windowsz is not applicable right?! > >In that case - true. There'll be fail : >"Compress device does not support this window size\n"); >So what is your proposal for that case? > We can set to window size to implementation default and add in diagnostic o= f used window sz for test run. No need to fail here I believe. Thanks Shally > > > > >> >> > >> >> >> >> > >> >> > >> >> > >> >> .... >> >> >> >> >> >+ >> >> >> >+ if (fread(data, data_to_read, 1, f) !=3D 1) { >> >> >> >+ RTE_LOG(ERR, USER1, "Input file could not= be read\n"); >> >> >> >+ goto err; >> >> >> >+ } >> >> >> >+ if (fseek(f, 0, SEEK_SET) !=3D 0) { >> >> >> >+ RTE_LOG(ERR, USER1, >> >> >> >+ "Size of input could not be calcu= lated\n"); >> >> >> >+ goto err; >> >> >> >+ } >> >> >> >+ remaining_data -=3D data_to_read; >> >> >> >+ data +=3D data_to_read; >> >> >> It looks like it will run 2nd time only if input file size < input >> >> >> data size in which case it will just keep filling input buffer >> >> >> with repeated >> >> data. >> >> >> Is that the intention here? >> >> > >> >> >TJ: Yes exactly. If test_data->input_data_sz is bigger than >> >> >actual_file_sz then we fill the buffer with repeated data from file >> >> >to fill >> >> whole buffer. >> >> I mentioned in one of the earlier reply, wont that then influence the >> >> compression behaviour and o/p? my suggestion was to work on actual >> >> user provided input to take perf to get actual perf for given content= . >> > >> >TJ: You right, but this solution is flexible. You can pass ' >> >--extended-input-sz" or not, so you can use original input data or exte= nd it >> if you want. >> Ok. but still not sure if it's really needed. Might be practically most = of the time >> it wont be exercised. No hard opinion on this though. >> >> Thanks >> Shally >> > >> >> >> >> > >> >> >> >> >> ... >> >> >> >> >> >+ if (data_addr =3D=3D NULL) { >> >> >> >+ RTE_LOG(ERR, USER1, "Could not >> >> >> >+ append data\n"); >> >> >> Since a new buffer per segment is allocated, so is it possible for >> >> >> append to fail? think, this check is redundant here. >> >> > >> >> >TJ: Yes, you're right, it should never fail. But I think it's good >> >> >coding practice >> >> to add the check just in case. >> >> > >> >> Unless it is called in data path which might cost perf a bit. >> > >> >TJ: prepare_bufs() is out of perf measurement, so shouldn't impact to >> >measurements. The performance measurement is inside >> >main_loop() only. >> > >> > >> >Br, Tomek >> > >> >> >> >> Thanks >> >> Shally >> >> >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ >> >> >> >+ rte_memcpy(data_addr, input_data_ptr, dat= a_sz); >> >> >> >+ input_data_ptr +=3D data_sz; >> >> >> >+ remaining_data -=3D data_sz; >> >> >> >+ >> >> >> >+ if (rte_pktmbuf_chain(test_data->decomp_b= ufs[i], >> >> >> >+ next_seg) < 0) { >> >> >> >+ RTE_LOG(ERR, USER1, "Could not ch= ain mbufs\n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ segs_per_mbuf++; >> >> >> >+ } >> >> >> >+ >> >> >> >+ /* Allocate data in output mbuf */ >> >> >> >+ test_data->comp_bufs[i] =3D >> >> >> >+ rte_pktmbuf_alloc(test_data->comp_buf_poo= l); >> >> >> >+ if (test_data->comp_bufs[i] =3D=3D NULL) { >> >> >> >+ RTE_LOG(ERR, USER1, "Could not allocate m= buf\n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ data_addr =3D (uint8_t *) rte_pktmbuf_append( >> >> >> >+ test_data->comp_bufs[i], >> >> >> >+ test_data->seg_sz); >> >> >> >+ if (data_addr =3D=3D NULL) { >> >> >> >+ RTE_LOG(ERR, USER1, "Could not append dat= a\n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ >> >> >> >+ /* Chain mbufs if needed for output mbufs */ >> >> >> >+ for (j =3D 1; j < segs_per_mbuf; j++) { >> >> >> >+ struct rte_mbuf *next_seg =3D >> >> >> >+ >> >> >> >+ rte_pktmbuf_alloc(test_data->comp_buf_pool); >> >> >> >+ >> >> >> >+ if (next_seg =3D=3D NULL) { >> >> >> >+ RTE_LOG(ERR, USER1, >> >> >> >+ "Could not allocate mbuf\= n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ >> >> >> >+ data_addr =3D (uint8_t *)rte_pktmbuf_appe= nd(next_seg, >> >> >> >+ test_data->seg_sz); >> >> >> >+ >> >> >> >+ if (data_addr =3D=3D NULL) { >> >> >> >+ RTE_LOG(ERR, USER1, "Could not ap= pend data\n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ >> >> >> >+ if (rte_pktmbuf_chain(test_data->comp_buf= s[i], >> >> >> >+ next_seg) < 0) { >> >> >> >+ RTE_LOG(ERR, USER1, "Could not ch= ain mbufs\n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ } >> >> >> >+ } >> >> >> >+ >> >> >> >+ return 0; >> >> >> >+} >> >> >> >+ >> >> >> >+static void >> >> >> >+free_bufs(struct comp_test_data *test_data) { >> >> >> >+ uint32_t i; >> >> >> >+ >> >> >> >+ for (i =3D 0; i < test_data->total_bufs; i++) { >> >> >> >+ rte_pktmbuf_free(test_data->comp_bufs[i]); >> >> >> >+ rte_pktmbuf_free(test_data->decomp_bufs[i]); >> >> >> >+ } >> >> >> >+ rte_free(test_data->comp_bufs); >> >> >> >+ rte_free(test_data->decomp_bufs); } >> >> >> >+ >> >> >> >+static int >> >> >> >+main_loop(struct comp_test_data *test_data, uint8_t level, >> >> >> >+ enum rte_comp_xform_type type, >> >> >> >+ uint8_t *output_data_ptr, >> >> >> >+ size_t *output_data_sz, >> >> >> >+ unsigned int benchmarking) { >> >> >> >+ uint8_t dev_id =3D test_data->cdev_id; >> >> >> >+ uint32_t i, iter, num_iter; >> >> >> >+ struct rte_comp_op **ops, **deq_ops; >> >> >> >+ void *priv_xform =3D NULL; >> >> >> >+ struct rte_comp_xform xform; >> >> >> >+ size_t output_size =3D 0; >> >> >> >+ struct rte_mbuf **input_bufs, **output_bufs; >> >> >> >+ int res =3D 0; >> >> >> >+ int allocated =3D 0; >> >> >> >+ >> >> >> >+ if (test_data =3D=3D NULL || !test_data->burst_sz) { >> >> >> >+ RTE_LOG(ERR, USER1, >> >> >> >+ "Unknow burst size\n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ >> >> >> >+ ops =3D rte_zmalloc_socket(NULL, >> >> >> >+ 2 * test_data->total_bufs * sizeof(struct rte_com= p_op *), >> >> >> >+ 0, rte_socket_id()); >> >> >> >+ >> >> >> >+ if (ops =3D=3D NULL) { >> >> >> >+ RTE_LOG(ERR, USER1, >> >> >> >+ "Can't allocate memory for ops strucures\= n"); >> >> >> >+ return -1; >> >> >> >+ } >> >> >> >+ >> >> >> >+ deq_ops =3D &ops[test_data->total_bufs]; >> >> >> >+ >> >> >> >+ if (type =3D=3D RTE_COMP_COMPRESS) { >> >> >> >+ xform =3D (struct rte_comp_xform) { >> >> >> >+ .type =3D RTE_COMP_COMPRESS, >> >> >> >+ .compress =3D { >> >> >> >+ .algo =3D RTE_COMP_ALGO_DEFLATE, >> >> >> >+ .deflate.huffman =3D test_data->h= uffman_enc, >> >> >> >+ .level =3D level, >> >> >> >+ .window_size =3D test_data->windo= w_sz, >> >> >> >+ .chksum =3D RTE_COMP_CHECKSUM_NON= E, >> >> >> >+ .hash_algo =3D RTE_COMP_HASH_ALGO= _NONE >> >> >> >+ } >> >> >> >+ }; >> >> >> >+ input_bufs =3D test_data->decomp_bufs; >> >> >> >+ output_bufs =3D test_data->comp_bufs; >> >> >> >+ } else { >> >> >> >+ xform =3D (struct rte_comp_xform) { >> >> >> >+ .type =3D RTE_COMP_DECOMPRESS, >> >> >> >+ .decompress =3D { >> >> >> >+ .algo =3D RTE_COMP_ALGO_DEFLATE, >> >> >> >+ .chksum =3D RTE_COMP_CHECKSUM_NON= E, >> >> >> >+ .window_size =3D test_data->windo= w_sz, >> >> >> >+ .hash_algo =3D RTE_COMP_HASH_ALGO= _NONE >> >> >> >+ } >> >> >> >+ }; >> >> >> >+ input_bufs =3D test_data->comp_bufs; >> >> >> >+ output_bufs =3D test_data->decomp_bufs; >> >> >> >+ } >> >> >> >+ >> >> >> >+ /* Create private xform */ >> >> >> >+ if (rte_compressdev_private_xform_create(dev_id, &xform, >> >> >> >+ &priv_xform) < 0) { >> >> >> >+ RTE_LOG(ERR, USER1, "Private xform could not be >> created\n"); >> >> >> >+ res =3D -1; >> >> >> >+ goto end; >> >> >> >+ } >> >> >> >+ >> >> >> >+ uint64_t tsc_start, tsc_end, tsc_duration; >> >> >> >+ >> >> >> >+ tsc_start =3D tsc_end =3D tsc_duration =3D 0; >> >> >> >+ if (benchmarking) { >> >> >> >+ tsc_start =3D rte_rdtsc(); >> >> >> >+ num_iter =3D test_data->num_iter; >> >> >> >+ } else >> >> >> >+ num_iter =3D 1; >> >> >> Looks like in same code we're doing benchmarking and functional >> >> validation. >> >> >> It can be reorganised to keep validation test separately like done >> >> >> in crypto_perf. >> >> > >> >> >TJ: Ok, makes sense. However in the interests of getting this into >> >> >the >> >> >18.11 release I'd like to defer this refactoring and the remainder >> >> >of your >> >> comments below to the next release. >> >> > >> >> > >> >> >Next comments - WIP >> >> > >> >> > >> >> >Br, Tomek