[dpdk-dev] eventdev: method for finding out unlink status

[dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

Hi,

In bug report https://bugs.dpdk.org/show_bug.cgi?id=60 we have been discussing
issues related to events ending up in wrong ports after calling
rte_event_port_unlink(). In addition of finding few bugs we have identified a
need for a new API call (or documentation extension) for an application to be
able to find out when an unlink() call has finished and no new events are
scheduled anymore to the particular event port. This is required e.g. when doing
clean-up after an application thread stops processing events.

The bug report discussion provides more background on the subject and Harry has
already proposed a new 'int32_t rte_event_unlinks_in_progress()' API as one
possible solution.

-Matias

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 30 Jul 2018 06:39:45 +0000
> From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> To: "dev@dpdk.org" <dev@dpdk.org>
> CC: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> Subject: [dpdk-dev] eventdev: method for finding out unlink status
> x-mailer: Apple Mail (2.3445.9.1)
> 
> 
> Hi,
> 
> In bug report https://bugs.dpdk.org/show_bug.cgi?id=60 we have been discussing
> issues related to events ending up in wrong ports after calling
> rte_event_port_unlink(). In addition of finding few bugs we have identified a
> need for a new API call (or documentation extension) for an application to be

>From HW perspective, documentation extension should be enough. adding 
"there may be pre-scheduled events and the application is responsible to process them"
on unlink(). Since dequeue() has which queue it is dequeue-ed from, the
application can allays make action based on that(i.e, Is the event
post/pre to unlink)

> able to find out when an unlink() call has finished and no new events are
> scheduled anymore to the particular event port. This is required e.g. when doing
> clean-up after an application thread stops processing events.

If thread stopping then it better to call dev_stop(). At least in HW
implementation, A given event port assigned to a new lcore other than
it previous one then we need to do some clean up at port level.

> 
> The bug report discussion provides more background on the subject and Harry has
> already proposed a new 'int32_t rte_event_unlinks_in_progress()' API as one
> possible solution.

Assuming stale event(s) can go the new linked port,
Does rte_event_unlink() takes consider amount of time in SW implementation.


> 
> -Matias
> 

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

>> 
>> In bug report https://bugs.dpdk.org/show_bug.cgi?id=60 we have been discussing
>> issues related to events ending up in wrong ports after calling
>> rte_event_port_unlink(). In addition of finding few bugs we have identified a
>> need for a new API call (or documentation extension) for an application to be
> 
> From HW perspective, documentation extension should be enough. adding 
> "there may be pre-scheduled events and the application is responsible to process them"
> on unlink(). Since dequeue() has which queue it is dequeue-ed from, the
> application can allays make action based on that(i.e, Is the event
> post/pre to unlink)

At least in case of SW eventdev the problem is how the application can know that
it has processed all pre-scheduled events. E.g. dequeue may return nothing but since
the scheduler is running as a separate process events may still end up to the unlinked
port asynchronously.

> 
>> able to find out when an unlink() call has finished and no new events are
>> scheduled anymore to the particular event port. This is required e.g. when doing
>> clean-up after an application thread stops processing events.
> 
> If thread stopping then it better to call dev_stop(). At least in HW
> implementation,

For an application doing dynamic load balancing stopping the whole eventdev is not an
option.

> A given event port assigned to a new lcore other than
> it previous one then we need to do some clean up at port level.

 In my case I'm mapping an event port per thread statically (basically thread_id == port_id),
so this shouldn't be an issue.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 30 Jul 2018 09:17:47 +0000
> From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Van Haaren, Harry"
>  <harry.van.haaren@intel.com>
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> x-mailer: Apple Mail (2.3445.9.1)
> 
> 
> >>
> >> In bug report https://bugs.dpdk.org/show_bug.cgi?id=60 we have been discussing
> >> issues related to events ending up in wrong ports after calling
> >> rte_event_port_unlink(). In addition of finding few bugs we have identified a
> >> need for a new API call (or documentation extension) for an application to be
> >
> > From HW perspective, documentation extension should be enough. adding
> > "there may be pre-scheduled events and the application is responsible to process them"
> > on unlink(). Since dequeue() has which queue it is dequeue-ed from, the
> > application can allays make action based on that(i.e, Is the event
> > post/pre to unlink)
> 
> At least in case of SW eventdev the problem is how the application can know that
> it has processed all pre-scheduled events. E.g. dequeue may return nothing but since
> the scheduler is running as a separate process events may still end up to the unlinked
> port asynchronously.

Can't we do, dequeue() in loop to get all the events from port. If
dequeue returns with zero event then ports are drained up. Right?

> 
> >
> >> able to find out when an unlink() call has finished and no new events are
> >> scheduled anymore to the particular event port. This is required e.g. when doing
> >> clean-up after an application thread stops processing events.
> >
> > If thread stopping then it better to call dev_stop(). At least in HW
> > implementation,
> 
> For an application doing dynamic load balancing stopping the whole eventdev is not an
> option.

OK. Makes sense. Doing unlink() and link() in fastpath is not a
problem. Changing core assignment to event port is problem without stop(). I guess, you
application or general would be OK with that constraint.

> 
> > A given event port assigned to a new lcore other than
> > it previous one then we need to do some clean up at port level.
> 
>  In my case I'm mapping an event port per thread statically (basically thread_id == port_id),
> so this shouldn't be an issue.
> 
> 

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Van Haaren, Harry]

> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, July 30, 2018 10:29 AM
> To: Elo, Matias (Nokia - FI/Espoo) <matias.elo@nokia.com>
> Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> 
> -----Original Message-----
> > Date: Mon, 30 Jul 2018 09:17:47 +0000
> > From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > CC: "dev@dpdk.org" <dev@dpdk.org>, "Van Haaren, Harry"
> >  <harry.van.haaren@intel.com>
> > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> > x-mailer: Apple Mail (2.3445.9.1)
> >
> >
> > >>
> > >> In bug report https://bugs.dpdk.org/show_bug.cgi?id=60 we have been
> discussing
> > >> issues related to events ending up in wrong ports after calling
> > >> rte_event_port_unlink(). In addition of finding few bugs we have
> identified a
> > >> need for a new API call (or documentation extension) for an application
> to be
> > >
> > > From HW perspective, documentation extension should be enough. adding
> > > "there may be pre-scheduled events and the application is responsible to
> process them"
> > > on unlink(). Since dequeue() has which queue it is dequeue-ed from, the
> > > application can allays make action based on that(i.e, Is the event
> > > post/pre to unlink)
> >
> > At least in case of SW eventdev the problem is how the application can know
> that
> > it has processed all pre-scheduled events. E.g. dequeue may return nothing
> but since
> > the scheduler is running as a separate process events may still end up to
> the unlinked
> > port asynchronously.
> 
> Can't we do, dequeue() in loop to get all the events from port. If
> dequeue returns with zero event then ports are drained up. Right?

Nope - because the scheduler might not have performed and "Acked" the
unlink(), and internally it has *just* scheduled an event, but it wasn't
available in the dequeue ring yet.

