All of lore.kernel.org
 help / color / mirror / Atom feed
* [dpdk-dev] eventdev DSW question
@ 2019-12-06  0:26 Venky Venkatesh
  2019-12-06  6:48 ` Jerin Jacob
  2019-12-06  8:34 ` Mattias Rönnblom
  0 siblings, 2 replies; 7+ messages in thread
From: Venky Venkatesh @ 2019-12-06  0:26 UTC (permalink / raw)
  To: dev

I see that the provision in 18.11 eventdev DSW for maximum number of queues
is

#define DSW_MAX_QUEUES (16)



   1. If the number of queues needed is to be increased to 7 bits (i.e.
   128) is there any issue (correctness, scale, performance) other than
   increased data structure size?
   2. I see that it is only used in the following structs:
      - struct dsw_evdev: struct dsw_queue queues[DSW_MAX_QUEUES];
      sizeof(struct dsw_queue) ~ DSW_MAX_FLOWS. So the total increase
      contribution here is (128-16)*DSW_MAX_FLOWS from about 0.5MB to 4MB
      - struct dsw_port: uint64_t queue_enqueued[DSW_MAX_QUEUES],
queue_dequeued[DSW_MAX_QUEUES];
      This increase is negligible (a few KB at most across all dsw_ports)
   3. So is it enough if I changed the above define? (In other words I hope
   there are no other hidden/implicit dependencies on the current value 16
   elsewhere in the code). Also I suppose the only way is to directly change
   this in the code, rite?

Thanks

-Venky

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] eventdev DSW question
  2019-12-06  0:26 [dpdk-dev] eventdev DSW question Venky Venkatesh
@ 2019-12-06  6:48 ` Jerin Jacob
  2019-12-06  8:34 ` Mattias Rönnblom
  1 sibling, 0 replies; 7+ messages in thread
From: Jerin Jacob @ 2019-12-06  6:48 UTC (permalink / raw)
  To: Venky Venkatesh; +Cc: dpdk-dev, mattias.ronnblom

+ DSW maintainer


On Fri, Dec 6, 2019 at 5:57 AM Venky Venkatesh
<vvenkatesh@paloaltonetworks.com> wrote:
>
> I see that the provision in 18.11 eventdev DSW for maximum number of queues
> is
>
> #define DSW_MAX_QUEUES (16)
>
>
>
>    1. If the number of queues needed is to be increased to 7 bits (i.e.
>    128) is there any issue (correctness, scale, performance) other than
>    increased data structure size?
>    2. I see that it is only used in the following structs:
>       - struct dsw_evdev: struct dsw_queue queues[DSW_MAX_QUEUES];
>       sizeof(struct dsw_queue) ~ DSW_MAX_FLOWS. So the total increase
>       contribution here is (128-16)*DSW_MAX_FLOWS from about 0.5MB to 4MB
>       - struct dsw_port: uint64_t queue_enqueued[DSW_MAX_QUEUES],
> queue_dequeued[DSW_MAX_QUEUES];
>       This increase is negligible (a few KB at most across all dsw_ports)
>    3. So is it enough if I changed the above define? (In other words I hope
>    there are no other hidden/implicit dependencies on the current value 16
>    elsewhere in the code). Also I suppose the only way is to directly change
>    this in the code, rite?
>
> Thanks
>
> -Venky

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] eventdev DSW question
  2019-12-06  0:26 [dpdk-dev] eventdev DSW question Venky Venkatesh
  2019-12-06  6:48 ` Jerin Jacob
@ 2019-12-06  8:34 ` Mattias Rönnblom
  2019-12-06 16:32   ` Venky Venkatesh
  1 sibling, 1 reply; 7+ messages in thread
From: Mattias Rönnblom @ 2019-12-06  8:34 UTC (permalink / raw)
  To: Venky Venkatesh, dev

On 2019-12-06 01:26, Venky Venkatesh wrote:
> I see that the provision in 18.11 eventdev DSW for maximum number of queues
> is
> 
> #define DSW_MAX_QUEUES (16)
> 
> 
> 
>     1. If the number of queues needed is to be increased to 7 bits (i.e.
>     128) is there any issue (correctness, scale, performance) other than
>     increased data structure size?

No.

>     2. I see that it is only used in the following structs:
>        - struct dsw_evdev: struct dsw_queue queues[DSW_MAX_QUEUES];
>        sizeof(struct dsw_queue) ~ DSW_MAX_FLOWS. So the total increase
>        contribution here is (128-16)*DSW_MAX_FLOWS from about 0.5MB to 4MB
>        - struct dsw_port: uint64_t queue_enqueued[DSW_MAX_QUEUES],
> queue_dequeued[DSW_MAX_QUEUES];
>        This increase is negligible (a few KB at most across all dsw_ports)

On x86_64, the size of the dsw_evdev-struct is increased by ~32 kB per 
queue added.

>     3. So is it enough if I changed the above define? (In other words I hope
>     there are no other hidden/implicit dependencies on the current value 16
>     elsewhere in the code). Also I suppose the only way is to directly change
>     this in the code, rite?
> 

Yes, and yes.

The original reason for having that define was that I thought 16 queues 
would be enough for anyone. As so many in the past that has utter such 
statements, I might well have been wrong.

/M

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] eventdev DSW question
  2019-12-06  8:34 ` Mattias Rönnblom
