[1/2] lib/eventdev: add usage hints to port configure API

Message ID 20210909125422.31144-2-harry.van.haaren@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series eventdev: add port usage hints |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Van Haaren, Harry Sept. 9, 2021, 12:54 p.m. UTC
  This commit introduces 3 flags to the port configuration flags.
These flags allow the application to indicate what type of work
is expected to be performed by an eventdev port.

The three new flags are
- RTE_EVENT_PORT_CFG_HINT_PRODUCER (mostly RTE_EVENT_OP_NEW events)
- RTE_EVENT_PORT_CFG_HINT_CONSUMER (mostly RTE_EVENT_OP_RELEASE events)
- RTE_EVENT_PORT_CFG_HINT_WORKER   (mostly RTE_EVENT_OP_FORWARD events)

These flags are only hints, and the PMDs must operate under the
assumption that any port can enqueue an event with any type of op.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/eventdev/rte_eventdev.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
  

Comments

Jerin Jacob Sept. 16, 2021, 4:59 a.m. UTC | #1
On Thu, Sep 9, 2021 at 6:24 PM Harry van Haaren
<harry.van.haaren@intel.com> wrote:
>
> This commit introduces 3 flags to the port configuration flags.
> These flags allow the application to indicate what type of work
> is expected to be performed by an eventdev port.
>
> The three new flags are
> - RTE_EVENT_PORT_CFG_HINT_PRODUCER (mostly RTE_EVENT_OP_NEW events)
> - RTE_EVENT_PORT_CFG_HINT_CONSUMER (mostly RTE_EVENT_OP_RELEASE events)
> - RTE_EVENT_PORT_CFG_HINT_WORKER   (mostly RTE_EVENT_OP_FORWARD events)
>
> These flags are only hints, and the PMDs must operate under the
> assumption that any port can enqueue an event with any type of op.

This change looks good to me.

+ @Mattias Rönnblom  @Erik Gabriel Carrillo  @Gujjar, Abhinandan S
@Pavan Nikhilesh  @Hemant Agrawal @Nipun Gupta  @Liang Ma @McDaniel,
Timothy
# Please change subject to evendev: ....
# Please submit the driver changes as well and updated performance
gain with the scheme.
# Please find below one comment