Aka, its racy behavior - and we need a way to retrieve this "Unlink Ack"
from the scheduler (which runs in another thread in event/sw).


> > >> able to find out when an unlink() call has finished and no new events are
> > >> scheduled anymore to the particular event port. This is required e.g.
> when doing
> > >> clean-up after an application thread stops processing events.
> > >
> > > If thread stopping then it better to call dev_stop(). At least in HW
> > > implementation,
> >
> > For an application doing dynamic load balancing stopping the whole eventdev
> is not an
> > option.
> 
> OK. Makes sense. Doing unlink() and link() in fastpath is not a
> problem.

Correct


> Changing core assignment to event port is problem without stop(). I
> guess, you
> application or general would be OK with that constraint.


I don't think that the eventdev API requires 1:1 Lcore / Port mapping, so really a
PMD should be able to handle any thread calling any port.

The event/sw PMD allows any thread to call dequeue/enqueue any port,
so long as it is not being accessed by another thread.


> > > A given event port assigned to a new lcore other than
> > > it previous one then we need to do some clean up at port level.
> >
> >  In my case I'm mapping an event port per thread statically (basically
> thread_id == port_id),
> > so this shouldn't be an issue.

This is the common case - but I don't think we should demand it.
There is a valid scale-down model which just polls *all* ports using
a single lcore, instead of unlink() of multiple ports.


For this "runtime scale down" use-case the missing information is being
able to identify when an unlink is complete. After that (and ensuring the
port buffer is empty) the application can be guaranteed that there are no
more events going to be sent to that port, and the application can take
the worker lcore out of its polling-loop and put it to sleep.

As mentioned before, I think an "unlinks_in_progress()" function is perhaps
the easiest way to achieve this functionality, as it allows relatively simple
tracking of unlinks() using an atomic counter in sw. (Implementation details
become complex when we have a separate core running event/sw, separate cores
polling, and a control-plane thread calling unlink...) 

I think the end result we're hoping for is something like pseudo code below,
(keep in mind that the event/sw has a service-core thread running it, so no
application code there):

int worker_poll = 1;

worker() {
  while(worker_poll) {
     // eventdev_dequeue_burst() etc
  }
  go_to_sleep(1);
}

control_plane_scale_down() {
  unlink(evdev, worker, queue_id);
  while(unlinks_in_progress(evdev) > 0)
      usleep(100);
  
  /* here we know that the unlink is complete.
   * so we can now stop the worker from polling */
  worker_poll = 0;
}

Hope my pseudo-code makes pseudo-sense :)

-Harry

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

> I don't think that the eventdev API requires 1:1 Lcore / Port mapping, so really a
> PMD should be able to handle any thread calling any port.
> 
> The event/sw PMD allows any thread to call dequeue/enqueue any port,
> so long as it is not being accessed by another thread.
> 
> 
>>>> A given event port assigned to a new lcore other than
>>>> it previous one then we need to do some clean up at port level.
>>> 
>>> In my case I'm mapping an event port per thread statically (basically
>> thread_id == port_id),
>>> so this shouldn't be an issue.
> 
> This is the common case - but I don't think we should demand it.
> There is a valid scale-down model which just polls *all* ports using
> a single lcore, instead of unlink() of multiple ports.

I agree, 1 : 1 Lcore / Port mapping shouldn't be required.


> I think the end result we're hoping for is something like pseudo code below,
> (keep in mind that the event/sw has a service-core thread running it, so no
> application code there):
> 
> int worker_poll = 1;
> 
> worker() {
>  while(worker_poll) {
>     // eventdev_dequeue_burst() etc
>  }
>  go_to_sleep(1);
> }
> 
> control_plane_scale_down() {
>  unlink(evdev, worker, queue_id);
>  while(unlinks_in_progress(evdev) > 0)
>      usleep(100);
> 
>  /* here we know that the unlink is complete.
>   * so we can now stop the worker from polling */
>  worker_poll = 0;
> }
> 
> Hope my pseudo-code makes pseudo-sense :)


Makes sense =)

One use case this API wouldn't support is if an application would
unlink only a subset of linked queues. In this case events could be still
arriving constantly after unlinks_in_progress() returns zero, so there is no way
for the application to know when all events from unlinked queues have been
processed. However, at least in our application this information is not needed.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 30 Jul 2018 09:38:01 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Elo, Matias (Nokia -
>  FI/Espoo)" <matias.elo@nokia.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>
> Subject: RE: [dpdk-dev] eventdev: method for finding out unlink status
> 
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, July 30, 2018 10:29 AM
> > To: Elo, Matias (Nokia - FI/Espoo) <matias.elo@nokia.com>
> > Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> >
> > -----Original Message-----
> > > Date: Mon, 30 Jul 2018 09:17:47 +0000
> > > From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > CC: "dev@dpdk.org" <dev@dpdk.org>, "Van Haaren, Harry"
> > >  <harry.van.haaren@intel.com>
> > > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> > > x-mailer: Apple Mail (2.3445.9.1)
> > >
> > >
> > > >>
> > > >> In bug report https://bugs.dpdk.org/show_bug.cgi?id=60 we have been
> > discussing
> > > >> issues related to events ending up in wrong ports after calling
> > > >> rte_event_port_unlink(). In addition of finding few bugs we have
> > identified a
> > > >> need for a new API call (or documentation extension) for an application
> > to be
> > > >
> > > > From HW perspective, documentation extension should be enough. adding
> > > > "there may be pre-scheduled events and the application is responsible to
> > process them"
> > > > on unlink(). Since dequeue() has which queue it is dequeue-ed from, the
> > > > application can allays make action based on that(i.e, Is the event
> > > > post/pre to unlink)
> > >
> > > At least in case of SW eventdev the problem is how the application can know
> > that
> > > it has processed all pre-scheduled events. E.g. dequeue may return nothing
> > but since
> > > the scheduler is running as a separate process events may still end up to
> > the unlinked
> > > port asynchronously.
> >
> > Can't we do, dequeue() in loop to get all the events from port. If
> > dequeue returns with zero event then ports are drained up. Right?
> 
> Nope - because the scheduler might not have performed and "Acked" the
> unlink(), and internally it has *just* scheduled an event, but it wasn't
> available in the dequeue ring yet.
> 
> Aka, its racy behavior - and we need a way to retrieve this "Unlink Ack"
> from the scheduler (which runs in another thread in event/sw).

OK. Some bits specific to event/sw. We will address it.

> 
> 
> > > >> able to find out when an unlink() call has finished and no new events are
> > > >> scheduled anymore to the particular event port. This is required e.g.
> > when doing
> > > >> clean-up after an application thread stops processing events.
> > > >
> > > > If thread stopping then it better to call dev_stop(). At least in HW
> > > > implementation,
> > >
> > > For an application doing dynamic load balancing stopping the whole eventdev
> > is not an
> > > option.
> >
> > OK. Makes sense. Doing unlink() and link() in fastpath is not a
> > problem.
> 
> Correct
> 
> 
> > Changing core assignment to event port is problem without stop(). I
> > guess, you
> > application or general would be OK with that constraint.
> 
> 
> I don't think that the eventdev API requires 1:1 Lcore / Port mapping, so really a
> PMD should be able to handle any thread calling any port.
> 
> The event/sw PMD allows any thread to call dequeue/enqueue any port,
> so long as it is not being accessed by another thread.

