On 2021-10-29 17:17, Jerin Jacob wrote: > On Fri, Oct 29, 2021 at 8:33 PM Mattias Rönnblom > <mattias.ronnb...@ericsson.com> wrote: >> On 2021-10-29 16:38, Jerin Jacob wrote: >>> On Tue, Oct 26, 2021 at 11:02 PM Mattias Rönnblom >>> <mattias.ronnb...@ericsson.com> wrote: >>>> Extend Eventdev API to allow for event devices which require various >>>> forms of internal processing to happen, even when events are not >>>> enqueued to or dequeued from a port. >>>> >>>> PATCH v1: >>>> - Adapt to the move of fastpath function pointers out of >>>> rte_eventdev struct >>>> - Attempt to clarify how often the application is expected to >>>> call rte_event_maintain() >>>> - Add trace point >>>> RFC v2: >>>> - Change rte_event_maintain() return type to be consistent >>>> with the documentation. >>>> - Remove unused typedef from eventdev_pmd.h. >>>> >>>> Signed-off-by: Mattias Rönnblom <mattias.ronnb...@ericsson.com> >>>> Tested-by: Richard Eklycke <richard.ekly...@ericsson.com> >>>> Tested-by: Liron Himi <lir...@marvell.com> >>>> --- >>>> >>>> +/** >>>> + * Maintain an event device. >>>> + * >>>> + * This function is only relevant for event devices which has the >>>> + * RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag set. Such devices require the >>>> + * application to call rte_event_maintain() on a port during periods >>>> + * which it is neither enqueuing nor dequeuing events from that >>>> + * port. >>> # We need to add "by the same core". Right? As other core such as >>> service core can not call rte_event_maintain() >> >> Do you mean by the same lcore thread that "owns" (dequeues and enqueues >> to) the port? Yes. I thought that was implicit, since eventdev port are >> not MT safe. I'll try to figure out some wording that makes that more clear. > OK. > >> >>> # Also, Incase of Adapters enqueue() happens, right? If so, either >>> above text is not correct. >>> # @Erik Gabriel Carrillo @Jayatheerthan, Jay @Gujjar, Abhinandan S >>> Please review 3/3 patch on adapter change. >>> Let me know you folks are OK with change or not or need more time to >>> analyze. >>> >>> If it need only for the adapter subsystem then can we make it an >>> internal API between DSW and adapters? >> >> No, it's needed for any producer-only eventdev ports, including any such >> ports used by the application. > > In that case, the code path in testeventdev, eventdev_pipeline, etc needs > to be updated. I am worried about the performance impact for the drivers they > don't have such limitations. > > Why not have an additional config option in port_config which says > it is a producer-only port by an application and takes care of the driver. > > In the current adapters code, you are calling maintain() when enqueue > returns zero. > In such a case, if the port is configured as producer and then > internally it can call maintain. > > Thoughts from other eventdev maintainers? > Cc+ @Van Haaren, Harry @Richardson, Bruce @Gujjar, Abhinandan S > @Jayatheerthan, Jay @Erik Gabriel Carrillo @McDaniel, Timothy @Pavan > Nikhilesh @Hemant Agrawal @Liang Ma >
One more thing to consider: should we add a "int op" parameter to rte_event_maintain()? It would also solve hack #2 in DSW eventdev API integration: forcing an output buffer flush. This is today done with a zero-sized rte_event_enqueue() call. You could have something like: #define RTE_EVENT_DEV_MAINT_FLUSH (1) int rte_event_maintain(int op); It would also allow future extensions of "maintain", without ABI breakage. Explicit flush is rare in real applications, in my experience, but useful for test cases. I suspect for DSW to work with the DPDK eventdev test suite, flushing buffered events (either zero-sized enqueue, repeated rte_event_maintain() calls, or a single of the rte_event_maintain(RTE_EVENT_DEV_MAINT_FLUSH) call [assuming the above API]) is required in the test code. >> >> Should rte_event_maintain() be marked experimental? I don't know how >> that works for inline functions. >> >> >>> + rte_event_maintain() is a low-overhead function and should be >>>> + * called at a high rate (e.g., in the applications poll loop). >>>> + * >>>> + * No port may be left unmaintained. >>>> + * >>>> + * rte_event_maintain() may be called on event devices which haven't >>>> + * set RTE_EVENT_DEV_CAP_REQUIRES_MAINT flag, in which case it is a >>>> + * no-operation. >>>> + * >>>> + * @param dev_id >>>> + * The identifier of the device. >>>> + * @param port_id >>>> + * The identifier of the event port. >>>> + * @return >>>> + * - 0 on success. >>>> + * - -EINVAL if *dev_id* or *port_id* is invalid >>>> + * >>>> + * @see RTE_EVENT_DEV_CAP_REQUIRES_MAINT >>>> + */ >>>> +static inline int >>>> +rte_event_maintain(uint8_t dev_id, uint8_t port_id) >>>> +{ >>>> + 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 >>>> + rte_eventdev_trace_maintain(dev_id, port_id); >>>> + >>>> + if (fp_ops->maintain != NULL) >>>> + fp_ops->maintain(port); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> #ifdef __cplusplus >>>> } >>>> #endif >>>> diff --git a/lib/eventdev/rte_eventdev_core.h >>>> b/lib/eventdev/rte_eventdev_core.h >>>> index 61d5ebdc44..61fa65cab3 100644 >>>> --- a/lib/eventdev/rte_eventdev_core.h >>>> +++ b/lib/eventdev/rte_eventdev_core.h >>>> @@ -29,6 +29,9 @@ typedef uint16_t (*event_dequeue_burst_t)(void *port, >>>> struct rte_event ev[], >>>> uint64_t timeout_ticks); >>>> /**< @internal Dequeue burst of events from port of a device */ >>>> >>>> +typedef void (*event_maintain_t)(void *port); >>>> +/**< @internal Maintains a port */ >>>> + >>>> typedef uint16_t (*event_tx_adapter_enqueue_t)(void *port, >>>> struct rte_event ev[], >>>> uint16_t nb_events); >>>> @@ -54,6 +57,8 @@ struct rte_event_fp_ops { >>>> /**< PMD dequeue function. */ >>>> event_dequeue_burst_t dequeue_burst; >>>> /**< PMD dequeue burst function. */ >>>> + event_maintain_t maintain; >>>> + /**< PMD port maintenance function. */ >>>> event_tx_adapter_enqueue_t txa_enqueue; >>>> /**< PMD Tx adapter enqueue function. */ >>>> event_tx_adapter_enqueue_t txa_enqueue_same_dest; >>>> diff --git a/lib/eventdev/rte_eventdev_trace_fp.h >>>> b/lib/eventdev/rte_eventdev_trace_fp.h >>>> index 5639e0b83a..c5a79a14d8 100644 >>>> --- a/lib/eventdev/rte_eventdev_trace_fp.h >>>> +++ b/lib/eventdev/rte_eventdev_trace_fp.h >>>> @@ -38,6 +38,13 @@ RTE_TRACE_POINT_FP( >>>> rte_trace_point_emit_ptr(enq_mode_cb); >>>> ) >>>> >>>> +RTE_TRACE_POINT_FP( >>>> + rte_eventdev_trace_maintain, >>>> + RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint8_t port_id), >>>> + rte_trace_point_emit_u8(dev_id); >>>> + rte_trace_point_emit_u8(port_id); >>>> +) >>>> + >>>> RTE_TRACE_POINT_FP( >>>> rte_eventdev_trace_eth_tx_adapter_enqueue, >>>> RTE_TRACE_POINT_ARGS(uint8_t dev_id, uint8_t port_id, void >>>> *ev_table, >>>> -- >>>> 2.25.1 >>>>