Re: [dpdk-dev] [RFC] service core concept header implementation

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Fri, 5 May 2017 15:48:02 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Richardson, Bruce"
>  <bruce.richardson@intel.com>, "hemant.agrawal@nxp.com"
>  <hemant.agrawal@nxp.com>, "nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
>  "Vangati, Narender" <narender.vangati@intel.com>, "Eads, Gage"
>  <gage.eads@intel.com>
> Subject: RE: [RFC] service core concept header implementation
> 
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> <snip email headers>
> > > Hi Harry,
> > >
> > > Overall it looks good. Some initial comments
> 
> Off to a good start! Replies inline, in general I think we're on the same page. This RFC was pretty "fresh", mostly to check if the community agrees with the general concept of service-cores. The implementation details (although going well) will have some churn I expect :)
> 
> I'd like opinions from others in the community - I'll leave this thread "open" for a while to get more feedback, and rework a v2 RFC then.
> 
> Thanks for your review - Harry
> 
> 
> > >
> > > Regarding step 4 and 5, It looks like,
> > > a) The lcore_id information is duplicate in step 4 and 5.
> 
> 
> Not really duplicated - keep in mind we do not want to have a 1:1 mapping, the goal of this is to allow e.g. 3 cores -> 5 services type mappings, where the work is shared between the cores. Hence, setting coremasks and setting lcores to use are very related, but not quite the same.
> 
> 
> > > b) On rte_eal_service_set_coremask() failure, you may the need
> > > inverse of rte_eal_service_use_lcore()(setting back to worker/normal
> > > lcore)
> 
> We can pass the "type" of lcore as an argument to a function:
> 
> #define RTE_EAL_SERVICE_TYPE_APPLICATION  0
> #define RTE_EAL_SERVICE_TYPE_SERVICE      1
> 
>   rte_eal_service_set_lcore_type(uint32_t type);
> 
> Allows adding more core types (if anybody can think of any) and avoids function-count bloat :)
> 
> 
> > > But I understand the need for step 5.
> > >
> > > How about changing the (4) and (5) as
> > >
> > > step 4) rte_eal_service_set_coremask
> > > step 5) rte_eal_service_apply(void)(or similar name) for error check and
> > > ensure at least on core is going to be running the service.
> 
> So the sequence would be:
> 
> for( i < app_n_service_cores )
>    rte_eal_service_set_lcore_type( SERVICE );
> 
> for( i < services_count )
>    rte_eal_serivce_set_coremask( app_decided_coremask_here );

I thought, The application may not need to set the explicit
rte_eal_service_set_lcore_type, it can be internally set by
rte_eal_service_set_coremask

i.e

for( i < services_count )
    rte_eal_service_set_coremask( app_decided_coremask_here );

int ret = rte_eal_service_apply();

rte_eal_service_set_coremask(app_decided_coremask_here)
{
	proposed_implementation;
	for_each_bit_set(app_decided_coremask_here)
		rte_eal_service_set_lcore_type(SERVICE);
}


> 
> int ret = rte_eal_service_apply();
> 
> if(ret) {
>    /* application can rte_panic(), or reset CPU cores to default APP state and continue */
> }
> 
> /* application profits from service-cores feature in EAL */
> 
> > 2) What would be the tear down sequence of the service core especially when
> > device stop or close happens?
> 
> Good question. Being able to "turn off" a single service while keeping other services running would be useful I think. That might require some extra tracking at the implementation layer, but perhaps something that's worth doing. Opinions and use-cases welcome here!

I think it makes sense to add "turn off" support for each service
functions as we are multiplexing the service core.


> 
> 
>