Yes. True. Eventdev API does not required 1:1 Lcore/Port mapping.
Just like event/sw requires some bits to clear "Unlink Ack". At least,
our HW implementation we need some bit clear when we change lcore to port
mapping. Currently we are doing it in stop() call, If there is a real valid use
case to change lcore to port mapping without stop, we would like to
propose and API to flush/clear state on Lcore/port mapping change.
It can be NOP for event/sw.

> 
> 
> > > > A given event port assigned to a new lcore other than
> > > > it previous one then we need to do some clean up at port level.
> > >
> > >  In my case I'm mapping an event port per thread statically (basically
> > thread_id == port_id),
> > > so this shouldn't be an issue.
> 
> This is the common case - but I don't think we should demand it.
> There is a valid scale-down model which just polls *all* ports using
> a single lcore, instead of unlink() of multiple ports.
> 
> 
> For this "runtime scale down" use-case the missing information is being
> able to identify when an unlink is complete. After that (and ensuring the
> port buffer is empty) the application can be guaranteed that there are no
> more events going to be sent to that port, and the application can take
> the worker lcore out of its polling-loop and put it to sleep.
> 
> As mentioned before, I think an "unlinks_in_progress()" function is perhaps
> the easiest way to achieve this functionality, as it allows relatively simple
> tracking of unlinks() using an atomic counter in sw. (Implementation details
> become complex when we have a separate core running event/sw, separate cores
> polling, and a control-plane thread calling unlink...)
> 
> I think the end result we're hoping for is something like pseudo code below,
> (keep in mind that the event/sw has a service-core thread running it, so no
> application code there):
> 
> int worker_poll = 1;
> 
> worker() {
>   while(worker_poll) {
>      // eventdev_dequeue_burst() etc
>   }
>   go_to_sleep(1);
> }
> 
> control_plane_scale_down() {
>   unlink(evdev, worker, queue_id);
>   while(unlinks_in_progress(evdev) > 0)
>       usleep(100);
> 
>   /* here we know that the unlink is complete.
>    * so we can now stop the worker from polling */
>   worker_poll = 0;
> }


Make sense. Instead of rte_event_is_unlink_in_progress(), How about
adding a callback in rte_event_port_unlink() which will be called on 
unlink completion. It will reduce the need for ONE more API.

Anyway it RC2 now, so we can not accept a new feature. So we will have
time for deprecation notice.


> 
> Hope my pseudo-code makes pseudo-sense :)
> 
> -Harry

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

>> For this "runtime scale down" use-case the missing information is being
>> able to identify when an unlink is complete. After that (and ensuring the
>> port buffer is empty) the application can be guaranteed that there are no
>> more events going to be sent to that port, and the application can take
>> the worker lcore out of its polling-loop and put it to sleep.
>> 
>> As mentioned before, I think an "unlinks_in_progress()" function is perhaps
>> the easiest way to achieve this functionality, as it allows relatively simple
>> tracking of unlinks() using an atomic counter in sw. (Implementation details
>> become complex when we have a separate core running event/sw, separate cores
>> polling, and a control-plane thread calling unlink...)
>> 
>> I think the end result we're hoping for is something like pseudo code below,
>> (keep in mind that the event/sw has a service-core thread running it, so no
>> application code there):
>> 
>> int worker_poll = 1;
>> 
>> worker() {
>>  while(worker_poll) {
>>     // eventdev_dequeue_burst() etc
>>  }
>>  go_to_sleep(1);
>> }
>> 
>> control_plane_scale_down() {
>>  unlink(evdev, worker, queue_id);
>>  while(unlinks_in_progress(evdev) > 0)
>>      usleep(100);
>> 
>>  /* here we know that the unlink is complete.
>>   * so we can now stop the worker from polling */
>>  worker_poll = 0;
>> }
> 
> 
> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
> adding a callback in rte_event_port_unlink() which will be called on 
> unlink completion. It will reduce the need for ONE more API.
> 
> Anyway it RC2 now, so we can not accept a new feature. So we will have
> time for deprecation notice.
> 

Both solutions should work but I would perhaps favor Harry's approach as it
requires less code in the application side and doesn't break backward
compatibility.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 30 Jul 2018 13:36:35 +0000
> From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "Van Haaren, Harry" <harry.van.haaren@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> x-mailer: Apple Mail (2.3445.9.1)
> 
> 
> >> For this "runtime scale down" use-case the missing information is being
> >> able to identify when an unlink is complete. After that (and ensuring the
> >> port buffer is empty) the application can be guaranteed that there are no
> >> more events going to be sent to that port, and the application can take
> >> the worker lcore out of its polling-loop and put it to sleep.
> >>
> >> As mentioned before, I think an "unlinks_in_progress()" function is perhaps
> >> the easiest way to achieve this functionality, as it allows relatively simple
> >> tracking of unlinks() using an atomic counter in sw. (Implementation details
> >> become complex when we have a separate core running event/sw, separate cores
> >> polling, and a control-plane thread calling unlink...)
> >>
> >> I think the end result we're hoping for is something like pseudo code below,
> >> (keep in mind that the event/sw has a service-core thread running it, so no
> >> application code there):
> >>
> >> int worker_poll = 1;
> >>
> >> worker() {
> >>  while(worker_poll) {
> >>     // eventdev_dequeue_burst() etc
> >>  }
> >>  go_to_sleep(1);
> >> }
> >>
> >> control_plane_scale_down() {
> >>  unlink(evdev, worker, queue_id);
> >>  while(unlinks_in_progress(evdev) > 0)
> >>      usleep(100);
> >>
> >>  /* here we know that the unlink is complete.
> >>   * so we can now stop the worker from polling */
> >>  worker_poll = 0;
> >> }
> >
> >
> > Make sense. Instead of rte_event_is_unlink_in_progress(), How about
> > adding a callback in rte_event_port_unlink() which will be called on
> > unlink completion. It will reduce the need for ONE more API.
> >
> > Anyway it RC2 now, so we can not accept a new feature. So we will have
> > time for deprecation notice.
> >
> 
> Both solutions should work but I would perhaps favor Harry's approach as it
> requires less code in the application side and doesn't break backward
> compatibility.

OK.

Does rte_event_port_unlink() returning -EBUSY will help?

while (rte_event_port_unlink() != nr_links)
	usleep(100);

I am trying to think, how can address this requirements without creating new API and/or less impact to other
drivers which don't have this requirements?

Are we calling this API in fastpath? or it is control thread as
mentioned in harry's pseudo code.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

