From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 5E18DA0096 for ; Mon, 3 Jun 2019 15:44:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 79A931B950; Mon, 3 Jun 2019 15:44:55 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by dpdk.org (Postfix) with ESMTP id 6E90AF64; Mon, 3 Jun 2019 15:44:53 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x53DZqUT007714; Mon, 3 Jun 2019 06:44:52 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pfpt0818; bh=PEmaq/7kZtuCZUDy391A+296MWoYqVeizoFZUGEUcGg=; b=kYJJoovpAmP3MnSW37uropMAAu8S74kkz7U2GkSXa7rUShWkTM4TGjnVmvrQTjB+SLJn ryKlygBdIzJwefKzZksfCKeIHkrtj60ciY09meobBlrMsiCDNHjRYI0niaY069gAa1KJ k/k8c4mYshNo9broC3pbTHHdYlwgBMuOio4KUgIAflGFrnnAl0+CAXJoiXprPXvGb0RB m3/WRCy2nvfFdWe6PBnu1QhgJBIMWDv+E2AV8/Pe8Bea5WHqfBfhUaX6q5eeG185Xsb9 lltdis0Q93ljPuyz92sTaqq7HWHeChxDiqw+UVeY78/hWq8H55RSUPS01O2x/9V9rJhQ lQ== Received: from sc-exch03.marvell.com ([199.233.58.183]) by mx0b-0016f401.pphosted.com with ESMTP id 2survk8bg5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Mon, 03 Jun 2019 06:44:51 -0700 Received: from SC-EXCH04.marvell.com (10.93.176.84) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Mon, 3 Jun 2019 06:44:49 -0700 Received: from NAM05-DM3-obe.outbound.protection.outlook.com (104.47.49.58) by SC-EXCH04.marvell.com (10.93.176.84) with Microsoft SMTP Server (TLS) id 15.0.1367.3 via Frontend Transport; Mon, 3 Jun 2019 06:44:48 -0700 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=PEmaq/7kZtuCZUDy391A+296MWoYqVeizoFZUGEUcGg=; b=aas+PTovPZhWr3fPOVwfmmgpd+Tf5ZcBWDE9dRZ8PaWs5KmiZApWXLJ42d+qPTo40vkikUjVlcJAeuwG33/TAeIb7KIIVtMqBeMDJrnurfsFfXPKygx0NpLaJkuFPil4jbwbKK/bsllPTB8tUI3o9NURoCwTPmlc3mF/ywwTKak= Received: from BN6PR1801MB2052.namprd18.prod.outlook.com (10.161.157.11) by BN6PR1801MB2004.namprd18.prod.outlook.com (10.161.153.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1943.22; Mon, 3 Jun 2019 13:44:43 +0000 Received: from BN6PR1801MB2052.namprd18.prod.outlook.com ([fe80::9170:db9e:4ac1:f76e]) by BN6PR1801MB2052.namprd18.prod.outlook.com ([fe80::9170:db9e:4ac1:f76e%5]) with mapi id 15.20.1943.018; Mon, 3 Jun 2019 13:44:43 +0000 From: Shally Verma To: "Trahe, Fiona" , "Jozwiak, TomaszX" , "dev@dpdk.org" , "stable@dpdk.org" CC: "Trybula, ArturX" Thread-Topic: [PATCH] app/test-compress-perf: fix reliance on integer endianness Thread-Index: AQHVDxVuwhlhcjj1yUi9K6Y9c9YDN6Z1JA+AgAAR+bCAAA9SgIAM5lqAgAfaneA= Date: Mon, 3 Jun 2019 13:44:43 +0000 Message-ID: References: <1558358764-32053-1-git-send-email-tomaszx.jozwiak@intel.com> <1558358764-32053-2-git-send-email-tomaszx.jozwiak@intel.com> <348A99DA5F5B7549AA880327E580B4358976C60C@IRSMSX101.ger.corp.intel.com> <348A99DA5F5B7549AA880327E580B435897749EF@IRSMSX101.ger.corp.intel.com> In-Reply-To: <348A99DA5F5B7549AA880327E580B435897749EF@IRSMSX101.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [115.113.156.3] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: ab47af7b-2420-471b-1eae-08d6e829a26e x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(7168020)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328)(7193020); SRVR:BN6PR1801MB2004; x-ms-traffictypediagnostic: BN6PR1801MB2004: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 0057EE387C x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(396003)(39850400004)(376002)(346002)(136003)(13464003)(189003)(199004)(316002)(6506007)(53546011)(2501003)(305945005)(3846002)(76116006)(2906002)(102836004)(73956011)(25786009)(66446008)(64756008)(66476007)(55236004)(66556008)(68736007)(486006)(66946007)(256004)(66066001)(7736002)(52536014)(6246003)(5660300002)(86362001)(71200400001)(71190400001)(14444005)(74316002)(110136005)(2201001)(6116002)(8936002)(81156014)(81166006)(4326008)(55016002)(8676002)(229853002)(6436002)(26005)(186003)(9686003)(33656002)(99286004)(14454004)(476003)(53936002)(7696005)(76176011)(478600001)(446003)(11346002); DIR:OUT; SFP:1101; SCL:1; SRVR:BN6PR1801MB2004; H:BN6PR1801MB2052.namprd18.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: marvell.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: rACbEiyrc2LRatIx4PnlfuTU75B+k/kncoLjJwIyrXcSnvScYRTvVIYkjnSEpns4OYSCUVir1nt44cZJ7rfBoWCN+B6g+KCyRmtxWKMSq3FJAkQisnKWd0k6ddkuHst7tSeQ3XB9LteBoPzSlLaWakqo3QSr2yGwIVUwaMxpwX4+2GQ4ITx+oBCde2dJMQ+DOUzTM3rW6NHd2HUdPX1URIgFc7X0cA/n15zNeoDK+jW3lbF4QJ/ArHHbYy4UzHFknIPWNsnWf1Q4d6Z8EagQlwQqspR3TFOUeiEAf7bynm6szNmY7JHMVg1i7WbyHkNATU8hGsrMsRFo+XV1rqpN4vHmD6fKv4U48txqCZcp4I20Dc8wQWD9yQE2mMpXRDBa41pSQQX1y0j2ExnODBdIabQ+dfTkZcRvXUOq+kI9fv8= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: ab47af7b-2420-471b-1eae-08d6e829a26e X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Jun 2019 13:44:43.3764 (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: shallyv@marvell.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1801MB2004 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-06-03_10:, , signatures=0 Subject: Re: [dpdk-dev] [PATCH] app/test-compress-perf: fix reliance on integer endianness 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: Trahe, Fiona > Sent: Wednesday, May 29, 2019 7:17 PM > To: Jozwiak, TomaszX ; Shally Verma > ; dev@dpdk.org; stable@dpdk.org > Cc: Trybula, ArturX ; Trahe, Fiona > > Subject: [EXT] RE: [PATCH] app/test-compress-perf: fix reliance on intege= r > endianness >=20 > External Email >=20 > ---------------------------------------------------------------------- > Hi Shally, Tomasz, >=20 > > > > > > -----Original Message----- > > > > > > From: Jozwiak, TomaszX > > > > > > Sent: Monday, May 20, 2019 2:26 PM > > > > > > To: dev@dpdk.org; Trahe, Fiona ; > > > > > > Jozwiak, TomaszX ; > > > > > > shallyv@marvell.com; stable@dpdk.org > > > > > > Subject: [PATCH] app/test-compress-perf: fix reliance on > > > > > > integer endianness > > > > > > > > > > > > This patch fixes coverity issue: > > > > > > Reliance on integer endianness (INCOMPATIBLE_CAST) in > > > > > parse_window_sz > > > > > > function. > > > > > > > > > > > > Coverity issue: 328524 > > > > > > Fixes: e0b6287c035d ("app/compress-perf: add parser") > > > > > > Cc: stable@dpdk.org > > > > > > > > > > > > Signed-off-by: Tomasz Jozwiak > > > > > > --- > > > > > > app/test-compress-perf/comp_perf_options_parse.c | 4 +++- > > > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/app/test-compress-perf/comp_perf_options_parse.c > > > > > > b/app/test-compress- perf/comp_perf_options_parse.c index > > > > > > 2fb6fb4..56ca580 100644 > > > > > > --- a/app/test-compress-perf/comp_perf_options_parse.c > > > > > > +++ b/app/test-compress-perf/comp_perf_options_parse.c > > > > > > @@ -364,13 +364,15 @@ parse_max_num_sgl_segs(struct > > > > > comp_test_data > > > > > > *test_data, const char *arg) static int > > > > > > parse_window_sz(struct comp_test_data *test_data, const char *a= rg) > { > > > > > > - int ret =3D parse_uint16_t((uint16_t *)&test_data->window_sz, > arg); > > > > > > + uint16_t tmp; > > > > > > + int ret =3D parse_uint16_t(&tmp, arg); > > > > > > > > > > > > if (ret) { > > > > > > RTE_LOG(ERR, USER1, "Failed to parse window > size\n"); > > > > > > return -1; > > > > > > } > > > > > > > > > > > > + test_data->window_sz =3D (int)tmp; > > > > > > return 0; > > > > > > } > > > > > [Fiona] I expect this fixes this coverity issue - but will it > > > > > result in another > > > one? > > > > > window_sz on the xform is uint8_t - so this int will get > > > > > truncated later, and there's no cast done at that point. > > > > > Would it be better to add a new parse_uint8_t fn and change > > > > > test-data- > > > > > >window_sz to a unit8_t? > > > > > Or add that cast? > > > > [Tomek] I measn it's ok. There's a check inside > > > > comp_perf_check_capabilities function. > > > > If the value from test_data->window_sz > cap->window_size we have > > > > a > > > fail. > > > > Also during parsing there's a check is value from command line > > > > between > > > > 0 and UINT16_MAX, so in my opinion all cases are tested. The point > > > > is there's only one place where we're parsing uint8_t value. > > > > parse_uint8_t function will be especially for that. > > > [Shally] What is window_sz in test data ?is it base 2 log of (actual > > > window > > > length) or actual window length in bytes? lib spec mention this as > > > struct rte_param_log2_range, so If test window size is actual window > > > length in bytes then I assume test perf should check for > > > test_data->window_sz > 2 pow cap->window_size but that doesn't look l= ike > the case. > > > So if it is log value, then coding wise typecasting here doesn't look= right. > > > Though it add need for extra function to parse_uint8, but that looks > > > like cleaner approach to use. > > [Tomek] I mean it's log 2 (please take a look at help usage function > > in comp_perf_options_parse.c:37 > > > > " --window-sz N: base two log value of compression window size\n" > > " (e.g.: 15 =3D> 32k, default: max supported by > PMD)\n" > > > > I mean it's ok, still. We have many types in command line and can be > > much more in the future. The idea is to parse them into a sort of > > common range value first ( it should be max range for all digital > > command line options - in our case there's uint16 or uint32_t) even if > > it's shorter like uint8_t or etc. We store these values in > > comp_test_data structure first. Next we check the ranges each of them. > > In case of window_sz this test is in comp_perf_check_capabilities > > function. That approach reduce a set of parsing functions we needed. > > Of course we can create separate parsing function for each of command > > line type value, but is this really needed ? :D Please let me know > > your thoughts - if this new parsing function will clear the code - > > I'll add this in v2 > [Fiona] ok, I reviewed again and see I'd misunderstood. > The param being parsed is intentionally not being stored in test_data str= uct as > uint8_t, but as an int because it uses -1 as a default value. And there a= re range > checks on the input, so an invalid value will never be passed to the PMD. > So I'm ok with the fix as is - it resolves the coverity issues reported o= n the param > parsing. >=20 > However there's a second issue, which coverity is likely to throw up afte= r the > above fix is applied - when the test_data value is later passed to the PM= D, it > should have a cast from int to unit8_t. > But that's a separate issue, not referred to by this coverity report, so = we'll send a > separate patch for it. > @Shally, are you ok with this? [Shally] No hard choices here. But why we cant app use 0 as default value i= nstead of -1? It would save us an additional typecast when passing to PMD Thanks Shally