Re: [dpdk-dev] [PATCH 1/3] eventdev: allow for event devices requiring maintenance

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

On 2021-10-29 16:38, Jerin Jacob wrote:
> On Tue, Oct 26, 2021 at 11:02 PM Mattias Rönnblom
> <mattias.ronnblom@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.ronnblom@ericsson.com>
>> Tested-by: Richard Eklycke <richard.eklycke@ericsson.com>
>> Tested-by: Liron Himi <lironh@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.


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


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