>>>> 
>>>> I think the end result we're hoping for is something like pseudo code below,
>>>> (keep in mind that the event/sw has a service-core thread running it, so no
>>>> application code there):
>>>> 
>>>> int worker_poll = 1;
>>>> 
>>>> worker() {
>>>> while(worker_poll) {
>>>>    // eventdev_dequeue_burst() etc
>>>> }
>>>> go_to_sleep(1);
>>>> }
>>>> 
>>>> control_plane_scale_down() {
>>>> unlink(evdev, worker, queue_id);
>>>> while(unlinks_in_progress(evdev) > 0)
>>>>     usleep(100);
>>>> 
>>>> /* here we know that the unlink is complete.
>>>>  * so we can now stop the worker from polling */
>>>> worker_poll = 0;
>>>> }
>>> 
>>> 
>>> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
>>> adding a callback in rte_event_port_unlink() which will be called on
>>> unlink completion. It will reduce the need for ONE more API.
>>> 
>>> Anyway it RC2 now, so we can not accept a new feature. So we will have
>>> time for deprecation notice.
>>> 
>> 
>> Both solutions should work but I would perhaps favor Harry's approach as it
>> requires less code in the application side and doesn't break backward
>> compatibility.
> 
> OK.
> 
> Does rte_event_port_unlink() returning -EBUSY will help?

It could perhaps work. The return value becomes a bit ambiguous though. E.g. how
to differentiate a delayed unlink completion from a scenario where the port & queues
have never been linked?

The implementation may also be more complex compared to a separate function but
Harry is a better person to answer this.

> 
> while (rte_event_port_unlink() != nr_links)
> 	usleep(100);
> 
> I am trying to think, how can address this requirements without creating new API and/or less impact to other
> drivers which don't have this requirements?

Wouldn't this function then just be NOP for the other drivers?

> 
> Are we calling this API in fastpath? or it is control thread as
> mentioned in harry's pseudo code.

In our use case it could be called also directly from the fast path by the worker thread.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Tue, 31 Jul 2018 08:09:05 +0000
> From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "Van Haaren, Harry" <harry.van.haaren@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> x-mailer: Apple Mail (2.3445.9.1)
> 
> 
> >>>>
> >>>> I think the end result we're hoping for is something like pseudo code below,
> >>>> (keep in mind that the event/sw has a service-core thread running it, so no
> >>>> application code there):
> >>>>
> >>>> int worker_poll = 1;
> >>>>
> >>>> worker() {
> >>>> while(worker_poll) {
> >>>>    // eventdev_dequeue_burst() etc
> >>>> }
> >>>> go_to_sleep(1);
> >>>> }
> >>>>
> >>>> control_plane_scale_down() {
> >>>> unlink(evdev, worker, queue_id);
> >>>> while(unlinks_in_progress(evdev) > 0)
> >>>>     usleep(100);
> >>>>
> >>>> /* here we know that the unlink is complete.
> >>>>  * so we can now stop the worker from polling */
> >>>> worker_poll = 0;
> >>>> }
> >>>
> >>>
> >>> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
> >>> adding a callback in rte_event_port_unlink() which will be called on
> >>> unlink completion. It will reduce the need for ONE more API.
> >>>
> >>> Anyway it RC2 now, so we can not accept a new feature. So we will have
> >>> time for deprecation notice.
> >>>
> >>
> >> Both solutions should work but I would perhaps favor Harry's approach as it
> >> requires less code in the application side and doesn't break backward
> >> compatibility.
> >
> > OK.
> >
> > Does rte_event_port_unlink() returning -EBUSY will help?
> 
> It could perhaps work. The return value becomes a bit ambiguous though. E.g. how
> to differentiate a delayed unlink completion from a scenario where the port & queues
> have never been linked?

Based on return code?

> 
> The implementation may also be more complex compared to a separate function but
> Harry is a better person to answer this.
> 
> >
> > while (rte_event_port_unlink() != nr_links)
> >       usleep(100);
> >
> > I am trying to think, how can address this requirements without creating new API and/or less impact to other
> > drivers which don't have this requirements?
> 
> Wouldn't this function then just be NOP for the other drivers?

If is used in fastpath, it will have function pointer overhead for other
drivers and southbound(driver) interface needs to add for holding one more function
pointer.

> 
> >
> > Are we calling this API in fastpath? or it is control thread as
> > mentioned in harry's pseudo code.
> 
> In our use case it could be called also directly from the fast path by the worker thread.

If it is in fastpath, I prefer to have single API(if possible) and avoid
creating new API for driver specific usecases(again if possible to do so)

> 

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

>>>>>> 
>>>>>> I think the end result we're hoping for is something like pseudo code below,
>>>>>> (keep in mind that the event/sw has a service-core thread running it, so no
>>>>>> application code there):
>>>>>> 
>>>>>> int worker_poll = 1;
>>>>>> 
>>>>>> worker() {
>>>>>> while(worker_poll) {
>>>>>>   // eventdev_dequeue_burst() etc
>>>>>> }
>>>>>> go_to_sleep(1);
>>>>>> }
>>>>>> 
>>>>>> control_plane_scale_down() {
>>>>>> unlink(evdev, worker, queue_id);
>>>>>> while(unlinks_in_progress(evdev) > 0)
>>>>>>    usleep(100);
>>>>>> 
>>>>>> /* here we know that the unlink is complete.
>>>>>> * so we can now stop the worker from polling */
>>>>>> worker_poll = 0;
>>>>>> }
>>>>> 
>>>>> 
>>>>> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
>>>>> adding a callback in rte_event_port_unlink() which will be called on
>>>>> unlink completion. It will reduce the need for ONE more API.
>>>>> 
>>>>> Anyway it RC2 now, so we can not accept a new feature. So we will have
>>>>> time for deprecation notice.
>>>>> 
>>>> 
>>>> Both solutions should work but I would perhaps favor Harry's approach as it
>>>> requires less code in the application side and doesn't break backward
>>>> compatibility.
>>> 
>>> OK.
>>> 
>>> Does rte_event_port_unlink() returning -EBUSY will help?
>> 
>> It could perhaps work. The return value becomes a bit ambiguous though. E.g. how
>> to differentiate a delayed unlink completion from a scenario where the port & queues
>> have never been linked?
> 
> Based on return code?

Yes, that works. I was thinking about the complexity of the implementation as it would
have to also track the pending unlink requests. But anyway, Harry is better answering
these questions since I guess he would be implementing this.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

