> -----Original Message-----
> From: Jerin Jacob <jerinjac...@gmail.com>
> Sent: Friday, February 10, 2023 7:36 PM
> To: Naga Harish K, S V <s.v.naga.haris...@intel.com>
> Cc: jer...@marvell.com; Carrillo, Erik G <erik.g.carri...@intel.com>; Gujjar,
> Abhinandan S <abhinandan.guj...@intel.com>; dev@dpdk.org;
> Jayatheerthan, Jay <jay.jayatheert...@intel.com>
> Subject: Re: [PATCH v5 2/3] eventdev/eth_tx: add params set/get APIs
>
> On Fri, Feb 10, 2023 at 7:17 PM Naga Harish K S V
> <s.v.naga.haris...@intel.com> wrote:
> >
> > The adapter runtime configuration parameters defined in the struct
> > rte_event_eth_tx_adapter_runtime_params can be configured and
> > retrieved using rte_event_eth_tx_adapter_runtime_params_set and
> rte_event_eth_tx_adapter_runtime_params_set()
>
> > rte_event_eth_tx_adapter_runtime_params_get respectively.
>
> rte_event_eth_tx_adapter_runtime_params_get()
>
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.haris...@intel.com>
>
> > --- a/doc/guides/prog_guide/event_ethernet_tx_adapter.rst
> > +++ b/doc/guides/prog_guide/event_ethernet_tx_adapter.rst
> > @@ -225,3 +225,12 @@ Stop function stops the adapter runtime function
> > from enqueueing any packets to the associated Tx queue. This API also
> > frees any packets that may have been buffered for this queue. All
> > inflight packets destined to the queue are freed by the adapter runtime
> until the queue is started again.
> > +
> > +Set/Get adapter runtime configuration parameters
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +The runtime configuration parameters of adapter can be set/get using
> > +``rte_event_eth_tx_adapter_runtime_params_set()`` and
> > +``rte_event_eth_tx_adapter_runtime_params_get()`` respectively. The
> > +parameters that can be set/get are defined in ``struct
> > +rte_event_eth_tx_adapter_runtime_params``.
> > diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c
> > b/lib/eventdev/rte_event_eth_tx_adapter.c
> > index cce50c3c18..131e11e01d 100644
> > --- a/lib/eventdev/rte_event_eth_tx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_tx_adapter.c
> > @@ -124,6 +124,8 @@ struct txa_service_data {
> > uint16_t dev_count;
> > /* Loop count to flush Tx buffers */
> > int loop_cnt;
> > + /* Loop count threshold to flush Tx buffers */
> > + uint16_t flush_threshold;
> > /* Per ethernet device structure */
> > struct txa_service_ethdev *txa_ethdev;
> > /* Statistics */
> > @@ -665,13 +667,14 @@ txa_service_func(void *args)
> > ret = 0;
> > }
> >
> > - if ((txa->loop_cnt++ & (TXA_FLUSH_THRESHOLD - 1)) == 0) {
> > + if (txa->loop_cnt++ == txa->flush_threshold) {
> >
> > struct txa_service_ethdev *tdi;
> > struct txa_service_queue_info *tqi;
> > struct rte_eth_dev *dev;
> > uint16_t i;
> >
> > + txa->loop_cnt = 0;
> > tdi = txa->txa_ethdev;
> > nb_tx = 0;
> >
> > @@ -769,6 +772,7 @@ txa_service_adapter_create_ext(uint8_t id, struct
> rte_eventdev *dev,
> > txa->service_id = TXA_INVALID_SERVICE_ID;
> > rte_spinlock_init(&txa->tx_lock);
> > txa_service_data_array[id] = txa;
> > + txa->flush_threshold = TXA_FLUSH_THRESHOLD;
> >
> > return 0;
> > }
> > @@ -1291,6 +1295,115 @@
> rte_event_eth_tx_adapter_stats_reset(uint8_t id)
> > return ret;
> > }
> >
> > +int
> > +rte_event_eth_tx_adapter_runtime_params_init(
> > + struct rte_event_eth_tx_adapter_runtime_params
> > +*txa_params) {
> > + if (txa_params == NULL)
> > + return -EINVAL;
> > +
> > + memset(txa_params, 0, sizeof(*txa_params));
> > + txa_params->max_nb_tx = TXA_MAX_NB_TX;
> > + txa_params->flush_threshold = TXA_FLUSH_THRESHOLD;
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +txa_caps_check(uint8_t id, struct txa_service_data *txa) {
> > + uint32_t caps = 0;
> > + struct rte_eth_dev *eth_dev = NULL;
> > + struct txa_service_ethdev *tdi;
> > + int i;
> > +
> > + if (!txa->dev_count)
> > + return -EINVAL;
> > +
> > + /* The eth_dev used is always the same type.
> > + * Hence first valid eth_dev is taken.
> > + */
> > + for (i = 0; i < txa->dev_count; i++) {
> > + tdi = &txa->txa_ethdev[i];
> > + if (tdi->nb_queues) {
> > + eth_dev = tdi->dev;
> > + break;
> > + }
> > + }
> > + if (eth_dev == NULL)
> > + return -EINVAL;
> > +
> > + if (txa_dev_caps_get(id))
> > + txa_dev_caps_get(id)(txa_evdev(id), eth_dev, &caps);
> > +
> > + if (caps & RTE_EVENT_ETH_TX_ADAPTER_CAP_INTERNAL_PORT)
> > + return -ENOTSUP;
> > +
> > + return 0;
> > +}
> > +
> > +int
> > +rte_event_eth_tx_adapter_runtime_params_set(uint8_t id,
> > + struct rte_event_eth_tx_adapter_runtime_params
> > +*txa_params) {
> > + struct txa_service_data *txa;
> > + int ret;
> > +
> > + if (txa_lookup())
> > + return -ENOMEM;
> > +
> > + TXA_CHECK_OR_ERR_RET(id);
> > +
> > + if (txa_params == NULL)
> > + return -EINVAL;
> > +
> > + txa = txa_service_id_to_data(id);
> > + if (txa == NULL)
> > + return -EINVAL;
> > +
> > + ret = txa_caps_check(id, txa);
> > + if (ret)
> > + return ret;
> > +
> > + rte_spinlock_lock(&txa->tx_lock);
> > + txa->flush_threshold = txa_params->flush_threshold;
> > + txa->max_nb_tx = txa_params->max_nb_tx;
> > + rte_spinlock_unlock(&txa->tx_lock);
> > +
> > + return 0;
> > +}
> > +
> > +int
> > +rte_event_eth_tx_adapter_runtime_params_get(uint8_t id,
> > + struct rte_event_eth_tx_adapter_runtime_params
> > +*txa_params) {
> > + struct txa_service_data *txa;
> > + int ret;
> > +
> > + if (txa_lookup())
> > + return -ENOMEM;
> > +
> > + TXA_CHECK_OR_ERR_RET(id);
> > +
> > + if (txa_params == NULL)
> > + return -EINVAL;
> > +
> > + txa = txa_service_id_to_data(id);
> > + if (txa == NULL)
> > + return -EINVAL;
> > +
> > + ret = txa_caps_check(id, txa);
> > + if (ret)
> > + return ret;
> > +
> > + rte_spinlock_lock(&txa->tx_lock);
> > + txa_params->flush_threshold = txa->flush_threshold;
> > + txa_params->max_nb_tx = txa->max_nb_tx;
> > + rte_spinlock_unlock(&txa->tx_lock);
> > +
> > + return 0;
> > +}
> > +
> > int
> > rte_event_eth_tx_adapter_stop(uint8_t id) { diff --git
> > a/lib/eventdev/rte_event_eth_tx_adapter.h
> > b/lib/eventdev/rte_event_eth_tx_adapter.h
> > index cd539af7ef..9b1ac2055e 100644
> > --- a/lib/eventdev/rte_event_eth_tx_adapter.h
> > +++ b/lib/eventdev/rte_event_eth_tx_adapter.h
> > @@ -37,6 +37,9 @@
> > * - rte_event_eth_tx_adapter_instance_get()
> > * - rte_event_eth_tx_adapter_queue_start()
> > * - rte_event_eth_tx_adapter_queue_stop()
> > + * - rte_event_eth_tx_adapter_runtime_params_get()
> > + * - rte_event_eth_tx_adapter_runtime_params_init()
> > + * - rte_event_eth_tx_adapter_runtime_params_set()
> > *
> > * The application creates the adapter using
> > * rte_event_eth_tx_adapter_create() or
> rte_event_eth_tx_adapter_create_ext().
> > @@ -103,6 +106,25 @@ struct rte_event_eth_tx_adapter_conf {
> > */
> > };
> >
> > +/**
> > + * Adapter runtime configuration parameters */ struct
> > +rte_event_eth_tx_adapter_runtime_params {
> > + uint32_t max_nb_tx;
> > + /**< The adapter can return early if it has processed at least
> > + * max_nb_tx mbufs. This isn't treated as a requirement; batching
> > may
> > + * cause the adapter to process more than max_nb_tx mbufs.
> > + * This is valid for service based SW adapter only.
>
> express with RTE_EVENT_*CAP_INTERNAL_PORT
>
> > + */
> > + uint16_t flush_threshold;
> > + /**< the number of service function iteration count to
> > + * flush buffered packets.
> > + * This is valid for service based SW adapter only.
>
> express with RTE_EVENT_*CAP_INTERNAL_PORT
>
>
> > + */
> > + uint16_t rsvd[29];
> > + /**< Reserved fields for future expansion */ };
> > +
> > /**
> > * Function type used for adapter configuration callback. The callback is
> > * used to fill in members of the struct
> > rte_event_eth_tx_adapter_conf, this @@ -516,6 +538,72 @@
> > __rte_experimental int rte_event_eth_tx_adapter_queue_stop(uint16_t
> > eth_dev_id, uint16_t tx_queue_id);
> >
> > +/**
> > + * Initialize the adapter runtime configuration parameters with
> > +default values
> > + *
> > + * @param txa_params
> > + * A pointer to structure of type struct
> > +rte_event_eth_tx_adapter_runtime_params
> > + *
> > + * @return
> > + * - 0: Success
> > + * - <0: Error code on failure
> > + */
> > +__rte_experimental
> > +int
> > +rte_event_eth_tx_adapter_runtime_params_init(
> > + struct rte_event_eth_tx_adapter_runtime_params
> > +*txa_params);
> > +
> > +/**
> > + * Set the runtime configuration parameters for adapter.
> > + *
> > + * In case not all fields are to be updated, the suggested way to use
> > +this
> > + * api is read the current values using
> > +rte_event_eth_tx_adapter_get_params(),
>
> api -> API
>
> Not relevent any more. Use
> rte_event_eth_tx_adapter_runtime_params_init().
> i.e This sentence can be removed.
>
>
> > + * modify the required parameters and then call
> > + * rte_event_eth_tx_adapter_runtime_params_set().
>
>
>
> > + *
> > + * This API is to be used after adding at least one queue to the
> > + adapter
> > + * and is supported only for service based adapter.
>
> This paragraph we can removed here as it is not generic API description.
>
This sequence is still needed as the "max_nb_tx" parameter of the adapter is
configured
during the addition of first queue to the adapter with the default value. If
params_set() is
called before adding the first queue, the params of the adapter will be
overwritten to the
default value later during the first queue add.
> > + *
> > + * @param id
> > + * Adapter identifier
> > + * @param params
> > + * A pointer to structure of type struct
> > +rte_event_eth_tx_adapter_runtime_params
> > + * with configuration parameter values. The reserved fields of this
> > +structure
> > + * must be initialized to zero and the valid fields need to be set
> appropriately.
> > + * This structure can be initialized using
> > + * rte_event_eth_tx_adapter_runtime_params_init() API to default
> > +values or
> > + * application may reset this structure and update required fields.
> > + *
> > + * @return
> > + * - 0: Success
> > + * - <0: Error code on failure
> > + */
> > +__rte_experimental
> > +int
> > +rte_event_eth_tx_adapter_runtime_params_set(uint8_t id,
> > + struct rte_event_eth_tx_adapter_runtime_params
> > +*params);
> > +
> > +/**
> > + * Get the runtime configuration parameters of adapter.
> > + *
> > + * This API is to be used after adding at least one queue to the
> > +adapter
> > + * and is supported only for service based adapter.
>
> This paragraph we can removed here as it is not generic API description.
>
Same as above.
> Similar comment to 3/3 patch. Also add chnagelog when sending next version
> to know what is changed.
>
> > + *
> > + * @param id
> > + * Adapter identifier
> > + * @param[out] params
> > + * A pointer to structure of type struct
> > +rte_event_eth_tx_adapter_runtime_params
> > + * containing valid Tx adapter parameters when return value is 0.
> > + *
> > + * @return
> > + * - 0: Success
> > + * - <0: Error code on failure
> > + */
> > +__rte_experimental
> > +int
> > +rte_event_eth_tx_adapter_runtime_params_get(uint8_t id,
> > + struct rte_event_eth_tx_adapter_runtime_params
> > +*params);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map index
> > ef9c3b86b2..7b93736dff 100644
> > --- a/lib/eventdev/version.map
> > +++ b/lib/eventdev/version.map
> > @@ -124,6 +124,9 @@ EXPERIMENTAL {
> > rte_event_eth_rx_adapter_runtime_params_get;
> > rte_event_eth_rx_adapter_runtime_params_init;
> > rte_event_eth_rx_adapter_runtime_params_set;
> > + rte_event_eth_tx_adapter_runtime_params_get;
> > + rte_event_eth_tx_adapter_runtime_params_init;
> > + rte_event_eth_tx_adapter_runtime_params_set;
> > rte_event_timer_remaining_ticks_get;
> > };
> >
> > --
> > 2.25.1
> >
