From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 2AD48A0577;
	Sun,  5 Apr 2020 20:35:22 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 73DB72BC7;
	Sun,  5 Apr 2020 20:35:21 +0200 (CEST)
Received: from EUR04-HE1-obe.outbound.protection.outlook.com
 (mail-eopbgr70085.outbound.protection.outlook.com [40.107.7.85])
 by dpdk.org (Postfix) with ESMTP id 8484BF12;
 Sun,  5 Apr 2020 20:35:19 +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=QatvC0piEGFRRgmF+f+BFJqjmF6i2Ikk9jusnbh70Yg=;
 b=AON1e5EnW2/lxXl2nZ0nwvxWEucGSwMHv0klyiFDzTYyZyZJI0dMZC+Lk7rW1GapqVSnjCjCAaMPiygN+VsaK0BomWjrf38y1/u9++JuhkhhoS1bHIgE6A7RJBI962fQnJWNRvJynhKNAD1/EJOPD7qZw1H3xwW1l/X3pMfg0PI=
Received: from AM5PR0402CA0007.eurprd04.prod.outlook.com
 (2603:10a6:203:90::17) by VI1PR0801MB2079.eurprd08.prod.outlook.com
 (2603:10a6:800:8e::11) with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.20; Sun, 5 Apr
 2020 18:35:16 +0000
Received: from VE1EUR03FT016.eop-EUR03.prod.protection.outlook.com
 (2603:10a6:203:90:cafe::d) by AM5PR0402CA0007.outlook.office365.com
 (2603:10a6:203:90::17) with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.15 via Frontend
 Transport; Sun, 5 Apr 2020 18:35:16 +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
 VE1EUR03FT016.mail.protection.outlook.com (10.152.18.115) with
 Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.2856.17 via Frontend Transport; Sun, 5 Apr 2020 18:35:15 +0000
Received: ("Tessian outbound 1425309d4c0b:v50");
 Sun, 05 Apr 2020 18:35:15 +0000
X-CR-MTA-TID: 64aa7808
Received: from 917bdd8fa4cb.2
 by 64aa7808-outbound-1.mta.getcheckrecipient.com id
 EB09D031-A8D2-46E6-9253-EDA622CAC75E.1; 
 Sun, 05 Apr 2020 18:35:10 +0000
Received: from EUR02-HE1-obe.outbound.protection.outlook.com
 by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 917bdd8fa4cb.2
 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384);
 Sun, 05 Apr 2020 18:35:10 +0000
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=JRuQIjWCZF6rDAxW3NMbpHdcsHvJbNP9BQxZPc5kztNXenTAVq1dH19/a34jOemfdasBjkiURpUwEhtHvDVw5STP1EK3Ym/N/TS13p5GK7sBNe1VqzRN3FzbOJdRt/yN5bEy67xLg702BM+7ohgkFZmWeHCy8YggXOiIWa0UIwFXQEJMI1q9qRHu4QkAhMHf6fYY4VsX0VAomXyEWBaq5KtbBeCtG8vhFRQRLRfTba5I+9Xmqvf5ymt5UbzgoJ5R0bTHt4OfAxmEW5iGtTIJQY8lpHI6ibmF4Tg9HHiD1fhjR2eseIFDRXpjdevTHhww22Het3bdPK/+UkGz77Ql4g==
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=QatvC0piEGFRRgmF+f+BFJqjmF6i2Ikk9jusnbh70Yg=;
 b=gaAnNgWIEbFgbPv2dz3q7R9wUs79WgdPo0bj+4RGZPQDi/8KutbUefQ+RWvkccvuXHPIdvkJyAtZHJJwVQL1LYvczpRs6i3dgwnWOmGhktte9hLqSPz+S+5xezIyto35RrZJ+D/cxkDIIP3VwKKvw2DqwzIiVnu9aNfX0WCZEpILMrDoUjq1YidnwtFmUxEUqXqh8fQN81nM5FpTXF4TivBQVUhTDooNL4DLhG3XSHlfY8taXtB64TgLOo0vdqfe3+3xXS+H75RgyFzsD3P+j3gidzvO4yUDWK53eAsr6PPo8FjNGBLNz35zObv0pfnxZaKQG+yMiw0FuJF1rAaRvw==
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=QatvC0piEGFRRgmF+f+BFJqjmF6i2Ikk9jusnbh70Yg=;
 b=AON1e5EnW2/lxXl2nZ0nwvxWEucGSwMHv0klyiFDzTYyZyZJI0dMZC+Lk7rW1GapqVSnjCjCAaMPiygN+VsaK0BomWjrf38y1/u9++JuhkhhoS1bHIgE6A7RJBI962fQnJWNRvJynhKNAD1/EJOPD7qZw1H3xwW1l/X3pMfg0PI=
