RE: [RFC PATCH] eventdev: adapter API to configure multiple Rx queues

["Naga Harish K, S V" <s.v.naga.harish.k@intel.com>]

> -----Original Message-----
> From: Shijith Thotton <sthotton@marvell.com>
> Sent: Thursday, January 16, 2025 11:58 AM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>; dev@dpdk.org
> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Pathak,
> Pravin <pravin.pathak@intel.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Sachin Saxena <sachin.saxena@nxp.com>;
> Mattias R_nnblom <mattias.ronnblom@ericsson.com>; Jerin Jacob
> <jerinj@marvell.com>; Liang Ma <liangma@liangbit.com>; Mccarthy, Peter
> <peter.mccarthy@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Amit Prakash Shukla
> <amitprakashs@marvell.com>; Burakov, Anatoly
> <anatoly.burakov@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 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 <sthotton@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