>>>>>>> 
>>>>>>> I think the end result we're hoping for is something like pseudo code below,
>>>>>>> (keep in mind that the event/sw has a service-core thread running it, so no
>>>>>>> application code there):
>>>>>>> 
>>>>>>> int worker_poll = 1;
>>>>>>> 
>>>>>>> worker() {
>>>>>>> while(worker_poll) {
>>>>>>>  // eventdev_dequeue_burst() etc
>>>>>>> }
>>>>>>> go_to_sleep(1);
>>>>>>> }
>>>>>>> 
>>>>>>> control_plane_scale_down() {
>>>>>>> unlink(evdev, worker, queue_id);
>>>>>>> while(unlinks_in_progress(evdev) > 0)
>>>>>>>   usleep(100);
>>>>>>> 
>>>>>>> /* here we know that the unlink is complete.
>>>>>>> * so we can now stop the worker from polling */
>>>>>>> worker_poll = 0;
>>>>>>> }
>>>>>> 
>>>>>> 
>>>>>> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
>>>>>> adding a callback in rte_event_port_unlink() which will be called on
>>>>>> unlink completion. It will reduce the need for ONE more API.
>>>>>> 
>>>>>> Anyway it RC2 now, so we can not accept a new feature. So we will have
>>>>>> time for deprecation notice.
>>>>>> 
>>>>> 
>>>>> Both solutions should work but I would perhaps favor Harry's approach as it
>>>>> requires less code in the application side and doesn't break backward
>>>>> compatibility.
>>>> 
>>>> OK.
>>>> 
>>>> Does rte_event_port_unlink() returning -EBUSY will help?
>>> 
>>> It could perhaps work. The return value becomes a bit ambiguous though. E.g. how
>>> to differentiate a delayed unlink completion from a scenario where the port & queues
>>> have never been linked?
>> 
>> Based on return code?
> 
> Yes, that works. I was thinking about the complexity of the implementation as it would
> have to also track the pending unlink requests. But anyway, Harry is better answering
> these questions since I guess he would be implementing this.


Hi Harry,

Have you had time to think about this?

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Van Haaren, Harry]

> From: Elo, Matias (Nokia - FI/Espoo) [mailto:matias.elo@nokia.com]
> Sent: Wednesday, August 8, 2018 11:05 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> 
> 
> >>>>>>>
> >>>>>>> I think the end result we're hoping for is something like pseudo
> code below,
> >>>>>>> (keep in mind that the event/sw has a service-core thread running
> it, so no
> >>>>>>> application code there):
> >>>>>>>
> >>>>>>> int worker_poll = 1;
> >>>>>>>
> >>>>>>> worker() {
> >>>>>>> while(worker_poll) {
> >>>>>>>  // eventdev_dequeue_burst() etc
> >>>>>>> }
> >>>>>>> go_to_sleep(1);
> >>>>>>> }
> >>>>>>>
> >>>>>>> control_plane_scale_down() {
> >>>>>>> unlink(evdev, worker, queue_id);
> >>>>>>> while(unlinks_in_progress(evdev) > 0)
> >>>>>>>   usleep(100);
> >>>>>>>
> >>>>>>> /* here we know that the unlink is complete.
> >>>>>>> * so we can now stop the worker from polling */
> >>>>>>> worker_poll = 0;
> >>>>>>> }
> >>>>>>
> >>>>>>
> >>>>>> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
> >>>>>> adding a callback in rte_event_port_unlink() which will be called on
> >>>>>> unlink completion. It will reduce the need for ONE more API.
> >>>>>>
> >>>>>> Anyway it RC2 now, so we can not accept a new feature. So we will
> have
> >>>>>> time for deprecation notice.
> >>>>>>
> >>>>>
> >>>>> Both solutions should work but I would perhaps favor Harry's approach
> as it
> >>>>> requires less code in the application side and doesn't break backward
> >>>>> compatibility.
> >>>>
> >>>> OK.
> >>>>
> >>>> Does rte_event_port_unlink() returning -EBUSY will help?
> >>>
> >>> It could perhaps work. The return value becomes a bit ambiguous though.
> E.g. how
> >>> to differentiate a delayed unlink completion from a scenario where the
> port & queues
> >>> have never been linked?
> >>
> >> Based on return code?
> >
> > Yes, that works. I was thinking about the complexity of the implementation
> as it would
> > have to also track the pending unlink requests. But anyway, Harry is
> better answering
> > these questions since I guess he would be implementing this.
> 
> 
> Hi Harry,
> 
> Have you had time to think about this?


Hey, Yes I'm just collecting my thoughts at the moment, I see a few small quirks;

1) I see the "return -EBUSY from port_unlink()" solution as overloading the rte_event_port_unlink() API.
We lose some self-documenting semantics of the code, see the following snippet @ 1) marker.

2) If some unlinks fail, and others are in progress, we cannot describe that in a single return.
See 2) marker in code below.


int ret = rte_event_port_unlink(dev, port, queues[], nb_queues);
while (ret == -EBUSY) {
   // 1) what args to pass here? It looks like we want to unlink again? 
   // 2) some unlinks fail, and others are -EBUSY: There is no appropriate ret code in that case
   ret = rte_event_port_unlink(...);
}


Contrast that to the following, which I feel is simpler and more descriptive:

int ret = rte_event_port_unlink(dev, port, queues[], nb_queues);

while (rte_event_port_unlink_in_progress(dev, port) > 0)
   rte_delay();


Here the port_unlink() call can sanity-check the unlinks, and return -EINVAL if invalid requests,
and we can detect other unlinks in progress too using the explicit API.

Regarding adding an API / function-pointer, is there actually a measurable cost there?
Are we willing to sacrifice code-readability and self-documentation?

-Harry

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Thu, 9 Aug 2018 13:14:40 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, Jerin Jacob
>  <jerin.jacob@caviumnetworks.com>
> Subject: RE: [dpdk-dev] eventdev: method for finding out unlink status
> 
> > From: Elo, Matias (Nokia - FI/Espoo) [mailto:matias.elo@nokia.com]
> > Sent: Wednesday, August 8, 2018 11:05 AM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: dev@dpdk.org; Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> >
> >
> > >>>>>>>
> > >>>>>>> I think the end result we're hoping for is something like pseudo
> > code below,
> > >>>>>>> (keep in mind that the event/sw has a service-core thread running
> > it, so no
> > >>>>>>> application code there):
> > >>>>>>>
> > >>>>>>> int worker_poll = 1;
> > >>>>>>>
> > >>>>>>> worker() {
> > >>>>>>> while(worker_poll) {
> > >>>>>>>  // eventdev_dequeue_burst() etc
> > >>>>>>> }
> > >>>>>>> go_to_sleep(1);
> > >>>>>>> }
> > >>>>>>>
> > >>>>>>> control_plane_scale_down() {
> > >>>>>>> unlink(evdev, worker, queue_id);
> > >>>>>>> while(unlinks_in_progress(evdev) > 0)
> > >>>>>>>   usleep(100);
> > >>>>>>>
> > >>>>>>> /* here we know that the unlink is complete.
> > >>>>>>> * so we can now stop the worker from polling */
> > >>>>>>> worker_poll = 0;
> > >>>>>>> }
> > >>>>>>
> > >>>>>>
> > >>>>>> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
> > >>>>>> adding a callback in rte_event_port_unlink() which will be called on
> > >>>>>> unlink completion. It will reduce the need for ONE more API.
> > >>>>>>
> > >>>>>> Anyway it RC2 now, so we can not accept a new feature. So we will
> > have
> > >>>>>> time for deprecation notice.
> > >>>>>>
> > >>>>>
> > >>>>> Both solutions should work but I would perhaps favor Harry's approach
> > as it
> > >>>>> requires less code in the application side and doesn't break backward
> > >>>>> compatibility.
> > >>>>
> > >>>> OK.
> > >>>>
> > >>>> Does rte_event_port_unlink() returning -EBUSY will help?
> > >>>
> > >>> It could perhaps work. The return value becomes a bit ambiguous though.
> > E.g. how
> > >>> to differentiate a delayed unlink completion from a scenario where the
> > port & queues
> > >>> have never been linked?
> > >>
> > >> Based on return code?
> > >
> > > Yes, that works. I was thinking about the complexity of the implementation
> > as it would
> > > have to also track the pending unlink requests. But anyway, Harry is
> > better answering
> > > these questions since I guess he would be implementing this.
> >
> >
> > Hi Harry,
> >
> > Have you had time to think about this?
> 
> 
> Hey, Yes I'm just collecting my thoughts at the moment, I see a few small quirks;
> 
> 1) I see the "return -EBUSY from port_unlink()" solution as overloading the rte_event_port_unlink() API.
> We lose some self-documenting semantics of the code, see the following snippet @ 1) marker.
> 
> 2) If some unlinks fail, and others are in progress, we cannot describe that in a single return.
> See 2) marker in code below.
> 
> 
> int ret = rte_event_port_unlink(dev, port, queues[], nb_queues);
> while (ret == -EBUSY) {
>    // 1) what args to pass here? It looks like we want to unlink again?

The same arguments.

>    // 2) some unlinks fail, and others are -EBUSY: There is no appropriate ret code in that case
>    ret = rte_event_port_unlink(...);

It is going to be boolean right? like
rte_event_port_unlink_in_progress(), So do we need additional return code to express partially completed?


> }