Received: from DBBPR08MB4646.eurprd08.prod.outlook.com (10.255.79.144) by
 DBBPR08MB4298.eurprd08.prod.outlook.com (20.179.42.206) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.2878.19; Sun, 5 Apr 2020 18:35:04 +0000
Received: from DBBPR08MB4646.eurprd08.prod.outlook.com
 ([fe80::1870:afc4:b90f:609d]) by DBBPR08MB4646.eurprd08.prod.outlook.com
 ([fe80::1870:afc4:b90f:609d%5]) with mapi id 15.20.2878.021; Sun, 5 Apr 2020
 18:35:04 +0000
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: "Van Haaren, Harry" <harry.van.haaren@intel.com>, Phil Yang
 <Phil.Yang@arm.com>, "thomas@monjalon.net" <thomas@monjalon.net>, "Ananyev,
 Konstantin" <konstantin.ananyev@intel.com>, "stephen@networkplumber.org"
 <stephen@networkplumber.org>, "maxime.coquelin@redhat.com"
 <maxime.coquelin@redhat.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "david.marchand@redhat.com" <david.marchand@redhat.com>,
 "jerinj@marvell.com" <jerinj@marvell.com>, "hemant.agrawal@nxp.com"
 <hemant.agrawal@nxp.com>, Gavin Hu <Gavin.Hu@arm.com>, Ruifeng Wang
 <Ruifeng.Wang@arm.com>, Joyce Kong <Joyce.Kong@arm.com>, nd <nd@arm.com>,
 "Stable@dpdk.org" <Stable@dpdk.org>, Honnappa Nagarahalli
 <Honnappa.Nagarahalli@arm.com>, nd <nd@arm.com>
Thread-Topic: [PATCH v3 08/12] service: remove redundant code
Thread-Index: AQHWC3jsxu0rlfDC3UGM+YGpX2PBRQ==
Date: Sun, 5 Apr 2020 18:35:04 +0000
Message-ID: <DBBPR08MB4646D2CAE03861DDF706EAB698C50@DBBPR08MB4646.eurprd08.prod.outlook.com>
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>
 <BYAPR11MB3143191EBB61DDEAF5991F03D7C70@BYAPR11MB3143.namprd11.prod.outlook.com>
In-Reply-To: <BYAPR11MB3143191EBB61DDEAF5991F03D7C70@BYAPR11MB3143.namprd11.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-ts-tracking-id: 0d93ca83-2b1f-46db-b37e-83842b75ce0e.0
x-checkrecipientchecked: true
Authentication-Results-Original: spf=none (sender IP is )
 smtp.mailfrom=Honnappa.Nagarahalli@arm.com; 
x-originating-ip: [70.113.25.165]
x-ms-publictraffictype: Email
X-MS-Office365-Filtering-HT: Tenant
X-MS-Office365-Filtering-Correlation-Id: 2b7cf7d8-9075-4062-8b34-08d7d99015ab
x-ms-traffictypediagnostic: DBBPR08MB4298:|DBBPR08MB4298:|VI1PR0801MB2079:
x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr
x-ms-exchange-transport-forked: True
X-Microsoft-Antispam-PRVS: <VI1PR0801MB20797AC1211180E484BAB71098C50@VI1PR0801MB2079.eurprd08.prod.outlook.com>
x-checkrecipientrouted: true
nodisclaimer: true
x-ms-oob-tlc-oobclassifiers: OLM:1443;OLM:1443;
x-forefront-prvs: 03648EFF89
X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en;
 SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DBBPR08MB4646.eurprd08.prod.outlook.com;
 PTR:; CAT:NONE; SFTY:;
 SFS:(10009020)(4636009)(136003)(376002)(396003)(39850400004)(366004)(346002)(5660300002)(7416002)(110136005)(54906003)(316002)(2906002)(66556008)(81166006)(66946007)(7696005)(66446008)(76116006)(66476007)(33656002)(52536014)(81156014)(8936002)(8676002)(64756008)(26005)(86362001)(6506007)(478600001)(4326008)(186003)(55016002)(9686003)(71200400001);
 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: Yv/LfgH8g2ljG2ht38aOPsu0f/kBrDj6IL3mXUUCr0/GOTeDeC4vwYSD8eTb4IpZO6Orqdi0isqAq9ZJ9I7/Dh4CVyGcD29mFtIsWFKokBXp0nB64NrZwQVflNMZsR706pm9gxNe11vqWZt42FNEvRZapRyHMZp6MPoytuT2x/r+P75JpuI9xSxu+Favm3z/L18dUA7Vji9MjkhaFdAidctf5bIy8W06xErjD3nFEUSbW7PCIG4/Ou+eXz88FQbmH+c8SUWy61nuA9plR35jphOKoO2du3URIEBpsH194eC5GLWQMq769LAbPThoFy6rPcIsScceYDfgxrTVj7zIWi62dQqfyCVi07+5ZV0jnL6TCr3ypQBpcABxTw5rtPIQmTMQor8zaQf4facJNV/RxMF9wOPzbSm92lymiqqXI44TRYMEkEbprlhtF0g5Ayna
