Re: [dpdk-dev] [PATCH v2] eventdev: amend comments for events limit and threshold

Re: [dpdk-dev] [PATCH v2] eventdev: amend comments for events limit and threshold

[Van Haaren, Harry]

> From: Nipun Gupta [mailto:nipun.gupta@nxp.com]
> Sent: Friday, February 10, 2017 3:50 PM
> To: dev@dpdk.org
> Cc: hemant.agrawal@nxp.com; jerin.jacob@caviumnetworks.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Eads, Gage <gage.eads@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Nipun Gupta <nipun.gupta@nxp.com>
> Subject: [PATCH v2] eventdev: amend comments for events limit and threshold
> 
> Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
> and 'new_event_threshold' of 'struct rte_event_port_conf' for open system
> configuration.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
> Changes for v2:
>  - Fix errors reported by check-git-log.sh
> 
>  lib/librte_eventdev/rte_eventdev.h | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index c2f9310..171e52e 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -404,11 +404,12 @@ struct rte_event_dev_config {
>  	 * @see RTE_EVENT_DEV_CFG_PER_DEQUEUE_TIMEOUT
>  	 */
>  	int32_t nb_events_limit;
> -	/**< Applies to *closed system* event dev only. This field indicates a
> -	 * limit to ethdev-like devices to limit the number of events injected
> -	 * into the system to not overwhelm core-to-core events.
> +	/**< In a *closed system* this field indicates a limit to ethdev-like
> +	 * devices to limit the number of events injected into the system to
> +	 * not overwhelm core-to-core events.
>  	 * This value cannot exceed the *max_num_events* which previously
> -	 * provided in rte_event_dev_info_get()
> +	 * provided in rte_event_dev_info_get().
> +	 * This should be set to '-1' for *open system*.


I don't think we should mention ethdev explicitly here - it applies to any
port that is attempting to enqueue work into a closed-system eventdev.

What do you think of the following wording? (Suggestion only, feel free to
re-word if required).

/**< In a closed system this field is the limit on the maximum number of events
     that can be inflight in the eventdev at a given time. The limit is required
     to ensure that the finite space in a closed system is not overwhelmed. The
     value cannot exceed the *max_num_events* as provided by rte_event_dev_info_get().
     This value should be set to -1 for open systems.
 */

>  	 */
>  	uint8_t nb_event_queues;
>  	/**< Number of event queues to configure on this device.
> @@ -633,7 +634,8 @@ struct rte_event_port_conf {
>  	 * can have a lower threshold so as not to overwhelm the device,
>  	 * while ports used for worker pools can have a higher threshold.
>  	 * This value cannot exceed the *nb_events_limit*
> -	 * which previously supplied to rte_event_dev_configure()
> +	 * which previously supplied to rte_event_dev_configure().
> +	 * This should be set to '-1' for *open system*.
>  	 */

Minor grammer issue (that was previously there too, but worth fixing anyway),
there is a _was_ missing from the sentence:

+   which was previously supplied to rte_event_dev_configure().

[dpdk-dev] [PATCH] eventdev: amend comments for nb_events_limit and new_event_threshold

[Nipun Gupta]

Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
and 'new_event_threshold' of 'struct rte_event_port_conf' for open system
configuration.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---
 lib/librte_eventdev/rte_eventdev.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index c2f9310..171e52e 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -404,11 +404,12 @@ struct rte_event_dev_config {
 	 * @see RTE_EVENT_DEV_CFG_PER_DEQUEUE_TIMEOUT
 	 */
 	int32_t nb_events_limit;
-	/**< Applies to *closed system* event dev only. This field indicates a
-	 * limit to ethdev-like devices to limit the number of events injected
-	 * into the system to not overwhelm core-to-core events.
+	/**< In a *closed system* this field indicates a limit to ethdev-like
+	 * devices to limit the number of events injected into the system to
+	 * not overwhelm core-to-core events.
 	 * This value cannot exceed the *max_num_events* which previously
-	 * provided in rte_event_dev_info_get()
+	 * provided in rte_event_dev_info_get().
+	 * This should be set to '-1' for *open system*.
 	 */
 	uint8_t nb_event_queues;
 	/**< Number of event queues to configure on this device.
@@ -633,7 +634,8 @@ struct rte_event_port_conf {
 	 * can have a lower threshold so as not to overwhelm the device,
 	 * while ports used for worker pools can have a higher threshold.
 	 * This value cannot exceed the *nb_events_limit*
-	 * which previously supplied to rte_event_dev_configure()
+	 * which previously supplied to rte_event_dev_configure().
+	 * This should be set to '-1' for *open system*.
 	 */
 	uint8_t dequeue_depth;
 	/**< Configure number of bulk dequeues for this event port.
-- 
1.9.1

[dpdk-dev] [PATCH v2] eventdev: amend comments for events limit and threshold

[Nipun Gupta]

Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
and 'new_event_threshold' of 'struct rte_event_port_conf' for open system
configuration.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---
Changes for v2:
 - Fix errors reported by check-git-log.sh

 lib/librte_eventdev/rte_eventdev.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index c2f9310..171e52e 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -404,11 +404,12 @@ struct rte_event_dev_config {
 	 * @see RTE_EVENT_DEV_CFG_PER_DEQUEUE_TIMEOUT
 	 */
 	int32_t nb_events_limit;
-	/**< Applies to *closed system* event dev only. This field indicates a
-	 * limit to ethdev-like devices to limit the number of events injected
-	 * into the system to not overwhelm core-to-core events.
+	/**< In a *closed system* this field indicates a limit to ethdev-like
+	 * devices to limit the number of events injected into the system to
+	 * not overwhelm core-to-core events.
 	 * This value cannot exceed the *max_num_events* which previously
-	 * provided in rte_event_dev_info_get()
+	 * provided in rte_event_dev_info_get().
+	 * This should be set to '-1' for *open system*.
 	 */
 	uint8_t nb_event_queues;
 	/**< Number of event queues to configure on this device.
@@ -633,7 +634,8 @@ struct rte_event_port_conf {
 	 * can have a lower threshold so as not to overwhelm the device,
 	 * while ports used for worker pools can have a higher threshold.
 	 * This value cannot exceed the *nb_events_limit*
-	 * which previously supplied to rte_event_dev_configure()
+	 * which previously supplied to rte_event_dev_configure().
+	 * This should be set to '-1' for *open system*.
 	 */
 	uint8_t dequeue_depth;
 	/**< Configure number of bulk dequeues for this event port.
-- 
1.9.1

Re: [dpdk-dev] [PATCH v2] eventdev: amend comments for events limit and threshold

[Nipun Gupta]

> -----Original Message-----
> From: Van Haaren, Harry [mailto:harry.van.haaren@intel.com]
> Sent: Friday, February 10, 2017 16:02
> To: Nipun Gupta <nipun.gupta@nxp.com>; dev@dpdk.org
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>;
> jerin.jacob@caviumnetworks.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Eads, Gage <gage.eads@intel.com>
> Subject: RE: [PATCH v2] eventdev: amend comments for events limit and
> threshold
> 
> > From: Nipun Gupta [mailto:nipun.gupta@nxp.com]
> > Sent: Friday, February 10, 2017 3:50 PM
> > To: dev@dpdk.org
> > Cc: hemant.agrawal@nxp.com; jerin.jacob@caviumnetworks.com;
> Richardson, Bruce
> > <bruce.richardson@intel.com>; Eads, Gage <gage.eads@intel.com>; Van
> Haaren, Harry
> > <harry.van.haaren@intel.com>; Nipun Gupta <nipun.gupta@nxp.com>
> > Subject: [PATCH v2] eventdev: amend comments for events limit and threshold
> >
> > Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
> > and 'new_event_threshold' of 'struct rte_event_port_conf' for open system
> > configuration.
> >
> > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> > ---
> > Changes for v2:
> >  - Fix errors reported by check-git-log.sh
> >
> >  lib/librte_eventdev/rte_eventdev.h | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev.h
> b/lib/librte_eventdev/rte_eventdev.h
> > index c2f9310..171e52e 100644
> > --- a/lib/librte_eventdev/rte_eventdev.h
> > +++ b/lib/librte_eventdev/rte_eventdev.h
> > @@ -404,11 +404,12 @@ struct rte_event_dev_config {
> >  	 * @see RTE_EVENT_DEV_CFG_PER_DEQUEUE_TIMEOUT
> >  	 */
> >  	int32_t nb_events_limit;
> > -	/**< Applies to *closed system* event dev only. This field indicates a
> > -	 * limit to ethdev-like devices to limit the number of events injected
> > -	 * into the system to not overwhelm core-to-core events.
> > +	/**< In a *closed system* this field indicates a limit to ethdev-like
> > +	 * devices to limit the number of events injected into the system to
> > +	 * not overwhelm core-to-core events.
> >  	 * This value cannot exceed the *max_num_events* which previously
> > -	 * provided in rte_event_dev_info_get()
> > +	 * provided in rte_event_dev_info_get().
> > +	 * This should be set to '-1' for *open system*.
> 
> 
> I don't think we should mention ethdev explicitly here - it applies to any
> port that is attempting to enqueue work into a closed-system eventdev.
> 
> What do you think of the following wording? (Suggestion only, feel free to
> re-word if required).
> 
> /**< In a closed system this field is the limit on the maximum number of events
>      that can be inflight in the eventdev at a given time. The limit is required
>      to ensure that the finite space in a closed system is not overwhelmed. The
>      value cannot exceed the *max_num_events* as provided by
> rte_event_dev_info_get().
>      This value should be set to -1 for open systems.
>  */

I agree with you Harry. For *closed systems*, this limit should be valid on all the
enqueues, whether from ethdev type devices or enqueue's from core.

BTW, do you use event limit both on event device and on event ports in your
software implementation? Because I think the limit may be only per event port.

Jerin,

Your views on this for open systems would be very helpful. Thanks.

> 
> >  	 */
> >  	uint8_t nb_event_queues;
> >  	/**< Number of event queues to configure on this device.
> > @@ -633,7 +634,8 @@ struct rte_event_port_conf {
> >  	 * can have a lower threshold so as not to overwhelm the device,
> >  	 * while ports used for worker pools can have a higher threshold.
> >  	 * This value cannot exceed the *nb_events_limit*
> > -	 * which previously supplied to rte_event_dev_configure()
> > +	 * which previously supplied to rte_event_dev_configure().
> > +	 * This should be set to '-1' for *open system*.
> >  	 */
> 
> Minor grammer issue (that was previously there too, but worth fixing anyway),
> there is a _was_ missing from the sentence:

Sure. I'll take care of this in v3.

> 
> +   which was previously supplied to rte_event_dev_configure().
> 

Re: [dpdk-dev] [PATCH v2] eventdev: amend comments for events limit and threshold

[Jerin Jacob]

On Sat, Feb 11, 2017 at 09:48:57AM +0000, Nipun Gupta wrote:
> 
> 
> > -----Original Message-----
> > From: Van Haaren, Harry [mailto:harry.van.haaren@intel.com]
> > Sent: Friday, February 10, 2017 16:02
> > To: Nipun Gupta <nipun.gupta@nxp.com>; dev@dpdk.org
> > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>;
> > jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > <bruce.richardson@intel.com>; Eads, Gage <gage.eads@intel.com>
> > Subject: RE: [PATCH v2] eventdev: amend comments for events limit and
> > threshold
> > 
> > > From: Nipun Gupta [mailto:nipun.gupta@nxp.com]
> > > Sent: Friday, February 10, 2017 3:50 PM
> > > To: dev@dpdk.org
> > > Cc: hemant.agrawal@nxp.com; jerin.jacob@caviumnetworks.com;
> > Richardson, Bruce
> > > <bruce.richardson@intel.com>; Eads, Gage <gage.eads@intel.com>; Van
> > Haaren, Harry
> > > <harry.van.haaren@intel.com>; Nipun Gupta <nipun.gupta@nxp.com>
> > > Subject: [PATCH v2] eventdev: amend comments for events limit and threshold
> > >
> > > Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
> > > and 'new_event_threshold' of 'struct rte_event_port_conf' for open system
> > > configuration.
> > >
> > > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> > > ---
> > > Changes for v2:
> > >  - Fix errors reported by check-git-log.sh
> > >
> > >  lib/librte_eventdev/rte_eventdev.h | 12 +++++++-----
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/librte_eventdev/rte_eventdev.h
> > b/lib/librte_eventdev/rte_eventdev.h
> > > index c2f9310..171e52e 100644
> > > --- a/lib/librte_eventdev/rte_eventdev.h
> > > +++ b/lib/librte_eventdev/rte_eventdev.h
> > > @@ -404,11 +404,12 @@ struct rte_event_dev_config {
> > >  	 * @see RTE_EVENT_DEV_CFG_PER_DEQUEUE_TIMEOUT
> > >  	 */
> > >  	int32_t nb_events_limit;
> > > -	/**< Applies to *closed system* event dev only. This field indicates a
> > > -	 * limit to ethdev-like devices to limit the number of events injected
> > > -	 * into the system to not overwhelm core-to-core events.
> > > +	/**< In a *closed system* this field indicates a limit to ethdev-like
> > > +	 * devices to limit the number of events injected into the system to
> > > +	 * not overwhelm core-to-core events.
> > >  	 * This value cannot exceed the *max_num_events* which previously
> > > -	 * provided in rte_event_dev_info_get()
> > > +	 * provided in rte_event_dev_info_get().
> > > +	 * This should be set to '-1' for *open system*.
> > 
> > 
> > I don't think we should mention ethdev explicitly here - it applies to any
> > port that is attempting to enqueue work into a closed-system eventdev.
> > 
> > What do you think of the following wording? (Suggestion only, feel free to
> > re-word if required).
> > 
> > /**< In a closed system this field is the limit on the maximum number of events
> >      that can be inflight in the eventdev at a given time. The limit is required
> >      to ensure that the finite space in a closed system is not overwhelmed. The
> >      value cannot exceed the *max_num_events* as provided by
> > rte_event_dev_info_get().
> >      This value should be set to -1 for open systems.
> >  */
> 
> I agree with you Harry. For *closed systems*, this limit should be valid on all the
> enqueues, whether from ethdev type devices or enqueue's from core.
> 
> BTW, do you use event limit both on event device and on event ports in your
> software implementation? Because I think the limit may be only per event port.
> 
> Jerin,
> 
> Your views on this for open systems would be very helpful. Thanks.

I think, none of the implementations can have _true_ infinite amount of inflight
buffers. In our implementation, we internal fixed size SRAM for storing
inflight events which are backed by DDR(To mimic the infinite space).
But both are fixed size and configurable.
That would translate to use only struct rte_event_dev_config.nb_events_limit in
our implementation.
If the implementation support port level configuration then it can
use struct rte_event_port_conf.new_event_threshold.

[dpdk-dev] [PATCH v3] eventdev: amend comments for events limit and threshold

[Nipun Gupta]

Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
and 'new_event_threshold' of 'struct rte_event_port_conf'.

Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---
Changes for v2:
 - Fix errors reported by check-git-log.sh

Changes for v3:
 - Updated nb_events_limit comment for closed systems
 - Fixed grammatical mistake on comment for new_event_threshold

 lib/librte_eventdev/rte_eventdev.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index c2f9310..b619160 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -404,11 +404,12 @@ struct rte_event_dev_config {
 	 * @see RTE_EVENT_DEV_CFG_PER_DEQUEUE_TIMEOUT
 	 */
 	int32_t nb_events_limit;
-	/**< Applies to *closed system* event dev only. This field indicates a
-	 * limit to ethdev-like devices to limit the number of events injected
-	 * into the system to not overwhelm core-to-core events.
-	 * This value cannot exceed the *max_num_events* which previously
-	 * provided in rte_event_dev_info_get()
+	/**< In a *closed system* this field is the limit on maximum number of
+	 * events that can be inflight in the eventdev at a given time. The
+	 * limit is required to ensure that the finite space in a closed system
+	 * is not overwhelmed. The value cannot exceed the *max_num_events*
+	 * as provided by rte_event_dev_info_get().
+	 * This value should be set to -1 for *open system*.
 	 */
 	uint8_t nb_event_queues;
 	/**< Number of event queues to configure on this device.
@@ -633,7 +634,8 @@ struct rte_event_port_conf {
 	 * can have a lower threshold so as not to overwhelm the device,
 	 * while ports used for worker pools can have a higher threshold.
 	 * This value cannot exceed the *nb_events_limit*
-	 * which previously supplied to rte_event_dev_configure()
+	 * which was previously supplied to rte_event_dev_configure().
+	 * This should be set to '-1' for *open system*.
 	 */
 	uint8_t dequeue_depth;
 	/**< Configure number of bulk dequeues for this event port.
-- 
1.9.1

Re: [dpdk-dev] [PATCH v3] eventdev: amend comments for events limit and threshold

[Van Haaren, Harry]

> From: Nipun Gupta [mailto:nipun.gupta@nxp.com]
> Sent: Tuesday, February 14, 2017 12:43 PM
> To: dev@dpdk.org
> Cc: hemant.agrawal@nxp.com; jerin.jacob@caviumnetworks.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Eads, Gage <gage.eads@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Nipun Gupta <nipun.gupta@nxp.com>
> Subject: [PATCH v3] eventdev: amend comments for events limit and threshold
> 
> Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
> and 'new_event_threshold' of 'struct rte_event_port_conf'.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

Re: [dpdk-dev] [PATCH v3] eventdev: amend comments for events limit and threshold

[Bruce Richardson]

On Wed, Feb 15, 2017 at 01:53:45PM +0000, Van Haaren, Harry wrote:
> > From: Nipun Gupta [mailto:nipun.gupta@nxp.com]
> > Sent: Tuesday, February 14, 2017 12:43 PM
> > To: dev@dpdk.org
> > Cc: hemant.agrawal@nxp.com; jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > <bruce.richardson@intel.com>; Eads, Gage <gage.eads@intel.com>; Van Haaren, Harry
> > <harry.van.haaren@intel.com>; Nipun Gupta <nipun.gupta@nxp.com>
> > Subject: [PATCH v3] eventdev: amend comments for events limit and threshold
> > 
> > Updated the comments on 'nb_events_limit' of 'struct rte_event_dev_config'
> > and 'new_event_threshold' of 'struct rte_event_port_conf'.
> > 
> > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
Applied to dpdk-next-eventdev

/Bruce