All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] doc: add enqueue depth for new event type
@ 2022-06-27  9:57 pbhagavatula
  2022-06-27  9:57 ` [PATCH 2/2] eventdev: add function to enq new events to the same queue pbhagavatula
                   ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: pbhagavatula @ 2022-06-27  9:57 UTC (permalink / raw)
  To: jerinj, Ray Kinsella; +Cc: dev, Pavan Nikhilesh

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

A new field ``max_event_port_enqueue_new_burst`` will be added to the
structure ``rte_event_dev_info``. The field defines the max enqueue
burst size of new events (OP_NEW) supported by the underlying event
device.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 doc/guides/rel_notes/deprecation.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 4e5b23c53d..071317e8e3 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -125,3 +125,8 @@ Deprecation Notices
   applications should be updated to use the ``dmadev`` library instead,
   with the underlying HW-functionality being provided by the ``ioat`` or
   ``idxd`` dma drivers
+
+* eventdev: The structure ``rte_event_dev_info`` will be extended to include the
+  max enqueue burst size of new events supported by the underlying event device.
+  A new field ``max_event_port_enqueue_new_burst`` will be added to the structure
+  ``rte_event_dev_info`` in DPDK 22.11.
-- 
2.25.1


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

* [PATCH 2/2] eventdev: add function to enq new events to the same queue
  2022-06-27  9:57 [PATCH 1/2] doc: add enqueue depth for new event type pbhagavatula
@ 2022-06-27  9:57 ` pbhagavatula
  2022-07-11 14:54 ` [PATCH 1/2] doc: add enqueue depth for new event type Jerin Jacob
  2022-07-12 15:01 ` Thomas Monjalon
  2 siblings, 0 replies; 27+ messages in thread
From: pbhagavatula @ 2022-06-27  9:57 UTC (permalink / raw)
  To: jerinj; +Cc: dev, Pavan Nikhilesh

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

Introduce new fastpath function to enqueue events with type *OP_NEW*
to the same destination event queue.
This function can be used as a hint to the PMD to use optimized the
enqueue sequence.

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 lib/eventdev/eventdev_pmd.h      |  5 +-
 lib/eventdev/eventdev_private.c  | 13 ++++++
 lib/eventdev/rte_eventdev.h      | 80 +++++++++++++++++++++++++++++++-
 lib/eventdev/rte_eventdev_core.h | 11 ++++-
 4 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/lib/eventdev/eventdev_pmd.h b/lib/eventdev/eventdev_pmd.h
index 69402668d8..f0bb97fb89 100644
--- a/lib/eventdev/eventdev_pmd.h
+++ b/lib/eventdev/eventdev_pmd.h
@@ -178,7 +178,10 @@ struct rte_eventdev {
 	/**< Pointer to PMD eth Tx adapter enqueue function. */
 	event_crypto_adapter_enqueue_t ca_enqueue;
 
-	uint64_t reserved_64s[4]; /**< Reserved for future fields */
+	event_enqueue_queue_burst_t enqueue_new_same_dest;
+	/**< PMD enqueue burst queue new function to same destination queue. */
+
+	uint64_t reserved_64s[3]; /**< Reserved for future fields */
 	void *reserved_ptrs[3];	  /**< Reserved for future fields */
 } __rte_cache_aligned;
 
diff --git a/lib/eventdev/eventdev_private.c b/lib/eventdev/eventdev_private.c
index 1d3d9d357e..53d1db281b 100644
--- a/lib/eventdev/eventdev_private.c
+++ b/lib/eventdev/eventdev_private.c
@@ -24,6 +24,17 @@ dummy_event_enqueue_burst(__rte_unused void *port,
 	return 0;
 }
 
+static uint16_t
+dummy_event_enqueue_queue_burst(__rte_unused void *port,
+				__rte_unused uint8_t queue,
+				__rte_unused const struct rte_event ev[],
+				__rte_unused uint16_t nb_events)
+{
+	RTE_EDEV_LOG_ERR(
+		"event enqueue burst requested for unconfigured event device");
+	return 0;
+}
+
 static uint16_t
 dummy_event_dequeue(__rte_unused void *port, __rte_unused struct rte_event *ev,
 		    __rte_unused uint64_t timeout_ticks)
@@ -90,6 +101,7 @@ event_dev_fp_ops_reset(struct rte_event_fp_ops *fp_op)
 		.enqueue_burst = dummy_event_enqueue_burst,
 		.enqueue_new_burst = dummy_event_enqueue_burst,
 		.enqueue_forward_burst = dummy_event_enqueue_burst,
+		.enqueue_new_same_dest = dummy_event_enqueue_queue_burst,
 		.dequeue = dummy_event_dequeue,
 		.dequeue_burst = dummy_event_dequeue_burst,
 		.maintain = dummy_event_maintain,
