[dpdk-dev] [PATCH v6 2/2] eal: add asynchronous request API to DPDK IPC

[Van Haaren, Harry]

> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Wednesday, March 28, 2018 9:55 AM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>; Thomas Monjalon
> <thomas at monjalon.net>
> Cc: Burakov, Anatoly <anatoly.burakov at intel.com>; dev at dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v6 2/2] eal: add asynchronous request API to
> DPDK IPC
> 
> Hi Thomas and Harry,

Hey,


> On 3/28/2018 4:22 PM, Van Haaren, Harry wrote:
> >> From: Thomas Monjalon [mailto:thomas at monjalon.net]
> >> Sent: Wednesday, March 28, 2018 8:30 AM
> >> To: Tan, Jianfeng <jianfeng.tan at intel.com>
> >> Cc: Burakov, Anatoly <anatoly.burakov at intel.com>; dev at dpdk.org; Ananyev,
> >> Konstantin <konstantin.ananyev at intel.com>; Van Haaren, Harry
> >> <harry.van.haaren at intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH v6 2/2] eal: add asynchronous request API
> to
> >> DPDK IPC
> >>
> >> 28/03/2018 04:08, Tan, Jianfeng:
> >>> Hi Thomas ,
> >>>
> >>> From: Thomas Monjalon [mailto:thomas at monjalon.net]
> >>>> 27/03/2018 15:59, Anatoly Burakov:
> >>>>> Under the hood, we create a separate thread to deal with replies to
> >>>>> asynchronous requests, that will just wait to be notified by the
> >>>>> main thread, or woken up on a timer.
> >>>> I really don't like that a library is creating a thread.
> >>>> We don't even know where the thread is created (which core).
> >>>> Can it be a rte_service? or in the interrupt thread?
> >>> Agree that we'd better not adding so many threads in a library.
> >>>
> >>> I was considering to merge all the threads into the interrupt thread,
> >> however, we don't have an interrupt thread in freebsd. Further, we don't
> >> implement alarm API in freebsd. That's why I tend to current
> implementation,
> >> and optimize it later.
> >>
> >> I would prefer we improve the current code now instead of polluting more
> >> with more uncontrolled threads.
> >>
> >>> For rte_service, it may be not a good idea to reply on it as it needs
> >> explicit API calls to setup.
> >>
> >> I don't see the issue of the explicit API.
> >> The IPC is a new service.
> 
> My concern is that not every DPDK application sets up rte_service, but
> IPC will be used for very fundamental functions, like memory allocation.
> We could not possibly ask all DPDK applications to add rte_service now.
> 
> And also take Harry's comments below into consideration, most likely, we
> will move these threads into interrupt thread now by adding

I don't suggest moving everything into interrupt thread.

We need to ensure the interrupt thread (aka, link status interrupt thread) is not
busy for too long. Certain work may cause un-acceptable delay in the interrupt
thread, hence perhaps we should have 2 core DPDK threads:

1) EAL interrupt thread: handles only interrupts, so is ~always blocked on some FDs

2) EAL "core" thread: handles the IPC work here, and any other irregular work that
   can take some time to complete. Adding all core DPDK work to this thread enables
   us to refactor and create a scalable / coherent design.


> > Although I do like to see new services, if we want to enable "core" dpdk
> functionality with Services, we need a proper designed solution for that.
> Service cores is not intended for "occasional" work - there is no method to
> block and sleep on a specific service until work becomes available, so this
> would imply a busy-polling. Using a service (hence busy polling) for
> rte_malloc()-based memory mapping requests is inefficient, and total
> overkill :)
> >
> > For this patch I suggest to use some blocking-read capable mechanism.
> 
> The problem here is that we add too many threads; blocking-read does not
> decrease # of threads.

To correct my statement just above - some method capable of blocking reads (as opposed to
busy polling). In my mind, this method *must* allow waiting on multiple FDs,
as to *require* only 1 thread. We could use more if it makes sense to do so.


> > The above said, in the longer term it would be good to have a design that
> allows new file-descriptors to be added to a "dpdk core" thread, which
> performs occasional lengthy work if the FD has data available.
> 
> Interrupt thread vs rte_service, which is the direction to go? We
> actually have some others threads, in vhost and even virtio-user; we can
> also avoid those threads if we have a clear direction.

I don't think that service is the correct hammer for this problem.

As to interrupt thread or creating a new thread, what makes more sense given
the current codebase? Is the current implementation an acceptable short-term
solution, that gets reworked to be more generic in future?