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 326B8A0597; Wed, 8 Apr 2020 12:15:31 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0FC941C13B; Wed, 8 Apr 2020 12:15:31 +0200 (CEST) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50072.outbound.protection.outlook.com [40.107.5.72]) by dpdk.org (Postfix) with ESMTP id 14BF81C0C2; Wed, 8 Apr 2020 12:15:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6u0UesNLU+gJbbcYxFd9FTCauWxCpoNxcNZ+F06z6Fs=; b=r9ASZKua2nVOJtlqqIvNSrYuu4BT+elrLoylvphNYn5yhiU60CEv0xPHsQmpRUY60zQrdOQgFdE8hC6udikXIPhSqCCkzeQr5J9p8FOa8z/3k/aTgGRxEpSorcYMFdDPLF+sXtNpNAT3ordRVKw/iLWu2M5CBWRlH7KGGpdJcps= Received: from VI1PR07CA0292.eurprd07.prod.outlook.com (2603:10a6:800:130::20) by AM6PR08MB3814.eurprd08.prod.outlook.com (2603:10a6:20b:88::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.20; Wed, 8 Apr 2020 10:15:27 +0000 Received: from VE1EUR03FT026.eop-EUR03.prod.protection.outlook.com (2603:10a6:800:130:cafe::1) by VI1PR07CA0292.outlook.office365.com (2603:10a6:800:130::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.6 via Frontend Transport; Wed, 8 Apr 2020 10:15:27 +0000 Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dpdk.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dpdk.org; dmarc=bestguesspass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT026.mail.protection.outlook.com (10.152.18.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2856.17 via Frontend Transport; Wed, 8 Apr 2020 10:15:26 +0000 Received: ("Tessian outbound af37c2b81632:v50"); Wed, 08 Apr 2020 10:15:26 +0000 X-CR-MTA-TID: 64aa7808 Received: from 82c978d20928.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 1E3C2352-6CF7-4B05-8087-1FB6587B30A7.1; Wed, 08 Apr 2020 10:15:21 +0000 Received: from EUR05-DB8-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 82c978d20928.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Wed, 08 Apr 2020 10:15:21 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QERN25sv0KK9gwDoRi2FfZzMUk6FJq/ksW4EDJ7NckdGWpIer40diD34GIR7cYbq39Uk+GPcRo0f+fyZM6eX1UgagrfFCuYPsDK3Pl9PxJSBzb6u8lB+kcbACi+k0727JfN5UkIq4LVVbrLjYRBkxw4KFeXH6IpHkCzlJTZsy4f4tOZffol0jBqHJD3bsDXioyIj/zWWzT2YZ7Fp9pUdiSgYoz/va5WtHjsOcp+Lk70aMqejtCUEiDWAAkgVgFGN2u8pknZRGsDohpzhGr+dtl4k9vMdpC4+DaQXVciYTjLJcYFm9Dk4CsO65LX2GZHsg1q4BZgUb6ebYLoxVmbdXA== 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=6u0UesNLU+gJbbcYxFd9FTCauWxCpoNxcNZ+F06z6Fs=; b=nrVExAruOTc9zO6/W7IbIMwv7H9oE+0ftUGrU2LWiMTMUIpvBbFQU6wjUNr4dRz2b3OC/Q9qJdeSZcuRjamMtzJmx+cc+w0G/uXfOD72A8U9f4Ly70HYJY/WiFf6HI2gWi+UNyF9lJBRpF1LvFnHPODfnd5HJmVhXL6EwRglNW/gE0Ka3poVy5V4ceIT2eDTXfpoVWIqhI0DzboleCYRR/Rl+N5qLk+lXZWWmbboMn8KeM8oUtjw0kpl/QVebK/aCKPzwHsrShkJXzzOQGRcdbMo4W5sOvg8sD05sgeyzAhaIMxXi2Oa1eC1iXdQ9o5XcrmjFnn5Ga8JzC8z53bLKw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=6u0UesNLU+gJbbcYxFd9FTCauWxCpoNxcNZ+F06z6Fs=; b=r9ASZKua2nVOJtlqqIvNSrYuu4BT+elrLoylvphNYn5yhiU60CEv0xPHsQmpRUY60zQrdOQgFdE8hC6udikXIPhSqCCkzeQr5J9p8FOa8z/3k/aTgGRxEpSorcYMFdDPLF+sXtNpNAT3ordRVKw/iLWu2M5CBWRlH7KGGpdJcps= Received: from VE1PR08MB4640.eurprd08.prod.outlook.com (10.255.27.75) by VE1PR08MB4687.eurprd08.prod.outlook.com (10.255.113.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.20; Wed, 8 Apr 2020 10:15:19 +0000 Received: from VE1PR08MB4640.eurprd08.prod.outlook.com ([fe80::7df3:e3c8:54f7:57c2]) by VE1PR08MB4640.eurprd08.prod.outlook.com ([fe80::7df3:e3c8:54f7:57c2%6]) with mapi id 15.20.2878.021; Wed, 8 Apr 2020 10:15:19 +0000 From: Phil Yang To: Honnappa Nagarahalli , "Van Haaren, Harry" , "thomas@monjalon.net" , "Ananyev, Konstantin" , "stephen@networkplumber.org" , "maxime.coquelin@redhat.com" , "dev@dpdk.org" CC: "david.marchand@redhat.com" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , Gavin Hu , Ruifeng Wang , Joyce Kong , nd , "Stable@dpdk.org" , nd , nd Thread-Topic: [PATCH v3 08/12] service: remove redundant code Thread-Index: AQHWCa8kD2NCO6hTmE64wOvdJh+5SKhq3noAgAQjCwA= Date: Wed, 8 Apr 2020 10:15:19 +0000 Message-ID: References: <1583999071-22872-1-git-send-email-phil.yang@arm.com> <1584407863-774-1-git-send-email-phil.yang@arm.com> <1584407863-774-9-git-send-email-phil.yang@arm.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: a626d439-947d-4dc1-9af7-a26bfe10728b.0 x-checkrecipientchecked: true Authentication-Results-Original: spf=none (sender IP is ) smtp.mailfrom=Phil.Yang@arm.com; x-originating-ip: [58.39.116.39] x-ms-publictraffictype: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 5c16e17a-2014-4892-6088-08d7dba5c226 x-ms-traffictypediagnostic: VE1PR08MB4687:|VE1PR08MB4687:|AM6PR08MB3814: x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr x-ms-exchange-transport-forked: True X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true nodisclaimer: true x-ms-oob-tlc-oobclassifiers: OLM:530;OLM:530; x-forefront-prvs: 0367A50BB1 X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VE1PR08MB4640.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(396003)(39860400002)(136003)(366004)(346002)(376002)(55016002)(478600001)(66556008)(66476007)(4326008)(71200400001)(26005)(110136005)(54906003)(316002)(5660300002)(9686003)(2906002)(66946007)(81166007)(64756008)(86362001)(66446008)(52536014)(76116006)(186003)(7416002)(8936002)(6506007)(53546011)(8676002)(7696005)(33656002)(81156014); DIR:OUT; SFP:1101; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: 3Qv5n86mYG61Ux6zmg3Z87vBUXOliMlJaqY+INWIZISD7gPuGEHE0NkBakG4TcbJOZALi7nnGiVOsyOubsTftMKLwFsuPHNc/n1IP8ZikBWjevV8gbBNxM+nOI9x7fxcO7Tj1IesmKIg0ynDIqC3hErWB2toe43BlyaSFzNsMqKpX2h1zULW1vmFDHGr7y84b7m7Qxwm253CymdTAlelUzDA0C8LmKhwfvSPwuEe4VGNuutJPhzSZy9np6GAJZZWGZSG2RZzmRS0zS/ZUHG2osWGX2GDdaYDjx6AqKY2hpSBFQOQE1tTtzD1KtGerK2uQ1RD60rmKD/Wvu4wEYT5sbsBfYnMJCBJsweX9mMFL7CctltOyRnRKWQn0L1mJe78t/tp+mZEmPj/Cu+CIfpqVmAoqcxYPHIS8Yb4OVlZFT2NGZPx8FenMcafy+V6R/0W x-ms-exchange-antispam-messagedata: v6olPd1A3MpswQzjRFO9+pI/oNzq/JXs+LvYvEDdAqQKddMV9IyMIxWNsNr3dIjH9yJnTnU4noiT0XaaUW4bWVvnVz5yYohQbKMlxywcSkoqB/Ku5nZPJ+p4pebhHieQ7TCs8E/suLynwetGWtZPYQ== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR08MB4687 Original-Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Phil.Yang@arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT026.eop-EUR03.prod.protection.outlook.com X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(136003)(346002)(376002)(39860400002)(396003)(46966005)(110136005)(26005)(450100002)(186003)(36906005)(8676002)(54906003)(26826003)(7696005)(356004)(4326008)(336012)(52536014)(478600001)(6506007)(82740400003)(47076004)(316002)(70586007)(70206006)(9686003)(81166007)(53546011)(5660300002)(81156014)(8936002)(86362001)(33656002)(2906002)(55016002); DIR:OUT; SFP:1101; X-MS-Office365-Filtering-Correlation-Id-Prvs: e4dc45dd-1b84-43ae-e2ff-08d7dba5bdce X-Forefront-PRVS: 0367A50BB1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: eh89Uz6cX6PGW1KdbAnzV/e9D0E4GOnsPhl774mWppQCLi7RWRY9KNCcu2gQehF3z7EQWdC67tnR1U6zkaNEYf2/GUYcXUZ2lo6BZPvdtx9YV8ci/KoAMWXpoesFA5kvyvyvZlnjG0p1fQ/HXSwZReVO05Gq7fs2IpkUWhXHwJuh6rXUSMFBMIfbhA996CuaoCRQ4fY3BxKs31MVUJCbP/SVcacMg2oK2RxQy/pEaNltzaOUpB472uqJ05TDsmBrVhHXOff+nNajUvaioK6z7XNIm9TNZuniM6UGygAgSnq/Brz+jyKIlO22TkuA8ybhB5ik9sCma68Zk2jdRCey+CPjHVyHQEcqwXr1sTrTynaNn495IHLxtUeF2oAyLEYGGjUI3O3cEQ2iAcQOs9meW02s6/E5lGnqnVchvKzvt9rWYKYY/FKY305EzzGI4Pqt8CNGs+TpOf96+6QSSI/Sj5+4E2Ucve2WVywXanp4t5hH/tYCDmEYGuTDfnyvAxfnDoI4Vy9VVzfjBAyF0Vqhaw== X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2020 10:15:26.8155 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 5c16e17a-2014-4892-6088-08d7dba5c226 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB3814 Subject: Re: [dpdk-dev] [PATCH v3 08/12] service: remove redundant code 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: Honnappa Nagarahalli > Sent: Monday, April 6, 2020 2:35 AM > To: Van Haaren, Harry ; Phil Yang > ; thomas@monjalon.net; Ananyev, Konstantin > ; stephen@networkplumber.org; > maxime.coquelin@redhat.com; dev@dpdk.org > Cc: david.marchand@redhat.com; jerinj@marvell.com; > hemant.agrawal@nxp.com; Gavin Hu ; Ruifeng Wang > ; Joyce Kong ; nd > ; Stable@dpdk.org; Honnappa Nagarahalli > ; nd > Subject: RE: [PATCH v3 08/12] service: remove redundant code >=20 > >=20 > > > > > > The service id validation is verified in the calling function, remove > > > the redundant code inside the service_update function. > > > > > > Fixes: 21698354c832 ("service: introduce service cores concept") > > > Cc: Stable@dpdk.org > > > > > > Signed-off-by: Phil Yang > > > Reviewed-by: Honnappa Nagarahalli > > > > > > Same comment as patch 7/12, is this really a "Fix"? This functionality = is not > > "broken" in the current code? And is there value in porting to stable?= I'd > see > > this as unnecessary churn. > > > > As before, it is a valid cleanup (thanks), and I'd like to take it for = new DPDK > > releases. > > > > Happy to Ack without Fixes or Cc Stable, if that's acceptable to you? > Agreed. Agreed.=20 >=20 > > > > > > > > > --- > > > lib/librte_eal/common/rte_service.c | 31 > > > ++++++++++++------------------- > > > 1 file changed, 12 insertions(+), 19 deletions(-) > > > > > > diff --git a/lib/librte_eal/common/rte_service.c > > > b/lib/librte_eal/common/rte_service.c > > > index 2117726..557b5a9 100644 > > > --- a/lib/librte_eal/common/rte_service.c > > > +++ b/lib/librte_eal/common/rte_service.c > > > @@ -552,21 +552,10 @@ rte_service_start_with_defaults(void) > > > } > > > > > > static int32_t > > > -service_update(struct rte_service_spec *service, uint32_t lcore, > > > +service_update(uint32_t sid, uint32_t lcore, > > > uint32_t *set, uint32_t *enabled) > 'set' parameter does not need be passed by reference, pass by value is > enough. Agreed. =20 >=20 > > > { > > > -uint32_t i; > > > -int32_t sid =3D -1; > > > - > > > -for (i =3D 0; i < RTE_SERVICE_NUM_MAX; i++) { > > > -if ((struct rte_service_spec *)&rte_services[i] =3D=3D service && > > > -service_valid(i)) { > > > -sid =3D i; > > > -break; > > > -} > > > -} > > > - > > > -if (sid =3D=3D -1 || lcore >=3D RTE_MAX_LCORE) > > > +if (lcore >=3D RTE_MAX_LCORE) > > > return -EINVAL; > The validations look somewhat inconsistent in service_update function, we > are validating some parameters and not some. > Suggest bringing the validation of the service id also into this function= and > remove it from the calling functions. Agreed. I will update it in the next version. >=20 > > > > > > if (!lcore_states[lcore].is_service_core) > > > @@ -598,19 +587,23 @@ service_update(struct rte_service_spec > *service, > > > uint32_t lcore, int32_t rte_service_map_lcore_set(uint32_t id, > > > uint32_t lcore, uint32_t enabled) { > > > -struct rte_service_spec_impl *s; > > > -SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); > > > +/* validate ID, or return error value */ > > > +if (id >=3D RTE_SERVICE_NUM_MAX || !service_valid(id)) > > > +return -EINVAL; > > > + > > > uint32_t on =3D enabled > 0; > We do not need the above line. 'enabled' can be passed directly to > 'service_update'. Agreed. >=20 > > > -return service_update(&s->spec, lcore, &on, 0); > > > +return service_update(id, lcore, &on, 0); > > > } > > > > > > int32_t > > > rte_service_map_lcore_get(uint32_t id, uint32_t lcore) { > > > -struct rte_service_spec_impl *s; > > > -SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); > > > +/* validate ID, or return error value */ > > > +if (id >=3D RTE_SERVICE_NUM_MAX || !service_valid(id)) > > > +return -EINVAL; > > > + > > > uint32_t enabled; > > > -int ret =3D service_update(&s->spec, lcore, 0, &enabled); > > > +int ret =3D service_update(id, lcore, 0, &enabled); > > > if (ret =3D=3D 0) > > > return enabled; > > > return ret; > > > -- > > > 2.7.4 >=20