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
>
>
> 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
> >>
>