@ 2019-12-06 16:32   ` Venky Venkatesh
  2019-12-06 20:37     ` Mattias Rönnblom
  0 siblings, 1 reply; 7+ messages in thread
From: Venky Venkatesh @ 2019-12-06 16:32 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev

Thanks Mattias for the clarifications.

1 more question: This time it is about the inflight accounting for DSW.
Here is my understanding: it seems to consider only the events which
are *inside
the scheduler* as in flight. I am trying to distinguish it from those which
have been currently given to cores by the scheduler. The latter are not
considered in flight since we dsw_port_return_credits as soon as
dsw_event_dequeue_burst . So if these events which are in core currently do
a FORWARD, there is a chance that those can fail. Ideally those FORWARDs
should not fail -- which can happen with the current design as some NEWs
can hog those credits freed up by the ones which have been dequeued by
cores. Is this design of DSW intentional or an omission? If it is an
omission I can work on a possible fix and run it by you.

Thanks
-Venky

On Fri, Dec 6, 2019 at 12:34 AM Mattias Rönnblom <
mattias.ronnblom@ericsson.com> wrote:

> On 2019-12-06 01:26, Venky Venkatesh wrote:
> > I see that the provision in 18.11 eventdev DSW for maximum number of
> queues
> > is
> >
> > #define DSW_MAX_QUEUES (16)
> >
> >
> >
> >     1. If the number of queues needed is to be increased to 7 bits (i.e.
> >     128) is there any issue (correctness, scale, performance) other than
> >     increased data structure size?
>
> No.
>
> >     2. I see that it is only used in the following structs:
> >        - struct dsw_evdev: struct dsw_queue queues[DSW_MAX_QUEUES];
> >        sizeof(struct dsw_queue) ~ DSW_MAX_FLOWS. So the total increase
> >        contribution here is (128-16)*DSW_MAX_FLOWS from about 0.5MB to
> 4MB
> >        - struct dsw_port: uint64_t queue_enqueued[DSW_MAX_QUEUES],
> > queue_dequeued[DSW_MAX_QUEUES];
> >        This increase is negligible (a few KB at most across all
> dsw_ports)
>
> On x86_64, the size of the dsw_evdev-struct is increased by ~32 kB per
> queue added.
>
> >     3. So is it enough if I changed the above define? (In other words I
> hope
> >     there are no other hidden/implicit dependencies on the current value
> 16
> >     elsewhere in the code). Also I suppose the only way is to directly
> change
> >     this in the code, rite?
> >
>
> Yes, and yes.
>
> The original reason for having that define was that I thought 16 queues
> would be enough for anyone. As so many in the past that has utter such
> statements, I might well have been wrong.
>
> /M
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] eventdev DSW question
  2019-12-06 16:32   ` Venky Venkatesh
@ 2019-12-06 20:37     ` Mattias Rönnblom
  2019-12-06 22:22       ` Venky Venkatesh
  0 siblings, 1 reply; 7+ messages in thread
From: Mattias Rönnblom @ 2019-12-06 20:37 UTC (permalink / raw)
  To: Venky Venkatesh; +Cc: dev

On 2019-12-06 17:32, Venky Venkatesh wrote:
> Thanks Mattias for the clarifications.
> 
> 1 more question: This time it is about the inflight accounting for DSW.
> Here is my understanding: it seems to consider only the events which
> are *inside
> the scheduler* as in flight.

Yes, like all event devices, I believe.

> I am trying to distinguish it from those which
> have been currently given to cores by the scheduler. The latter are not
> considered in flight since we dsw_port_return_credits as soon as
> dsw_event_dequeue_burst.

A new dequeue means an implicit release of all unreleased events 
dequeued in the previous call. It's standard Eventdev semantics.

> So if these events which are in core currently do
> a FORWARD, there is a chance that those can fail. Ideally those FORWARDs
> should not fail -- which can happen with the current design as some NEWs
> can hog those credits freed up by the ones which have been dequeued by
> cores.

What you do to avoid this situation is set the new_event_threshold 
low-enough, so NEW events don't block FORWARDed ones.

> Is this design of DSW intentional or an omission? If it is an
> omission I can work on a possible fix and run it by you.
> 

This is not really a DSW design, but rather how Eventdev works.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] eventdev DSW question
  2019-12-06 20:37     ` Mattias Rönnblom
