From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 3BA537CE5 for ; Wed, 20 Sep 2017 16:53:30 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Sep 2017 07:53:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,421,1500966000"; d="scan'208";a="1197200516" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga001.fm.intel.com with ESMTP; 20 Sep 2017 07:53:28 -0700 Received: from irsmsx155.ger.corp.intel.com (163.33.192.3) by IRSMSX152.ger.corp.intel.com (163.33.192.66) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 20 Sep 2017 15:53:05 +0100 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.22]) by irsmsx155.ger.corp.intel.com ([169.254.14.70]) with mapi id 14.03.0319.002; Wed, 20 Sep 2017 15:53:05 +0100 From: "Van Haaren, Harry" To: Pavan Nikhilesh Bhagavatula , "Thomas Monjalon" CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] eal: added new `rte_lcore_is_service_lcore` API. Thread-Index: AQHTHCIRejLk4K1hBEGkGqXEqpO/LKKZoOZA///444CAADSUsIAAB/qAgBw0fACAABEogP///IqAgAARKhD///PFAAAB0TgAAAO0doAA94ZBcA== Date: Wed, 20 Sep 2017 14:53:04 +0000 Message-ID: References: <1503501027-11046-1-git-send-email-pbhagavatula@caviumnetworks.com> <20170915145933.GA16776@PBHAGAVATULA-LT> <6875069.BRSTqJL15t@xps> <20170915173740.GA21540@PBHAGAVATULA-LT> In-Reply-To: <20170915173740.GA21540@PBHAGAVATULA-LT> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjJmMWFlMzctZjUxNC00YWQ1LWJlZTgtYmVhZGViNzE4M2I4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImE5UWxcL2dtOW55WHI1akJQenJsSkhJZGVua28zS0lzbm5idG9Gdnp6Y3g4PSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] eal: added new `rte_lcore_is_service_lcore` API. 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: Wed, 20 Sep 2017 14:53:31 -0000 > From: Pavan Nikhilesh Bhagavatula [mailto:pbhagavatula@caviumnetworks.com= ] > Sent: Friday, September 15, 2017 6:38 PM > To: Thomas Monjalon > Cc: Van Haaren, Harry ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] eal: added new `rte_lcore_is_service_lcor= e` > API. >=20 > On Fri, Sep 15, 2017 at 05:51:36PM +0200, Thomas Monjalon wrote: > > 15/09/2017 16:59, Pavan Nikhilesh Bhagavatula: > > > On Fri, Sep 15, 2017 at 02:44:57PM +0000, Van Haaren, Harry wrote: > > > > > > > > > We could also choose to add this function to rte_service.h ? > > > > > > > > > > > > Yes that is an option, and OK with me. > > > > > > > > > > > > @Pavan what do you think of adding it to service.h, implement i= n .c > and add > > > > > to .map? > > > > > > > > > > > > > > > > The ROLE_SERVICE/ROLE_RTE defines the role of a lcore so it made = sense > to put > > > > > it in rte_lcore.h as lcore properties are accessed mostly through= this > header. > > > > > I'm fine with adding it to service.h as suggested by Harry. > > > > > > > > > > -Pavan > > > > > > > > *as suggested by Thomas ;) > > > > > > > > Initially I thought it made more sense in lcore.h too, however the > application > > > > should only require knowing if core X is a service core if it cares= about > > > > services / service-cores, hence I'm fine with rte_service.h too. > > > > > > > > -Harry > > > > > > > Agreed, will spin up a v2. > > > > The most difficult is to find a good name for this function :) >=20 > If not rte_lcore_is_service_core then how about rte_lcore_is_role_service= ? > But this would need a sibling api rte_lcore_is_role_rte (or a better one)= which > is satisfied by rte_lcore_is_enabled :( > IMO when role was limited to RTE & OFF rte_lcore_is_enabled fits now with > new role SERVICE it looks out of place cause even service lcores are > "enabled". > Modifying rte_lcore_is_enabled would be a huge task (API change) as it is= used > widely in many places. Hey all, I've been thinking a little, and adding the "is service core" functionality= in the rte_service_* namespace might be the wrong place. The function name certain= ly doesn't roll off the tongue ( rte_service_lcore_has_service_role() ?? ) What if we add a new function to rte_lcore.h? The implementation could be i= n a new file, rte_lcore.c, to avoid "static inline" in a control-path function. In my eyes, this approach is the cleanest as it allows re-use of the same f= unction for various types, including SERVICE, RTE, OFF etc.=20 /** Probes if the calling core has a specific role. * @retval 1 If the core has role matching the *role* passed in * @retval 0 If the core's role does not match *role* passed in */ int rte_lcore_has_role(enum rte_lcore_role_t role); Application code becomes pretty self-documenting: if (rte_lcore_has_role(ROLE_SERVICE)) { // do something } Thoughts? -Harry