I was thinking like this,
while (rte_event_port_unlink() == -EBUSY)
{
	rte_delay();
}

> 
> 
> Contrast that to the following, which I feel is simpler and more descriptive:
> 
> int ret = rte_event_port_unlink(dev, port, queues[], nb_queues);
> 
> while (rte_event_port_unlink_in_progress(dev, port) > 0)
>    rte_delay();
> 
> 
> Here the port_unlink() call can sanity-check the unlinks, and return -EINVAL if invalid requests,
> and we can detect other unlinks in progress too using the explicit API.
> 
> Regarding adding an API / function-pointer, is there actually a measurable cost there?
> Are we willing to sacrifice code-readability and self-documentation?

I am fine either approach, at minimum, you can still return -EBUSY so
that loop look like this,

int ret = rte_event_port_unlink(dev, port, queues[], nb_queues);
while (ret == -EBUSY && rte_event_port_unlink_in_progress(dev, port) > 0)
	rte_delay();

So that, rte_event_port_unlink_in_progress() wont be called for other
drivers for normal cases.

# Other than that, I am still not able to understand, why not
application wait until rte_event_port_unlink() returns.

# What in real word use case, application can, do other than waiting
to complete rte_event_port_unlink(). If we try to put some logic in like,

while (rte_event_port_unlink_in_progress(dev, port) > 0){
	do_something();
}

The do_something() will not be called in some platform at all.

# Any idea on what will be the real world use case, where rte_event_port_unlink() called in fastpath?









> 
> -Harry

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

> 
> # Other than that, I am still not able to understand, why not
> application wait until rte_event_port_unlink() returns.

Making rte_event_port_unlink() blocking would be troublesome if one doesn’t care
about unlink completion. E.g. doing dynamic load balancing.

> 
> # What in real word use case, application can, do other than waiting
> to complete rte_event_port_unlink(). If we try to put some logic in like,
> 
> while (rte_event_port_unlink_in_progress(dev, port) > 0){
> 	do_something();
> }
> 
> The do_something() will not be called in some platform at all.
> 
> # Any idea on what will be the real world use case, where rte_event_port_unlink() called in fastpath?

In our application this could be used for example to pause scheduling of new events while
working on an “expensive” event to minimise delays. It is also needed when destroying
queues, though calling this fast path is debatable (our application enables creating /
destroying queues at runtime).

These are perhaps not the best examples but I would be very cautious to make a function
blocking if there is even a small probability that it could be called from the fast path.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Fri, 10 Aug 2018 14:24:02 +0000
> From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "Van Haaren, Harry" <harry.van.haaren@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> x-mailer: Apple Mail (2.3445.9.1)
> 
> 
> >
> > # Other than that, I am still not able to understand, why not
> > application wait until rte_event_port_unlink() returns.
> 
> Making rte_event_port_unlink() blocking would be troublesome if one doesn’t care
> about unlink completion. E.g. doing dynamic load balancing.

By making it as blocking(i.e the rte_event_port_unlink() returns when
unlink() completed) forcing everyone to care about unlink completion.
Right?

> 
> >
> > # What in real word use case, application can, do other than waiting
> > to complete rte_event_port_unlink(). If we try to put some logic in like,
> >
> > while (rte_event_port_unlink_in_progress(dev, port) > 0){
> >       do_something();
> > }
> >
> > The do_something() will not be called in some platform at all.
> >
> > # Any idea on what will be the real world use case, where rte_event_port_unlink() called in fastpath?
> 
> In our application this could be used for example to pause scheduling of new events while
> working on an “expensive” event to minimise delays. It is also needed when destroying
> queues, though calling this fast path is debatable (our application enables creating /
> destroying queues at runtime).

If I understand it correctly, Your current issue is, SW driver is
not waiting for to complete the unlink() operation so that in your application you are
seeing some abnormalities.

> 
> These are perhaps not the best examples but I would be very cautious to make a function
> blocking if there is even a small probability that it could be called from the fast path.

Let assume even if it is called in fastpath, what else, we can really do
other that calling rte_pause() in loop. realistically? after issuing
unlink() operation.

> 

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Van Haaren, Harry]

> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Friday, August 10, 2018 3:52 PM
> To: Elo, Matias (Nokia - FI/Espoo) <matias.elo@nokia.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> 
> -----Original Message-----
> > Date: Fri, 10 Aug 2018 14:24:02 +0000
> > From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > CC: "Van Haaren, Harry" <harry.van.haaren@intel.com>, "dev@dpdk.org"
> >  <dev@dpdk.org>
> > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> > x-mailer: Apple Mail (2.3445.9.1)
> >
> >
> > >
> > > # Other than that, I am still not able to understand, why not
> > > application wait until rte_event_port_unlink() returns.
> >
> > Making rte_event_port_unlink() blocking would be troublesome if one
> doesn’t care
> > about unlink completion. E.g. doing dynamic load balancing.
> 
> By making it as blocking(i.e the rte_event_port_unlink() returns when
> unlink() completed) forcing everyone to care about unlink completion.
> Right?

I'm not sure I understand the issue here.
Is anybody suggesting to make unlink() blocking?

For certain PMDs, perhaps it must be a synchronous handled unlink().
For other PMDs (eg event/sw) there are multiple threads involved,
so it must be async. Hence, APIs should be async to avoid blocking the caller.

With an async API, if you don't want the async behaviuor, it is
easy to build the sync version: call it in a loop, optionally with a delay().