@ 2019-12-06 22:22       ` Venky Venkatesh
  2019-12-07 19:35         ` Mattias Rönnblom
  0 siblings, 1 reply; 7+ messages in thread
From: Venky Venkatesh @ 2019-12-06 22:22 UTC (permalink / raw)
  To: Mattias Rönnblom; +Cc: dev

To my understanding, per eventdev API, events are considered in flight
between NEW to RELEASE (implicit/explicit). Now consider an event (event-1)
going thru the following stages:

   1. NEW from core-3
   2. dequeued by core-1
   3. FORWARD
   4. core-1 does a next dequeue
   5. dequeued by core-2
   6. RELEASE by core-2/implicit release on next dequeue by core-2

The way I understand DSW implementation this event would use credit at step
1 AND step 3 while releasing in step2 -- right now credit usage is for
non_release (i.e NEW and FORWARD). So if between step-2 and step-3 another
core puts in a NEW of event-2 that could utilize all the credits of the
system and could thus fail step-3 of event-1.
This to my knowledge is not conformant with eventdev. One way to address
this is to track the credits for that which are currently in core and not
make those credits available to NEW but only for FORWARDs ... there are
more details of course.

Hope this explains
Thanks
-Venky



On Fri, Dec 6, 2019 at 12:37 PM Mattias Rönnblom <
mattias.ronnblom@ericsson.com> wrote:

> On 2019-12-06 17:32, Venky Venkatesh wrote:
> > Thanks Mattias for the clarifications.
> >
> > 1 more question: This time it is about the inflight accounting for DSW.
> > Here is my understanding: it seems to consider only the events which
> > are *inside
> > the scheduler* as in flight.
>
> Yes, like all event devices, I believe.
>
> > I am trying to distinguish it from those which
> > have been currently given to cores by the scheduler. The latter are not
> > considered in flight since we dsw_port_return_credits as soon as
> > dsw_event_dequeue_burst.
>
> A new dequeue means an implicit release of all unreleased events
> dequeued in the previous call. It's standard Eventdev semantics.
>
> > So if these events which are in core currently do
> > a FORWARD, there is a chance that those can fail. Ideally those FORWARDs
> > should not fail -- which can happen with the current design as some NEWs
> > can hog those credits freed up by the ones which have been dequeued by
> > cores.
>
> What you do to avoid this situation is set the new_event_threshold
> low-enough, so NEW events don't block FORWARDed ones.
>
> > Is this design of DSW intentional or an omission? If it is an
> > omission I can work on a possible fix and run it by you.
> >
>
> This is not really a DSW design, but rather how Eventdev works.
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] eventdev DSW question
  2019-12-06 22:22       ` Venky Venkatesh
@ 2019-12-07 19:35         ` Mattias Rönnblom
  0 siblings, 0 replies; 7+ messages in thread