x-ms-exchange-antispam-messagedata: SvDKlGZfLn8yv5kg9nN69bht1vGuX6gkNAyx7vC7tROK+aOq0STi1MlLDIqBbeBIJ+7x+6FJ9/acIZgA06WuNn2LC34sgQvjhiZvvAmDmR9jnrmTFmJ6Blm9YsvoNCtM1MXMomAoOlXaNwu8cmHScw==
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR08MB4298
Original-Authentication-Results: spf=none (sender IP is )
 smtp.mailfrom=Honnappa.Nagarahalli@arm.com; 
X-EOPAttributedMessage: 0
X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT016.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)(346002)(39850400004)(376002)(396003)(136003)(46966005)(47076004)(7696005)(52536014)(9686003)(336012)(450100002)(2906002)(26005)(55016002)(5660300002)(186003)(82740400003)(8936002)(8676002)(478600001)(70586007)(26826003)(6506007)(70206006)(81166006)(86362001)(81156014)(54906003)(33656002)(4326008)(36906005)(316002)(110136005)(356004);
 DIR:OUT; SFP:1101; 
X-MS-Office365-Filtering-Correlation-Id-Prvs: e39624ec-c726-4c2d-d89b-08d7d9900ef6
X-Forefront-PRVS: 03648EFF89
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: hE73UsVoJIg9vxMuBa4brDEk9+FwV8anv/vw20inTxnkMWvHFxIjdJZvrWpkqR51heVeDkq5//EhShGb22oTsDX7wTu2f5idRxCXEnE2iALYiQ77IFG//o2gNWb9i2iZnw0G7oMD4FkfeI5UJaercxFSLjHfT0PrHBZMyDgsspbh7n42wGHyY+TA8NTGLq/2wyLNLRBrIA+lMEP3oFnDA1itP7+3RylW/2StgboZ0OafDtUR/V5FM/GO9PP3c3L1a58gqb8oRYI4SoRyE0KbS9ZsfeHTkZs7Qj21Cna7OclfaEWX3hBZZxdy2Afk49iTArLfG4JULdIOXwcM3Z3yOKrxVO0owjI8ukVtgQOtIyL3Uvc3iOXHx9nePS0KZi7RSX7+D0KOSwLKH55BjxpZeH4g6M/KWa0kmWtPgdeqoHvZVRveML4vQvQiJoD+iNywCxdYH2A067d4jCeFG9EzTR5FChl8XjhnEmCGr4ZEV9gLSmKCX+lDNvAETPD2JnSfmpjxomF9cnhwSD9X6tqQmg==
X-OriginatorOrg: arm.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2020 18:35:15.6825 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 2b7cf7d8-9075-4062-8b34-08d7d99015ab
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: VI1PR0801MB2079
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

<snip>

> >
> > 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 <phil.yang@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>=20
>=20
> 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.
>=20
> As before, it is a valid cleanup (thanks), and I'd like to take it for ne=
w DPDK
> releases.
>=20
> Happy to Ack without Fixes or Cc Stable, if that's acceptable to you?
Agreed.

>=20
>=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 enou=
gh.

> >  {
> > -	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 a=
re validating some parameters and not some.
Suggest bringing the validation of the service id also into this function a=
nd remove it from the calling functions.

> >
> >  	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'.

> > -	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