> > > # What in real word use case, application can, do other than waiting
> > > to complete rte_event_port_unlink(). If we try to put some logic in
> like,
> > >
> > > while (rte_event_port_unlink_in_progress(dev, port) > 0){
> > >       do_something();
> > > }
> > >
> > > The do_something() will not be called in some platform at all.
> > >
> > > # Any idea on what will be the real world use case, where
> rte_event_port_unlink() called in fastpath?
> >
> > In our application this could be used for example to pause scheduling of
> new events while
> > working on an “expensive” event to minimise delays. It is also needed when
> destroying
> > queues, though calling this fast path is debatable (our application
> enables creating /
> > destroying queues at runtime).
> 
> If I understand it correctly, Your current issue is, SW driver is
> not waiting for to complete the unlink() operation so that in your
> application you are seeing some abnormalities.

To be more specific:

The issue is that the application cannot reliably know when the unlink()
has completed. As such, the application doesn't know when it can put
a core to sleep instead of busy polling.

Waiting for unlink() to return does not give this info - the
scheduler core may schedule more events until it "acks" the unlink.

Checking that dequeue() == 0 events isn't reliable either, the buffering
in the PMD could hide events.

It seems logical to add an API that allows the user to query the status
of the PMDs unlink() progress. This provides the application with the
information that it needs to reliably determine when a worker is no
longer being scheduled events of a specific type, at which point it
can put its worker to sleep.


> > These are perhaps not the best examples but I would be very cautious to
> make a function
> > blocking if there is even a small probability that it could be called from
> the fast path.
> 
> Let assume even if it is called in fastpath, what else, we can really do
> other that calling rte_pause() in loop. realistically? after issuing
> unlink() operation.

A CPU core could be polling two queues:

for_each(queue) {
  if (unlinks_in_progress() != 0)
    continue;
  // poll queue and handle packets
}

or we could have the unlinks in progress flush buffers:

if (unlinks_in_progress() != 0) {
  tx_flush_packets();
}

I don't think that making unlinks_in_progress() blocking is acceptable,
it feels like too strong a limitation.

If a PMD doesn't support unlinking, we can have the eventdev.c layer
return an error code (or zero..?) to avoid burden on PMDs that don't care.


I've detailed why I think overloading the return value of unlink()
(eg to -EBUSY) isn't a good solution here:
http://mails.dpdk.org/archives/dev/2018-August/109550.html

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Jerin Jacob]

-----Original Message-----
> Date: Fri, 10 Aug 2018 16:55:31 +0000
> From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Elo, Matias (Nokia -
>  FI/Espoo)" <matias.elo@nokia.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>
> Subject: RE: [dpdk-dev] eventdev: method for finding out unlink status
> 
> 
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Friday, August 10, 2018 3:52 PM
> > To: Elo, Matias (Nokia - FI/Espoo) <matias.elo@nokia.com>
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> >
> > -----Original Message-----
> > > Date: Fri, 10 Aug 2018 14:24:02 +0000
> > > From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > CC: "Van Haaren, Harry" <harry.van.haaren@intel.com>, "dev@dpdk.org"
> > >  <dev@dpdk.org>
> > > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> > > x-mailer: Apple Mail (2.3445.9.1)
> > >
> > >
> > > >
> > > > # Other than that, I am still not able to understand, why not
> > > > application wait until rte_event_port_unlink() returns.
> > >
> > > Making rte_event_port_unlink() blocking would be troublesome if one
> > doesn’t care
> > > about unlink completion. E.g. doing dynamic load balancing.
> >
> > By making it as blocking(i.e the rte_event_port_unlink() returns when
> > unlink() completed) forcing everyone to care about unlink completion.
> > Right?
> 
> I'm not sure I understand the issue here.
> Is anybody suggesting to make unlink() blocking?
> 
> For certain PMDs, perhaps it must be a synchronous handled unlink().
> For other PMDs (eg event/sw) there are multiple threads involved,
> so it must be async. Hence, APIs should be async to avoid blocking the caller.
> 
> With an async API, if you don't want the async behaviuor, it is
> easy to build the sync version: call it in a loop, optionally with a delay().

Correct. My point was, rte_event_port_unlink() can be blocking as it 
is a slow path API(does not really matter how long it waits).
If you think, it can be called in fastpath and/or application can
leverage some cpu cycles on completing the async call then you can add
at the cost of new API unlinks_in_progress() and make sure to update the documentation
about unlink() that it can be async call(currently it is documented as a sync
call).

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Elo, Matias (Nokia - FI/Espoo)]

>> 
>> I'm not sure I understand the issue here.
>> Is anybody suggesting to make unlink() blocking?
>> 
>> For certain PMDs, perhaps it must be a synchronous handled unlink().
>> For other PMDs (eg event/sw) there are multiple threads involved,
>> so it must be async. Hence, APIs should be async to avoid blocking the caller.
>> 
>> With an async API, if you don't want the async behaviuor, it is
>> easy to build the sync version: call it in a loop, optionally with a delay().
> 
> Correct. My point was, rte_event_port_unlink() can be blocking as it 
> is a slow path API(does not really matter how long it waits).
> If you think, it can be called in fastpath and/or application can
> leverage some cpu cycles on completing the async call then you can add
> at the cost of new API unlinks_in_progress() and make sure to update the documentation
> about unlink() that it can be async call(currently it is documented as a sync
> call).

Did you come to an agreement how to solve this issue? Any solution (e.g. make
rte_event_port_unlink() blocking with SW eventdev) would be welcomed since this
issue is currently blocking my work with eventdev.

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Van Haaren, Harry]

> From: Elo, Matias (Nokia - FI/Espoo) [mailto:matias.elo@nokia.com]
> Sent: Wednesday, September 5, 2018 8:49 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> 
> 
> >>
> >> I'm not sure I understand the issue here.
> >> Is anybody suggesting to make unlink() blocking?
> >>
> >> For certain PMDs, perhaps it must be a synchronous handled unlink().
> >> For other PMDs (eg event/sw) there are multiple threads involved,
> >> so it must be async. Hence, APIs should be async to avoid blocking the
> caller.
> >>
> >> With an async API, if you don't want the async behaviuor, it is
> >> easy to build the sync version: call it in a loop, optionally with a
> delay().
> >
> > Correct. My point was, rte_event_port_unlink() can be blocking as it
> > is a slow path API(does not really matter how long it waits).
> > If you think, it can be called in fastpath and/or application can
> > leverage some cpu cycles on completing the async call then you can add
> > at the cost of new API unlinks_in_progress() and make sure to update the
> documentation
> > about unlink() that it can be async call(currently it is documented as a
> sync
> > call).
> 
> Did you come to an agreement how to solve this issue? Any solution (e.g.
> make
> rte_event_port_unlink() blocking with SW eventdev) would be welcomed since
> this
> issue is currently blocking my work with eventdev.


I'll work on a patch and document async behavior as Jerin suggested.

I think we may still need to drain the port after unlinks_in_progress() returns
zero, as there may still be events in the port buffers until all unlinks are done.

That would make the final application code-path as follows:


/* control path core */
unlink();
while(unlinks_in_progress() > 0) {
    delay();
}