>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
>  lib/eventdev/rte_eventdev.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> index a9c496fb62..159b580938 100644
> --- a/lib/eventdev/rte_eventdev.h
> +++ b/lib/eventdev/rte_eventdev.h
> @@ -709,6 +709,29 @@ rte_event_queue_attr_get(uint8_t dev_id, uint8_t queue_id, uint32_t attr_id,
>   *
>   *  @see rte_event_port_setup(), rte_event_port_link()
>   */
> +#define RTE_EVENT_PORT_CFG_HINT_PRODUCER       (1ULL << 2)
> +/**< Hint that this event port will primarily enqueue events to the system.
> + * A PMD can optimize its internal workings by assuming that this port is
> + * primarily going to enqueue NEW events. Note that this flag is only a hint.

IMO, We can explicitly add the following line in each comment.

PMDs must operate under the assumption that any port can enqueue an
event with any type of op.

> + *
> + *  @see rte_event_port_setup()
> + */
> +#define RTE_EVENT_PORT_CFG_HINT_CONSUMER       (1ULL << 3)
> +/**< Hint that this event port will primarily dequeue events from the system.
> + * A PMD can optimize its internal workings by assuming that this port is
> + * primarily going to consume events, and not enqueue FORWARD or RELEASE events.
> + * Note that this flag is only a hint.
> + *
> + *  @see rte_event_port_setup()
> + */
> +#define RTE_EVENT_PORT_CFG_HINT_WORKER         (1ULL << 4)
> +/**< Hint that this event port will primarily events to the system.
> + * A PMD can optimize its internal workings by assuming that this port is
> + * primarily going to FORWARD events, and not enqueue NEW or RELEASE events
> + * often. Note that this flag is only a hint.
> + *
> + *  @see rte_event_port_setup()
> + */
>
>  /** Event port configuration structure */
>  struct rte_event_port_conf {
> --
> 2.30.2
>
  
Van Haaren, Harry Sept. 16, 2021, 11:53 a.m. UTC | #2
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, September 16, 2021 5:59 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dpdk-dev <dev@dpdk.org>; Pathak, Pravin <pravin.pathak@intel.com>;
> McDaniel, Timothy <timothy.mcdaniel@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 1/2] lib/eventdev: add usage hints to port
> configure API
> 
> On Thu, Sep 9, 2021 at 6:24 PM Harry van Haaren
> <harry.van.haaren@intel.com> wrote:
> >
> > This commit introduces 3 flags to the port configuration flags.
> > These flags allow the application to indicate what type of work
> > is expected to be performed by an eventdev port.
> >
> > The three new flags are
> > - RTE_EVENT_PORT_CFG_HINT_PRODUCER (mostly RTE_EVENT_OP_NEW
> events)
> > - RTE_EVENT_PORT_CFG_HINT_CONSUMER (mostly RTE_EVENT_OP_RELEASE
> events)
> > - RTE_EVENT_PORT_CFG_HINT_WORKER   (mostly RTE_EVENT_OP_FORWARD
> events)
> >
> > These flags are only hints, and the PMDs must operate under the
> > assumption that any port can enqueue an event with any type of op.
> 
> This change looks good to me.
> 
> + @Mattias Rönnblom  @Erik Gabriel Carrillo  @Gujjar, Abhinandan S
> @Pavan Nikhilesh  @Hemant Agrawal @Nipun Gupta  @Liang Ma @McDaniel,
> Timothy

Thanks for review Jerin. I didn't see some these folks on CC, so +CC a few.
Any concerns folks about adding hints for producer/worker/consumer?

> # Please change subject to evendev: ....

Yes, will do.

> # Please submit the driver changes as well and updated performance
> gain with the scheme.

Yes, that makes sense. I'll work with DLB driver maintainers to get these
changes up ASAP.

> # Please find below one comment

<snip details>

> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > ---
> >  lib/eventdev/rte_eventdev.h | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
> > index a9c496fb62..159b580938 100644
> > --- a/lib/eventdev/rte_eventdev.h
> > +++ b/lib/eventdev/rte_eventdev.h
> > @@ -709,6 +709,29 @@ rte_event_queue_attr_get(uint8_t dev_id, uint8_t
> queue_id, uint32_t attr_id,
> >   *
> >   *  @see rte_event_port_setup(), rte_event_port_link()
> >   */
> > +#define RTE_EVENT_PORT_CFG_HINT_PRODUCER       (1ULL << 2)
> > +/**< Hint that this event port will primarily enqueue events to the system.
> > + * A PMD can optimize its internal workings by assuming that this port is
> > + * primarily going to enqueue NEW events. Note that this flag is only a hint.
> 
> IMO, We can explicitly add the following line in each comment.
> 
> PMDs must operate under the assumption that any port can enqueue an
> event with any type of op.

Yes - good clarification to make it very clear each time.

<snip remaining patch>
  

Patch

diff --git a/lib/eventdev/rte_eventdev.h b/lib/eventdev/rte_eventdev.h
index a9c496fb62..159b580938 100644
--- a/lib/eventdev/rte_eventdev.h
+++ b/lib/eventdev/rte_eventdev.h
@@ -709,6 +709,29 @@  rte_event_queue_attr_get(uint8_t dev_id, uint8_t queue_id, uint32_t attr_id,
  *
  *  @see rte_event_port_setup(), rte_event_port_link()
  */
+#define RTE_EVENT_PORT_CFG_HINT_PRODUCER       (1ULL << 2)
+/**< Hint that this event port will primarily enqueue events to the system.
+ * A PMD can optimize its internal workings by assuming that this port is
+ * primarily going to enqueue NEW events. Note that this flag is only a hint.
+ *
+ *  @see rte_event_port_setup()
+ */
+#define RTE_EVENT_PORT_CFG_HINT_CONSUMER       (1ULL << 3)
+/**< Hint that this event port will primarily dequeue events from the system.
+ * A PMD can optimize its internal workings by assuming that this port is
+ * primarily going to consume events, and not enqueue FORWARD or RELEASE events.
+ * Note that this flag is only a hint.
+ *
+ *  @see rte_event_port_setup()
+ */
+#define RTE_EVENT_PORT_CFG_HINT_WORKER         (1ULL << 4)
+/**< Hint that this event port will primarily events to the system.
+ * A PMD can optimize its internal workings by assuming that this port is
+ * primarily going to FORWARD events, and not enqueue NEW or RELEASE events
+ * often. Note that this flag is only a hint.
+ *
+ *  @see rte_event_port_setup()
+ */
 
 /** Event port configuration structure */
 struct rte_event_port_conf {