From: Mattias Rönnblom @ 2019-12-07 19:35 UTC (permalink / raw)
  To: Venky Venkatesh; +Cc: dev

On 2019-12-06 23:22, Venky Venkatesh wrote:
> To my understanding, per eventdev API, events are considered in flight
> between NEW to RELEASE (implicit/explicit).

DSW considers events to be in flight between time of enqueue (on the 
source port), to the time of release (on the destination port). This is
regardless if they are FORWARD or NEW-type events.

The SW event device only counts NEW events toward the max number of 
in-flights events limitation. It will however release the atomic context 
and return the credits at step 4 below. Since there's no centralized 
point which can backpressure FORWARD events (which I think is how SW 
avoids being overwhelmed by FORWARDed events), DSW needs to ensure that 
the max_num_events is never exceeded.

> Now consider an event (event-1) going thru the following stages:
> 
>     1. NEW from core-3
>     2. dequeued by core-1
>     3. FORWARD
>     4. core-1 does a next dequeue

The event is RELEASEd already at this point.

>     5. dequeued by core-2
>     6. RELEASE by core-2/implicit release on next dequeue by core-2
> 
> The way I understand DSW implementation this event would use credit at step
> 1 AND step 3 while releasing in step2 -- right now credit usage is for
> non_release (i.e NEW and FORWARD). So if between step-2 and step-3 another
> core puts in a NEW of event-2 that could utilize all the credits of the
> system and could thus fail step-3 of event-1.

The NEW events producer ports' new_event_threshold should be 
significantly lower than the maximum numbers of in-flight events, and 
thus leave credits for FORWARDed events.

> This to my knowledge is not conformant with eventdev. One way to address
> this is to track the credits for that which are currently in core and not
> make those credits available to NEW but only for FORWARDs ... there are
> more details of course.
> 
> Hope this explains
> Thanks
> -Venky
> 
> 
> 
> On Fri, Dec 6, 2019 at 12:37 PM Mattias Rönnblom <
> mattias.ronnblom@ericsson.com> wrote:
> 
>> On 2019-12-06 17:32, Venky Venkatesh wrote:
>>> Thanks Mattias for the clarifications.
>>>
>>> 1 more question: This time it is about the inflight accounting for DSW.
>>> Here is my understanding: it seems to consider only the events which
>>> are *inside
>>> the scheduler* as in flight.
>>
>> Yes, like all event devices, I believe.
>>
>>> I am trying to distinguish it from those which
>>> have been currently given to cores by the scheduler. The latter are not
>>> considered in flight since we dsw_port_return_credits as soon as
>>> dsw_event_dequeue_burst.
>>
>> A new dequeue means an implicit release of all unreleased events
>> dequeued in the previous call. It's standard Eventdev semantics.
>>
>>> So if these events which are in core currently do
>>> a FORWARD, there is a chance that those can fail. Ideally those FORWARDs
>>> should not fail -- which can happen with the current design as some NEWs
>>> can hog those credits freed up by the ones which have been dequeued by
>>> cores.
>>
>> What you do to avoid this situation is set the new_event_threshold
>> low-enough, so NEW events don't block FORWARDed ones.
>>
>>> Is this design of DSW intentional or an omission? If it is an
>>> omission I can work on a possible fix and run it by you.
>>>
>>
>> This is not really a DSW design, but rather how Eventdev works.
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-12-07 19:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-06  0:26 [dpdk-dev] eventdev DSW question Venky Venkatesh
2019-12-06  6:48 ` Jerin Jacob
2019-12-06  8:34 ` Mattias Rönnblom
2019-12-06 16:32   ` Venky Venkatesh
2019-12-06 20:37     ` Mattias Rönnblom
2019-12-06 22:22       ` Venky Venkatesh
2019-12-07 19:35         ` Mattias Rönnblom

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.