> -----Original Message-----
> From: Shijith Thotton <sthot...@marvell.com>
> Sent: Friday, January 24, 2025 3:30 PM
> To: Naga Harish K, S V <s.v.naga.haris...@intel.com>; dev@dpdk.org
> Cc: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; Pathak,
> Pravin <pravin.pat...@intel.com>; Hemant Agrawal
> <hemant.agra...@nxp.com>; Sachin Saxena <sachin.sax...@nxp.com>;
> Mattias R_nnblom <mattias.ronnb...@ericsson.com>; Jerin Jacob
> <jer...@marvell.com>; Liang Ma <lian...@liangbit.com>; Mccarthy, Peter
> <peter.mccar...@intel.com>; Van Haaren, Harry
> <harry.van.haa...@intel.com>; Carrillo, Erik G <erik.g.carri...@intel.com>;
> Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; Amit Prakash Shukla
> <amitpraka...@marvell.com>; Burakov, Anatoly
> <anatoly.bura...@intel.com>
> Subject: RE: [RFC PATCH] eventdev: adapter API to configure multiple Rx
> queues
>
> >> >> >> >> >>> This RFC introduces a new API,
> >> >> >> >> >>> rte_event_eth_rx_adapter_queues_add(),
> >> >> >> >> >>> designed to enhance the flexibility of configuring
> >> >> >> >> >>> multiple Rx queues in eventdev Rx adapter.
> >> >> >> >> >>>
> >> >> >> >> >>> The existing rte_event_eth_rx_adapter_queue_add() API
> >> >> >> >> >>> supports adding multiple queues by specifying
> >> >> >> >> >>> rx_queue_id = -1, but it lacks the ability to
> >> >> >> >> >apply
> >> >> >> >> >>> specific configurations to each of the added queues.
> >> >> >> >> >>>
> >> >> >> >> >>
> >> >> >> >> >>The application can still use the existing
> >> >> >> >> >>rte_event_eth_rx_adapter_queue_add() API in a loop with
> >> >> >> >> >>different configurations for different queues.
> >> >> >> >> >>
> >> >> >> >> >>The proposed API is not enabling new features that cannot
> >> >> >> >> >>be achieved with the existing API.
> >> >> >> >> >>Adding new APIs without much usefulness causes unnecessary
> >> >> >> >> >>complexity/confusion for users.
> >> >> >> >> >>
> >> >>
> >> >> The eth_rx_adapter_queue_add eventdev PMD operation can be
> updated
> >> to
> >> >> support burst mode. Internally, both the new and existing APIs can
> >> >> utilize this updated operation. This enables applications to use
> >> >> either API and achieve
> >> >the
> >> >> same results while adding a single queue. For adding multiple RX
> >> >> queues to
> >> >the
> >> >> adapter, the new API can be used as it is not supported by the old API.
> >> >>
> >> >
> >> >Not all platforms implement the eventdev PMD operation for
> >> >eth_rx_adapter_queue_add, so this does not apply to all platforms.
> >> >
> >>
> >> Yes, but there are hardware PMDs that implement
> >eth_rx_adapter_queue_add
> >> op, and I am looking for a solution that works for both cases.
> >>
> >> The idea is to use the new eventdev PMD operation
> >> (eth_rx_adapter_queues_add) within the
> >> rte_event_eth_rx_adapter_queue_add() API. The parameters of this API
> >> can be easily mapped to and supported by the new PMD operation.
> >>
> >
> >This requires a change to the rte_event_eth_rx_adapter_queue_add()
> >stable API parameters.
> >This is an ABI breakage and may not be possible now.
> >It requires changes to many current applications that are using the
> >rte_event_eth_rx_adapter_queue_add() stable API.
> >
>
> What I meant by mapping was to retain the stable API parameters as they are.
> Internally, the API can use the proposed eventdev PMD operation
> (eth_rx_adapter_queues_add) without causing an ABI break, as shown below.
>
> int rte_event_eth_rx_adapter_queue_add(uint8_t id, uint16_t eth_dev_id,
> int32_t rx_queue_id,
> const struct rte_event_eth_rx_adapter_queue_conf *conf) {
> if (rx_queue_id == -1)
> dev->dev_ops->eth_rx_adapter_queues_add)(
> dev, &rte_eth_devices[eth_dev_id], 0,
> conf, 0);
> else
> dev->dev_ops->eth_rx_adapter_queues_add)(
> dev, &rte_eth_devices[eth_dev_id], &rx_queue_id,
> conf, 1);
> }
>
> With above change, old op (eth_rx_adapter_queue_add) can be removed as
> both API (stable and proposed) will be using eth_rx_adapter_queues_add.
>
The whole idea is not to have the proposed API, as it does not add any new
feature but just a combination of existing API.
It is already discussed in the previous threads.
The internal implementation details are not a concern.
> >> typedef int (*eventdev_eth_rx_adapter_queues_add_t)(
> >> const struct rte_eventdev *dev,
> >> const struct rte_eth_dev *eth_dev,
> >> int32_t rx_queue_id[],
> >> const struct rte_event_eth_rx_adapter_queue_conf queue_conf[],
> >> uint16_t nb_rx_queues);
> >>
> >> With this, the old PMD op (eth_rx_adapter_queue_add) can be removed.
> >>
> >> >> >> >> >
> >> >> >> >> >The new API was introduced because the existing API does
> >> >> >> >> >not support adding multiple queues with specific configurations.
> >> >> >> >> >It serves as a burst variant of the existing API, like many
> >> >> >> >> >other APIs in
> >> >> DPDK.
> >> >> >> >> >
> >> >> >> >
> >> >> >> >The other burst APIs may be there for dataplane
> >> >> >> >functionalities, but may not be for the control plane
> >> >> >> >functionalities.
> >> >> >> >
> >> >> >>
> >> >> >> rte_acl_add_rules() is an example of burst API in control path.
> >> >> >>
> >> >> >
> >> >> >I mean, In general, burst APIs are for data-plane functions.
> >> >> >This may be one of the rare cases where a burst API is in the control
> path.
> >> >> >
> >> >> >> >> >For better clarity, the API can be renamed to
> >> >> >> >> >rte_event_eth_rx_adapter_queue_add_burst() if needed.
> >> >> >> >> >
> >> >> >> >> >In hardware, adding each queue individually incurs
> >> >> >> >> >significant overheads, such as mailbox operations. A burst
> >> >> >> >> >API helps to amortize this overhead. Since real- world
> >> >> >> >> >applications often call the API with specific queue_ids,
> >> >> >> >> >the burst API can provide considerable
> >> >> benefits.
> >> >> >> >> >Testing shows a 75% reduction in time when adding multiple
> >> >> >> >> >queues to the RX adapter using the burst API on our platform.
> >> >> >> >> >
> >> >> >> >
> >> >> >> > As batching helps for a particular hardware device, this may
> >> >> >> >not be applicable for all platforms/cases.
> >> >> >> > Since queue_add is a control plane operation, latency may not
> >be
> >> >> >> >a concern.
> >> >> >>
> >> >> >> In certain use cases, these APIs can be considered semi-fast path.
> >> >> >> For
> >> >> >instance,
> >> >> >> in an application that hotplugs a port on demand, configuring
> >> >> >> all available queues simultaneously can significantly reduce latency.
> >> >> >>
> >> >> >
> >> >> >As said earlier, this latency reduction (when trying to add
> >> >> >multiple RX queues to the Event Ethernet Rx adapter) may not
> >> >> >apply to all
> >> >> platforms/cases.
> >> >> >This API is not for configuring queues but for adding the queues
> >> >> >to the RX adapter.
> >> >> >
> >> >> >> >How to specify a particular set(specific queue_ids) of
> >> >> >> >rx_queues that has a non- zero start index with the new proposed
> API?
> >> >> >>
> >> >> >> In the proposed API,
> >> >> >> int rte_event_eth_rx_adapter_queues_add(
> >> >> >> uint8_t id, uint16_t eth_dev_id, int32_t
> >> >> >> rx_queue_id[],
> >> >> >> const struct
> >> >> >> rte_event_eth_rx_adapter_queue_conf
> conf[],
> >> >> >> uint16_t nb_rx_queues); rx_queues_id is
> >> >> >> an array containing the receive queues ids, which can start
> >> >> >> from a non-zero value. The array index is used solely to locate
> >> >> >> the corresponding queue_conf. For example, rx_queues_id[i] will
> >> >> >> use
> >conf[i].
> >> >> >>
> >> >> >
> >> >> >Ok
> >> >> >
> >> >> >> > Since this is still not possible with the proposed API, the
> >> >> >> >existing queue_add API needs to be used with specific
> >> >> >> >queue_ids and their configurations.
> >> >> >> >
> >> >> >> >> >I can modify the old API implementation to act as a wrapper
> >> >> >> >> >around the burst API, with number of queues equal to 1. If
> >> >> >> >> >concerns remain, we can explore deprecation as an alternative.
> >> >> >> >> >
> >> >> >> >>
> >> >> >> >> Please let me know if you have any suggestions/feedback on
> >> >> >> >> what I said above.
> >> >> >> >
> >> >> >> >Still feel the new proposed API can be avoided as it looks
> >> >> >> >like a different combination of existing API instead of adding
> >> >> >> >some new
> >> >features.
> >> >> >> >
> >> >> >> >> If not, I can go ahead and send v1.
> >> >> >> >>
> >> >> >> >> >>> The proposed API, rte_event_eth_rx_adapter_queues_add,
> >> >> >> >> >>> addresses this limitation by:
> >> >> >> >> >>>
> >> >> >> >> >>> - Enabling users to specify an array of rx_queue_id
> >> >> >> >> >>> values
> >> alongside
> >> >> >> >> >>> individual configurations for each queue.
> >> >> >> >> >>>
> >> >> >> >> >>> - Supporting a nb_rx_queues argument to define the
> >> >> >> >> >>> number of queues
> >> >> >> >to
> >> >> >> >> >>> configure. When set to 0, the API applies a common
> >> >> >> >> >>> configuration
> >> >> to
> >> >> >> >> >>> all queues, similar to the existing rx_queue_id = -1
> >> >> >> >> >>> behavior.
> >> >> >> >> >>>
> >> >> >> >> >>> This enhancement allows for more granular control when
> >> >> >> >> >>> configuring
> >> >> >> >> >multiple
> >> >> >> >> >>> Rx queues. Additionally, the API can act as a
> >> >> >> >> >>> replacement for the older API, offering both flexibility
> >> >> >> >> >>> and improved
> >> functionality.
> >> >> >> >> >>>
> >> >> >> >> >>> Signed-off-by: Shijith Thotton <sthot...@marvell.com>
> >> >> >> >> >>> ---
> >> >> >> >> >>> lib/eventdev/eventdev_pmd.h | 34
> >> >> >> >> +++++++++++++++++++++++++
> >> >> >> >> >>> lib/eventdev/rte_event_eth_rx_adapter.h | 34
> >> >> >> >> >>> +++++++++++++++++++++++++
> >> >> >> >> >>> 2 files changed, 68 insertions(+)
> >> >> >> >> >>>
> >> >> >> >> >>> diff --git a/lib/eventdev/eventdev_pmd.h
> >> >> >> >> >>> b/lib/eventdev/eventdev_pmd.h index
> >> 36148f8d86..2e458a9779
> >> >> >> >> 100644
> >> >> >> >> >>> --- a/lib/eventdev/eventdev_pmd.h
> >> >> >> >> >>> +++ b/lib/eventdev/eventdev_pmd.h
> >> >> >> >> >>> @@ -25,6 +25,7 @@
> >> >> >> >> >>> #include <rte_mbuf_dyn.h>
> >> >> >> >> >>>
> >> >> >> >> >>> #include "event_timer_adapter_pmd.h"
> >> >> >> >> >>> +#include "rte_event_eth_rx_adapter.h"
> >> >> >> >> >>> #include "rte_eventdev.h"
> >> >> >> >> >>>
> >> >> >> >> >>> #ifdef __cplusplus
> >> >> >> >> >>> @@ -708,6 +709,37 @@ typedef int
> >> >> >> >> >>> (*eventdev_eth_rx_adapter_queue_add_t)(
> >> >> >> >> >>> int32_t rx_queue_id,
> >> >> >> >> >>> const struct
> rte_event_eth_rx_adapter_queue_conf
> >> >> >> >> >>> *queue_conf);
> >> >> >> >> >>>
> >> >> >> >> >>> +/**
> >> >> >> >> >>> + * Add ethernet Rx queues to event device. This
> >> >> >> >> >>> +callback is invoked if
> >> >> >> >> >>> + * the caps returned from
> >> >> >> >> >>> +rte_eventdev_eth_rx_adapter_caps_get(,
> >> >> >> >> >>> +eth_port_id)
> >> >> >> >> >>> + * has RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT
> >set.
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param dev
> >> >> >> >> >>> + * Event device pointer
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param eth_dev
> >> >> >> >> >>> + * Ethernet device pointer
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param rx_queue_id
> >> >> >> >> >>> + * Ethernet device receive queue index array
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param queue_conf
> >> >> >> >> >>> + * Additional configuration structure array
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param nb_rx_queues
> >> >> >> >> >>> + * Number of ethernet device receive queues
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @return
> >> >> >> >> >>> + * - 0: Success, ethernet receive queues added
> >> >> >> >> >>> successfully.
> >> >> >> >> >>> + * - <0: Error code returned by the driver function.
> >> >> >> >> >>> + */
> >> >> >> >> >>> +typedef int (*eventdev_eth_rx_adapter_queues_add_t)(
> >> >> >> >> >>> + const struct rte_eventdev *dev,
> >> >> >> >> >>> + const struct rte_eth_dev *eth_dev,
> >> >> >> >> >>> + int32_t rx_queue_id[],
> >> >> >> >> >>> + const struct
> rte_event_eth_rx_adapter_queue_conf
> >> >> >> >> >>> queue_conf[],
> >> >> >> >> >>> + uint16_t nb_rx_queues);
> >> >> >> >> >>> +
> >> >> >> >> >>> /**
> >> >> >> >> >>> * Delete ethernet Rx queues from event device. This
> >> >> >> >> >>> callback is
> >> >> >invoked
> >> >> >> if
> >> >> >> >> >>> * the caps returned from
> >> >> >> >> >>> eventdev_eth_rx_adapter_caps_get(,
> >> >> >> >> >eth_port_id)
> >> >> >> >> >>> @@ -1578,6 +1610,8 @@ struct eventdev_ops {
> >> >> >> >> >>> /**< Get ethernet Rx adapter capabilities */
> >> >> >> >> >>> eventdev_eth_rx_adapter_queue_add_t
> >> >> >eth_rx_adapter_queue_add;
> >> >> >> >> >>> /**< Add Rx queues to ethernet Rx adapter */
> >> >> >> >> >>> + eventdev_eth_rx_adapter_queues_add_t
> >> >> >> >> >>> eth_rx_adapter_queues_add;
> >> >> >> >> >>> + /**< Add Rx queues to ethernet Rx adapter */
> >> >> >> >> >>> eventdev_eth_rx_adapter_queue_del_t
> >> >> >eth_rx_adapter_queue_del;
> >> >> >> >> >>> /**< Delete Rx queues from ethernet Rx adapter */
> >> >> >> >> >>> eventdev_eth_rx_adapter_queue_conf_get_t
> >> >> >> >> >>> eth_rx_adapter_queue_conf_get; diff --git
> >> >> >> >> >>> a/lib/eventdev/rte_event_eth_rx_adapter.h
> >> >> >> >> >>> b/lib/eventdev/rte_event_eth_rx_adapter.h
> >> >> >> >> >>> index 9237e198a7..9a5c560b67 100644
> >> >> >> >> >>> --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> >> >> >> >> >>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> >> >> >> >> >>> @@ -553,6 +553,40 @@ int
> >> >> >> >> rte_event_eth_rx_adapter_queue_add(uint8_t
> >> >> >> >> >>> id,
> >> >> >> >> >>> int32_t rx_queue_id,
> >> >> >> >> >>> const struct
> >> >> >rte_event_eth_rx_adapter_queue_conf
> >> >> >> >> >>> *conf);
> >> >> >> >> >>>
> >> >> >> >> >>> +/**
> >> >> >> >> >>> + * Add multiple receive queues to an event adapter.
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param id
> >> >> >> >> >>> + * Adapter identifier.
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param eth_dev_id
> >> >> >> >> >>> + * Port identifier of Ethernet device.
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param rx_queue_id
> >> >> >> >> >>> + * Array of Ethernet device receive queue indices.
> >> >> >> >> >>> + * If nb_rx_queues is 0, then rx_queue_id is ignored.
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param conf
> >> >> >> >> >>> + * Array of additional configuration structures of
> >> >> >> >> >>> +type
> >> >> >> >> >>> + * *rte_event_eth_rx_adapter_queue_conf*. conf[i] is
> >> >> >> >> >>> +used for
> >> >> >> >> >>> rx_queue_id[i].
> >> >> >> >> >>> + * If nb_rx_queues is 0, then conf[0] is used for all Rx
> >> >> >> >> >>> queues.
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @param nb_rx_queues
> >> >> >> >> >>> + * Number of receive queues to add.
> >> >> >> >> >>> + * If nb_rx_queues is 0, then all Rx queues configured
> >> >> >> >> >>> +for
> >> >> >> >> >>> + * the device are added with the same configuration in
> conf[0].
> >> >> >> >> >>> + * @see RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ
> >> >> >> >> >>> + *
> >> >> >> >> >>> + * @return
> >> >> >> >> >>> + * - 0: Success, Receive queues added correctly.
> >> >> >> >> >>> + * - <0: Error code on failure.
> >> >> >> >> >>> + */
> >> >> >> >> >>> +__rte_experimental
> >> >> >> >> >>> +int rte_event_eth_rx_adapter_queues_add(
> >> >> >> >> >>> + uint8_t id, uint16_t eth_dev_id,
> int32_t
> >> >> >> >> >>> rx_queue_id[],
> >> >> >> >> >>> + const struct
> >> >> >> rte_event_eth_rx_adapter_queue_conf
> >> >> >> >> >>> conf[],
> >> >> >> >> >>> + uint16_t nb_rx_queues);
> >> >> >> >> >>> +
> >> >> >> >> >>> /**
> >> >> >> >> >>> * Delete receive queue from an event adapter.
> >> >> >> >> >>> *
> >> >> >> >> >>> --
> >> >> >> >> >>> 2.25.1