@@ -111,6 +123,7 @@ event_dev_fp_ops_set(struct rte_event_fp_ops *fp_op,
 	fp_op->enqueue_burst = dev->enqueue_burst;
 	fp_op->enqueue_new_burst = dev->enqueue_new_burst;
 	fp_op->enqueue_forward_burst = dev->enqueue_forward_burst;
+	fp_op->enqueue_new_same_dest = dev->enqueue_new_same_dest;
 	fp_op->dequeue = dev->dequeue;
 	fp_op->dequeue_burst = dev->dequeue_burst;
 	fp_op->maintain = dev->maintain;
diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index 6a6f6ea4c1..2aa563740b 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -425,8 +425,9 @@ struct rte_event_dev_info {
 	 * A device that does not support bulk dequeue will set this as 1.
 	 */
 	uint32_t max_event_port_enqueue_depth;
-	/**< Maximum number of events can be enqueued at a time from an
-	 * event port by this device.
+	/**< Maximum number of events that can be enqueued at a time to a
+	 * event port by this device, applicable for rte_event::op is either
+	 * *RTE_EVENT_OP_FORWARD* or *RTE_EVENT_OP_RELEASE*
 	 * A device that does not support bulk enqueue will set this as 1.
 	 */
 	uint8_t max_event_port_links;
@@ -446,6 +447,12 @@ struct rte_event_dev_info {
 	 * device. These ports and queues are not accounted for in
 	 * max_event_ports or max_event_queues.
 	 */
+	int16_t max_event_port_enqueue_new_burst;
+	/**< Maximum number of events that can be enqueued at a time to a
+	 * event port by this device, applicable when rte_event::op is set to
+	 * *RTE_EVENT_OP_NEW*.
+	 * A device with no limits will set this value to -1.
+	 */
 };
 
 /**
@@ -2082,6 +2089,75 @@ rte_event_enqueue_forward_burst(uint8_t dev_id, uint8_t port_id,
 					 fp_ops->enqueue_forward_burst);
 }
 
+/**
+ * Enqueue a burst of events objects of operation type *RTE_EVENT_OP_NEW* on
+ * an event device designated by its *dev_id* through the event port specified
+ * by *port_id* to the same queue specified by *queue_id*.
+ *
+ * Provides the same functionality as rte_event_enqueue_burst(), expect that
+ * application can use this API when the all objects in the burst contains
+ * the enqueue operation of the type *RTE_EVENT_OP_NEW* and are destined to the
+ * same queue. This specialized function can provide the additional hint to the
+ * PMD and optimize if possible.
+ *
+ * The rte_event_enqueue_new_queue_burst() result is undefined if the enqueue
+ * burst has event object of operation type != RTE_EVENT_OP_NEW.
+ *
+ * @param dev_id
+ *   The identifier of the device.
+ * @param port_id
+ *   The identifier of the event port.
+ * @param queue_id
+ *   The identifier of the event port.
+ * @param ev
+ *   Points to an array of *nb_events* objects of type *rte_event* structure
+ *   which contain the event object enqueue operations to be processed.
+ * @param nb_events
+ *   The number of event objects to enqueue, typically number of
+ *   rte_event_port_attr_get(...RTE_EVENT_PORT_ATTR_ENQ_DEPTH...)
+ *   available for this port.
+ *
+ * @return
+ *   The number of event objects actually enqueued on the event device. The
+ *   return value can be less than the value of the *nb_events* parameter when
+ *   the event devices queue is full or if invalid parameters are specified in a
+ *   *rte_event*. If the return value is less than *nb_events*, the remaining
+ *   events at the end of ev[] are not consumed and the caller has to take care
+ *   of them, and rte_errno is set accordingly. Possible errno values include:
+ *   - EINVAL   The port ID is invalid, device ID is invalid, an event's queue
+ *              ID is invalid, or an event's sched type doesn't match the
+ *              capabilities of the destination queue.
+ *   - ENOSPC   The event port was backpressured and unable to enqueue
+ *              one or more events. This error code is only applicable to
+ *              closed systems.
+ * @see rte_event_port_attr_get(), RTE_EVENT_PORT_ATTR_ENQ_DEPTH
+ * @see rte_event_enqueue_burst()
+ */
+static inline uint16_t
+rte_event_enqueue_new_queue_burst(uint8_t dev_id, uint8_t port_id,
+				  uint8_t queue_id, const struct rte_event ev[],
+				  uint16_t nb_events)
+{
+	const struct rte_event_fp_ops *fp_ops;
+	void *port;
+
+	fp_ops = &rte_event_fp_ops[dev_id];
+	port = fp_ops->data[port_id];
+#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
+	if (dev_id >= RTE_EVENT_MAX_DEVS ||
+	    port_id >= RTE_EVENT_MAX_PORTS_PER_DEV) {
+		rte_errno = EINVAL;
+		return 0;
+	}
+
+	if (port == NULL) {
+		rte_errno = EINVAL;
+		return 0;
+	}
+#endif
+	return fp_ops->enqueue_new_same_dest(port, queue_id, ev, nb_events);
+}
+
 /**
  * Dequeue a burst of events objects or an event object from the event port
  * designated by its *event_port_id*, on an event device designated
diff --git a/lib/eventdev/rte_eventdev_core.h b/lib/eventdev/rte_eventdev_core.h
index c328bdbc82..4d7d27e82d 100644
--- a/lib/eventdev/rte_eventdev_core.h
+++ b/lib/eventdev/rte_eventdev_core.h
@@ -20,6 +20,13 @@ typedef uint16_t (*event_enqueue_burst_t)(void *port,
 					  uint16_t nb_events);
 /**< @internal Enqueue burst of events on port of a device */
 
+typedef uint16_t (*event_enqueue_queue_burst_t)(void *port, uint8_t queue_id,
+						const struct rte_event ev[],
+						uint16_t nb_events);
+/**< @internal Enqueue burst of events on port of a device to a specific
+ * event queue.
+ */
+
 typedef uint16_t (*event_dequeue_t)(void *port, struct rte_event *ev,
 				    uint64_t timeout_ticks);
 /**< @internal Dequeue event from port of a device */
@@ -65,7 +72,9 @@ struct rte_event_fp_ops {
 	/**< PMD Tx adapter enqueue same destination function. */
 	event_crypto_adapter_enqueue_t ca_enqueue;
 	/**< PMD Crypto adapter enqueue function. */
-	uintptr_t reserved[6];
+	event_enqueue_queue_burst_t enqueue_new_same_dest;
+	/**< PMD enqueue burst new function to same destination queue. */
+	uintptr_t reserved[5];
 } __rte_cache_aligned;
 
 extern struct rte_event_fp_ops rte_event_fp_ops[RTE_EVENT_MAX_DEVS];
-- 
2.25.1


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

* Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-06-27  9:57 [PATCH 1/2] doc: add enqueue depth for new event type pbhagavatula
  2022-06-27  9:57 ` [PATCH 2/2] eventdev: add function to enq new events to the same queue pbhagavatula
@ 2022-07-11 14:54 ` Jerin Jacob
  2022-07-12 15:01 ` Thomas Monjalon
  2 siblings, 0 replies; 27+ messages in thread
From: Jerin Jacob @ 2022-07-11 14:54 UTC (permalink / raw)
  To: Pavan Nikhilesh; +Cc: Jerin Jacob, Ray Kinsella, dpdk-dev

On Mon, Jun 27, 2022 at 3:29 PM <pbhagavatula@marvell.com> wrote:
>
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> A new field ``max_event_port_enqueue_new_burst`` will be added to the
> structure ``rte_event_dev_info``. The field defines the max enqueue
> burst size of new events (OP_NEW) supported by the underlying event
> device.
>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>


> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 4e5b23c53d..071317e8e3 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -125,3 +125,8 @@ Deprecation Notices
>    applications should be updated to use the ``dmadev`` library instead,
>    with the underlying HW-functionality being provided by the ``ioat`` or
>    ``idxd`` dma drivers
> +
> +* eventdev: The structure ``rte_event_dev_info`` will be extended to include the
> +  max enqueue burst size of new events supported by the underlying event device.
> +  A new field ``max_event_port_enqueue_new_burst`` will be added to the structure
> +  ``rte_event_dev_info`` in DPDK 22.11.
> --
> 2.25.1
>

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

* Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-06-27  9:57 [PATCH 1/2] doc: add enqueue depth for new event type pbhagavatula
  2022-06-27  9:57 ` [PATCH 2/2] eventdev: add function to enq new events to the same queue pbhagavatula
  2022-07-11 14:54 ` [PATCH 1/2] doc: add enqueue depth for new event type Jerin Jacob
@ 2022-07-12 15:01 ` Thomas Monjalon
  2022-07-12 18:11   ` [EXT] " Pavan Nikhilesh Bhagavatula
  2 siblings, 1 reply; 27+ messages in thread
From: Thomas Monjalon @ 2022-07-12 15:01 UTC (permalink / raw)
  To: Pavan Nikhilesh; +Cc: jerinj, Ray Kinsella, dev, pbhagavatula

I'm not your teacher, but please consider Cc'ing people outside of your company.


27/06/2022 11:57, pbhagavatula@marvell.com:
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> A new field ``max_event_port_enqueue_new_burst`` will be added to the
> structure ``rte_event_dev_info``. The field defines the max enqueue
> burst size of new events (OP_NEW) supported by the underlying event
> device.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 4e5b23c53d..071317e8e3 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -125,3 +125,8 @@ Deprecation Notices
>    applications should be updated to use the ``dmadev`` library instead,
>    with the underlying HW-functionality being provided by the ``ioat`` or
>    ``idxd`` dma drivers
> +
> +* eventdev: The structure ``rte_event_dev_info`` will be extended to include the
> +  max enqueue burst size of new events supported by the underlying event device.
> +  A new field ``max_event_port_enqueue_new_burst`` will be added to the structure
> +  ``rte_event_dev_info`` in DPDK 22.11.
> 






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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-12 15:01 ` Thomas Monjalon
@ 2022-07-12 18:11   ` Pavan Nikhilesh Bhagavatula
  2022-07-12 20:47     ` Thomas Monjalon
                       ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-12 18:11 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, mattias.ronnblom, liangma,
	peter.mccarthy, harry.van.haaren, erik.g.carrillo,
	abhinandan.gujjar, jay.jayatheerthan, mdr, anatoly.burakov

+Cc
timothy.mcdaniel@intel.com;
hemant.agrawal@nxp.com;
sachin.saxena@oss.nxp.com;
mattias.ronnblom@ericsson.com;
jerinj@marvell.com;
liangma@liangbit.com;
peter.mccarthy@intel.com;
harry.van.haaren@intel.com;
erik.g.carrillo@intel.com;
abhinandan.gujjar@intel.com;
jay.jayatheerthan@intel.com;
mdr@ashroe.eu;
anatoly.burakov@intel.com;


> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, July 12, 2022 8:31 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>
> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
> 
> External Email
> 
> ----------------------------------------------------------------------
> I'm not your teacher, but please consider Cc'ing people outside of your
> company.
> 

I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's useless for
sending deprecation notices.

Maybe we should update it to include lib/driver maintainers when diff sees deprecation.rst

> 
> 27/06/2022 11:57, pbhagavatula@marvell.com:
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > A new field ``max_event_port_enqueue_new_burst`` will be added to the
> > structure ``rte_event_dev_info``. The field defines the max enqueue
> > burst size of new events (OP_NEW) supported by the underlying event
> > device.
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > index 4e5b23c53d..071317e8e3 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -125,3 +125,8 @@ Deprecation Notices
> >    applications should be updated to use the ``dmadev`` library instead,
> >    with the underlying HW-functionality being provided by the ``ioat`` or
> >    ``idxd`` dma drivers
> > +
> > +* eventdev: The structure ``rte_event_dev_info`` will be extended to
> include the
> > +  max enqueue burst size of new events supported by the underlying
> event device.
> > +  A new field ``max_event_port_enqueue_new_burst`` will be added to
> the structure
> > +  ``rte_event_dev_info`` in DPDK 22.11.
> >
> 
> 
> 
> 


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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-12 18:11   ` [EXT] " Pavan Nikhilesh Bhagavatula
@ 2022-07-12 20:47     ` Thomas Monjalon
  2022-07-13  3:15     ` Hemant Agrawal
  2022-07-13  9:08     ` Mattias Rönnblom
  2 siblings, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2022-07-12 20:47 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, mattias.ronnblom, liangma,
	peter.mccarthy, harry.van.haaren, erik.g.carrillo,
	abhinandan.gujjar, jay.jayatheerthan, mdr, anatoly.burakov

12/07/2022 20:11, Pavan Nikhilesh Bhagavatula:
> +Cc
> timothy.mcdaniel@intel.com;
> hemant.agrawal@nxp.com;
> sachin.saxena@oss.nxp.com;
> mattias.ronnblom@ericsson.com;
> jerinj@marvell.com;
> liangma@liangbit.com;
> peter.mccarthy@intel.com;
> harry.van.haaren@intel.com;
> erik.g.carrillo@intel.com;
> abhinandan.gujjar@intel.com;
> jay.jayatheerthan@intel.com;
> mdr@ashroe.eu;
> anatoly.burakov@intel.com;
> 
> From: Thomas Monjalon <thomas@monjalon.net>
> > I'm not your teacher, but please consider Cc'ing people outside of your
> > company.
> 
> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's useless for
> sending deprecation notices.
> 
> Maybe we should update it to include lib/driver maintainers when diff sees deprecation.rst

It cannot be an automatic process.
You must think who can be interested for each deprecation notice.
3 acks are required, and preferably from different companies.



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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-12 18:11   ` [EXT] " Pavan Nikhilesh Bhagavatula
  2022-07-12 20:47     ` Thomas Monjalon
@ 2022-07-13  3:15     ` Hemant Agrawal
  2022-07-13  9:08     ` Mattias Rönnblom
  2 siblings, 0 replies; 27+ messages in thread
From: Hemant Agrawal @ 2022-07-13  3:15 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Sachin Saxena (OSS),
	mattias.ronnblom, liangma, peter.mccarthy, harry.van.haaren,
	erik.g.carrillo, abhinandan.gujjar, jay.jayatheerthan, mdr,
	anatoly.burakov

> > 27/06/2022 11:57, pbhagavatula@marvell.com:
> > > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >
> > > A new field ``max_event_port_enqueue_new_burst`` will be added to
> > > the structure ``rte_event_dev_info``. The field defines the max
> > > enqueue burst size of new events (OP_NEW) supported by the
> > > underlying event device.
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>


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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-12 18:11   ` [EXT] " Pavan Nikhilesh Bhagavatula
  2022-07-12 20:47     ` Thomas Monjalon
  2022-07-13  3:15     ` Hemant Agrawal
@ 2022-07-13  9:08     ` Mattias Rönnblom
  2022-07-13 10:40       ` Pavan Nikhilesh Bhagavatula
  2 siblings, 1 reply; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-13  9:08 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov

On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> +Cc
> timothy.mcdaniel@intel.com;
> hemant.agrawal@nxp.com;
> sachin.saxena@oss.nxp.com;
> mattias.ronnblom@ericsson.com;
> jerinj@marvell.com;
> liangma@liangbit.com;
> peter.mccarthy@intel.com;
> harry.van.haaren@intel.com;
> erik.g.carrillo@intel.com;
> abhinandan.gujjar@intel.com;
> jay.jayatheerthan@intel.com;
> mdr@ashroe.eu;
> anatoly.burakov@intel.com;
> 
> 
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Tuesday, July 12, 2022 8:31 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>> <pbhagavatula@marvell.com>
>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> I'm not your teacher, but please consider Cc'ing people outside of your
>> company.
>>
> 
> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's useless for
> sending deprecation notices.
> 
> Maybe we should update it to include lib/driver maintainers when diff sees deprecation.rst
> 
>>
>> 27/06/2022 11:57, pbhagavatula@marvell.com:
>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>
>>> A new field ``max_event_port_enqueue_new_burst`` will be added to the
>>> structure ``rte_event_dev_info``. The field defines the max enqueue
>>> burst size of new events (OP_NEW) supported by the underlying event
>>> device.
>>>
>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>> ---
>>>   doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>> b/doc/guides/rel_notes/deprecation.rst
>>> index 4e5b23c53d..071317e8e3 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -125,3 +125,8 @@ Deprecation Notices
>>>     applications should be updated to use the ``dmadev`` library instead,
>>>     with the underlying HW-functionality being provided by the ``ioat`` or
>>>     ``idxd`` dma drivers
>>> +
>>> +* eventdev: The structure ``rte_event_dev_info`` will be extended to
>> include the
>>> +  max enqueue burst size of new events supported by the underlying
>> event device.
>>> +  A new field ``max_event_port_enqueue_new_burst`` will be added to
>> the structure
>>> +  ``rte_event_dev_info`` in DPDK 22.11.
>>>
>>
>>
>>
>>

Why is this needed, or useful? Why isn't called something with 
"enqueue_depth" in it, like the already-present field?

I think I would rather remove all fields related to the max 
enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If 
you have some HW limit on the maximum number of new events it can 
accept, have the driver retry until backpressure occurs.


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-13  9:08     ` Mattias Rönnblom
@ 2022-07-13 10:40       ` Pavan Nikhilesh Bhagavatula
  2022-07-13 12:15         ` Mattias Rönnblom
  0 siblings, 1 reply; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-13 10:40 UTC (permalink / raw)
  To: Mattias Rönnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov



> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Wednesday, July 13, 2022 2:39 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> Monjalon <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; peter.mccarthy@intel.com;
> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> anatoly.burakov@intel.com
> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> > +Cc
> > timothy.mcdaniel@intel.com;
> > hemant.agrawal@nxp.com;
> > sachin.saxena@oss.nxp.com;
> > mattias.ronnblom@ericsson.com;
> > jerinj@marvell.com;
> > liangma@liangbit.com;
> > peter.mccarthy@intel.com;
> > harry.van.haaren@intel.com;
> > erik.g.carrillo@intel.com;
> > abhinandan.gujjar@intel.com;
> > jay.jayatheerthan@intel.com;
> > mdr@ashroe.eu;
> > anatoly.burakov@intel.com;
> >
> >
> >> -----Original Message-----
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >> Sent: Tuesday, July 12, 2022 8:31 PM
> >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> >> <pbhagavatula@marvell.com>
> >> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> >>
> >> External Email
> >>
> >> ----------------------------------------------------------------------
> >> I'm not your teacher, but please consider Cc'ing people outside of your
> >> company.
> >>
> >
> > I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's
> useless for
> > sending deprecation notices.
> >
> > Maybe we should update it to include lib/driver maintainers when diff sees
> deprecation.rst
> >
> >>
> >> 27/06/2022 11:57, pbhagavatula@marvell.com:
> >>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>
> >>> A new field ``max_event_port_enqueue_new_burst`` will be added to
> the
> >>> structure ``rte_event_dev_info``. The field defines the max enqueue
> >>> burst size of new events (OP_NEW) supported by the underlying event
> >>> device.
> >>>
> >>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>> ---
> >>>   doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>   1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >> b/doc/guides/rel_notes/deprecation.rst
> >>> index 4e5b23c53d..071317e8e3 100644
> >>> --- a/doc/guides/rel_notes/deprecation.rst
> >>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>> @@ -125,3 +125,8 @@ Deprecation Notices
> >>>     applications should be updated to use the ``dmadev`` library instead,
> >>>     with the underlying HW-functionality being provided by the ``ioat`` or
> >>>     ``idxd`` dma drivers
> >>> +
> >>> +* eventdev: The structure ``rte_event_dev_info`` will be extended to
> >> include the
> >>> +  max enqueue burst size of new events supported by the underlying
> >> event device.
> >>> +  A new field ``max_event_port_enqueue_new_burst`` will be added
> to
> >> the structure
> >>> +  ``rte_event_dev_info`` in DPDK 22.11.
> >>>
> >>
> >>
> >>
> >>
> 
> Why is this needed, or useful? Why isn't called something with
> "enqueue_depth" in it, like the already-present field?
> 

This is for a case where enqueues with OP_FORWARD/OP_RELEASE only supports
enqueue depth of 1.
Where as OP_NEW supports higher burst size.

This is already tied into capability description :
 
#define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
/**< Event device is capable of operating in burst mode for enqueue(forward,
 * release) and dequeue operation. If this capability is not set, application
 * still uses the rte_event_dequeue_burst() and rte_event_enqueue_burst() but
 * PMD accepts only one event at a time.
 *
 * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
 */

> I think I would rather remove all fields related to the max
> enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If
> you have some HW limit on the maximum number of new events it can
> accept, have the driver retry until backpressure occurs.


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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-13 10:40       ` Pavan Nikhilesh Bhagavatula
@ 2022-07-13 12:15         ` Mattias Rönnblom
  2022-07-14  6:32           ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-13 12:15 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov

On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> 
> 
>> -----Original Message-----
>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Sent: Wednesday, July 13, 2022 2:39 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>> Monjalon <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; peter.mccarthy@intel.com;
>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>> anatoly.burakov@intel.com
>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>> type
>>
>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
>>> +Cc
>>> timothy.mcdaniel@intel.com;
>>> hemant.agrawal@nxp.com;
>>> sachin.saxena@oss.nxp.com;
>>> mattias.ronnblom@ericsson.com;
>>> jerinj@marvell.com;
>>> liangma@liangbit.com;
>>> peter.mccarthy@intel.com;
>>> harry.van.haaren@intel.com;
>>> erik.g.carrillo@intel.com;
>>> abhinandan.gujjar@intel.com;
>>> jay.jayatheerthan@intel.com;
>>> mdr@ashroe.eu;
>>> anatoly.burakov@intel.com;
>>>
>>>
>>>> -----Original Message-----
>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>> Sent: Tuesday, July 12, 2022 8:31 PM
>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>>>> <pbhagavatula@marvell.com>
>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>> type
>>>>
>>>> External Email
>>>>
>>>> ----------------------------------------------------------------------
>>>> I'm not your teacher, but please consider Cc'ing people outside of your
>>>> company.
>>>>
>>>
>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's
>> useless for
>>> sending deprecation notices.
>>>
>>> Maybe we should update it to include lib/driver maintainers when diff sees
>> deprecation.rst
>>>
>>>>
>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>
>>>>> A new field ``max_event_port_enqueue_new_burst`` will be added to
>> the
>>>>> structure ``rte_event_dev_info``. The field defines the max enqueue
>>>>> burst size of new events (OP_NEW) supported by the underlying event
>>>>> device.
>>>>>
>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>> ---
>>>>>    doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>    1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>> index 4e5b23c53d..071317e8e3 100644
>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
>>>>>      applications should be updated to use the ``dmadev`` library instead,
>>>>>      with the underlying HW-functionality being provided by the ``ioat`` or
>>>>>      ``idxd`` dma drivers
>>>>> +
>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be extended to
>>>> include the
>>>>> +  max enqueue burst size of new events supported by the underlying
>>>> event device.
>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be added
>> to
>>>> the structure
>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
>>>>>
>>>>
>>>>
>>>>
>>>>
>>
>> Why is this needed, or useful? Why isn't called something with
>> "enqueue_depth" in it, like the already-present field?
>>
> 
> This is for a case where enqueues with OP_FORWARD/OP_RELEASE only supports
> enqueue depth of 1.

I assume it's for other cases as well? Any case when the event device 
has a max forward enqueue depth != max new enqueue depth?

I don't see why an event device would have such low max limit on the 
number events enqueued. If the underlying hardware has some limitations, 
why not let the driver loop until back pressure occurs? Then you can 
amortize the function call overhead and potentially other software 
operations (credit system management etc) over multiple events. Plus, 
the application doesn't need a special case for new events versus 
forward type events, or this-versus-that event device.

> Where as OP_NEW supports higher burst size.
> 
> This is already tied into capability description :
>   
> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> /**< Event device is capable of operating in burst mode for enqueue(forward,
>   * release) and dequeue operation. If this capability is not set, application
>   * still uses the rte_event_dequeue_burst() and rte_event_enqueue_burst() but
>   * PMD accepts only one event at a time.
>   *
>   * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
>   */
> 
>> I think I would rather remove all fields related to the max
>> enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If
>> you have some HW limit on the maximum number of new events it can
>> accept, have the driver retry until backpressure occurs.
> 


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-13 12:15         ` Mattias Rönnblom
@ 2022-07-14  6:32           ` Pavan Nikhilesh Bhagavatula
  2022-07-14  9:45             ` Van Haaren, Harry
  2022-07-14 10:43             ` Mattias Rönnblom
  0 siblings, 2 replies; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-14  6:32 UTC (permalink / raw)
  To: Mattias Rönnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov



> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Wednesday, July 13, 2022 5:45 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> Monjalon <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; peter.mccarthy@intel.com;
> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> anatoly.burakov@intel.com
> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> >
> >
> >> -----Original Message-----
> >> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >> Sent: Wednesday, July 13, 2022 2:39 PM
> >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> >> Monjalon <thomas@monjalon.net>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> Hemant
> >> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >> liangma@liangbit.com; peter.mccarthy@intel.com;
> >> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >> anatoly.burakov@intel.com
> >> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >> type
> >>
> >> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> >>> +Cc
> >>> timothy.mcdaniel@intel.com;
> >>> hemant.agrawal@nxp.com;
> >>> sachin.saxena@oss.nxp.com;
> >>> mattias.ronnblom@ericsson.com;
> >>> jerinj@marvell.com;
> >>> liangma@liangbit.com;
> >>> peter.mccarthy@intel.com;
> >>> harry.van.haaren@intel.com;
> >>> erik.g.carrillo@intel.com;
> >>> abhinandan.gujjar@intel.com;
> >>> jay.jayatheerthan@intel.com;
> >>> mdr@ashroe.eu;
> >>> anatoly.burakov@intel.com;
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>> Sent: Tuesday, July 12, 2022 8:31 PM
> >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> >>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> >>>> <pbhagavatula@marvell.com>
> >>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> >> type
> >>>>
> >>>> External Email
> >>>>
> >>>> ----------------------------------------------------------------------
> >>>> I'm not your teacher, but please consider Cc'ing people outside of your
> >>>> company.
> >>>>
> >>>
> >>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's
> >> useless for
> >>> sending deprecation notices.
> >>>
> >>> Maybe we should update it to include lib/driver maintainers when diff
> sees
> >> deprecation.rst
> >>>
> >>>>
> >>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
> >>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>
> >>>>> A new field ``max_event_port_enqueue_new_burst`` will be added
> to
> >> the
> >>>>> structure ``rte_event_dev_info``. The field defines the max enqueue
> >>>>> burst size of new events (OP_NEW) supported by the underlying
> event
> >>>>> device.
> >>>>>
> >>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>> ---
> >>>>>    doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>    1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>> index 4e5b23c53d..071317e8e3 100644
> >>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>> @@ -125,3 +125,8 @@ Deprecation Notices
> >>>>>      applications should be updated to use the ``dmadev`` library
> instead,
> >>>>>      with the underlying HW-functionality being provided by the ``ioat``
> or
> >>>>>      ``idxd`` dma drivers
> >>>>> +
> >>>>> +* eventdev: The structure ``rte_event_dev_info`` will be extended
> to
> >>>> include the
> >>>>> +  max enqueue burst size of new events supported by the
> underlying
> >>>> event device.
> >>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
> added
> >> to
> >>>> the structure
> >>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>
> >> Why is this needed, or useful? Why isn't called something with
> >> "enqueue_depth" in it, like the already-present field?
> >>
> >
> > This is for a case where enqueues with OP_FORWARD/OP_RELEASE only
> supports
> > enqueue depth of 1.
> 
> I assume it's for other cases as well? Any case when the event device
> has a max forward enqueue depth != max new enqueue depth?
> 

Yes, generally new events have much more flexibility than forwards event.

> I don't see why an event device would have such low max limit on the
> number events enqueued. 

It depends on the number of scheduling contexts a given event port can track.
Anyway this would align to the current existing feature definitions. The existing
API only defines the enqueue size of fwd and release events i.e. scheduling contexts
already tracked by event device.
NEW is always a special case as we are adding new scheduling contexts to the event 
device, the idea of this patch is to specify that NEW events wouldn’t have the same
restrictions of fwd/release events.

This would also allow us to craft optimized APIs such as 
https://patches.dpdk.org/project/dpdk/patch/20220627095702.8047-2-pbhagavatula@marvell.com/


>If the underlying hardware has some limitations,
> why not let the driver loop until back pressure occurs? Then you can
> amortize the function call overhead and potentially other software
> operations (credit system management etc) over multiple events. Plus,
> the application doesn't need a special case for new events versus
> forward type events, or this-versus-that event device.
> 
> > Where as OP_NEW supports higher burst size.
> >
> > This is already tied into capability description :
> >
> > #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> > /**< Event device is capable of operating in burst mode for
> enqueue(forward,
> >   * release) and dequeue operation. If this capability is not set, application
> >   * still uses the rte_event_dequeue_burst() and
> rte_event_enqueue_burst() but
> >   * PMD accepts only one event at a time.
> >   *
> >   * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
> >   */
> >
> >> I think I would rather remove all fields related to the max
> >> enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If
> >> you have some HW limit on the maximum number of new events it can
> >> accept, have the driver retry until backpressure occurs.
> >


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14  6:32           ` Pavan Nikhilesh Bhagavatula
@ 2022-07-14  9:45             ` Van Haaren, Harry
  2022-07-14 10:53               ` Mattias Rönnblom
  2022-07-14 10:43             ` Mattias Rönnblom
  1 sibling, 1 reply; 27+ messages in thread
From: Van Haaren, Harry @ 2022-07-14  9:45 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, mattias.ronnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly

> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Sent: Thursday, July 14, 2022 7:33 AM
> To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella <mdr@ashroe.eu>;
> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van Haaren,
> Harry <harry.van.haaren@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
> 
> 
> 
> > -----Original Message-----
> > From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> > Sent: Wednesday, July 13, 2022 5:45 PM
> > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> > Monjalon <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
> > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > liangma@liangbit.com; peter.mccarthy@intel.com;
> > harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> > abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> > anatoly.burakov@intel.com
> > Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> > type
> >
> > On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> > >> Sent: Wednesday, July 13, 2022 2:39 PM
> > >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> > >> Monjalon <thomas@monjalon.net>
> > >> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > >> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> > Hemant
> > >> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > >> liangma@liangbit.com; peter.mccarthy@intel.com;
> > >> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> > >> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> > >> anatoly.burakov@intel.com
> > >> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> > event
> > >> type
> > >>
> > >> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> > >>> +Cc
> > >>> timothy.mcdaniel@intel.com;
> > >>> hemant.agrawal@nxp.com;
> > >>> sachin.saxena@oss.nxp.com;
> > >>> mattias.ronnblom@ericsson.com;
> > >>> jerinj@marvell.com;
> > >>> liangma@liangbit.com;
> > >>> peter.mccarthy@intel.com;
> > >>> harry.van.haaren@intel.com;
> > >>> erik.g.carrillo@intel.com;
> > >>> abhinandan.gujjar@intel.com;
> > >>> jay.jayatheerthan@intel.com;
> > >>> mdr@ashroe.eu;
> > >>> anatoly.burakov@intel.com;
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Thomas Monjalon <thomas@monjalon.net>
> > >>>> Sent: Tuesday, July 12, 2022 8:31 PM
> > >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > >>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > >>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> > >>>> <pbhagavatula@marvell.com>
> > >>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> > >> type
> > >>>>
> > >>>> External Email
> > >>>>
> > >>>> ----------------------------------------------------------------------
> > >>>> I'm not your teacher, but please consider Cc'ing people outside of your
> > >>>> company.
> > >>>>
> > >>>
> > >>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's
> > >> useless for
> > >>> sending deprecation notices.
> > >>>
> > >>> Maybe we should update it to include lib/driver maintainers when diff
> > sees
> > >> deprecation.rst
> > >>>
> > >>>>
> > >>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
> > >>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >>>>>
> > >>>>> A new field ``max_event_port_enqueue_new_burst`` will be added
> > to
> > >> the
> > >>>>> structure ``rte_event_dev_info``. The field defines the max enqueue
> > >>>>> burst size of new events (OP_NEW) supported by the underlying
> > event
> > >>>>> device.
> > >>>>>
> > >>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >>>>> ---
> > >>>>>    doc/guides/rel_notes/deprecation.rst | 5 +++++
> > >>>>>    1 file changed, 5 insertions(+)
> > >>>>>
> > >>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> > >>>> b/doc/guides/rel_notes/deprecation.rst
> > >>>>> index 4e5b23c53d..071317e8e3 100644
> > >>>>> --- a/doc/guides/rel_notes/deprecation.rst
> > >>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> > >>>>> @@ -125,3 +125,8 @@ Deprecation Notices
> > >>>>>      applications should be updated to use the ``dmadev`` library
> > instead,
> > >>>>>      with the underlying HW-functionality being provided by the ``ioat``
> > or
> > >>>>>      ``idxd`` dma drivers
> > >>>>> +
> > >>>>> +* eventdev: The structure ``rte_event_dev_info`` will be extended
> > to
> > >>>> include the
> > >>>>> +  max enqueue burst size of new events supported by the
> > underlying
> > >>>> event device.
> > >>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
> > added
> > >> to
> > >>>> the structure
> > >>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
> > >>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>
> > >> Why is this needed, or useful? Why isn't called something with
> > >> "enqueue_depth" in it, like the already-present field?
> > >>
> > >
> > > This is for a case where enqueues with OP_FORWARD/OP_RELEASE only
> > supports
> > > enqueue depth of 1.
> >
> > I assume it's for other cases as well? Any case when the event device
> > has a max forward enqueue depth != max new enqueue depth?
> >
> 
> Yes, generally new events have much more flexibility than forwards event.
> 
> > I don't see why an event device would have such low max limit on the
> > number events enqueued.
> 
> It depends on the number of scheduling contexts a given event port can track.
> Anyway this would align to the current existing feature definitions. The existing
> API only defines the enqueue size of fwd and release events i.e. scheduling contexts
> already tracked by event device.
> NEW is always a special case as we are adding new scheduling contexts to the event
> device, the idea of this patch is to specify that NEW events wouldn’t have the same
> restrictions of fwd/release events.
> 
> This would also allow us to craft optimized APIs such as
> https://patches.dpdk.org/project/dpdk/patch/20220627095702.8047-2-
> pbhagavatula@marvell.com/

I've reviewed the above; I'm not in favour of adding even more APIs to the Eventdev level.
Adding even more enq APIs just overloads applications with options; today we already have
multiple APIs;
rte_event_enqueue_burst()
rte_event_enqueue_new_burst()
rte_event_enqueue_forward_burst()

Now the suggestion is to add another one,
rte_event_enqueue_new_queue_burst()?

For an Ethdev analogy: we have rx_burst() and tx_burst(). There is no tx_to_same_dest_ip_burst().

The driver already has all knowledge required if "all events going to same destination",
so it can optimize for that case already internally. I think Mattias was asking similar questions,
around PMD having enough info today already.

Pushing more APIs and complexity to Application level doesn't seem a good direction to me.


> >If the underlying hardware has some limitations,
> > why not let the driver loop until back pressure occurs? Then you can
> > amortize the function call overhead and potentially other software
> > operations (credit system management etc) over multiple events. Plus,
> > the application doesn't need a special case for new events versus
> > forward type events, or this-versus-that event device.
> >
> > > Where as OP_NEW supports higher burst size.
> > >
> > > This is already tied into capability description :
> > >
> > > #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> > > /**< Event device is capable of operating in burst mode for
> > enqueue(forward,
> > >   * release) and dequeue operation. If this capability is not set, application
> > >   * still uses the rte_event_dequeue_burst() and
> > rte_event_enqueue_burst() but
> > >   * PMD accepts only one event at a time.
> > >   *
> > >   * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
> > >   */
> > >
> > >> I think I would rather remove all fields related to the max
> > >> enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If
> > >> you have some HW limit on the maximum number of new events it can
> > >> accept, have the driver retry until backpressure occurs.
> > >


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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14  6:32           ` Pavan Nikhilesh Bhagavatula
  2022-07-14  9:45             ` Van Haaren, Harry
@ 2022-07-14 10:43             ` Mattias Rönnblom
  2022-07-14 16:42               ` Pavan Nikhilesh Bhagavatula
  1 sibling, 1 reply; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-14 10:43 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov

On 2022-07-14 08:32, Pavan Nikhilesh Bhagavatula wrote:
> 
> 
>> -----Original Message-----
>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Sent: Wednesday, July 13, 2022 5:45 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>> Monjalon <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; peter.mccarthy@intel.com;
>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>> anatoly.burakov@intel.com
>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>> type
>>
>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>> Sent: Wednesday, July 13, 2022 2:39 PM
>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>>>> Monjalon <thomas@monjalon.net>
>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>> Hemant
>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>> anatoly.burakov@intel.com
>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>> event
>>>> type
>>>>
>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
>>>>> +Cc
>>>>> timothy.mcdaniel@intel.com;
>>>>> hemant.agrawal@nxp.com;
>>>>> sachin.saxena@oss.nxp.com;
>>>>> mattias.ronnblom@ericsson.com;
>>>>> jerinj@marvell.com;
>>>>> liangma@liangbit.com;
>>>>> peter.mccarthy@intel.com;
>>>>> harry.van.haaren@intel.com;
>>>>> erik.g.carrillo@intel.com;
>>>>> abhinandan.gujjar@intel.com;
>>>>> jay.jayatheerthan@intel.com;
>>>>> mdr@ashroe.eu;
>>>>> anatoly.burakov@intel.com;
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>>>>>> <pbhagavatula@marvell.com>
>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>>>> type
>>>>>>
>>>>>> External Email
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>> I'm not your teacher, but please consider Cc'ing people outside of your
>>>>>> company.
>>>>>>
>>>>>
>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's
>>>> useless for
>>>>> sending deprecation notices.
>>>>>
>>>>> Maybe we should update it to include lib/driver maintainers when diff
>> sees
>>>> deprecation.rst
>>>>>
>>>>>>
>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>
>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be added
>> to
>>>> the
>>>>>>> structure ``rte_event_dev_info``. The field defines the max enqueue
>>>>>>> burst size of new events (OP_NEW) supported by the underlying
>> event
>>>>>>> device.
>>>>>>>
>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>> ---
>>>>>>>     doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>>     1 file changed, 5 insertions(+)
>>>>>>>
>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>> index 4e5b23c53d..071317e8e3 100644
>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
>>>>>>>       applications should be updated to use the ``dmadev`` library
>> instead,
>>>>>>>       with the underlying HW-functionality being provided by the ``ioat``
>> or
>>>>>>>       ``idxd`` dma drivers
>>>>>>> +
>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be extended
>> to
>>>>>> include the
>>>>>>> +  max enqueue burst size of new events supported by the
>> underlying
>>>>>> event device.
>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
>> added
>>>> to
>>>>>> the structure
>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>> Why is this needed, or useful? Why isn't called something with
>>>> "enqueue_depth" in it, like the already-present field?
>>>>
>>>
>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE only
>> supports
>>> enqueue depth of 1.
>>
>> I assume it's for other cases as well? Any case when the event device
>> has a max forward enqueue depth != max new enqueue depth?
>>
> 
> Yes, generally new events have much more flexibility than forwards event.
> 
>> I don't see why an event device would have such low max limit on the
>> number events enqueued.
> 
> It depends on the number of scheduling contexts a given event port can track.

I have no idea what a "scheduling context" is (it's not something 
related to the Eventdev APIs), but if you have a shortage of them (for 
the moment, or always), the driver would just return a short count from 
the enqueue burst function. What use has the application of knowing that 
the event device can accept at most a certain number of events?

> Anyway this would align to the current existing feature definitions. The existing
> API only defines the enqueue size of fwd and release events i.e. scheduling contexts
> already tracked by event device.

The documentation of max_event_port_enqueue_depth says:
"Maximum number of events can be enqueued at a time from an event port 
by this device."

 From what I can tell, it says nothing about new versus forward, or 
release type events.

> NEW is always a special case as we are adding new scheduling contexts to the event
> device, the idea of this patch is to specify that NEW events wouldn’t have the same
> restrictions of fwd/release events.
> 
> This would also allow us to craft optimized APIs such as
> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-ef12ffaf4bdb568f&q=1&e=f0a92ad3-1167-448d-be14-0987e939f019&u=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20220627095702.8047-2-pbhagavatula%40marvell.com%2F
> 
> 
>> If the underlying hardware has some limitations,
>> why not let the driver loop until back pressure occurs? Then you can

You didn't answer this question. Why not let the driver loop, until you 
for some reason or the other can't accept more events?

>> amortize the function call overhead and potentially other software
>> operations (credit system management etc) over multiple events. Plus,
>> the application doesn't need a special case for new events versus
>> forward type events, or this-versus-that event device.
>>
>>> Where as OP_NEW supports higher burst size.
>>>
>>> This is already tied into capability description :
>>>
>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
>>> /**< Event device is capable of operating in burst mode for
>> enqueue(forward,
>>>    * release) and dequeue operation. If this capability is not set, application
>>>    * still uses the rte_event_dequeue_burst() and
>> rte_event_enqueue_burst() but
>>>    * PMD accepts only one event at a time.
>>>    *
>>>    * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
>>>    */
>>>
>>>> I think I would rather remove all fields related to the max
>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If
>>>> you have some HW limit on the maximum number of new events it can
>>>> accept, have the driver retry until backpressure occurs.
>>>
> 


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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14  9:45             ` Van Haaren, Harry
@ 2022-07-14 10:53               ` Mattias Rönnblom
  2022-07-14 14:44                 ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-14 10:53 UTC (permalink / raw)
  To: Van Haaren, Harry, Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly

On 2022-07-14 11:45, Van Haaren, Harry wrote:
>> -----Original Message-----
>> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>> Sent: Thursday, July 14, 2022 7:33 AM
>> To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
>> <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella <mdr@ashroe.eu>;
>> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van Haaren,
>> Harry <harry.van.haaren@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
>> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
>> <jay.jayatheerthan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
>> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
>>
>>
>>
>>> -----Original Message-----
>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>> Sent: Wednesday, July 13, 2022 5:45 PM
>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>>> Monjalon <thomas@monjalon.net>
>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>> anatoly.burakov@intel.com
>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>>> type
>>>
>>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>> Sent: Wednesday, July 13, 2022 2:39 PM
>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>>>>> Monjalon <thomas@monjalon.net>
>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>>> Hemant
>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>> anatoly.burakov@intel.com
>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>>> event
>>>>> type
>>>>>
>>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
>>>>>> +Cc
>>>>>> timothy.mcdaniel@intel.com;
>>>>>> hemant.agrawal@nxp.com;
>>>>>> sachin.saxena@oss.nxp.com;
>>>>>> mattias.ronnblom@ericsson.com;
>>>>>> jerinj@marvell.com;
>>>>>> liangma@liangbit.com;
>>>>>> peter.mccarthy@intel.com;
>>>>>> harry.van.haaren@intel.com;
>>>>>> erik.g.carrillo@intel.com;
>>>>>> abhinandan.gujjar@intel.com;
>>>>>> jay.jayatheerthan@intel.com;
>>>>>> mdr@ashroe.eu;
>>>>>> anatoly.burakov@intel.com;
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>>>>>>> <pbhagavatula@marvell.com>
>>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>>>>> type
>>>>>>>
>>>>>>> External Email
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>> I'm not your teacher, but please consider Cc'ing people outside of your
>>>>>>> company.
>>>>>>>
>>>>>>
>>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like it's
>>>>> useless for
>>>>>> sending deprecation notices.
>>>>>>
>>>>>> Maybe we should update it to include lib/driver maintainers when diff
>>> sees
>>>>> deprecation.rst
>>>>>>
>>>>>>>
>>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
>>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>>
>>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be added
>>> to
>>>>> the
>>>>>>>> structure ``rte_event_dev_info``. The field defines the max enqueue
>>>>>>>> burst size of new events (OP_NEW) supported by the underlying
>>> event
>>>>>>>> device.
>>>>>>>>
>>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>> ---
>>>>>>>>     doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>>>     1 file changed, 5 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> index 4e5b23c53d..071317e8e3 100644
>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
>>>>>>>>       applications should be updated to use the ``dmadev`` library
>>> instead,
>>>>>>>>       with the underlying HW-functionality being provided by the ``ioat``
>>> or
>>>>>>>>       ``idxd`` dma drivers
>>>>>>>> +
>>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be extended
>>> to
>>>>>>> include the
>>>>>>>> +  max enqueue burst size of new events supported by the
>>> underlying
>>>>>>> event device.
>>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
>>> added
>>>>> to
>>>>>>> the structure
>>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>> Why is this needed, or useful? Why isn't called something with
>>>>> "enqueue_depth" in it, like the already-present field?
>>>>>
>>>>
>>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE only
>>> supports
>>>> enqueue depth of 1.
>>>
>>> I assume it's for other cases as well? Any case when the event device
>>> has a max forward enqueue depth != max new enqueue depth?
>>>
>>
>> Yes, generally new events have much more flexibility than forwards event.
>>
>>> I don't see why an event device would have such low max limit on the
>>> number events enqueued.
>>
>> It depends on the number of scheduling contexts a given event port can track.
>> Anyway this would align to the current existing feature definitions. The existing
>> API only defines the enqueue size of fwd and release events i.e. scheduling contexts
>> already tracked by event device.
>> NEW is always a special case as we are adding new scheduling contexts to the event
>> device, the idea of this patch is to specify that NEW events wouldn’t have the same
>> restrictions of fwd/release events.
>>
>> This would also allow us to craft optimized APIs such as
>> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-6bd5fd62af0f8992&q=1&e=c4f1686f-725e-48bf-aa62-069489e74009&u=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20220627095702.8047-2-
>> pbhagavatula@marvell.com/
> 
> I've reviewed the above; I'm not in favour of adding even more APIs to the Eventdev level.
> Adding even more enq APIs just overloads applications with options; today we already have
> multiple APIs;
> rte_event_enqueue_burst()
> rte_event_enqueue_new_burst()
> rte_event_enqueue_forward_burst()
> 
> Now the suggestion is to add another one,
> rte_event_enqueue_new_queue_burst()?
> 

Why not three new ones? And why not also 
rte_event_queue(_new_|_forward_|)(_queue_|)priority_burst()?

Plus maybe you want functions that enqueue to the same flow id as well?

It's like you can almost hear the combinatorial explosion go off. :)

Btw, isn't this the problem that the event vector functionality is 
supposed to solve? Enqueue many similar events to the same destination.

> For an Ethdev analogy: we have rx_burst() and tx_burst(). There is no tx_to_same_dest_ip_burst().
> 
> The driver already has all knowledge required if "all events going to same destination",
> so it can optimize for that case already internally. I think Mattias was asking similar questions,
> around PMD having enough info today already.
> 
> Pushing more APIs and complexity to Application level doesn't seem a good direction to me.
> 
> 
>>> If the underlying hardware has some limitations,
>>> why not let the driver loop until back pressure occurs? Then you can
>>> amortize the function call overhead and potentially other software
>>> operations (credit system management etc) over multiple events. Plus,
>>> the application doesn't need a special case for new events versus
>>> forward type events, or this-versus-that event device.
>>>
>>>> Where as OP_NEW supports higher burst size.
>>>>
>>>> This is already tied into capability description :
>>>>
>>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
>>>> /**< Event device is capable of operating in burst mode for
>>> enqueue(forward,
>>>>    * release) and dequeue operation. If this capability is not set, application
>>>>    * still uses the rte_event_dequeue_burst() and
>>> rte_event_enqueue_burst() but
>>>>    * PMD accepts only one event at a time.
>>>>    *
>>>>    * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
>>>>    */
>>>>
>>>>> I think I would rather remove all fields related to the max
>>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I see. If
>>>>> you have some HW limit on the maximum number of new events it can
>>>>> accept, have the driver retry until backpressure occurs.
>>>>
> 


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 10:53               ` Mattias Rönnblom
@ 2022-07-14 14:44                 ` Pavan Nikhilesh Bhagavatula
  2022-07-15  7:43                   ` Mattias Rönnblom
  0 siblings, 1 reply; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-14 14:44 UTC (permalink / raw)
  To: Mattias Rönnblom, Van Haaren, Harry, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly



> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Thursday, July 14, 2022 4:24 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Pavan Nikhilesh
> Bhagavatula <pbhagavatula@marvell.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; McDaniel, Timothy
> <timothy.mcdaniel@intel.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> On 2022-07-14 11:45, Van Haaren, Harry wrote:
> >> -----Original Message-----
> >> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> >> Sent: Thursday, July 14, 2022 7:33 AM
> >> To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas
> Monjalon
> >> <thomas@monjalon.net>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>;
> >> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
> Hemant
> >> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> Van Haaren,
> >> Harry <harry.van.haaren@intel.com>; Carrillo, Erik G
> <erik.g.carrillo@intel.com>;
> >> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> >> <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> >> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>> Sent: Wednesday, July 13, 2022 5:45 PM
> >>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> >>> Monjalon <thomas@monjalon.net>
> >>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> Hemant
> >>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >>> liangma@liangbit.com; peter.mccarthy@intel.com;
> >>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >>> anatoly.burakov@intel.com
> >>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >>> type
> >>>
> >>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>>> Sent: Wednesday, July 13, 2022 2:39 PM
> >>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> Thomas
> >>>>> Monjalon <thomas@monjalon.net>
> >>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> >>> Hemant
> >>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
> >>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >>>>> anatoly.burakov@intel.com
> >>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >>> event
> >>>>> type
> >>>>>
> >>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> >>>>>> +Cc
> >>>>>> timothy.mcdaniel@intel.com;
> >>>>>> hemant.agrawal@nxp.com;
> >>>>>> sachin.saxena@oss.nxp.com;
> >>>>>> mattias.ronnblom@ericsson.com;
> >>>>>> jerinj@marvell.com;
> >>>>>> liangma@liangbit.com;
> >>>>>> peter.mccarthy@intel.com;
> >>>>>> harry.van.haaren@intel.com;
> >>>>>> erik.g.carrillo@intel.com;
> >>>>>> abhinandan.gujjar@intel.com;
> >>>>>> jay.jayatheerthan@intel.com;
> >>>>>> mdr@ashroe.eu;
> >>>>>> anatoly.burakov@intel.com;
> >>>>>>
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
> >>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> >>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> >>>>>>> <pbhagavatula@marvell.com>
> >>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >>>>> type
> >>>>>>>
> >>>>>>> External Email
> >>>>>>>
> >>>>>>> ----------------------------------------------------------------------
> >>>>>>> I'm not your teacher, but please consider Cc'ing people outside of
> your
> >>>>>>> company.
> >>>>>>>
> >>>>>>
> >>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like
> it's
> >>>>> useless for
> >>>>>> sending deprecation notices.
> >>>>>>
> >>>>>> Maybe we should update it to include lib/driver maintainers when
> diff
> >>> sees
> >>>>> deprecation.rst
> >>>>>>
> >>>>>>>
> >>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
> >>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>>>
> >>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be
> added
> >>> to
> >>>>> the
> >>>>>>>> structure ``rte_event_dev_info``. The field defines the max
> enqueue
> >>>>>>>> burst size of new events (OP_NEW) supported by the underlying
> >>> event
> >>>>>>>> device.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>>> ---
> >>>>>>>>     doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>>>>     1 file changed, 5 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> index 4e5b23c53d..071317e8e3 100644
> >>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
> >>>>>>>>       applications should be updated to use the ``dmadev`` library
> >>> instead,
> >>>>>>>>       with the underlying HW-functionality being provided by the
> ``ioat``
> >>> or
> >>>>>>>>       ``idxd`` dma drivers
> >>>>>>>> +
> >>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be
> extended
> >>> to
> >>>>>>> include the
> >>>>>>>> +  max enqueue burst size of new events supported by the
> >>> underlying
> >>>>>>> event device.
> >>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
> >>> added
> >>>>> to
> >>>>>>> the structure
> >>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>> Why is this needed, or useful? Why isn't called something with
> >>>>> "enqueue_depth" in it, like the already-present field?
> >>>>>
> >>>>
> >>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE
> only
> >>> supports
> >>>> enqueue depth of 1.
> >>>
> >>> I assume it's for other cases as well? Any case when the event device
> >>> has a max forward enqueue depth != max new enqueue depth?
> >>>
> >>
> >> Yes, generally new events have much more flexibility than forwards
> event.
> >>
> >>> I don't see why an event device would have such low max limit on the
> >>> number events enqueued.
> >>
> >> It depends on the number of scheduling contexts a given event port can
> track.
> >> Anyway this would align to the current existing feature definitions. The
> existing
> >> API only defines the enqueue size of fwd and release events i.e.
> scheduling contexts
> >> already tracked by event device.
> >> NEW is always a special case as we are adding new scheduling contexts to
> the event
> >> device, the idea of this patch is to specify that NEW events wouldn’t have
> the same
> >> restrictions of fwd/release events.
> >>
> >> This would also allow us to craft optimized APIs such as
> >> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-2D313273af-
> 2D454445555731-2D6bd5fd62af0f8992-26q-3D1-26e-3Dc4f1686f-2D725e-
> 2D48bf-2Daa62-2D069489e74009-26u-3Dhttps-253A-252F-
> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
> 252F20220627095702.8047-2D2-
> 2D&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=-Mr6gUdwMnOZbXSlWeHhYpTVt7UdA-
> VHDmIC_MuUjne3U5nqwFeoOatsEX10pzow&s=Khz6GF4271RAiO7-
> 5BY1J2u0BvT8CwWvfwvRSnzeeeM&e=
> >> pbhagavatula@marvell.com/
> >
> > I've reviewed the above; I'm not in favour of adding even more APIs to the
> Eventdev level.
> > Adding even more enq APIs just overloads applications with options; today
> we already have
> > multiple APIs;
> > rte_event_enqueue_burst()
> > rte_event_enqueue_new_burst()
> > rte_event_enqueue_forward_burst()
> >
> > Now the suggestion is to add another one,
> > rte_event_enqueue_new_queue_burst()?
> >
> 
> Why not three new ones? And why not also
> rte_event_queue(_new_|_forward_|)(_queue_|)priority_burst()?
> 
> Plus maybe you want functions that enqueue to the same flow id as well?
> 
> It's like you can almost hear the combinatorial explosion go off. :)
> 
> Btw, isn't this the problem that the event vector functionality is
> supposed to solve? Enqueue many similar events to the same destination.

Maybe we could move this to rte_event_enqueue_new_burst() by adding an
additional flags param?

This is already done for an existing API rte_event_eth_tx_adapter_enqueue
where flags is used to signify same Tx queue destination.

* @param flags
 *  RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_ flags.
 *  #RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST signifies that all the packets
 *  which are enqueued are destined for the same Ethernet port & Tx queue.
static inline uint16_t
rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
				uint8_t port_id,
				struct rte_event ev[],
				uint16_t nb_events,
				const uint8_t flags)



> 
> > For an Ethdev analogy: we have rx_burst() and tx_burst(). There is no
> tx_to_same_dest_ip_burst().
> >
> > The driver already has all knowledge required if "all events going to same
> destination",
> > so it can optimize for that case already internally. I think Mattias was asking
> similar questions,
> > around PMD having enough info today already.
> >
> > Pushing more APIs and complexity to Application level doesn't seem a good
> direction to me.
> >
> >
> >>> If the underlying hardware has some limitations,
> >>> why not let the driver loop until back pressure occurs? Then you can
> >>> amortize the function call overhead and potentially other software
> >>> operations (credit system management etc) over multiple events. Plus,
> >>> the application doesn't need a special case for new events versus
> >>> forward type events, or this-versus-that event device.
> >>>
> >>>> Where as OP_NEW supports higher burst size.
> >>>>
> >>>> This is already tied into capability description :
> >>>>
> >>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> >>>> /**< Event device is capable of operating in burst mode for
> >>> enqueue(forward,
> >>>>    * release) and dequeue operation. If this capability is not set,
> application
> >>>>    * still uses the rte_event_dequeue_burst() and
> >>> rte_event_enqueue_burst() but
> >>>>    * PMD accepts only one event at a time.
> >>>>    *
> >>>>    * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
> >>>>    */
> >>>>
> >>>>> I think I would rather remove all fields related to the max
> >>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I see.
> If
> >>>>> you have some HW limit on the maximum number of new events it
> can
> >>>>> accept, have the driver retry until backpressure occurs.
> >>>>
> >


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 10:43             ` Mattias Rönnblom
@ 2022-07-14 16:42               ` Pavan Nikhilesh Bhagavatula
  2022-07-14 16:53                 ` Van Haaren, Harry
  2022-07-15  7:56                 ` Mattias Rönnblom
  0 siblings, 2 replies; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-14 16:42 UTC (permalink / raw)
  To: Mattias Rönnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov



> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Thursday, July 14, 2022 4:13 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> Monjalon <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; peter.mccarthy@intel.com;
> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> anatoly.burakov@intel.com
> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> On 2022-07-14 08:32, Pavan Nikhilesh Bhagavatula wrote:
> >
> >
> >> -----Original Message-----
> >> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >> Sent: Wednesday, July 13, 2022 5:45 PM
> >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> >> Monjalon <thomas@monjalon.net>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> Hemant
> >> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >> liangma@liangbit.com; peter.mccarthy@intel.com;
> >> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >> anatoly.burakov@intel.com
> >> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >> type
> >>
> >> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>> Sent: Wednesday, July 13, 2022 2:39 PM
> >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> Thomas
> >>>> Monjalon <thomas@monjalon.net>
> >>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> >> Hemant
> >>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >>>> liangma@liangbit.com; peter.mccarthy@intel.com;
> >>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >>>> anatoly.burakov@intel.com
> >>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >> event
> >>>> type
> >>>>
> >>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> >>>>> +Cc
> >>>>> timothy.mcdaniel@intel.com;
> >>>>> hemant.agrawal@nxp.com;
> >>>>> sachin.saxena@oss.nxp.com;
> >>>>> mattias.ronnblom@ericsson.com;
> >>>>> jerinj@marvell.com;
> >>>>> liangma@liangbit.com;
> >>>>> peter.mccarthy@intel.com;
> >>>>> harry.van.haaren@intel.com;
> >>>>> erik.g.carrillo@intel.com;
> >>>>> abhinandan.gujjar@intel.com;
> >>>>> jay.jayatheerthan@intel.com;
> >>>>> mdr@ashroe.eu;
> >>>>> anatoly.burakov@intel.com;
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
> >>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> >>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> >>>>>> <pbhagavatula@marvell.com>
> >>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >>>> type
> >>>>>>
> >>>>>> External Email
> >>>>>>
> >>>>>> ----------------------------------------------------------------------
> >>>>>> I'm not your teacher, but please consider Cc'ing people outside of
> your
> >>>>>> company.
> >>>>>>
> >>>>>
> >>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like
> it's
> >>>> useless for
> >>>>> sending deprecation notices.
> >>>>>
> >>>>> Maybe we should update it to include lib/driver maintainers when diff
> >> sees
> >>>> deprecation.rst
> >>>>>
> >>>>>>
> >>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
> >>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>>
> >>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be
> added
> >> to
> >>>> the
> >>>>>>> structure ``rte_event_dev_info``. The field defines the max
> enqueue
> >>>>>>> burst size of new events (OP_NEW) supported by the underlying
> >> event
> >>>>>>> device.
> >>>>>>>
> >>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>> ---
> >>>>>>>     doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>>>     1 file changed, 5 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>> index 4e5b23c53d..071317e8e3 100644
> >>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
> >>>>>>>       applications should be updated to use the ``dmadev`` library
> >> instead,
> >>>>>>>       with the underlying HW-functionality being provided by the
> ``ioat``
> >> or
> >>>>>>>       ``idxd`` dma drivers
> >>>>>>> +
> >>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be
> extended
> >> to
> >>>>>> include the
> >>>>>>> +  max enqueue burst size of new events supported by the
> >> underlying
> >>>>>> event device.
> >>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
> >> added
> >>>> to
> >>>>>> the structure
> >>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>> Why is this needed, or useful? Why isn't called something with
> >>>> "enqueue_depth" in it, like the already-present field?
> >>>>
> >>>
> >>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE only
> >> supports
> >>> enqueue depth of 1.
> >>
> >> I assume it's for other cases as well? Any case when the event device
> >> has a max forward enqueue depth != max new enqueue depth?
> >>
> >
> > Yes, generally new events have much more flexibility than forwards event.
> >
> >> I don't see why an event device would have such low max limit on the
> >> number events enqueued.
> >
> > It depends on the number of scheduling contexts a given event port can
> track.
> 
> I have no idea what a "scheduling context" is (it's not something
> related to the Eventdev APIs), but if you have a shortage of them (for
> the moment, or always), the driver would just return a short count from
> the enqueue burst function. What use has the application of knowing that
> the event device can accept at most a certain number of events?
> 
> > Anyway this would align to the current existing feature definitions. The
> existing
> > API only defines the enqueue size of fwd and release events i.e.
> scheduling contexts
> > already tracked by event device.
> 
> The documentation of max_event_port_enqueue_depth says:
> "Maximum number of events can be enqueued at a time from an event port
> by this device."
> 
>  From what I can tell, it says nothing about new versus forward, or
> release type events.
> 
> > NEW is always a special case as we are adding new scheduling contexts to
> the event
> > device, the idea of this patch is to specify that NEW events wouldn’t have
> the same
> > restrictions of fwd/release events.
> >
> > This would also allow us to craft optimized APIs such as
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-2D313273af-
> 2D454445555731-2Def12ffaf4bdb568f-26q-3D1-26e-3Df0a92ad3-2D1167-
> 2D448d-2Dbe14-2D0987e939f019-26u-3Dhttps-253A-252F-
> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
> 252F20220627095702.8047-2D2-2Dpbhagavatula-2540marvell.com-
> 252F&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=iazRQtbylIuGXRfj2NPpf_wwG-
> ppSwFOPKclj5zcwu2pQG9b7ovePiSBn9k4PjQZ&s=sTrEcWbst59oK_jYe6jXhya
> CWwELib6Yr-3d9BccQt4&e=
> >
> >
> >> If the underlying hardware has some limitations,
> >> why not let the driver loop until back pressure occurs? Then you can
> 
> You didn't answer this question. Why not let the driver loop, until you
> for some reason or the other can't accept more events?

CNXK event driver cannot accept forwarding(enq) more than one event that has
been dequeued. Enqueueing more than one event for forwarding/releasing 
is a violation from HW perspective, this is currently announced by BURST capability.
But It can enqueue a burst if new events.

If you see the current example implementation we pick the worker based on 
BURST capability for optimizing the enqueue/dequeue by providing a hint
to the driver layer.

Although, we could live with aggregating the events at driver layer based on
queue. We would still require announce burst capability for new events i.e. 
changes to the info structure.

> 
> >> amortize the function call overhead and potentially other software
> >> operations (credit system management etc) over multiple events. Plus,
> >> the application doesn't need a special case for new events versus
> >> forward type events, or this-versus-that event device.
> >>
> >>> Where as OP_NEW supports higher burst size.
> >>>
> >>> This is already tied into capability description :
> >>>
> >>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> >>> /**< Event device is capable of operating in burst mode for
> >> enqueue(forward,
> >>>    * release) and dequeue operation. If this capability is not set,
> application
> >>>    * still uses the rte_event_dequeue_burst() and
> >> rte_event_enqueue_burst() but
> >>>    * PMD accepts only one event at a time.
> >>>    *
> >>>    * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
> >>>    */
> >>>
> >>>> I think I would rather remove all fields related to the max
> >>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I see.
> If
> >>>> you have some HW limit on the maximum number of new events it can
> >>>> accept, have the driver retry until backpressure occurs.
> >>>
> >


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 16:42               ` Pavan Nikhilesh Bhagavatula
@ 2022-07-14 16:53                 ` Van Haaren, Harry
  2022-07-14 16:57                   ` Van Haaren, Harry
  2022-07-14 18:01                   ` Pavan Nikhilesh Bhagavatula
  2022-07-15  7:56                 ` Mattias Rönnblom
  1 sibling, 2 replies; 27+ messages in thread
From: Van Haaren, Harry @ 2022-07-14 16:53 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, mattias.ronnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly

> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Sent: Thursday, July 14, 2022 5:42 PM
> To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella <mdr@ashroe.eu>;
> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van Haaren,
> Harry <harry.van.haaren@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type

<snip old conversation>

> > >> If the underlying hardware has some limitations,
> > >> why not let the driver loop until back pressure occurs? Then you can
> >
> > You didn't answer this question. Why not let the driver loop, until you
> > for some reason or the other can't accept more events?
> 
> CNXK event driver cannot accept forwarding(enq) more than one event that has
> been dequeued. Enqueueing more than one event for forwarding/releasing
> is a violation from HW perspective, this is currently announced by BURST capability.
> But It can enqueue a burst if new events.

Can't the driver just backpressure NEW events? that's what the event/sw driver
does in order to limit "new" inflight events. App attempts to enq FWD/REL, no 
problem. App enqueues burst of NEW (and there's only N spaces) then the
first N events pass, and the rest are returned to the application.

> If you see the current example implementation we pick the worker based on
> BURST capability for optimizing the enqueue/dequeue by providing a hint
> to the driver layer.

Please provide a link to the code? Others are not familiar with the CNXK driver,
or the sample code you're referring to...


> Although, we could live with aggregating the events at driver layer based on
> queue. We would still require announce burst capability for new events i.e.
> changes to the info structure.

As per above, I still don't see a reason why this HW optimization/limitation
needs to be pushed to the application layer. Why can the driver not handle
things by allowing/backpressure to the events it can/can't handle?


In this email thread[1] you've suggested reworking the rx_burst API with a
flag to indicate "same destination". This still pushes the problem to the application,
and exposes more HW/PMD specific options. This impl is *slightly* better because it
wont' require new APIs for each mode, but also *breaks all existing apps*!?

I'm just not understanding why the application needs to change, and why it
cannot be optimized/handled in the driver layer.

[1] http://mails.dpdk.org/archives/dev/2022-July/246717.html

<snip old conversation>

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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 16:53                 ` Van Haaren, Harry
@ 2022-07-14 16:57                   ` Van Haaren, Harry
  2022-07-15  8:13                     ` Mattias Rönnblom
  2022-07-17 12:38                     ` Thomas Monjalon
  2022-07-14 18:01                   ` Pavan Nikhilesh Bhagavatula
  1 sibling, 2 replies; 27+ messages in thread
From: Van Haaren, Harry @ 2022-07-14 16:57 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, mattias.ronnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly

> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Thursday, July 14, 2022 5:54 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; mattias.ronnblom
> <mattias.ronnblom@ericsson.com>; Thomas Monjalon <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella <mdr@ashroe.eu>;
> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <Peter.Mccarthy@intel.com>; Carrillo, Erik
> G <Erik.G.Carrillo@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>;
> Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
> 
> > -----Original Message-----
> > From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Sent: Thursday, July 14, 2022 5:42 PM
> > To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> > <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>;
> > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
> > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van
> Haaren,
> > Harry <harry.van.haaren@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> > Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
> 
> <snip old conversation>
> 
> > > >> If the underlying hardware has some limitations,
> > > >> why not let the driver loop until back pressure occurs? Then you can
> > >
> > > You didn't answer this question. Why not let the driver loop, until you
> > > for some reason or the other can't accept more events?
> >
> > CNXK event driver cannot accept forwarding(enq) more than one event that has
> > been dequeued. Enqueueing more than one event for forwarding/releasing
> > is a violation from HW perspective, this is currently announced by BURST
> capability.
> > But It can enqueue a burst if new events.
> 
> Can't the driver just backpressure NEW events? that's what the event/sw driver
> does in order to limit "new" inflight events. App attempts to enq FWD/REL, no
> problem. App enqueues burst of NEW (and there's only N spaces) then the
> first N events pass, and the rest are returned to the application.
> 
> > If you see the current example implementation we pick the worker based on
> > BURST capability for optimizing the enqueue/dequeue by providing a hint
> > to the driver layer.
> 
> Please provide a link to the code? Others are not familiar with the CNXK driver,
> or the sample code you're referring to...
> 
> 
> > Although, we could live with aggregating the events at driver layer based on
> > queue. We would still require announce burst capability for new events i.e.
> > changes to the info structure.
> 
> As per above, I still don't see a reason why this HW optimization/limitation
> needs to be pushed to the application layer. Why can the driver not handle
> things by allowing/backpressure to the events it can/can't handle?
> 
> 
> In this email thread[1] you've suggested reworking the rx_burst API with a
> flag to indicate "same destination". This still pushes the problem to the application,
> and exposes more HW/PMD specific options. This impl is *slightly* better because it
> wont' require new APIs for each mode, but also *breaks all existing apps*!?
> 
> I'm just not understanding why the application needs to change, and why it
> cannot be optimized/handled in the driver layer.
> 
> [1] http://mails.dpdk.org/archives/dev/2022-July/246717.html
> 
> <snip old conversation>


Let me be very clear, but also try to help here;
  I'm not in favour of the changes as proposed here, and feel that pushing
  the problem to Application API is NOT the right solution.

But *just in case* there is a *genuine* reason that we need an API/ABI
change, and that can be justified clearly in the upcoming weeks, then I'd
prefer not have problems with the deprecation notice not being included.

This Ack is *only* to allow possible API/ABI changes in 22.11, and not for
the proposal above.

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


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 16:53                 ` Van Haaren, Harry
  2022-07-14 16:57                   ` Van Haaren, Harry
@ 2022-07-14 18:01                   ` Pavan Nikhilesh Bhagavatula
  2022-07-15  7:49                     ` Van Haaren, Harry
  1 sibling, 1 reply; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-14 18:01 UTC (permalink / raw)
  To: Van Haaren, Harry, mattias.ronnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly



> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: Thursday, July 14, 2022 10:24 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; McDaniel, Timothy
> <timothy.mcdaniel@intel.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> > -----Original Message-----
> > From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Sent: Thursday, July 14, 2022 5:42 PM
> > To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas
> Monjalon
> > <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>;
> > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
> Hemant
> > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van
> Haaren,
> > Harry <harry.van.haaren@intel.com>; Carrillo, Erik G
> <erik.g.carrillo@intel.com>;
> > Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> <snip old conversation>
> 
> > > >> If the underlying hardware has some limitations,
> > > >> why not let the driver loop until back pressure occurs? Then you can
> > >
> > > You didn't answer this question. Why not let the driver loop, until you
> > > for some reason or the other can't accept more events?
> >
> > CNXK event driver cannot accept forwarding(enq) more than one event
> that has
> > been dequeued. Enqueueing more than one event for
> forwarding/releasing
> > is a violation from HW perspective, this is currently announced by BURST
> capability.
> > But It can enqueue a burst if new events.
> 
> Can't the driver just backpressure NEW events? that's what the event/sw
> driver
> does in order to limit "new" inflight events. App attempts to enq FWD/REL,
> no
> problem. App enqueues burst of NEW (and there's only N spaces) then the
> first N events pass, and the rest are returned to the application.
> 

Yes, driver can backpressure NEW events, in-fact that’s what we do today even
with burst size 1 as we need to check if target queue has space.

The main problem is app needs to know that enqueue NEW supports 
burst of events even when capability doesn't report BURST support.

Currently burst check is done as follows:
http://git.dpdk.org/dpdk/tree/app/test-eventdev/test_perf_common.c#n545
http://git.dpdk.org/dpdk/tree/app/test-eventdev/evt_common.h#n99

> > If you see the current example implementation we pick the worker based
> on
> > BURST capability for optimizing the enqueue/dequeue by providing a hint
> > to the driver layer.
> 
> Please provide a link to the code? Others are not familiar with the CNXK
> driver,
> or the sample code you're referring to...
> 

See above.

> 
> > Although, we could live with aggregating the events at driver layer based
> on
> > queue. We would still require announce burst capability for new events i.e.
> > changes to the info structure.
> 
> As per above, I still don't see a reason why this HW optimization/limitation
> needs to be pushed to the application layer. Why can the driver not handle
> things by allowing/backpressure to the events it can/can't handle?
> 

We can handle aggregation in the driver i.e. the new API is not needed although 
doing so is inefficient, our synthetic benchmark shows ~20% drop.

The main issue is that application needs to know that burst enqueue is supported
for event with op_type as NEW even when capability doesn’t report BURST support.
I think this can only be done if driver reports it via info structure.

> 
> In this email thread[1] you've suggested reworking the rx_burst API with a
> flag to indicate "same destination". This still pushes the problem to the
> application,
> and exposes more HW/PMD specific options. This impl is *slightly* better
> because it
> wont' require new APIs for each mode, but also *breaks all existing apps*!?
> 
> I'm just not understanding why the application needs to change, and why it
> cannot be optimized/handled in the driver layer.
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__mails.dpdk.org_archives_dev_2022-
> 2DJuly_246717.html&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjt
> KCMVsB-fmvgGV3o-
> g_fjLhk5Pupi9ijohpc&m=CyiCnnBdRFmd0maK3yHCkM7_3fDnVGGCeHteXAb
> I6DvehYrkk6BvyrMsV_NKsUGs&s=SbdMMotdrG_yzjCRgJc7h_Oq9Jtfl_8V06
> QsyPqUfro&e=
> 
> <snip old conversation>

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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 14:44                 ` Pavan Nikhilesh Bhagavatula
@ 2022-07-15  7:43                   ` Mattias Rönnblom
  0 siblings, 0 replies; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-15  7:43 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Van Haaren, Harry, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly

On 2022-07-14 16:44, Pavan Nikhilesh Bhagavatula wrote:
> 
> 
>> -----Original Message-----
>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Sent: Thursday, July 14, 2022 4:24 PM
>> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Pavan Nikhilesh
>> Bhagavatula <pbhagavatula@marvell.com>; Thomas Monjalon
>> <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>; dev@dpdk.org; McDaniel, Timothy
>> <timothy.mcdaniel@intel.com>; Hemant Agrawal
>> <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
>> Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
>> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
>> <jay.jayatheerthan@intel.com>; Burakov, Anatoly
>> <anatoly.burakov@intel.com>
>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>> type
>>
>> On 2022-07-14 11:45, Van Haaren, Harry wrote:
>>>> -----Original Message-----
>>>> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>> Sent: Thursday, July 14, 2022 7:33 AM
>>>> To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas
>> Monjalon
>>>> <thomas@monjalon.net>
>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>;
>>>> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
>> Hemant
>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
>> Van Haaren,
>>>> Harry <harry.van.haaren@intel.com>; Carrillo, Erik G
>> <erik.g.carrillo@intel.com>;
>>>> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
>>>> <jay.jayatheerthan@intel.com>; Burakov, Anatoly
>> <anatoly.burakov@intel.com>
>>>> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>> type
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>> Sent: Wednesday, July 13, 2022 5:45 PM
>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>>>>> Monjalon <thomas@monjalon.net>
>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>> Hemant
>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>> anatoly.burakov@intel.com
>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>> event
>>>>> type
>>>>>
>>>>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>>>> Sent: Wednesday, July 13, 2022 2:39 PM
>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>> Thomas
>>>>>>> Monjalon <thomas@monjalon.net>
>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>>>>> Hemant
>>>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>>>> anatoly.burakov@intel.com
>>>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>>>>> event
>>>>>>> type
>>>>>>>
>>>>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
>>>>>>>> +Cc
>>>>>>>> timothy.mcdaniel@intel.com;
>>>>>>>> hemant.agrawal@nxp.com;
>>>>>>>> sachin.saxena@oss.nxp.com;
>>>>>>>> mattias.ronnblom@ericsson.com;
>>>>>>>> jerinj@marvell.com;
>>>>>>>> liangma@liangbit.com;
>>>>>>>> peter.mccarthy@intel.com;
>>>>>>>> harry.van.haaren@intel.com;
>>>>>>>> erik.g.carrillo@intel.com;
>>>>>>>> abhinandan.gujjar@intel.com;
>>>>>>>> jay.jayatheerthan@intel.com;
>>>>>>>> mdr@ashroe.eu;
>>>>>>>> anatoly.burakov@intel.com;
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
>>>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>>>>>>>>> <pbhagavatula@marvell.com>
>>>>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>> event
>>>>>>> type
>>>>>>>>>
>>>>>>>>> External Email
>>>>>>>>>
>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>> I'm not your teacher, but please consider Cc'ing people outside of
>> your
>>>>>>>>> company.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like
>> it's
>>>>>>> useless for
>>>>>>>> sending deprecation notices.
>>>>>>>>
>>>>>>>> Maybe we should update it to include lib/driver maintainers when
>> diff
>>>>> sees
>>>>>>> deprecation.rst
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
>>>>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>>>>
>>>>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be
>> added
>>>>> to
>>>>>>> the
>>>>>>>>>> structure ``rte_event_dev_info``. The field defines the max
>> enqueue
>>>>>>>>>> burst size of new events (OP_NEW) supported by the underlying
>>>>> event
>>>>>>>>>> device.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>>>> ---
>>>>>>>>>>      doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>>>>>      1 file changed, 5 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> index 4e5b23c53d..071317e8e3 100644
>>>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
>>>>>>>>>>        applications should be updated to use the ``dmadev`` library
>>>>> instead,
>>>>>>>>>>        with the underlying HW-functionality being provided by the
>> ``ioat``
>>>>> or
>>>>>>>>>>        ``idxd`` dma drivers
>>>>>>>>>> +
>>>>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be
>> extended
>>>>> to
>>>>>>>>> include the
>>>>>>>>>> +  max enqueue burst size of new events supported by the
>>>>> underlying
>>>>>>>>> event device.
>>>>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
>>>>> added
>>>>>>> to
>>>>>>>>> the structure
>>>>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>> Why is this needed, or useful? Why isn't called something with
>>>>>>> "enqueue_depth" in it, like the already-present field?
>>>>>>>
>>>>>>
>>>>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE
>> only
>>>>> supports
>>>>>> enqueue depth of 1.
>>>>>
>>>>> I assume it's for other cases as well? Any case when the event device
>>>>> has a max forward enqueue depth != max new enqueue depth?
>>>>>
>>>>
>>>> Yes, generally new events have much more flexibility than forwards
>> event.
>>>>
>>>>> I don't see why an event device would have such low max limit on the
>>>>> number events enqueued.
>>>>
>>>> It depends on the number of scheduling contexts a given event port can
>> track.
>>>> Anyway this would align to the current existing feature definitions. The
>> existing
>>>> API only defines the enqueue size of fwd and release events i.e.
>> scheduling contexts
>>>> already tracked by event device.
>>>> NEW is always a special case as we are adding new scheduling contexts to
>> the event
>>>> device, the idea of this patch is to specify that NEW events wouldn’t have
>> the same
>>>> restrictions of fwd/release events.
>>>>
>>>> This would also allow us to craft optimized APIs such as
>>>> https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-2D313273af-
>> 2D454445555731-2D6bd5fd62af0f8992-26q-3D1-26e-3Dc4f1686f-2D725e-
>> 2D48bf-2Daa62-2D069489e74009-26u-3Dhttps-253A-252F-
>> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
>> 252F20220627095702.8047-2D2-
>> 2D&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
>> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=-Mr6gUdwMnOZbXSlWeHhYpTVt7UdA-
>> VHDmIC_MuUjne3U5nqwFeoOatsEX10pzow&s=Khz6GF4271RAiO7-
>> 5BY1J2u0BvT8CwWvfwvRSnzeeeM&e=
>>>> pbhagavatula@marvell.com/
>>>
>>> I've reviewed the above; I'm not in favour of adding even more APIs to the
>> Eventdev level.
>>> Adding even more enq APIs just overloads applications with options; today
>> we already have
>>> multiple APIs;
>>> rte_event_enqueue_burst()
>>> rte_event_enqueue_new_burst()
>>> rte_event_enqueue_forward_burst()
>>>
>>> Now the suggestion is to add another one,
>>> rte_event_enqueue_new_queue_burst()?
>>>
>>
>> Why not three new ones? And why not also
>> rte_event_queue(_new_|_forward_|)(_queue_|)priority_burst()?
>>
>> Plus maybe you want functions that enqueue to the same flow id as well?
>>
>> It's like you can almost hear the combinatorial explosion go off. :)
>>
>> Btw, isn't this the problem that the event vector functionality is
>> supposed to solve? Enqueue many similar events to the same destination.
> 
> Maybe we could move this to rte_event_enqueue_new_burst() by adding an
> additional flags param?
> 

This sounds better. Extending this idea, you could add a 
rte_event_enqueue_burst_ex(), which would include a flags parameters, 
specifying which fields were invariant across the burst, including the 
op type (and also sub type, priority, queue id, flow id, and/or sched 
type). Mark the op-specific functions obsolete.

That would move the explosion to the driver instead, where it could be 
better confined.

> This is already done for an existing API rte_event_eth_tx_adapter_enqueue
> where flags is used to signify same Tx queue destination.
> 
> * @param flags
>   *  RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_ flags.
>   *  #RTE_EVENT_ETH_TX_ADAPTER_ENQUEUE_SAME_DEST signifies that all the packets
>   *  which are enqueued are destined for the same Ethernet port & Tx queue.
> static inline uint16_t
> rte_event_eth_tx_adapter_enqueue(uint8_t dev_id,
> 				uint8_t port_id,
> 				struct rte_event ev[],
> 				uint16_t nb_events,
> 				const uint8_t flags)
> 
> 
> 
>>
>>> For an Ethdev analogy: we have rx_burst() and tx_burst(). There is no
>> tx_to_same_dest_ip_burst().
>>>
>>> The driver already has all knowledge required if "all events going to same
>> destination",
>>> so it can optimize for that case already internally. I think Mattias was asking
>> similar questions,
>>> around PMD having enough info today already.
>>>
>>> Pushing more APIs and complexity to Application level doesn't seem a good
>> direction to me.
>>>
>>>
>>>>> If the underlying hardware has some limitations,
>>>>> why not let the driver loop until back pressure occurs? Then you can
>>>>> amortize the function call overhead and potentially other software
>>>>> operations (credit system management etc) over multiple events. Plus,
>>>>> the application doesn't need a special case for new events versus
>>>>> forward type events, or this-versus-that event device.
>>>>>
>>>>>> Where as OP_NEW supports higher burst size.
>>>>>>
>>>>>> This is already tied into capability description :
>>>>>>
>>>>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
>>>>>> /**< Event device is capable of operating in burst mode for
>>>>> enqueue(forward,
>>>>>>     * release) and dequeue operation. If this capability is not set,
>> application
>>>>>>     * still uses the rte_event_dequeue_burst() and
>>>>> rte_event_enqueue_burst() but
>>>>>>     * PMD accepts only one event at a time.
>>>>>>     *
>>>>>>     * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
>>>>>>     */
>>>>>>
>>>>>>> I think I would rather remove all fields related to the max
>>>>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I see.
>> If
>>>>>>> you have some HW limit on the maximum number of new events it
>> can
>>>>>>> accept, have the driver retry until backpressure occurs.
>>>>>>
>>>
> 


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 18:01                   ` Pavan Nikhilesh Bhagavatula
@ 2022-07-15  7:49                     ` Van Haaren, Harry
  2022-07-15 13:09                       ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 27+ messages in thread
From: Van Haaren, Harry @ 2022-07-15  7:49 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, mattias.ronnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly

> -----Original Message-----
> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> Sent: Thursday, July 14, 2022 7:01 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; mattias.ronnblom
> <mattias.ronnblom@ericsson.com>; Thomas Monjalon <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella <mdr@ashroe.eu>;
> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Carrillo, Erik
> G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>;
> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
> 
> 
> 
> > -----Original Message-----
> > From: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Sent: Thursday, July 14, 2022 10:24 PM
> > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> > mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> > <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > <mdr@ashroe.eu>; dev@dpdk.org; McDaniel, Timothy
> > <timothy.mcdaniel@intel.com>; Hemant Agrawal
> > <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> > Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> > <anatoly.burakov@intel.com>
> > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> > type
> >
> > > -----Original Message-----
> > > From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > > Sent: Thursday, July 14, 2022 5:42 PM
> > > To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas
> > Monjalon
> > > <thomas@monjalon.net>
> > > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > <mdr@ashroe.eu>;
> > > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
> > Hemant
> > > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van
> > Haaren,
> > > Harry <harry.van.haaren@intel.com>; Carrillo, Erik G
> > <erik.g.carrillo@intel.com>;
> > > Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> > <anatoly.burakov@intel.com>
> > > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> > type
> >
> > <snip old conversation>
> >
> > > > >> If the underlying hardware has some limitations,
> > > > >> why not let the driver loop until back pressure occurs? Then you can
> > > >
> > > > You didn't answer this question. Why not let the driver loop, until you
> > > > for some reason or the other can't accept more events?
> > >
> > > CNXK event driver cannot accept forwarding(enq) more than one event
> > that has
> > > been dequeued. Enqueueing more than one event for
> > forwarding/releasing
> > > is a violation from HW perspective, this is currently announced by BURST
> > capability.
> > > But It can enqueue a burst if new events.
> >
> > Can't the driver just backpressure NEW events? that's what the event/sw
> > driver
> > does in order to limit "new" inflight events. App attempts to enq FWD/REL,
> > no
> > problem. App enqueues burst of NEW (and there's only N spaces) then the
> > first N events pass, and the rest are returned to the application.
> >
> 
> Yes, driver can backpressure NEW events, in-fact that’s what we do today even
> with burst size 1 as we need to check if target queue has space.
> 
> The main problem is app needs to know that enqueue NEW supports
> burst of events even when capability doesn't report BURST support.

If this is the "main problem", then 2 steps:
1) Let the driver report it supports BURST, and app will try to use it
2A) Let user enq bursts of FWD/REL, and accept only 1 (expecting app to retry with rest of burst, as is common)
2B) Put a retry loop inside the PMD, until actual backpressure is hit in HW, then return to App.


> 
> Currently burst check is done as follows:
> http://git.dpdk.org/dpdk/tree/app/test-eventdev/test_perf_common.c#n545
> http://git.dpdk.org/dpdk/tree/app/test-eventdev/evt_common.h#n99
> 
> > > If you see the current example implementation we pick the worker based
> > on
> > > BURST capability for optimizing the enqueue/dequeue by providing a hint
> > > to the driver layer.
> >
> > Please provide a link to the code? Others are not familiar with the CNXK
> > driver,
> > or the sample code you're referring to...
> >
> 
> See above.
> 
> >
> > > Although, we could live with aggregating the events at driver layer based
> > on
> > > queue. We would still require announce burst capability for new events i.e.
> > > changes to the info structure.
> >
> > As per above, I still don't see a reason why this HW optimization/limitation
> > needs to be pushed to the application layer. Why can the driver not handle
> > things by allowing/backpressure to the events it can/can't handle?
> >
> 
> We can handle aggregation in the driver i.e. the new API is not needed although
> doing so is inefficient, our synthetic benchmark shows ~20% drop.
> 
> The main issue is that application needs to know that burst enqueue is supported
> for event with op_type as NEW even when capability doesn’t report BURST support.
> I think this can only be done if driver reports it via info structure.

See above suggestion; the application should already be burst-capable (if it wants to be)
and hence there's "nothing to do" at the app level, if the PMD is reworked to 1 and 2B?


> > In this email thread[1] you've suggested reworking the rx_burst API with a
> > flag to indicate "same destination". This still pushes the problem to the
> > application,
> > and exposes more HW/PMD specific options. This impl is *slightly* better
> > because it
> > wont' require new APIs for each mode, but also *breaks all existing apps*!?
> >
> > I'm just not understanding why the application needs to change, and why it
> > cannot be optimized/handled in the driver layer.
> >
> > [1] https://urldefense.proofpoint.com/v2/url?u=http-
> > 3A__mails.dpdk.org_archives_dev_2022-
> > 2DJuly_246717.html&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjt
> > KCMVsB-fmvgGV3o-
> > g_fjLhk5Pupi9ijohpc&m=CyiCnnBdRFmd0maK3yHCkM7_3fDnVGGCeHteXAb
> > I6DvehYrkk6BvyrMsV_NKsUGs&s=SbdMMotdrG_yzjCRgJc7h_Oq9Jtfl_8V06
> > QsyPqUfro&e=
> >
> > <snip old conversation>

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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 16:42               ` Pavan Nikhilesh Bhagavatula
  2022-07-14 16:53                 ` Van Haaren, Harry
@ 2022-07-15  7:56                 ` Mattias Rönnblom
  2022-07-15 13:16                   ` Pavan Nikhilesh Bhagavatula
  1 sibling, 1 reply; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-15  7:56 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov

On 2022-07-14 18:42, Pavan Nikhilesh Bhagavatula wrote:
> 
> 
>> -----Original Message-----
>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Sent: Thursday, July 14, 2022 4:13 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>> Monjalon <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; peter.mccarthy@intel.com;
>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>> anatoly.burakov@intel.com
>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>> type
>>
>> On 2022-07-14 08:32, Pavan Nikhilesh Bhagavatula wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>> Sent: Wednesday, July 13, 2022 5:45 PM
>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>>>> Monjalon <thomas@monjalon.net>
>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>> Hemant
>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>> anatoly.burakov@intel.com
>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>> event
>>>> type
>>>>
>>>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>>> Sent: Wednesday, July 13, 2022 2:39 PM
>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>> Thomas
>>>>>> Monjalon <thomas@monjalon.net>
>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>>>> Hemant
>>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>>> anatoly.burakov@intel.com
>>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>>>> event
>>>>>> type
>>>>>>
>>>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
>>>>>>> +Cc
>>>>>>> timothy.mcdaniel@intel.com;
>>>>>>> hemant.agrawal@nxp.com;
>>>>>>> sachin.saxena@oss.nxp.com;
>>>>>>> mattias.ronnblom@ericsson.com;
>>>>>>> jerinj@marvell.com;
>>>>>>> liangma@liangbit.com;
>>>>>>> peter.mccarthy@intel.com;
>>>>>>> harry.van.haaren@intel.com;
>>>>>>> erik.g.carrillo@intel.com;
>>>>>>> abhinandan.gujjar@intel.com;
>>>>>>> jay.jayatheerthan@intel.com;
>>>>>>> mdr@ashroe.eu;
>>>>>>> anatoly.burakov@intel.com;
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
>>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>>>>>>>> <pbhagavatula@marvell.com>
>>>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>> event
>>>>>> type
>>>>>>>>
>>>>>>>> External Email
>>>>>>>>
>>>>>>>> ----------------------------------------------------------------------
>>>>>>>> I'm not your teacher, but please consider Cc'ing people outside of
>> your
>>>>>>>> company.
>>>>>>>>
>>>>>>>
>>>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks like
>> it's
>>>>>> useless for
>>>>>>> sending deprecation notices.
>>>>>>>
>>>>>>> Maybe we should update it to include lib/driver maintainers when diff
>>>> sees
>>>>>> deprecation.rst
>>>>>>>
>>>>>>>>
>>>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
>>>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>>>
>>>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be
>> added
>>>> to
>>>>>> the
>>>>>>>>> structure ``rte_event_dev_info``. The field defines the max
>> enqueue
>>>>>>>>> burst size of new events (OP_NEW) supported by the underlying
>>>> event
>>>>>>>>> device.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>>> ---
>>>>>>>>>      doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>>>>      1 file changed, 5 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>> index 4e5b23c53d..071317e8e3 100644
>>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
>>>>>>>>>        applications should be updated to use the ``dmadev`` library
>>>> instead,
>>>>>>>>>        with the underlying HW-functionality being provided by the
>> ``ioat``
>>>> or
>>>>>>>>>        ``idxd`` dma drivers
>>>>>>>>> +
>>>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be
>> extended
>>>> to
>>>>>>>> include the
>>>>>>>>> +  max enqueue burst size of new events supported by the
>>>> underlying
>>>>>>>> event device.
>>>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
>>>> added
>>>>>> to
>>>>>>>> the structure
>>>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>> Why is this needed, or useful? Why isn't called something with
>>>>>> "enqueue_depth" in it, like the already-present field?
>>>>>>
>>>>>
>>>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE only
>>>> supports
>>>>> enqueue depth of 1.
>>>>
>>>> I assume it's for other cases as well? Any case when the event device
>>>> has a max forward enqueue depth != max new enqueue depth?
>>>>
>>>
>>> Yes, generally new events have much more flexibility than forwards event.
>>>
>>>> I don't see why an event device would have such low max limit on the
>>>> number events enqueued.
>>>
>>> It depends on the number of scheduling contexts a given event port can
>> track.
>>
>> I have no idea what a "scheduling context" is (it's not something
>> related to the Eventdev APIs), but if you have a shortage of them (for
>> the moment, or always), the driver would just return a short count from
>> the enqueue burst function. What use has the application of knowing that
>> the event device can accept at most a certain number of events?
>>
>>> Anyway this would align to the current existing feature definitions. The
>> existing
>>> API only defines the enqueue size of fwd and release events i.e.
>> scheduling contexts
>>> already tracked by event device.
>>
>> The documentation of max_event_port_enqueue_depth says:
>> "Maximum number of events can be enqueued at a time from an event port
>> by this device."
>>
>>   From what I can tell, it says nothing about new versus forward, or
>> release type events.
>>
>>> NEW is always a special case as we are adding new scheduling contexts to
>> the event
>>> device, the idea of this patch is to specify that NEW events wouldn’t have
>> the same
>>> restrictions of fwd/release events.
>>>
>>> This would also allow us to craft optimized APIs such as
>>> https://urldefense.proofpoint.com/v2/url?u=https-
>> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-2D313273af-
>> 2D454445555731-2Def12ffaf4bdb568f-26q-3D1-26e-3Df0a92ad3-2D1167-
>> 2D448d-2Dbe14-2D0987e939f019-26u-3Dhttps-253A-252F-
>> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
>> 252F20220627095702.8047-2D2-2Dpbhagavatula-2540marvell.com-
>> 252F&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
>> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=iazRQtbylIuGXRfj2NPpf_wwG-
>> ppSwFOPKclj5zcwu2pQG9b7ovePiSBn9k4PjQZ&s=sTrEcWbst59oK_jYe6jXhya
>> CWwELib6Yr-3d9BccQt4&e=
>>>
>>>
>>>> If the underlying hardware has some limitations,
>>>> why not let the driver loop until back pressure occurs? Then you can
>>
>> You didn't answer this question. Why not let the driver loop, until you
>> for some reason or the other can't accept more events?
> 
> CNXK event driver cannot accept forwarding(enq) more than one event that has
> been dequeued. Enqueueing more than one event for forwarding/releasing
> is a violation from HW perspective, this is currently announced by BURST capability.
> But It can enqueue a burst if new events.
> 

OK, so the limitation we are discussing here is not really related to 
enqueue bursts, but the number of allowed events that can be in-flight 
(in-progress) on the eventdev port?

So what happens if you announce some larger max enqueue depth in 
dev_info? If the application can't dequeue more than one event at a 
time, which means it can't possible enqueue more than one forward event 
at a time. Or am I missing something? Even with implicit release turned 
off, the PMD can deny the application having more than one outstanding 
event.

One issue is head-of-line blocking if you mix new and forward events, 
but that you can solve on the application level by separating new and 
forward bursts. That problem already exists, but for back pressure 
scenarios, where new events are disallowed, but forward are not.

> If you see the current example implementation we pick the worker based on
> BURST capability for optimizing the enqueue/dequeue by providing a hint
> to the driver layer.
> 

What example? What does "pick the worker" mean?

> Although, we could live with aggregating the events at driver layer based on
> queue. We would still require announce burst capability for new events i.e.
> changes to the info structure.
> 
>>
>>>> amortize the function call overhead and potentially other software
>>>> operations (credit system management etc) over multiple events. Plus,
>>>> the application doesn't need a special case for new events versus
>>>> forward type events, or this-versus-that event device.
>>>>
>>>>> Where as OP_NEW supports higher burst size.
>>>>>
>>>>> This is already tied into capability description :
>>>>>
>>>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
>>>>> /**< Event device is capable of operating in burst mode for
>>>> enqueue(forward,
>>>>>     * release) and dequeue operation. If this capability is not set,
>> application
>>>>>     * still uses the rte_event_dequeue_burst() and
>>>> rte_event_enqueue_burst() but
>>>>>     * PMD accepts only one event at a time.
>>>>>     *
>>>>>     * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
>>>>>     */
>>>>>
>>>>>> I think I would rather remove all fields related to the max
>>>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I see.
>> If
>>>>>> you have some HW limit on the maximum number of new events it can
>>>>>> accept, have the driver retry until backpressure occurs.
>>>>>
>>>
> 


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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 16:57                   ` Van Haaren, Harry
@ 2022-07-15  8:13                     ` Mattias Rönnblom
  2022-07-17 12:38                     ` Thomas Monjalon
  1 sibling, 0 replies; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-15  8:13 UTC (permalink / raw)
  To: Van Haaren, Harry, Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly

On 2022-07-14 18:57, Van Haaren, Harry wrote:
>> -----Original Message-----
>> From: Van Haaren, Harry
>> Sent: Thursday, July 14, 2022 5:54 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; mattias.ronnblom
>> <mattias.ronnblom@ericsson.com>; Thomas Monjalon <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella <mdr@ashroe.eu>;
>> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; Mccarthy, Peter <Peter.Mccarthy@intel.com>; Carrillo, Erik
>> G <Erik.G.Carrillo@intel.com>; Gujjar, Abhinandan S
>> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>;
>> Burakov, Anatoly <anatoly.burakov@intel.com>
>> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
>>
>>> -----Original Message-----
>>> From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>> Sent: Thursday, July 14, 2022 5:42 PM
>>> To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
>>> <thomas@monjalon.net>
>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>;
>>> dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van
>> Haaren,
>>> Harry <harry.van.haaren@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
>>> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
>>> <jay.jayatheerthan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
>>> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
>>
>> <snip old conversation>
>>
>>>>>> If the underlying hardware has some limitations,
>>>>>> why not let the driver loop until back pressure occurs? Then you can
>>>>
>>>> You didn't answer this question. Why not let the driver loop, until you
>>>> for some reason or the other can't accept more events?
>>>
>>> CNXK event driver cannot accept forwarding(enq) more than one event that has
>>> been dequeued. Enqueueing more than one event for forwarding/releasing
>>> is a violation from HW perspective, this is currently announced by BURST
>> capability.
>>> But It can enqueue a burst if new events.
>>
>> Can't the driver just backpressure NEW events? that's what the event/sw driver
>> does in order to limit "new" inflight events. App attempts to enq FWD/REL, no
>> problem. App enqueues burst of NEW (and there's only N spaces) then the
>> first N events pass, and the rest are returned to the application.
>>
>>> If you see the current example implementation we pick the worker based on
>>> BURST capability for optimizing the enqueue/dequeue by providing a hint
>>> to the driver layer.
>>
>> Please provide a link to the code? Others are not familiar with the CNXK driver,
>> or the sample code you're referring to...
>>
>>
>>> Although, we could live with aggregating the events at driver layer based on
>>> queue. We would still require announce burst capability for new events i.e.
>>> changes to the info structure.
>>
>> As per above, I still don't see a reason why this HW optimization/limitation
>> needs to be pushed to the application layer. Why can the driver not handle
>> things by allowing/backpressure to the events it can/can't handle?
>>
>>
>> In this email thread[1] you've suggested reworking the rx_burst API with a
>> flag to indicate "same destination". This still pushes the problem to the application,
>> and exposes more HW/PMD specific options. This impl is *slightly* better because it
>> wont' require new APIs for each mode, but also *breaks all existing apps*!?
>>
>> I'm just not understanding why the application needs to change, and why it
>> cannot be optimized/handled in the driver layer.
>>
>> [1] https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-e92a9cef34b4dac6&q=1&e=fb252b76-35e7-4476-b756-c64849e9bf54&u=http%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2022-July%2F246717.html
>>
>> <snip old conversation>
> 
> 
> Let me be very clear, but also try to help here;
>    I'm not in favour of the changes as proposed here, and feel that pushing
>    the problem to Application API is NOT the right solution.
> 
> But *just in case* there is a *genuine* reason that we need an API/ABI
> change, and that can be justified clearly in the upcoming weeks, then I'd
> prefer not have problems with the deprecation notice not being included.

Quantifying the performance benefits would be helpful, I think.

> 
> This Ack is *only* to allow possible API/ABI changes in 22.11, and not for
> the proposal above.
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> 


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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-15  7:49                     ` Van Haaren, Harry
@ 2022-07-15 13:09                       ` Pavan Nikhilesh Bhagavatula
  0 siblings, 0 replies; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-15 13:09 UTC (permalink / raw)
  To: Van Haaren, Harry, mattias.ronnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, McDaniel, Timothy,
	Hemant Agrawal, sachin.saxena, liangma, Mccarthy, Peter,
	Carrillo, Erik G, Gujjar, Abhinandan S, Jayatheerthan, Jay,
	Burakov, Anatoly



> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: Friday, July 15, 2022 1:20 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; McDaniel, Timothy
> <timothy.mcdaniel@intel.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> > -----Original Message-----
> > From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > Sent: Thursday, July 14, 2022 7:01 PM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>; mattias.ronnblom
> > <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>;
> > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
> Hemant
> > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> Carrillo, Erik
> > G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>;
> > Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> > <anatoly.burakov@intel.com>
> > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> >
> >
> >
> > > -----Original Message-----
> > > From: Van Haaren, Harry <harry.van.haaren@intel.com>
> > > Sent: Thursday, July 14, 2022 10:24 PM
> > > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> > > mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> > > <thomas@monjalon.net>
> > > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > > <mdr@ashroe.eu>; dev@dpdk.org; McDaniel, Timothy
> > > <timothy.mcdaniel@intel.com>; Hemant Agrawal
> > > <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> > > Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> > > <anatoly.burakov@intel.com>
> > > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> > > type
> > >
> > > > -----Original Message-----
> > > > From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > > > Sent: Thursday, July 14, 2022 5:42 PM
> > > > To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas
> > > Monjalon
> > > > <thomas@monjalon.net>
> > > > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > > <mdr@ashroe.eu>;
> > > > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>;
> > > Hemant
> > > > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > > > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>;
> Van
> > > Haaren,
> > > > Harry <harry.van.haaren@intel.com>; Carrillo, Erik G
> > > <erik.g.carrillo@intel.com>;
> > > > Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan,
> Jay
> > > > <jay.jayatheerthan@intel.com>; Burakov, Anatoly
> > > <anatoly.burakov@intel.com>
> > > > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> > > type
> > >
> > > <snip old conversation>
> > >
> > > > > >> If the underlying hardware has some limitations,
> > > > > >> why not let the driver loop until back pressure occurs? Then you
> can
> > > > >
> > > > > You didn't answer this question. Why not let the driver loop, until you
> > > > > for some reason or the other can't accept more events?
> > > >
> > > > CNXK event driver cannot accept forwarding(enq) more than one event
> > > that has
> > > > been dequeued. Enqueueing more than one event for
> > > forwarding/releasing
> > > > is a violation from HW perspective, this is currently announced by
> BURST
> > > capability.
> > > > But It can enqueue a burst if new events.
> > >
> > > Can't the driver just backpressure NEW events? that's what the event/sw
> > > driver
> > > does in order to limit "new" inflight events. App attempts to enq
> FWD/REL,
> > > no
> > > problem. App enqueues burst of NEW (and there's only N spaces) then
> the
> > > first N events pass, and the rest are returned to the application.
> > >
> >
> > Yes, driver can backpressure NEW events, in-fact that’s what we do today
> even
> > with burst size 1 as we need to check if target queue has space.
> >
> > The main problem is app needs to know that enqueue NEW supports
> > burst of events even when capability doesn't report BURST support.
> 
> If this is the "main problem", then 2 steps:
> 1) Let the driver report it supports BURST, and app will try to use it
> 2A) Let user enq bursts of FWD/REL, and accept only 1 (expecting app to
> retry with rest of burst, as is common)
> 2B) Put a retry loop inside the PMD, until actual backpressure is hit in HW,
> then return to App.
> 

Yeah, we could announce capability as burst supported, then set
max_dequeue_depth as 1
max_enqueue_depth as -1 (Infinite as open eventdev)

But this might be slightly misleading as -1 would be applicable only for OP_NEW.

We cannot put a retry loop for OP_FWD/OP_RELEASE as it simply doesn’t make sense
from HW PoV. 
In CNXK each event port tracks only one scheduling context (held on dequeue), 
and OP_FWD/OP_RELEASE translate to commands to the device to operate on the 
the scheduling context. There can be only one pending command per a "scheduling context"
until the next dequeue.

I understand it can be done as stated above but it might mislead an application writer as he 
might interpret the max_enqueue_depth to be applicable for OP_FWD/OP_NEW unless
explicitly stated.



> 
> >
> > Currently burst check is done as follows:
> > https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__git.dpdk.org_dpdk_tree_app_test-2Deventdev_test-5Fperf-
> 5Fcommon.c-
> 23n545&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-
> g_fjLhk5Pupi9ijohpc&m=vLB2LhvU8CB6ljBToaWpY30DLKEj1ELSFBx7CWAgF2
> MXInVg3fFQXw0iPtu_nKou&s=N4paenk2QVADf4igi-erY6XS9Ya-
> m5iMC19m2IMvpAM&e=
> > https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__git.dpdk.org_dpdk_tree_app_test-2Deventdev_evt-5Fcommon.h-
> 23n99&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> fmvgGV3o-
> g_fjLhk5Pupi9ijohpc&m=vLB2LhvU8CB6ljBToaWpY30DLKEj1ELSFBx7CWAgF2
> MXInVg3fFQXw0iPtu_nKou&s=eB2IU-
> ixeBz3473hwsL4erScT5XReSe3vk72hnku3zM&e=
> >
> > > > If you see the current example implementation we pick the worker
> based
> > > on
> > > > BURST capability for optimizing the enqueue/dequeue by providing a
> hint
> > > > to the driver layer.
> > >
> > > Please provide a link to the code? Others are not familiar with the CNXK
> > > driver,
> > > or the sample code you're referring to...
> > >
> >
> > See above.
> >
> > >
> > > > Although, we could live with aggregating the events at driver layer
> based
> > > on
> > > > queue. We would still require announce burst capability for new events
> i.e.
> > > > changes to the info structure.
> > >
> > > As per above, I still don't see a reason why this HW
> optimization/limitation
> > > needs to be pushed to the application layer. Why can the driver not
> handle
> > > things by allowing/backpressure to the events it can/can't handle?
> > >
> >
> > We can handle aggregation in the driver i.e. the new API is not needed
> although
> > doing so is inefficient, our synthetic benchmark shows ~20% drop.
> >
> > The main issue is that application needs to know that burst enqueue is
> supported
> > for event with op_type as NEW even when capability doesn’t report BURST
> support.
> > I think this can only be done if driver reports it via info structure.
> 
> See above suggestion; the application should already be burst-capable (if it
> wants to be)
> and hence there's "nothing to do" at the app level, if the PMD is reworked to
> 1 and 2B?
> 
> 
> > > In this email thread[1] you've suggested reworking the rx_burst API with
> a
> > > flag to indicate "same destination". This still pushes the problem to the
> > > application,
> > > and exposes more HW/PMD specific options. This impl is *slightly* better
> > > because it
> > > wont' require new APIs for each mode, but also *breaks all existing
> apps*!?
> > >
> > > I'm just not understanding why the application needs to change, and why
> it
> > > cannot be optimized/handled in the driver layer.
> > >
> > > [1] https://urldefense.proofpoint.com/v2/url?u=http-
> > > 3A__mails.dpdk.org_archives_dev_2022-
> > >
> 2DJuly_246717.html&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjt
> > > KCMVsB-fmvgGV3o-
> > >
> g_fjLhk5Pupi9ijohpc&m=CyiCnnBdRFmd0maK3yHCkM7_3fDnVGGCeHteXAb
> > >
> I6DvehYrkk6BvyrMsV_NKsUGs&s=SbdMMotdrG_yzjCRgJc7h_Oq9Jtfl_8V06
> > > QsyPqUfro&e=
> > >
> > > <snip old conversation>

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

* RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-15  7:56                 ` Mattias Rönnblom
@ 2022-07-15 13:16                   ` Pavan Nikhilesh Bhagavatula
  2022-07-17 20:23                     ` Mattias Rönnblom
  0 siblings, 1 reply; 27+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2022-07-15 13:16 UTC (permalink / raw)
  To: Mattias Rönnblom, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov



> -----Original Message-----
> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Sent: Friday, July 15, 2022 1:26 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> Monjalon <thomas@monjalon.net>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> liangma@liangbit.com; peter.mccarthy@intel.com;
> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> anatoly.burakov@intel.com
> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
> type
> 
> On 2022-07-14 18:42, Pavan Nikhilesh Bhagavatula wrote:
> >
> >
> >> -----Original Message-----
> >> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >> Sent: Thursday, July 14, 2022 4:13 PM
> >> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
> >> Monjalon <thomas@monjalon.net>
> >> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> Hemant
> >> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >> liangma@liangbit.com; peter.mccarthy@intel.com;
> >> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >> anatoly.burakov@intel.com
> >> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> event
> >> type
> >>
> >> On 2022-07-14 08:32, Pavan Nikhilesh Bhagavatula wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>> Sent: Wednesday, July 13, 2022 5:45 PM
> >>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> Thomas
> >>>> Monjalon <thomas@monjalon.net>
> >>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> >> Hemant
> >>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >>>> liangma@liangbit.com; peter.mccarthy@intel.com;
> >>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >>>> anatoly.burakov@intel.com
> >>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >> event
> >>>> type
> >>>>
> >>>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> >>>>>> Sent: Wednesday, July 13, 2022 2:39 PM
> >>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
> >> Thomas
> >>>>>> Monjalon <thomas@monjalon.net>
> >>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
> >>>> Hemant
> >>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> >>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
> >>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
> >>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
> >>>>>> anatoly.burakov@intel.com
> >>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >>>> event
> >>>>>> type
> >>>>>>
> >>>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
> >>>>>>> +Cc
> >>>>>>> timothy.mcdaniel@intel.com;
> >>>>>>> hemant.agrawal@nxp.com;
> >>>>>>> sachin.saxena@oss.nxp.com;
> >>>>>>> mattias.ronnblom@ericsson.com;
> >>>>>>> jerinj@marvell.com;
> >>>>>>> liangma@liangbit.com;
> >>>>>>> peter.mccarthy@intel.com;
> >>>>>>> harry.van.haaren@intel.com;
> >>>>>>> erik.g.carrillo@intel.com;
> >>>>>>> abhinandan.gujjar@intel.com;
> >>>>>>> jay.jayatheerthan@intel.com;
> >>>>>>> mdr@ashroe.eu;
> >>>>>>> anatoly.burakov@intel.com;
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
> >>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> >>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> >>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> >>>>>>>> <pbhagavatula@marvell.com>
> >>>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
> >> event
> >>>>>> type
> >>>>>>>>
> >>>>>>>> External Email
> >>>>>>>>
> >>>>>>>> ----------------------------------------------------------------------
> >>>>>>>> I'm not your teacher, but please consider Cc'ing people outside of
> >> your
> >>>>>>>> company.
> >>>>>>>>
> >>>>>>>
> >>>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks
> like
> >> it's
> >>>>>> useless for
> >>>>>>> sending deprecation notices.
> >>>>>>>
> >>>>>>> Maybe we should update it to include lib/driver maintainers when
> diff
> >>>> sees
> >>>>>> deprecation.rst
> >>>>>>>
> >>>>>>>>
> >>>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
> >>>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>>>>
> >>>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be
> >> added
> >>>> to
> >>>>>> the
> >>>>>>>>> structure ``rte_event_dev_info``. The field defines the max
> >> enqueue
> >>>>>>>>> burst size of new events (OP_NEW) supported by the underlying
> >>>> event
> >>>>>>>>> device.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>>>>>>> ---
> >>>>>>>>>      doc/guides/rel_notes/deprecation.rst | 5 +++++
> >>>>>>>>>      1 file changed, 5 insertions(+)
> >>>>>>>>>
> >>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>> b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>> index 4e5b23c53d..071317e8e3 100644
> >>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
> >>>>>>>>>        applications should be updated to use the ``dmadev`` library
> >>>> instead,
> >>>>>>>>>        with the underlying HW-functionality being provided by the
> >> ``ioat``
> >>>> or
> >>>>>>>>>        ``idxd`` dma drivers
> >>>>>>>>> +
> >>>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be
> >> extended
> >>>> to
> >>>>>>>> include the
> >>>>>>>>> +  max enqueue burst size of new events supported by the
> >>>> underlying
> >>>>>>>> event device.
> >>>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
> >>>> added
> >>>>>> to
> >>>>>>>> the structure
> >>>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>> Why is this needed, or useful? Why isn't called something with
> >>>>>> "enqueue_depth" in it, like the already-present field?
> >>>>>>
> >>>>>
> >>>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE
> only
> >>>> supports
> >>>>> enqueue depth of 1.
> >>>>
> >>>> I assume it's for other cases as well? Any case when the event device
> >>>> has a max forward enqueue depth != max new enqueue depth?
> >>>>
> >>>
> >>> Yes, generally new events have much more flexibility than forwards
> event.
> >>>
> >>>> I don't see why an event device would have such low max limit on the
> >>>> number events enqueued.
> >>>
> >>> It depends on the number of scheduling contexts a given event port can
> >> track.
> >>
> >> I have no idea what a "scheduling context" is (it's not something
> >> related to the Eventdev APIs), but if you have a shortage of them (for
> >> the moment, or always), the driver would just return a short count from
> >> the enqueue burst function. What use has the application of knowing that
> >> the event device can accept at most a certain number of events?
> >>
> >>> Anyway this would align to the current existing feature definitions. The
> >> existing
> >>> API only defines the enqueue size of fwd and release events i.e.
> >> scheduling contexts
> >>> already tracked by event device.
> >>
> >> The documentation of max_event_port_enqueue_depth says:
> >> "Maximum number of events can be enqueued at a time from an event
> port
> >> by this device."
> >>
> >>   From what I can tell, it says nothing about new versus forward, or
> >> release type events.
> >>
> >>> NEW is always a special case as we are adding new scheduling contexts
> to
> >> the event
> >>> device, the idea of this patch is to specify that NEW events wouldn’t
> have
> >> the same
> >>> restrictions of fwd/release events.
> >>>
> >>> This would also allow us to craft optimized APIs such as
> >>> https://urldefense.proofpoint.com/v2/url?u=https-
> >> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-
> 2D313273af-
> >> 2D454445555731-2Def12ffaf4bdb568f-26q-3D1-26e-3Df0a92ad3-2D1167-
> >> 2D448d-2Dbe14-2D0987e939f019-26u-3Dhttps-253A-252F-
> >> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
> >> 252F20220627095702.8047-2D2-2Dpbhagavatula-2540marvell.com-
> >> 252F&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
> >> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=iazRQtbylIuGXRfj2NPpf_wwG-
> >>
> ppSwFOPKclj5zcwu2pQG9b7ovePiSBn9k4PjQZ&s=sTrEcWbst59oK_jYe6jXhya
> >> CWwELib6Yr-3d9BccQt4&e=
> >>>
> >>>
> >>>> If the underlying hardware has some limitations,
> >>>> why not let the driver loop until back pressure occurs? Then you can
> >>
> >> You didn't answer this question. Why not let the driver loop, until you
> >> for some reason or the other can't accept more events?
> >
> > CNXK event driver cannot accept forwarding(enq) more than one event
> that has
> > been dequeued. Enqueueing more than one event for
> forwarding/releasing
> > is a violation from HW perspective, this is currently announced by BURST
> capability.
> > But It can enqueue a burst if new events.
> >
> 
> OK, so the limitation we are discussing here is not really related to
> enqueue bursts, but the number of allowed events that can be in-flight
> (in-progress) on the eventdev port?

Yes that’s correct.

In CNXK each event port tracks only one scheduling context (held on dequeue), 
and OP_FWD/OP_RELEASE translate to commands to the device to operate on the 
the scheduling context. There can be only one pending command per a "scheduling context"
until the next dequeue.

> 
> So what happens if you announce some larger max enqueue depth in
> dev_info? If the application can't dequeue more than one event at a
> time, which means it can't possible enqueue more than one forward event
> at a time. Or am I missing something? Even with implicit release turned
> off, the PMD can deny the application having more than one outstanding
> event.

My intention was to cleanly differentiate between OP_FWD and OP_NEW as it might mislead 
an application writer as he  might interpret the max_enqueue_depth to be applicable for 
OP_FWD/OP_NEW unless explicitly stated.

> 
> One issue is head-of-line blocking if you mix new and forward events,
> but that you can solve on the application level by separating new and
> forward bursts. That problem already exists, but for back pressure
> scenarios, where new events are disallowed, but forward are not.
> 
> > If you see the current example implementation we pick the worker based
> on
> > BURST capability for optimizing the enqueue/dequeue by providing a hint
> > to the driver layer.
> >
> 
> What example? What does "pick the worker" mean?

Pick worker functions:
http://git.dpdk.org/dpdk/tree/app/test-eventdev/test_perf_common.c#n545
http://git.dpdk.org/dpdk/tree/app/test-eventdev/evt_common.h#n99

> 
> > Although, we could live with aggregating the events at driver layer based
> on
> > queue. We would still require announce burst capability for new events i.e.
> > changes to the info structure.
> >
> >>
> >>>> amortize the function call overhead and potentially other software
> >>>> operations (credit system management etc) over multiple events. Plus,
> >>>> the application doesn't need a special case for new events versus
> >>>> forward type events, or this-versus-that event device.
> >>>>
> >>>>> Where as OP_NEW supports higher burst size.
> >>>>>
> >>>>> This is already tied into capability description :
> >>>>>
> >>>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
> >>>>> /**< Event device is capable of operating in burst mode for
> >>>> enqueue(forward,
> >>>>>     * release) and dequeue operation. If this capability is not set,
> >> application
> >>>>>     * still uses the rte_event_dequeue_burst() and
> >>>> rte_event_enqueue_burst() but
> >>>>>     * PMD accepts only one event at a time.
> >>>>>     *
> >>>>>     * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
> >>>>>     */
> >>>>>
> >>>>>> I think I would rather remove all fields related to the max
> >>>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I
> see.
> >> If
> >>>>>> you have some HW limit on the maximum number of new events it
> can
> >>>>>> accept, have the driver retry until backpressure occurs.
> >>>>>
> >>>
> >


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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-14 16:57                   ` Van Haaren, Harry
  2022-07-15  8:13                     ` Mattias Rönnblom
@ 2022-07-17 12:38                     ` Thomas Monjalon
  1 sibling, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2022-07-17 12:38 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Van Haaren, Harry
  Cc: mattias. ronnblom, dev, Jerin Jacob Kollanukkaran, Ray Kinsella,
	dev, McDaniel, Timothy, Hemant Agrawal, sachin.saxena, liangma,
	Mccarthy, Peter, Carrillo, Erik G, Gujjar, Abhinandan S,
	Jayatheerthan, Jay, Burakov, Anatoly

14/07/2022 18:57, Van Haaren, Harry:
> > -----Original Message-----
> > From: Van Haaren, Harry
> > Sent: Thursday, July 14, 2022 5:54 PM
> > To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; mattias.ronnblom
> > <mattias.ronnblom@ericsson.com>; Thomas Monjalon <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella <mdr@ashroe.eu>;
> > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
> > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > liangma@liangbit.com; Mccarthy, Peter <Peter.Mccarthy@intel.com>; Carrillo, Erik
> > G <Erik.G.Carrillo@intel.com>; Gujjar, Abhinandan S
> > <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay <jay.jayatheerthan@intel.com>;
> > Burakov, Anatoly <anatoly.burakov@intel.com>
> > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
> > 
> > > -----Original Message-----
> > > From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
> > > Sent: Thursday, July 14, 2022 5:42 PM
> > > To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Thomas Monjalon
> > > <thomas@monjalon.net>
> > > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
> > <mdr@ashroe.eu>;
> > > dev@dpdk.org; McDaniel, Timothy <timothy.mcdaniel@intel.com>; Hemant
> > > Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
> > > liangma@liangbit.com; Mccarthy, Peter <peter.mccarthy@intel.com>; Van
> > Haaren,
> > > Harry <harry.van.haaren@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> > > Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Subject: RE: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
> > 
> > <snip old conversation>
> > 
> > > > >> If the underlying hardware has some limitations,
> > > > >> why not let the driver loop until back pressure occurs? Then you can
> > > >
> > > > You didn't answer this question. Why not let the driver loop, until you
> > > > for some reason or the other can't accept more events?
> > >
> > > CNXK event driver cannot accept forwarding(enq) more than one event that has
> > > been dequeued. Enqueueing more than one event for forwarding/releasing
> > > is a violation from HW perspective, this is currently announced by BURST
> > capability.
> > > But It can enqueue a burst if new events.
> > 
> > Can't the driver just backpressure NEW events? that's what the event/sw driver
> > does in order to limit "new" inflight events. App attempts to enq FWD/REL, no
> > problem. App enqueues burst of NEW (and there's only N spaces) then the
> > first N events pass, and the rest are returned to the application.
> > 
> > > If you see the current example implementation we pick the worker based on
> > > BURST capability for optimizing the enqueue/dequeue by providing a hint
> > > to the driver layer.
> > 
> > Please provide a link to the code? Others are not familiar with the CNXK driver,
> > or the sample code you're referring to...
> > 
> > 
> > > Although, we could live with aggregating the events at driver layer based on
> > > queue. We would still require announce burst capability for new events i.e.
> > > changes to the info structure.
> > 
> > As per above, I still don't see a reason why this HW optimization/limitation
> > needs to be pushed to the application layer. Why can the driver not handle
> > things by allowing/backpressure to the events it can/can't handle?
> > 
> > 
> > In this email thread[1] you've suggested reworking the rx_burst API with a
> > flag to indicate "same destination". This still pushes the problem to the application,
> > and exposes more HW/PMD specific options. This impl is *slightly* better because it
> > wont' require new APIs for each mode, but also *breaks all existing apps*!?
> > 
> > I'm just not understanding why the application needs to change, and why it
> > cannot be optimized/handled in the driver layer.
> > 
> > [1] http://mails.dpdk.org/archives/dev/2022-July/246717.html
> > 
> > <snip old conversation>
> 
> 
> Let me be very clear, but also try to help here;
>   I'm not in favour of the changes as proposed here, and feel that pushing
>   the problem to Application API is NOT the right solution.
> 
> But *just in case* there is a *genuine* reason that we need an API/ABI
> change, and that can be justified clearly in the upcoming weeks, then I'd
> prefer not have problems with the deprecation notice not being included.
> 
> This Ack is *only* to allow possible API/ABI changes in 22.11, and not for
> the proposal above.
> 
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

It doesn't make sense to add a deprecation notice
if the direction is not agreed.

Let's discuss further and ask for techboard help if needed.
In general, I would not be surprised that it's time for a cleanup in this API.



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

* Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event type
  2022-07-15 13:16                   ` Pavan Nikhilesh Bhagavatula
@ 2022-07-17 20:23                     ` Mattias Rönnblom
  0 siblings, 0 replies; 27+ messages in thread
From: Mattias Rönnblom @ 2022-07-17 20:23 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Thomas Monjalon
  Cc: Jerin Jacob Kollanukkaran, Ray Kinsella, dev, timothy.mcdaniel,
	Hemant Agrawal, sachin.saxena, liangma, peter.mccarthy,
	harry.van.haaren, erik.g.carrillo, abhinandan.gujjar,
	jay.jayatheerthan, anatoly.burakov

On 2022-07-15 15:16, Pavan Nikhilesh Bhagavatula wrote:
> 
> 
>> -----Original Message-----
>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Sent: Friday, July 15, 2022 1:26 PM
>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>> Monjalon <thomas@monjalon.net>
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com; Hemant
>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>> liangma@liangbit.com; peter.mccarthy@intel.com;
>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>> anatoly.burakov@intel.com
>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new event
>> type
>>
>> On 2022-07-14 18:42, Pavan Nikhilesh Bhagavatula wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>> Sent: Thursday, July 14, 2022 4:13 PM
>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Thomas
>>>> Monjalon <thomas@monjalon.net>
>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>> Hemant
>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>> anatoly.burakov@intel.com
>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>> event
>>>> type
>>>>
>>>> On 2022-07-14 08:32, Pavan Nikhilesh Bhagavatula wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>>> Sent: Wednesday, July 13, 2022 5:45 PM
>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>> Thomas
>>>>>> Monjalon <thomas@monjalon.net>
>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>>>> Hemant
>>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>>> anatoly.burakov@intel.com
>>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>>>> event
>>>>>> type
>>>>>>
>>>>>> On 2022-07-13 12:40, Pavan Nikhilesh Bhagavatula wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>>>>>>> Sent: Wednesday, July 13, 2022 2:39 PM
>>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>>>> Thomas
>>>>>>>> Monjalon <thomas@monjalon.net>
>>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; timothy.mcdaniel@intel.com;
>>>>>> Hemant
>>>>>>>> Agrawal <hemant.agrawal@nxp.com>; sachin.saxena@oss.nxp.com;
>>>>>>>> liangma@liangbit.com; peter.mccarthy@intel.com;
>>>>>>>> harry.van.haaren@intel.com; erik.g.carrillo@intel.com;
>>>>>>>> abhinandan.gujjar@intel.com; jay.jayatheerthan@intel.com;
>>>>>>>> anatoly.burakov@intel.com
>>>>>>>> Subject: Re: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>>>>>> event
>>>>>>>> type
>>>>>>>>
>>>>>>>> On 2022-07-12 20:11, Pavan Nikhilesh Bhagavatula wrote:
>>>>>>>>> +Cc
>>>>>>>>> timothy.mcdaniel@intel.com;
>>>>>>>>> hemant.agrawal@nxp.com;
>>>>>>>>> sachin.saxena@oss.nxp.com;
>>>>>>>>> mattias.ronnblom@ericsson.com;
>>>>>>>>> jerinj@marvell.com;
>>>>>>>>> liangma@liangbit.com;
>>>>>>>>> peter.mccarthy@intel.com;
>>>>>>>>> harry.van.haaren@intel.com;
>>>>>>>>> erik.g.carrillo@intel.com;
>>>>>>>>> abhinandan.gujjar@intel.com;
>>>>>>>>> jay.jayatheerthan@intel.com;
>>>>>>>>> mdr@ashroe.eu;
>>>>>>>>> anatoly.burakov@intel.com;
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>>>>> Sent: Tuesday, July 12, 2022 8:31 PM
>>>>>>>>>> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
>>>>>>>>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ray Kinsella
>>>>>>>>>> <mdr@ashroe.eu>; dev@dpdk.org; Pavan Nikhilesh Bhagavatula
>>>>>>>>>> <pbhagavatula@marvell.com>
>>>>>>>>>> Subject: [EXT] Re: [PATCH 1/2] doc: add enqueue depth for new
>>>> event
>>>>>>>> type
>>>>>>>>>>
>>>>>>>>>> External Email
>>>>>>>>>>
>>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>>> I'm not your teacher, but please consider Cc'ing people outside of
>>>> your
>>>>>>>>>> company.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I generally add --to-cmd=./devtools/get-maintainer.sh but looks
>> like
>>>> it's
>>>>>>>> useless for
>>>>>>>>> sending deprecation notices.
>>>>>>>>>
>>>>>>>>> Maybe we should update it to include lib/driver maintainers when
>> diff
>>>>>> sees
>>>>>>>> deprecation.rst
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 27/06/2022 11:57, pbhagavatula@marvell.com:
>>>>>>>>>>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>>>>>
>>>>>>>>>>> A new field ``max_event_port_enqueue_new_burst`` will be
>>>> added
>>>>>> to
>>>>>>>> the
>>>>>>>>>>> structure ``rte_event_dev_info``. The field defines the max
>>>> enqueue
>>>>>>>>>>> burst size of new events (OP_NEW) supported by the underlying
>>>>>> event
>>>>>>>>>>> device.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>>>>>>>>>>> ---
>>>>>>>>>>>       doc/guides/rel_notes/deprecation.rst | 5 +++++
>>>>>>>>>>>       1 file changed, 5 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>>> index 4e5b23c53d..071317e8e3 100644
>>>>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>>>>> @@ -125,3 +125,8 @@ Deprecation Notices
>>>>>>>>>>>         applications should be updated to use the ``dmadev`` library
>>>>>> instead,
>>>>>>>>>>>         with the underlying HW-functionality being provided by the
>>>> ``ioat``
>>>>>> or
>>>>>>>>>>>         ``idxd`` dma drivers
>>>>>>>>>>> +
>>>>>>>>>>> +* eventdev: The structure ``rte_event_dev_info`` will be
>>>> extended
>>>>>> to
>>>>>>>>>> include the
>>>>>>>>>>> +  max enqueue burst size of new events supported by the
>>>>>> underlying
>>>>>>>>>> event device.
>>>>>>>>>>> +  A new field ``max_event_port_enqueue_new_burst`` will be
>>>>>> added
>>>>>>>> to
>>>>>>>>>> the structure
>>>>>>>>>>> +  ``rte_event_dev_info`` in DPDK 22.11.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>> Why is this needed, or useful? Why isn't called something with
>>>>>>>> "enqueue_depth" in it, like the already-present field?
>>>>>>>>
>>>>>>>
>>>>>>> This is for a case where enqueues with OP_FORWARD/OP_RELEASE
>> only
>>>>>> supports
>>>>>>> enqueue depth of 1.
>>>>>>
>>>>>> I assume it's for other cases as well? Any case when the event device
>>>>>> has a max forward enqueue depth != max new enqueue depth?
>>>>>>
>>>>>
>>>>> Yes, generally new events have much more flexibility than forwards
>> event.
>>>>>
>>>>>> I don't see why an event device would have such low max limit on the
>>>>>> number events enqueued.
>>>>>
>>>>> It depends on the number of scheduling contexts a given event port can
>>>> track.
>>>>
>>>> I have no idea what a "scheduling context" is (it's not something
>>>> related to the Eventdev APIs), but if you have a shortage of them (for
>>>> the moment, or always), the driver would just return a short count from
>>>> the enqueue burst function. What use has the application of knowing that
>>>> the event device can accept at most a certain number of events?
>>>>
>>>>> Anyway this would align to the current existing feature definitions. The
>>>> existing
>>>>> API only defines the enqueue size of fwd and release events i.e.
>>>> scheduling contexts
>>>>> already tracked by event device.
>>>>
>>>> The documentation of max_event_port_enqueue_depth says:
>>>> "Maximum number of events can be enqueued at a time from an event
>> port
>>>> by this device."
>>>>
>>>>    From what I can tell, it says nothing about new versus forward, or
>>>> release type events.
>>>>
>>>>> NEW is always a special case as we are adding new scheduling contexts
>> to
>>>> the event
>>>>> device, the idea of this patch is to specify that NEW events wouldn’t
>> have
>>>> the same
>>>>> restrictions of fwd/release events.
>>>>>
>>>>> This would also allow us to craft optimized APIs such as
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-
>>>> 3A__protect2.fireeye.com_v1_url-3Fk-3D31323334-2D501d5122-
>> 2D313273af-
>>>> 2D454445555731-2Def12ffaf4bdb568f-26q-3D1-26e-3Df0a92ad3-2D1167-
>>>> 2D448d-2Dbe14-2D0987e939f019-26u-3Dhttps-253A-252F-
>>>> 252Fpatches.dpdk.org-252Fproject-252Fdpdk-252Fpatch-
>>>> 252F20220627095702.8047-2D2-2Dpbhagavatula-2540marvell.com-
>>>> 252F&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=E3SgYMjtKCMVsB-
>>>> fmvgGV3o-g_fjLhk5Pupi9ijohpc&m=iazRQtbylIuGXRfj2NPpf_wwG-
>>>>
>> ppSwFOPKclj5zcwu2pQG9b7ovePiSBn9k4PjQZ&s=sTrEcWbst59oK_jYe6jXhya
>>>> CWwELib6Yr-3d9BccQt4&e=
>>>>>
>>>>>
>>>>>> If the underlying hardware has some limitations,
>>>>>> why not let the driver loop until back pressure occurs? Then you can
>>>>
>>>> You didn't answer this question. Why not let the driver loop, until you
>>>> for some reason or the other can't accept more events?
>>>
>>> CNXK event driver cannot accept forwarding(enq) more than one event
>> that has
>>> been dequeued. Enqueueing more than one event for
>> forwarding/releasing
>>> is a violation from HW perspective, this is currently announced by BURST
>> capability.
>>> But It can enqueue a burst if new events.
>>>
>>
>> OK, so the limitation we are discussing here is not really related to
>> enqueue bursts, but the number of allowed events that can be in-flight
>> (in-progress) on the eventdev port?
> 
> Yes that’s correct.
> 

OK.

Can't this be communicate to the application by setting the max dequeue 
depth to 1?

> In CNXK each event port tracks only one scheduling context (held on dequeue),
> and OP_FWD/OP_RELEASE translate to commands to the device to operate on the
> the scheduling context. There can be only one pending command per a "scheduling context"
> until the next dequeue.
> 
>>
>> So what happens if you announce some larger max enqueue depth in
>> dev_info? If the application can't dequeue more than one event at a
>> time, which means it can't possible enqueue more than one forward event
>> at a time. Or am I missing something? Even with implicit release turned
>> off, the PMD can deny the application having more than one outstanding
>> event.
> 
> My intention was to cleanly differentiate between OP_FWD and OP_NEW as it might mislead
> an application writer as he  might interpret the max_enqueue_depth to be applicable for
> OP_FWD/OP_NEW unless explicitly stated.
>
Yeah, but why? How is this information useful?

The only scenario I can think of is an application employing a fd.io VPP 
style SIMD-heavy "vectorized" design, with the per-burst processing 
progressing as a series of loops, one per layer (or "node"). If the 
event device can only hand you a single event at a time, then such an 
application would suffer, performance wise. In such a case, what is 
relevant is the maximum *dequeue* depth.

For one-event-at-a-time type applications, it will just be a case of an 
unnecessary loop, which cost will be trivial.

>>
>> One issue is head-of-line blocking if you mix new and forward events,
>> but that you can solve on the application level by separating new and
>> forward bursts. That problem already exists, but for back pressure
>> scenarios, where new events are disallowed, but forward are not.
>>
>>> If you see the current example implementation we pick the worker based
>> on
>>> BURST capability for optimizing the enqueue/dequeue by providing a hint
>>> to the driver layer.
>>>
>>
>> What example? What does "pick the worker" mean?
> 
> Pick worker functions:
> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-afc8d38dd7668c15&q=1&e=8100a5ca-5a51-46b2-91e1-1b6a49519b24&u=http%3A%2F%2Fgit.dpdk.org%2Fdpdk%2Ftree%2Fapp%2Ftest-eventdev%2Ftest_perf_common.c%23n545
> https://protect2.fireeye.com/v1/url?k=31323334-501d5122-313273af-454445555731-bb3f76042845f72f&q=1&e=8100a5ca-5a51-46b2-91e1-1b6a49519b24&u=http%3A%2F%2Fgit.dpdk.org%2Fdpdk%2Ftree%2Fapp%2Ftest-eventdev%2Fevt_common.h%23n99
> 
>>
>>> Although, we could live with aggregating the events at driver layer based
>> on
>>> queue. We would still require announce burst capability for new events i.e.
>>> changes to the info structure.
>>>
>>>>
>>>>>> amortize the function call overhead and potentially other software
>>>>>> operations (credit system management etc) over multiple events. Plus,
>>>>>> the application doesn't need a special case for new events versus
>>>>>> forward type events, or this-versus-that event device.
>>>>>>
>>>>>>> Where as OP_NEW supports higher burst size.
>>>>>>>
>>>>>>> This is already tied into capability description :
>>>>>>>
>>>>>>> #define RTE_EVENT_DEV_CAP_BURST_MODE          (1ULL << 4)
>>>>>>> /**< Event device is capable of operating in burst mode for
>>>>>> enqueue(forward,
>>>>>>>      * release) and dequeue operation. If this capability is not set,
>>>> application
>>>>>>>      * still uses the rte_event_dequeue_burst() and
>>>>>> rte_event_enqueue_burst() but
>>>>>>>      * PMD accepts only one event at a time.
>>>>>>>      *
>>>>>>>      * @see rte_event_dequeue_burst() rte_event_enqueue_burst()
>>>>>>>      */
>>>>>>>
>>>>>>>> I think I would rather remove all fields related to the max
>>>>>>>> enqueue/dequeue burst sizes. They serve no purpose, as far as I
>> see.
>>>> If
>>>>>>>> you have some HW limit on the maximum number of new events it
>> can
>>>>>>>> accept, have the driver retry until backpressure occurs.
>>>>>>>
>>>>>
>>>
> 


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

end of thread, other threads:[~2022-07-17 20:23 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27  9:57 [PATCH 1/2] doc: add enqueue depth for new event type pbhagavatula
2022-06-27  9:57 ` [PATCH 2/2] eventdev: add function to enq new events to the same queue pbhagavatula
2022-07-11 14:54 ` [PATCH 1/2] doc: add enqueue depth for new event type Jerin Jacob
2022-07-12 15:01 ` Thomas Monjalon
2022-07-12 18:11   ` [EXT] " Pavan Nikhilesh Bhagavatula
2022-07-12 20:47     ` Thomas Monjalon
2022-07-13  3:15     ` Hemant Agrawal
2022-07-13  9:08     ` Mattias Rönnblom
2022-07-13 10:40       ` Pavan Nikhilesh Bhagavatula
2022-07-13 12:15         ` Mattias Rönnblom
2022-07-14  6:32           ` Pavan Nikhilesh Bhagavatula
2022-07-14  9:45             ` Van Haaren, Harry
2022-07-14 10:53               ` Mattias Rönnblom
2022-07-14 14:44                 ` Pavan Nikhilesh Bhagavatula
2022-07-15  7:43                   ` Mattias Rönnblom
2022-07-14 10:43             ` Mattias Rönnblom
2022-07-14 16:42               ` Pavan Nikhilesh Bhagavatula
2022-07-14 16:53                 ` Van Haaren, Harry
2022-07-14 16:57                   ` Van Haaren, Harry
2022-07-15  8:13                     ` Mattias Rönnblom
2022-07-17 12:38                     ` Thomas Monjalon
2022-07-14 18:01                   ` Pavan Nikhilesh Bhagavatula
2022-07-15  7:49                     ` Van Haaren, Harry
2022-07-15 13:09                       ` Pavan Nikhilesh Bhagavatula
2022-07-15  7:56                 ` Mattias Rönnblom
2022-07-15 13:16                   ` Pavan Nikhilesh Bhagavatula
2022-07-17 20:23                     ` 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.