/* we know there are no more events being scheduled to the port.
   Could flag to data-path core here that unlinks are acked */

while(dequeue_burst(events, ... ) > 0) {
    free_events();
}

/* now we know there are no events in the port itself either.
   Application can perform whatever it wants with the data-path core */

The final dequeueing could also be handled in the data-path core,
and only break out of its loop when the unlink-ack and zero packets returned from dequeue.


I'll send a v1 patchset for your reviews - I think that might make things clearer.

Thanks, -Harry

Re: [dpdk-dev] eventdev: method for finding out unlink status

[Liang, Ma]

On 30 Jul 16:06, Jerin Jacob wrote:
> -----Original Message-----
> > Date: Mon, 30 Jul 2018 09:38:01 +0000
> > From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, "Elo, Matias (Nokia -
> >  FI/Espoo)" <matias.elo@nokia.com>
> > CC: "dev@dpdk.org" <dev@dpdk.org>
> > Subject: RE: [dpdk-dev] eventdev: method for finding out unlink status
> > 
> > 
> > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > > Sent: Monday, July 30, 2018 10:29 AM
> > > To: Elo, Matias (Nokia - FI/Espoo) <matias.elo@nokia.com>
> > > Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>
> > > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> > >
> > > -----Original Message-----
> > > > Date: Mon, 30 Jul 2018 09:17:47 +0000
> > > > From: "Elo, Matias (Nokia - FI/Espoo)" <matias.elo@nokia.com>
> > > > To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > > CC: "dev@dpdk.org" <dev@dpdk.org>, "Van Haaren, Harry"
> > > >  <harry.van.haaren@intel.com>
> > > > Subject: Re: [dpdk-dev] eventdev: method for finding out unlink status
> > > > x-mailer: Apple Mail (2.3445.9.1)
> > > >
> > > >
> > > > >>
> > > > >> In bug report https://bugs.dpdk.org/show_bug.cgi?id=60 we have been
> > > discussing
> > > > >> issues related to events ending up in wrong ports after calling
> > > > >> rte_event_port_unlink(). In addition of finding few bugs we have
> > > identified a
> > > > >> need for a new API call (or documentation extension) for an application
> > > to be
> > > > >
> > > > > From HW perspective, documentation extension should be enough. adding
> > > > > "there may be pre-scheduled events and the application is responsible to
> > > process them"
> > > > > on unlink(). Since dequeue() has which queue it is dequeue-ed from, the
> > > > > application can allays make action based on that(i.e, Is the event
> > > > > post/pre to unlink)
> > > >
> > > > At least in case of SW eventdev the problem is how the application can know
> > > that
> > > > it has processed all pre-scheduled events. E.g. dequeue may return nothing
> > > but since
> > > > the scheduler is running as a separate process events may still end up to
> > > the unlinked
> > > > port asynchronously.
> > >
> > > Can't we do, dequeue() in loop to get all the events from port. If
> > > dequeue returns with zero event then ports are drained up. Right?
> > 
> > Nope - because the scheduler might not have performed and "Acked" the
> > unlink(), and internally it has *just* scheduled an event, but it wasn't
> > available in the dequeue ring yet.
> > 
> > Aka, its racy behavior - and we need a way to retrieve this "Unlink Ack"
> > from the scheduler (which runs in another thread in event/sw).
> 
> OK. Some bits specific to event/sw. We will address it.
BTW: OPDL is not support unlink in runtime. so if we need suggest user do a query to the CAP bits first. 
> 
> > 
> > 
> > > > >> able to find out when an unlink() call has finished and no new events are
> > > > >> scheduled anymore to the particular event port. This is required e.g.
> > > when doing
> > > > >> clean-up after an application thread stops processing events.
> > > > >
> > > > > If thread stopping then it better to call dev_stop(). At least in HW
> > > > > implementation,
> > > >
> > > > For an application doing dynamic load balancing stopping the whole eventdev
> > > is not an
> > > > option.
> > >
> > > OK. Makes sense. Doing unlink() and link() in fastpath is not a
> > > problem.
> > 
> > Correct
> > 
> > 
> > > Changing core assignment to event port is problem without stop(). I
> > > guess, you
> > > application or general would be OK with that constraint.
> > 
> > 
> > I don't think that the eventdev API requires 1:1 Lcore / Port mapping, so really a
> > PMD should be able to handle any thread calling any port.
> > 
> > The event/sw PMD allows any thread to call dequeue/enqueue any port,
> > so long as it is not being accessed by another thread.
> 
> Yes. True. Eventdev API does not required 1:1 Lcore/Port mapping.
> Just like event/sw requires some bits to clear "Unlink Ack". At least,
> our HW implementation we need some bit clear when we change lcore to port
> mapping. Currently we are doing it in stop() call, If there is a real valid use
> case to change lcore to port mapping without stop, we would like to
> propose and API to flush/clear state on Lcore/port mapping change.
> It can be NOP for event/sw.
> 
> > 
> > 
> > > > > A given event port assigned to a new lcore other than
> > > > > it previous one then we need to do some clean up at port level.
> > > >
> > > >  In my case I'm mapping an event port per thread statically (basically
> > > thread_id == port_id),
> > > > so this shouldn't be an issue.
> > 
> > This is the common case - but I don't think we should demand it.
> > There is a valid scale-down model which just polls *all* ports using
> > a single lcore, instead of unlink() of multiple ports.
> > 
> > 
> > For this "runtime scale down" use-case the missing information is being
> > able to identify when an unlink is complete. After that (and ensuring the
> > port buffer is empty) the application can be guaranteed that there are no
> > more events going to be sent to that port, and the application can take
> > the worker lcore out of its polling-loop and put it to sleep.
> > 
> > As mentioned before, I think an "unlinks_in_progress()" function is perhaps
> > the easiest way to achieve this functionality, as it allows relatively simple
> > tracking of unlinks() using an atomic counter in sw. (Implementation details
> > become complex when we have a separate core running event/sw, separate cores
> > polling, and a control-plane thread calling unlink...)
> > 
> > I think the end result we're hoping for is something like pseudo code below,
> > (keep in mind that the event/sw has a service-core thread running it, so no
> > application code there):
> > 
> > int worker_poll = 1;
> > 
> > worker() {
> >   while(worker_poll) {
> >      // eventdev_dequeue_burst() etc
> >   }
> >   go_to_sleep(1);
> > }
> > 
> > control_plane_scale_down() {
> >   unlink(evdev, worker, queue_id);
> >   while(unlinks_in_progress(evdev) > 0)
> >       usleep(100);
> > 
> >   /* here we know that the unlink is complete.
> >    * so we can now stop the worker from polling */
> >   worker_poll = 0;
> > }
> 
> 
> Make sense. Instead of rte_event_is_unlink_in_progress(), How about
> adding a callback in rte_event_port_unlink() which will be called on 
> unlink completion. It will reduce the need for ONE more API.
> 
> Anyway it RC2 now, so we can not accept a new feature. So we will have
> time for deprecation notice.
> 
> 
> > 
> > Hope my pseudo-code makes pseudo-sense :)
> > 
> > -Harry