[v2,1/3] eventdev/eth_rx: add params set/get APIs
Checks
Commit Message
The adapter configuration parameters defined in the
``struct rte_event_eth_rx_adapter_runtime_params`` can be configured
and retrieved using ``rte_event_eth_rx_adapter_runtime_params_set`` and
``rte_event_eth_tx_adapter_runtime_params_get`` respectively.
Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
---
app/test/test_event_eth_rx_adapter.c | 92 +++++++++++++++++++
.../prog_guide/event_ethernet_rx_adapter.rst | 20 ++++
lib/eventdev/rte_event_eth_rx_adapter.c | 88 +++++++++++++++++-
lib/eventdev/rte_event_eth_rx_adapter.h | 70 +++++++++++++-
lib/eventdev/version.map | 2 +
5 files changed, 270 insertions(+), 2 deletions(-)
Comments
On Mon, Jan 23, 2023 at 11:35 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> The adapter configuration parameters defined in the
> ``struct rte_event_eth_rx_adapter_runtime_params`` can be configured
> and retrieved using ``rte_event_eth_rx_adapter_runtime_params_set`` and
> ``rte_event_eth_tx_adapter_runtime_params_get`` respectively.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> index 461eca566f..2207d6ffc3 100644
> --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> @@ -185,6 +185,26 @@ flags for handling received packets, event queue identifier, scheduler type,
> event priority, polling frequency of the receive queue and flow identifier
> in struct ``rte_event_eth_rx_adapter_queue_conf``.
>
> +Set/Get adapter runtime configuration parameters
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The runtime configuration parameters of adapter can be set/read using
> +``rte_event_eth_rx_adapter_runtime_params_set()`` and
> +``rte_event_eth_rx_adapter_runtime_params_get()`` respectively. The parameters that
> +can be set/read are defined in ``struct rte_event_eth_rx_adapter_runtime_params``.
Good.
> +
> +``rte_event_eth_rx_adapter_create()`` or
> +``rte_event_eth_rx_adapter_create_with_params()`` configures the adapter with
> +default value for maximum packets processed per request to 128.
> +``rte_event_eth_rx_adapter_runtime_params_set()`` function allows to reconfigure
> +maximum number of packets processed by adapter per service request. This is
> +alternative to configuring the maximum packets processed per request by adapter
> +by using ``rte_event_eth_rx_adapter_create_ext()`` with parameter
> +``rte_event_eth_rx_adapter_conf::max_nb_rx``.
This paragraph is not needed IMO. As it is specific to a driver, and
we can keep Doxygen comment only.
> +
> +``rte_event_eth_rx_adapter_runtime_parmas_get()`` function retrieves the configuration
> +parameters that are defined in ``struct rte_event_eth_rx_adapter_runtime_params``.
Good.
> +
> Getting and resetting Adapter queue stats
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index 34aa87379e..d8f3e750b7 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -35,6 +35,8 @@
> #define MAX_VECTOR_NS 1E9
> #define MIN_VECTOR_NS 1E5
>
> +#define RXA_NB_RX_WORK_DEFAULT 128
> +
> #define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32
> #define ETH_RX_ADAPTER_MEM_NAME_LEN 32
>
> @@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t dev_id,
> }
>
> conf->event_port_id = port_id;
> - conf->max_nb_rx = 128;
> + conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT;
> if (started)
> ret = rte_event_dev_start(dev_id);
> rx_adapter->default_cb_arg = 1;
> @@ -3436,6 +3438,90 @@ rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
> return -EINVAL;
> }
> +
> +int
> +rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
> + struct rte_event_eth_rx_adapter_runtime_params *params)
> +{
> + struct event_eth_rx_adapter *rxa;
> + int ret;
> +
> + if (params == NULL)
> + return -EINVAL;
> +
> + if (rxa_memzone_lookup())
> + return -ENOMEM;
Introduce an adapter callback and move SW adapter related logic under
callback handler.
> +
> + rxa = rxa_id_to_adapter(id);
> + if (rxa == NULL)
> + return -EINVAL;
> +
> + ret = rxa_caps_check(rxa);
> + if (ret)
> + return ret;
> +
> + rte_spinlock_lock(&rxa->rx_lock);
> + rxa->max_nb_rx = params->max_nb_rx;
> + rte_spinlock_unlock(&rxa->rx_lock);
> +
> + return 0;
> +}
> +
> +int
> +rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
> + struct rte_event_eth_rx_adapter_runtime_params *params)
> +{
> + struct event_eth_rx_adapter *rxa;
> + int ret;
> +
> + if (params == NULL)
> + return -EINVAL;
Introduce an adapter callback and move SW adapter related logic under
callback handler.
> +
> + if (rxa_memzone_lookup())
> + return -ENOMEM;
+
> + rxa = rxa_id_to_adapter(id);
> + if (rxa == NULL)
> + return -EINVAL;
> +
> + ret = rxa_caps_check(rxa);
> + if (ret)
> + return ret;
> +
> + params->max_nb_rx = rxa->max_nb_rx;
> +
> + return 0;
> +}
> +
> +/* RX-adapter telemetry callbacks */
> #define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d, #s, stats.s)
>
> static int
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h b/lib/eventdev/rte_event_eth_rx_adapter.h
> index f4652f40e8..214ffd018c 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> @@ -39,10 +39,21 @@
> * - rte_event_eth_rx_adapter_queue_stats_reset()
> * - rte_event_eth_rx_adapter_event_port_get()
> * - rte_event_eth_rx_adapter_instance_get()
> + * - rte_event_eth_rx_adapter_runtime_params_get()
> + * - rte_event_eth_rx_adapter_runtime_params_set()
> *
> * The application creates an ethernet to event adapter using
> * rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
> * or rte_event_eth_rx_adapter_create_with_params() functions.
> + *
> + * rte_event_eth_rx_adapter_create() or rte_event_eth_adapter_create_with_params()
> + * configures the adapter with default value of maximum packets processed per
> + * iteration to RXA_NB_RX_WORK_DEFAULT(128).
> + * rte_event_eth_rx_adapter_runtime_params_set() allows to re-configure maximum
> + * packets processed per iteration. This is alternative to using
> + * rte_event_eth_rx_adapter_create_ext() with parameter
> + * rte_event_eth_rx_adapter_conf::max_nb_rx
Move this to Doxygen comment against max_nb_rx
> + *
> * The adapter needs to know which ethernet rx queues to poll for mbufs as well
> * as event device parameters such as the event queue identifier, event
> * priority and scheduling type that the adapter should use when constructing
> @@ -299,6 +310,19 @@ struct rte_event_eth_rx_adapter_params {
> /**< flag to indicate that event buffer is separate for each queue */
> };
>
> +/**
> + * Adapter configuration parameters
> + */
> +struct rte_event_eth_rx_adapter_runtime_params {
> + uint32_t max_nb_rx;
> + /**< The adapter can return early if it has processed at least
> + * max_nb_rx mbufs. This isn't treated as a requirement; batching may
> + * cause the adapter to process more than max_nb_rx mbufs.
Also tell it is valid only for INTERNAL PORT capablity is set.
> + */
> + uint32_t rsvd[15];
> + /**< Reserved fields for future use */
Introduce rte_event_eth_rx_adapter_runtime_params_init() to make sure
rsvd is zero.
> +};
> +
> /**
> *
> * Callback function invoked by the SW adapter before it continues
> @@ -377,7 +401,7 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
> * Create a new ethernet Rx event adapter with the specified identifier.
> * This function uses an internal configuration function that creates an event
> * port. This default function reconfigures the event device with an
> - * additional event port and setups up the event port using the port_config
> + * additional event port and setup the event port using the port_config
> * parameter passed into this function. In case the application needs more
> * control in configuration of the service, it should use the
> * rte_event_eth_rx_adapter_create_ext() version.
> @@ -743,6 +767,50 @@ rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
> uint16_t rx_queue_id,
> uint8_t *rxa_inst_id);
>
Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, January 24, 2023 10:00 AM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan,
> Jay <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
>
> On Mon, Jan 23, 2023 at 11:35 PM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > The adapter configuration parameters defined in the ``struct
> > rte_event_eth_rx_adapter_runtime_params`` can be configured and
> > retrieved using ``rte_event_eth_rx_adapter_runtime_params_set`` and
> > ``rte_event_eth_tx_adapter_runtime_params_get`` respectively.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
>
> > diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > index 461eca566f..2207d6ffc3 100644
> > --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > @@ -185,6 +185,26 @@ flags for handling received packets, event queue
> > identifier, scheduler type, event priority, polling frequency of the
> > receive queue and flow identifier in struct
> ``rte_event_eth_rx_adapter_queue_conf``.
> >
> > +Set/Get adapter runtime configuration parameters
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +The runtime configuration parameters of adapter can be set/read using
> > +``rte_event_eth_rx_adapter_runtime_params_set()`` and
> > +``rte_event_eth_rx_adapter_runtime_params_get()`` respectively. The
> > +parameters that can be set/read are defined in ``struct
> rte_event_eth_rx_adapter_runtime_params``.
>
> Good.
>
> > +
> > +``rte_event_eth_rx_adapter_create()`` or
> > +``rte_event_eth_rx_adapter_create_with_params()`` configures the
> > +adapter with default value for maximum packets processed per request to
> 128.
> > +``rte_event_eth_rx_adapter_runtime_params_set()`` function allows to
> > +reconfigure maximum number of packets processed by adapter per
> > +service request. This is alternative to configuring the maximum
> > +packets processed per request by adapter by using
> > +``rte_event_eth_rx_adapter_create_ext()`` with parameter
> ``rte_event_eth_rx_adapter_conf::max_nb_rx``.
>
> This paragraph is not needed IMO. As it is specific to a driver, and we can keep
> Doxygen comment only.
>
>
> > +
> > +``rte_event_eth_rx_adapter_runtime_parmas_get()`` function retrieves
> > +the configuration parameters that are defined in ``struct
> rte_event_eth_rx_adapter_runtime_params``.
>
> Good.
>
> > +
> > Getting and resetting Adapter queue stats
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index 34aa87379e..d8f3e750b7 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -35,6 +35,8 @@
> > #define MAX_VECTOR_NS 1E9
> > #define MIN_VECTOR_NS 1E5
> >
> > +#define RXA_NB_RX_WORK_DEFAULT 128
> > +
> > #define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32
> > #define ETH_RX_ADAPTER_MEM_NAME_LEN 32
> >
> > @@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t dev_id,
> > }
> >
> > conf->event_port_id = port_id;
> > - conf->max_nb_rx = 128;
> > + conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT;
> > if (started)
> > ret = rte_event_dev_start(dev_id);
> > rx_adapter->default_cb_arg = 1; @@ -3436,6 +3438,90 @@
> > rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
> > return -EINVAL;
> > }
>
> > +
> > +int
> > +rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
> > + struct rte_event_eth_rx_adapter_runtime_params
> > +*params) {
> > + struct event_eth_rx_adapter *rxa;
> > + int ret;
> > +
> > + if (params == NULL)
> > + return -EINVAL;
> > +
> > + if (rxa_memzone_lookup())
> > + return -ENOMEM;
>
> Introduce an adapter callback and move SW adapter related logic under callback
> handler.
>
>
Do you mean introducing eventdev PMD callback for HW implementation?
There are no adapter callback currently for service based SW Implementation.
> > +
> > + rxa = rxa_id_to_adapter(id);
> > + if (rxa == NULL)
> > + return -EINVAL;
> > +
> > + ret = rxa_caps_check(rxa);
> > + if (ret)
> > + return ret;
> > +
> > + rte_spinlock_lock(&rxa->rx_lock);
> > + rxa->max_nb_rx = params->max_nb_rx;
> > + rte_spinlock_unlock(&rxa->rx_lock);
> > +
> > + return 0;
> > +}
> > +
> > +int
> > +rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
> > + struct rte_event_eth_rx_adapter_runtime_params
> > +*params) {
> > + struct event_eth_rx_adapter *rxa;
> > + int ret;
> > +
> > + if (params == NULL)
> > + return -EINVAL;
>
>
> Introduce an adapter callback and move SW adapter related logic under callback
> handler.
>
>
Same as above
> > +
> > + if (rxa_memzone_lookup())
> > + return -ENOMEM;
> +
> > + rxa = rxa_id_to_adapter(id);
> > + if (rxa == NULL)
> > + return -EINVAL;
> > +
> > + ret = rxa_caps_check(rxa);
> > + if (ret)
> > + return ret;
> > +
> > + params->max_nb_rx = rxa->max_nb_rx;
> > +
> > + return 0;
> > +}
> > +
> > +/* RX-adapter telemetry callbacks */
> > #define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d, #s,
> > stats.s)
> >
> > static int
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h
> > b/lib/eventdev/rte_event_eth_rx_adapter.h
> > index f4652f40e8..214ffd018c 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> > @@ -39,10 +39,21 @@
> > * - rte_event_eth_rx_adapter_queue_stats_reset()
> > * - rte_event_eth_rx_adapter_event_port_get()
> > * - rte_event_eth_rx_adapter_instance_get()
> > + * - rte_event_eth_rx_adapter_runtime_params_get()
> > + * - rte_event_eth_rx_adapter_runtime_params_set()
> > *
> > * The application creates an ethernet to event adapter using
> > * rte_event_eth_rx_adapter_create_ext() or
> rte_event_eth_rx_adapter_create()
> > * or rte_event_eth_rx_adapter_create_with_params() functions.
> > + *
> > + * rte_event_eth_rx_adapter_create() or
> > + rte_event_eth_adapter_create_with_params()
> > + * configures the adapter with default value of maximum packets
> > + processed per
> > + * iteration to RXA_NB_RX_WORK_DEFAULT(128).
> > + * rte_event_eth_rx_adapter_runtime_params_set() allows to
> > + re-configure maximum
> > + * packets processed per iteration. This is alternative to using
> > + * rte_event_eth_rx_adapter_create_ext() with parameter
> > + * rte_event_eth_rx_adapter_conf::max_nb_rx
>
> Move this to Doxygen comment against max_nb_rx
>
> > + *
> > * The adapter needs to know which ethernet rx queues to poll for mbufs as
> well
> > * as event device parameters such as the event queue identifier, event
> > * priority and scheduling type that the adapter should use when
> > constructing @@ -299,6 +310,19 @@ struct
> rte_event_eth_rx_adapter_params {
> > /**< flag to indicate that event buffer is separate for each
> > queue */ };
> >
> > +/**
> > + * Adapter configuration parameters
> > + */
> > +struct rte_event_eth_rx_adapter_runtime_params {
> > + uint32_t max_nb_rx;
> > + /**< The adapter can return early if it has processed at least
> > + * max_nb_rx mbufs. This isn't treated as a requirement; batching may
> > + * cause the adapter to process more than max_nb_rx mbufs.
>
> Also tell it is valid only for INTERNAL PORT capablity is set.
>
Do you mean, it is valid only for INTERNAL PORT capability is 'not' set?
> > + */
> > + uint32_t rsvd[15];
> > + /**< Reserved fields for future use */
>
> Introduce rte_event_eth_rx_adapter_runtime_params_init() to make sure rsvd is
> zero.
>
The reserved fields are not used by the adapter or application. Not sure Is it necessary to Introduce
a new API to clear reserved fields.
> > +};
> > +
> > /**
> > *
> > * Callback function invoked by the SW adapter before it continues @@
> > -377,7 +401,7 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id,
> uint8_t dev_id,
> > * Create a new ethernet Rx event adapter with the specified identifier.
> > * This function uses an internal configuration function that creates an event
> > * port. This default function reconfigures the event device with an
> > - * additional event port and setups up the event port using the
> > port_config
> > + * additional event port and setup the event port using the
> > + port_config
> > * parameter passed into this function. In case the application needs more
> > * control in configuration of the service, it should use the
> > * rte_event_eth_rx_adapter_create_ext() version.
> > @@ -743,6 +767,50 @@ rte_event_eth_rx_adapter_instance_get(uint16_t
> eth_dev_id,
> > uint16_t rx_queue_id,
> > uint8_t *rxa_inst_id);
> >
On Tue, Jan 24, 2023 at 6:37 PM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Tuesday, January 24, 2023 10:00 AM
> > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> > Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan,
> > Jay <jay.jayatheerthan@intel.com>
> > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> >
> > On Mon, Jan 23, 2023 at 11:35 PM Naga Harish K S V
> > <s.v.naga.harish.k@intel.com> wrote:
> > >
> > > The adapter configuration parameters defined in the ``struct
> > > rte_event_eth_rx_adapter_runtime_params`` can be configured and
> > > retrieved using ``rte_event_eth_rx_adapter_runtime_params_set`` and
> > > ``rte_event_eth_tx_adapter_runtime_params_get`` respectively.
> > >
> > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> >
> > > diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > index 461eca566f..2207d6ffc3 100644
> > > --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > @@ -185,6 +185,26 @@ flags for handling received packets, event queue
> > > identifier, scheduler type, event priority, polling frequency of the
> > > receive queue and flow identifier in struct
> > ``rte_event_eth_rx_adapter_queue_conf``.
> > >
> > > +Set/Get adapter runtime configuration parameters
> > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > +
> > > +The runtime configuration parameters of adapter can be set/read using
> > > +``rte_event_eth_rx_adapter_runtime_params_set()`` and
> > > +``rte_event_eth_rx_adapter_runtime_params_get()`` respectively. The
> > > +parameters that can be set/read are defined in ``struct
> > rte_event_eth_rx_adapter_runtime_params``.
> >
> > Good.
> >
> > > +
> > > +``rte_event_eth_rx_adapter_create()`` or
> > > +``rte_event_eth_rx_adapter_create_with_params()`` configures the
> > > +adapter with default value for maximum packets processed per request to
> > 128.
> > > +``rte_event_eth_rx_adapter_runtime_params_set()`` function allows to
> > > +reconfigure maximum number of packets processed by adapter per
> > > +service request. This is alternative to configuring the maximum
> > > +packets processed per request by adapter by using
> > > +``rte_event_eth_rx_adapter_create_ext()`` with parameter
> > ``rte_event_eth_rx_adapter_conf::max_nb_rx``.
> >
> > This paragraph is not needed IMO. As it is specific to a driver, and we can keep
> > Doxygen comment only.
> >
> >
> > > +
> > > +``rte_event_eth_rx_adapter_runtime_parmas_get()`` function retrieves
> > > +the configuration parameters that are defined in ``struct
> > rte_event_eth_rx_adapter_runtime_params``.
> >
> > Good.
> >
> > > +
> > > Getting and resetting Adapter queue stats
> > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > index 34aa87379e..d8f3e750b7 100644
> > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > @@ -35,6 +35,8 @@
> > > #define MAX_VECTOR_NS 1E9
> > > #define MIN_VECTOR_NS 1E5
> > >
> > > +#define RXA_NB_RX_WORK_DEFAULT 128
> > > +
> > > #define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32
> > > #define ETH_RX_ADAPTER_MEM_NAME_LEN 32
> > >
> > > @@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t dev_id,
> > > }
> > >
> > > conf->event_port_id = port_id;
> > > - conf->max_nb_rx = 128;
> > > + conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT;
> > > if (started)
> > > ret = rte_event_dev_start(dev_id);
> > > rx_adapter->default_cb_arg = 1; @@ -3436,6 +3438,90 @@
> > > rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
> > > return -EINVAL;
> > > }
> >
> > > +
> > > +int
> > > +rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
> > > + struct rte_event_eth_rx_adapter_runtime_params
> > > +*params) {
> > > + struct event_eth_rx_adapter *rxa;
> > > + int ret;
> > > +
> > > + if (params == NULL)
> > > + return -EINVAL;
> > > +
> > > + if (rxa_memzone_lookup())
> > > + return -ENOMEM;
> >
> > Introduce an adapter callback and move SW adapter related logic under callback
> > handler.
> >
> >
> Do you mean introducing eventdev PMD callback for HW implementation?
Yes.
> There are no adapter callback currently for service based SW Implementation.
>
> > > +
> > > + rxa = rxa_id_to_adapter(id);
> > > + if (rxa == NULL)
> > > + return -EINVAL;
> > > +
> > > + ret = rxa_caps_check(rxa);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + rte_spinlock_lock(&rxa->rx_lock);
> > > + rxa->max_nb_rx = params->max_nb_rx;
> > > + rte_spinlock_unlock(&rxa->rx_lock);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +int
> > > +rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
> > > + struct rte_event_eth_rx_adapter_runtime_params
> > > +*params) {
> > > + struct event_eth_rx_adapter *rxa;
> > > + int ret;
> > > +
> > > + if (params == NULL)
> > > + return -EINVAL;
> >
> >
> > Introduce an adapter callback and move SW adapter related logic under callback
> > handler.
> >
> >
>
> Same as above
>
> > > +
> > > + if (rxa_memzone_lookup())
> > > + return -ENOMEM;
> > +
> > > + rxa = rxa_id_to_adapter(id);
> > > + if (rxa == NULL)
> > > + return -EINVAL;
> > > +
> > > + ret = rxa_caps_check(rxa);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + params->max_nb_rx = rxa->max_nb_rx;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* RX-adapter telemetry callbacks */
> > > #define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d, #s,
> > > stats.s)
> > >
> > > static int
> > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > index f4652f40e8..214ffd018c 100644
> > > --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > @@ -39,10 +39,21 @@
> > > * - rte_event_eth_rx_adapter_queue_stats_reset()
> > > * - rte_event_eth_rx_adapter_event_port_get()
> > > * - rte_event_eth_rx_adapter_instance_get()
> > > + * - rte_event_eth_rx_adapter_runtime_params_get()
> > > + * - rte_event_eth_rx_adapter_runtime_params_set()
> > > *
> > > * The application creates an ethernet to event adapter using
> > > * rte_event_eth_rx_adapter_create_ext() or
> > rte_event_eth_rx_adapter_create()
> > > * or rte_event_eth_rx_adapter_create_with_params() functions.
> > > + *
> > > + * rte_event_eth_rx_adapter_create() or
> > > + rte_event_eth_adapter_create_with_params()
> > > + * configures the adapter with default value of maximum packets
> > > + processed per
> > > + * iteration to RXA_NB_RX_WORK_DEFAULT(128).
> > > + * rte_event_eth_rx_adapter_runtime_params_set() allows to
> > > + re-configure maximum
> > > + * packets processed per iteration. This is alternative to using
> > > + * rte_event_eth_rx_adapter_create_ext() with parameter
> > > + * rte_event_eth_rx_adapter_conf::max_nb_rx
> >
> > Move this to Doxygen comment against max_nb_rx
> >
> > > + *
> > > * The adapter needs to know which ethernet rx queues to poll for mbufs as
> > well
> > > * as event device parameters such as the event queue identifier, event
> > > * priority and scheduling type that the adapter should use when
> > > constructing @@ -299,6 +310,19 @@ struct
> > rte_event_eth_rx_adapter_params {
> > > /**< flag to indicate that event buffer is separate for each
> > > queue */ };
> > >
> > > +/**
> > > + * Adapter configuration parameters
> > > + */
> > > +struct rte_event_eth_rx_adapter_runtime_params {
> > > + uint32_t max_nb_rx;
> > > + /**< The adapter can return early if it has processed at least
> > > + * max_nb_rx mbufs. This isn't treated as a requirement; batching may
> > > + * cause the adapter to process more than max_nb_rx mbufs.
> >
> > Also tell it is valid only for INTERNAL PORT capablity is set.
> >
>
> Do you mean, it is valid only for INTERNAL PORT capability is 'not' set?
Yes.
>
> > > + */
> > > + uint32_t rsvd[15];
> > > + /**< Reserved fields for future use */
> >
> > Introduce rte_event_eth_rx_adapter_runtime_params_init() to make sure rsvd is
> > zero.
> >
>
> The reserved fields are not used by the adapter or application. Not sure Is it necessary to Introduce
> a new API to clear reserved fields.
When adapter starts using new fileds(when we add new fieds in future),
the old applicaiton which
is not using rte_event_eth_rx_adapter_runtime_params_init() may have
junk value and then adapter
implementation will behave bad.
>
> > > +};
> > > +
> > > /**
> > > *
> > > * Callback function invoked by the SW adapter before it continues @@
> > > -377,7 +401,7 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id,
> > uint8_t dev_id,
> > > * Create a new ethernet Rx event adapter with the specified identifier.
> > > * This function uses an internal configuration function that creates an event
> > > * port. This default function reconfigures the event device with an
> > > - * additional event port and setups up the event port using the
> > > port_config
> > > + * additional event port and setup the event port using the
> > > + port_config
> > > * parameter passed into this function. In case the application needs more
> > > * control in configuration of the service, it should use the
> > > * rte_event_eth_rx_adapter_create_ext() version.
> > > @@ -743,6 +767,50 @@ rte_event_eth_rx_adapter_instance_get(uint16_t
> > eth_dev_id,
> > > uint16_t rx_queue_id,
> > > uint8_t *rxa_inst_id);
> > >
Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, January 25, 2023 9:42 AM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
>
> On Tue, Jan 24, 2023 at 6:37 PM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > Hi Jerin,
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Tuesday, January 24, 2023 10:00 AM
> > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>
> > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > >
> > > On Mon, Jan 23, 2023 at 11:35 PM Naga Harish K S V
> > > <s.v.naga.harish.k@intel.com> wrote:
> > > >
> > > > The adapter configuration parameters defined in the ``struct
> > > > rte_event_eth_rx_adapter_runtime_params`` can be configured and
> > > > retrieved using ``rte_event_eth_rx_adapter_runtime_params_set``
> > > > and ``rte_event_eth_tx_adapter_runtime_params_get`` respectively.
> > > >
> > > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > >
> > > > diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > index 461eca566f..2207d6ffc3 100644
> > > > --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > @@ -185,6 +185,26 @@ flags for handling received packets, event
> > > > queue identifier, scheduler type, event priority, polling
> > > > frequency of the receive queue and flow identifier in struct
> > > ``rte_event_eth_rx_adapter_queue_conf``.
> > > >
> > > > +Set/Get adapter runtime configuration parameters
> > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +The runtime configuration parameters of adapter can be set/read
> > > > +using ``rte_event_eth_rx_adapter_runtime_params_set()`` and
> > > > +``rte_event_eth_rx_adapter_runtime_params_get()`` respectively.
> > > > +The parameters that can be set/read are defined in ``struct
> > > rte_event_eth_rx_adapter_runtime_params``.
> > >
> > > Good.
> > >
> > > > +
> > > > +``rte_event_eth_rx_adapter_create()`` or
> > > > +``rte_event_eth_rx_adapter_create_with_params()`` configures the
> > > > +adapter with default value for maximum packets processed per
> > > > +request to
> > > 128.
> > > > +``rte_event_eth_rx_adapter_runtime_params_set()`` function allows
> > > > +to reconfigure maximum number of packets processed by adapter per
> > > > +service request. This is alternative to configuring the maximum
> > > > +packets processed per request by adapter by using
> > > > +``rte_event_eth_rx_adapter_create_ext()`` with parameter
> > > ``rte_event_eth_rx_adapter_conf::max_nb_rx``.
> > >
> > > This paragraph is not needed IMO. As it is specific to a driver, and
> > > we can keep Doxygen comment only.
> > >
> > >
> > > > +
> > > > +``rte_event_eth_rx_adapter_runtime_parmas_get()`` function
> > > > +retrieves the configuration parameters that are defined in
> > > > +``struct
> > > rte_event_eth_rx_adapter_runtime_params``.
> > >
> > > Good.
> > >
> > > > +
> > > > Getting and resetting Adapter queue stats
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >
> > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > index 34aa87379e..d8f3e750b7 100644
> > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > @@ -35,6 +35,8 @@
> > > > #define MAX_VECTOR_NS 1E9
> > > > #define MIN_VECTOR_NS 1E5
> > > >
> > > > +#define RXA_NB_RX_WORK_DEFAULT 128
> > > > +
> > > > #define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32
> > > > #define ETH_RX_ADAPTER_MEM_NAME_LEN 32
> > > >
> > > > @@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t
> dev_id,
> > > > }
> > > >
> > > > conf->event_port_id = port_id;
> > > > - conf->max_nb_rx = 128;
> > > > + conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT;
> > > > if (started)
> > > > ret = rte_event_dev_start(dev_id);
> > > > rx_adapter->default_cb_arg = 1; @@ -3436,6 +3438,90 @@
> > > > rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
> > > > return -EINVAL;
> > > > }
> > >
> > > > +
> > > > +int
> > > > +rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
> > > > + struct rte_event_eth_rx_adapter_runtime_params
> > > > +*params) {
> > > > + struct event_eth_rx_adapter *rxa;
> > > > + int ret;
> > > > +
> > > > + if (params == NULL)
> > > > + return -EINVAL;
> > > > +
> > > > + if (rxa_memzone_lookup())
> > > > + return -ENOMEM;
> > >
> > > Introduce an adapter callback and move SW adapter related logic
> > > under callback handler.
> > >
> > >
> > Do you mean introducing eventdev PMD callback for HW implementation?
>
> Yes.
>
Can this be taken care as and when the HW implementation is required?
> > There are no adapter callback currently for service based SW
> Implementation.
> >
> > > > +
> > > > + rxa = rxa_id_to_adapter(id);
> > > > + if (rxa == NULL)
> > > > + return -EINVAL;
> > > > +
> > > > + ret = rxa_caps_check(rxa);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + rte_spinlock_lock(&rxa->rx_lock);
> > > > + rxa->max_nb_rx = params->max_nb_rx;
> > > > + rte_spinlock_unlock(&rxa->rx_lock);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +int
> > > > +rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
> > > > + struct rte_event_eth_rx_adapter_runtime_params
> > > > +*params) {
> > > > + struct event_eth_rx_adapter *rxa;
> > > > + int ret;
> > > > +
> > > > + if (params == NULL)
> > > > + return -EINVAL;
> > >
> > >
> > > Introduce an adapter callback and move SW adapter related logic
> > > under callback handler.
> > >
> > >
> >
> > Same as above
> >
> > > > +
> > > > + if (rxa_memzone_lookup())
> > > > + return -ENOMEM;
> > > +
> > > > + rxa = rxa_id_to_adapter(id);
> > > > + if (rxa == NULL)
> > > > + return -EINVAL;
> > > > +
> > > > + ret = rxa_caps_check(rxa);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + params->max_nb_rx = rxa->max_nb_rx;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +/* RX-adapter telemetry callbacks */
> > > > #define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d, #s,
> > > > stats.s)
> > > >
> > > > static int
> > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > index f4652f40e8..214ffd018c 100644
> > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > @@ -39,10 +39,21 @@
> > > > * - rte_event_eth_rx_adapter_queue_stats_reset()
> > > > * - rte_event_eth_rx_adapter_event_port_get()
> > > > * - rte_event_eth_rx_adapter_instance_get()
> > > > + * - rte_event_eth_rx_adapter_runtime_params_get()
> > > > + * - rte_event_eth_rx_adapter_runtime_params_set()
> > > > *
> > > > * The application creates an ethernet to event adapter using
> > > > * rte_event_eth_rx_adapter_create_ext() or
> > > rte_event_eth_rx_adapter_create()
> > > > * or rte_event_eth_rx_adapter_create_with_params() functions.
> > > > + *
> > > > + * rte_event_eth_rx_adapter_create() or
> > > > + rte_event_eth_adapter_create_with_params()
> > > > + * configures the adapter with default value of maximum packets
> > > > + processed per
> > > > + * iteration to RXA_NB_RX_WORK_DEFAULT(128).
> > > > + * rte_event_eth_rx_adapter_runtime_params_set() allows to
> > > > + re-configure maximum
> > > > + * packets processed per iteration. This is alternative to using
> > > > + * rte_event_eth_rx_adapter_create_ext() with parameter
> > > > + * rte_event_eth_rx_adapter_conf::max_nb_rx
> > >
> > > Move this to Doxygen comment against max_nb_rx
> > >
> > > > + *
> > > > * The adapter needs to know which ethernet rx queues to poll for
> > > > mbufs as
> > > well
> > > > * as event device parameters such as the event queue identifier,
> event
> > > > * priority and scheduling type that the adapter should use when
> > > > constructing @@ -299,6 +310,19 @@ struct
> > > rte_event_eth_rx_adapter_params {
> > > > /**< flag to indicate that event buffer is separate for
> > > > each queue */ };
> > > >
> > > > +/**
> > > > + * Adapter configuration parameters */ struct
> > > > +rte_event_eth_rx_adapter_runtime_params {
> > > > + uint32_t max_nb_rx;
> > > > + /**< The adapter can return early if it has processed at least
> > > > + * max_nb_rx mbufs. This isn't treated as a requirement; batching
> may
> > > > + * cause the adapter to process more than max_nb_rx mbufs.
> > >
> > > Also tell it is valid only for INTERNAL PORT capablity is set.
> > >
> >
> > Do you mean, it is valid only for INTERNAL PORT capability is 'not' set?
>
> Yes.
>
> >
> > > > + */
> > > > + uint32_t rsvd[15];
> > > > + /**< Reserved fields for future use */
> > >
> > > Introduce rte_event_eth_rx_adapter_runtime_params_init() to make
> > > sure rsvd is zero.
> > >
> >
> > The reserved fields are not used by the adapter or application. Not
> > sure Is it necessary to Introduce a new API to clear reserved fields.
>
> When adapter starts using new fileds(when we add new fieds in future), the
> old applicaiton which is not using
> rte_event_eth_rx_adapter_runtime_params_init() may have junk value and
> then adapter implementation will behave bad.
>
>
does it mean, the application doesn't re-compile for the new DPDK?
When some of the reserved fields are used in the future, the application also may need to be recompiled along with DPDK right?
As the application also may need to use the newly consumed reserved fields?
> >
> > > > +};
> > > > +
> > > > /**
> > > > *
> > > > * Callback function invoked by the SW adapter before it
> > > > continues @@
> > > > -377,7 +401,7 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t
> > > > id,
> > > uint8_t dev_id,
> > > > * Create a new ethernet Rx event adapter with the specified
> identifier.
> > > > * This function uses an internal configuration function that creates an
> event
> > > > * port. This default function reconfigures the event device with
> > > > an
> > > > - * additional event port and setups up the event port using the
> > > > port_config
> > > > + * additional event port and setup the event port using the
> > > > + port_config
> > > > * parameter passed into this function. In case the application needs
> more
> > > > * control in configuration of the service, it should use the
> > > > * rte_event_eth_rx_adapter_create_ext() version.
> > > > @@ -743,6 +767,50 @@
> > > > rte_event_eth_rx_adapter_instance_get(uint16_t
> > > eth_dev_id,
> > > > uint16_t rx_queue_id,
> > > > uint8_t *rxa_inst_id);
> > > >
On Wed, Jan 25, 2023 at 3:22 PM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, January 25, 2023 9:42 AM
> > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> > Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> > Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> >
> > On Tue, Jan 24, 2023 at 6:37 PM Naga Harish K, S V
> > <s.v.naga.harish.k@intel.com> wrote:
> > >
> > > Hi Jerin,
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Tuesday, January 24, 2023 10:00 AM
> > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > <jay.jayatheerthan@intel.com>
> > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > > >
> > > > On Mon, Jan 23, 2023 at 11:35 PM Naga Harish K S V
> > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > >
> > > > > The adapter configuration parameters defined in the ``struct
> > > > > rte_event_eth_rx_adapter_runtime_params`` can be configured and
> > > > > retrieved using ``rte_event_eth_rx_adapter_runtime_params_set``
> > > > > and ``rte_event_eth_tx_adapter_runtime_params_get`` respectively.
> > > > >
> > > > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > > >
> > > > > diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > index 461eca566f..2207d6ffc3 100644
> > > > > --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > @@ -185,6 +185,26 @@ flags for handling received packets, event
> > > > > queue identifier, scheduler type, event priority, polling
> > > > > frequency of the receive queue and flow identifier in struct
> > > > ``rte_event_eth_rx_adapter_queue_conf``.
> > > > >
> > > > > +Set/Get adapter runtime configuration parameters
> > > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > +
> > > > > +The runtime configuration parameters of adapter can be set/read
> > > > > +using ``rte_event_eth_rx_adapter_runtime_params_set()`` and
> > > > > +``rte_event_eth_rx_adapter_runtime_params_get()`` respectively.
> > > > > +The parameters that can be set/read are defined in ``struct
> > > > rte_event_eth_rx_adapter_runtime_params``.
> > > >
> > > > Good.
> > > >
> > > > > +
> > > > > +``rte_event_eth_rx_adapter_create()`` or
> > > > > +``rte_event_eth_rx_adapter_create_with_params()`` configures the
> > > > > +adapter with default value for maximum packets processed per
> > > > > +request to
> > > > 128.
> > > > > +``rte_event_eth_rx_adapter_runtime_params_set()`` function allows
> > > > > +to reconfigure maximum number of packets processed by adapter per
> > > > > +service request. This is alternative to configuring the maximum
> > > > > +packets processed per request by adapter by using
> > > > > +``rte_event_eth_rx_adapter_create_ext()`` with parameter
> > > > ``rte_event_eth_rx_adapter_conf::max_nb_rx``.
> > > >
> > > > This paragraph is not needed IMO. As it is specific to a driver, and
> > > > we can keep Doxygen comment only.
> > > >
> > > >
> > > > > +
> > > > > +``rte_event_eth_rx_adapter_runtime_parmas_get()`` function
> > > > > +retrieves the configuration parameters that are defined in
> > > > > +``struct
> > > > rte_event_eth_rx_adapter_runtime_params``.
> > > >
> > > > Good.
> > > >
> > > > > +
> > > > > Getting and resetting Adapter queue stats
> > > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > >
> > > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > index 34aa87379e..d8f3e750b7 100644
> > > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > @@ -35,6 +35,8 @@
> > > > > #define MAX_VECTOR_NS 1E9
> > > > > #define MIN_VECTOR_NS 1E5
> > > > >
> > > > > +#define RXA_NB_RX_WORK_DEFAULT 128
> > > > > +
> > > > > #define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32
> > > > > #define ETH_RX_ADAPTER_MEM_NAME_LEN 32
> > > > >
> > > > > @@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t
> > dev_id,
> > > > > }
> > > > >
> > > > > conf->event_port_id = port_id;
> > > > > - conf->max_nb_rx = 128;
> > > > > + conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT;
> > > > > if (started)
> > > > > ret = rte_event_dev_start(dev_id);
> > > > > rx_adapter->default_cb_arg = 1; @@ -3436,6 +3438,90 @@
> > > > > rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
> > > > > return -EINVAL;
> > > > > }
> > > >
> > > > > +
> > > > > +int
> > > > > +rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
> > > > > + struct rte_event_eth_rx_adapter_runtime_params
> > > > > +*params) {
> > > > > + struct event_eth_rx_adapter *rxa;
> > > > > + int ret;
> > > > > +
> > > > > + if (params == NULL)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + if (rxa_memzone_lookup())
> > > > > + return -ENOMEM;
> > > >
> > > > Introduce an adapter callback and move SW adapter related logic
> > > > under callback handler.
> > > >
> > > >
> > > Do you mean introducing eventdev PMD callback for HW implementation?
> >
> > Yes.
> >
>
> Can this be taken care as and when the HW implementation is required?
OK. As long as when case INTERNAL PORT it return -ENOSUP now.
>
> > > There are no adapter callback currently for service based SW
> > Implementation.
> > >
> > > > > +
> > > > > + rxa = rxa_id_to_adapter(id);
> > > > > + if (rxa == NULL)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + ret = rxa_caps_check(rxa);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + rte_spinlock_lock(&rxa->rx_lock);
> > > > > + rxa->max_nb_rx = params->max_nb_rx;
> > > > > + rte_spinlock_unlock(&rxa->rx_lock);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +int
> > > > > +rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
> > > > > + struct rte_event_eth_rx_adapter_runtime_params
> > > > > +*params) {
> > > > > + struct event_eth_rx_adapter *rxa;
> > > > > + int ret;
> > > > > +
> > > > > + if (params == NULL)
> > > > > + return -EINVAL;
> > > >
> > > >
> > > > Introduce an adapter callback and move SW adapter related logic
> > > > under callback handler.
> > > >
> > > >
> > >
> > > Same as above
> > >
> > > > > +
> > > > > + if (rxa_memzone_lookup())
> > > > > + return -ENOMEM;
> > > > +
> > > > > + rxa = rxa_id_to_adapter(id);
> > > > > + if (rxa == NULL)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + ret = rxa_caps_check(rxa);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + params->max_nb_rx = rxa->max_nb_rx;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +/* RX-adapter telemetry callbacks */
> > > > > #define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d, #s,
> > > > > stats.s)
> > > > >
> > > > > static int
> > > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > index f4652f40e8..214ffd018c 100644
> > > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > @@ -39,10 +39,21 @@
> > > > > * - rte_event_eth_rx_adapter_queue_stats_reset()
> > > > > * - rte_event_eth_rx_adapter_event_port_get()
> > > > > * - rte_event_eth_rx_adapter_instance_get()
> > > > > + * - rte_event_eth_rx_adapter_runtime_params_get()
> > > > > + * - rte_event_eth_rx_adapter_runtime_params_set()
> > > > > *
> > > > > * The application creates an ethernet to event adapter using
> > > > > * rte_event_eth_rx_adapter_create_ext() or
> > > > rte_event_eth_rx_adapter_create()
> > > > > * or rte_event_eth_rx_adapter_create_with_params() functions.
> > > > > + *
> > > > > + * rte_event_eth_rx_adapter_create() or
> > > > > + rte_event_eth_adapter_create_with_params()
> > > > > + * configures the adapter with default value of maximum packets
> > > > > + processed per
> > > > > + * iteration to RXA_NB_RX_WORK_DEFAULT(128).
> > > > > + * rte_event_eth_rx_adapter_runtime_params_set() allows to
> > > > > + re-configure maximum
> > > > > + * packets processed per iteration. This is alternative to using
> > > > > + * rte_event_eth_rx_adapter_create_ext() with parameter
> > > > > + * rte_event_eth_rx_adapter_conf::max_nb_rx
> > > >
> > > > Move this to Doxygen comment against max_nb_rx
> > > >
> > > > > + *
> > > > > * The adapter needs to know which ethernet rx queues to poll for
> > > > > mbufs as
> > > > well
> > > > > * as event device parameters such as the event queue identifier,
> > event
> > > > > * priority and scheduling type that the adapter should use when
> > > > > constructing @@ -299,6 +310,19 @@ struct
> > > > rte_event_eth_rx_adapter_params {
> > > > > /**< flag to indicate that event buffer is separate for
> > > > > each queue */ };
> > > > >
> > > > > +/**
> > > > > + * Adapter configuration parameters */ struct
> > > > > +rte_event_eth_rx_adapter_runtime_params {
> > > > > + uint32_t max_nb_rx;
> > > > > + /**< The adapter can return early if it has processed at least
> > > > > + * max_nb_rx mbufs. This isn't treated as a requirement; batching
> > may
> > > > > + * cause the adapter to process more than max_nb_rx mbufs.
> > > >
> > > > Also tell it is valid only for INTERNAL PORT capablity is set.
> > > >
> > >
> > > Do you mean, it is valid only for INTERNAL PORT capability is 'not' set?
> >
> > Yes.
> >
> > >
> > > > > + */
> > > > > + uint32_t rsvd[15];
> > > > > + /**< Reserved fields for future use */
> > > >
> > > > Introduce rte_event_eth_rx_adapter_runtime_params_init() to make
> > > > sure rsvd is zero.
> > > >
> > >
> > > The reserved fields are not used by the adapter or application. Not
> > > sure Is it necessary to Introduce a new API to clear reserved fields.
> >
> > When adapter starts using new fileds(when we add new fieds in future), the
> > old applicaiton which is not using
> > rte_event_eth_rx_adapter_runtime_params_init() may have junk value and
> > then adapter implementation will behave bad.
> >
> >
>
> does it mean, the application doesn't re-compile for the new DPDK?
Yes. No need recompile if ABI not breaking.
> When some of the reserved fields are used in the future, the application also may need to be recompiled along with DPDK right?
> As the application also may need to use the newly consumed reserved fields?
The problematic case is:
Adapter implementation of 23.07(Assuming there is change params) field
needs to work with application of 23.03.
rte_event_eth_rx_adapter_runtime_params_init() will sove that.
>
> > >
> > > > > +};
> > > > > +
> > > > > /**
> > > > > *
> > > > > * Callback function invoked by the SW adapter before it
> > > > > continues @@
> > > > > -377,7 +401,7 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t
> > > > > id,
> > > > uint8_t dev_id,
> > > > > * Create a new ethernet Rx event adapter with the specified
> > identifier.
> > > > > * This function uses an internal configuration function that creates an
> > event
> > > > > * port. This default function reconfigures the event device with
> > > > > an
> > > > > - * additional event port and setups up the event port using the
> > > > > port_config
> > > > > + * additional event port and setup the event port using the
> > > > > + port_config
> > > > > * parameter passed into this function. In case the application needs
> > more
> > > > > * control in configuration of the service, it should use the
> > > > > * rte_event_eth_rx_adapter_create_ext() version.
> > > > > @@ -743,6 +767,50 @@
> > > > > rte_event_eth_rx_adapter_instance_get(uint16_t
> > > > eth_dev_id,
> > > > > uint16_t rx_queue_id,
> > > > > uint8_t *rxa_inst_id);
> > > > >
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, January 25, 2023 4:08 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
>
> On Wed, Jan 25, 2023 at 3:22 PM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > Hi Jerin,
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Wednesday, January 25, 2023 9:42 AM
> > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>
> > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > >
> > > On Tue, Jan 24, 2023 at 6:37 PM Naga Harish K, S V
> > > <s.v.naga.harish.k@intel.com> wrote:
> > > >
> > > > Hi Jerin,
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Tuesday, January 24, 2023 10:00 AM
> > > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > > <jay.jayatheerthan@intel.com>
> > > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get
> > > > > APIs
> > > > >
> > > > > On Mon, Jan 23, 2023 at 11:35 PM Naga Harish K S V
> > > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > > >
> > > > > > The adapter configuration parameters defined in the ``struct
> > > > > > rte_event_eth_rx_adapter_runtime_params`` can be configured
> > > > > > and retrieved using
> > > > > > ``rte_event_eth_rx_adapter_runtime_params_set``
> > > > > > and ``rte_event_eth_tx_adapter_runtime_params_get``
> respectively.
> > > > > >
> > > > > > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > > > >
> > > > > > diff --git
> > > > > > a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > > b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > > index 461eca566f..2207d6ffc3 100644
> > > > > > --- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > > +++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
> > > > > > @@ -185,6 +185,26 @@ flags for handling received packets,
> > > > > > event queue identifier, scheduler type, event priority,
> > > > > > polling frequency of the receive queue and flow identifier in
> > > > > > struct
> > > > > ``rte_event_eth_rx_adapter_queue_conf``.
> > > > > >
> > > > > > +Set/Get adapter runtime configuration parameters
> > > > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > > +
> > > > > > +The runtime configuration parameters of adapter can be
> > > > > > +set/read using
> > > > > > +``rte_event_eth_rx_adapter_runtime_params_set()`` and
> ``rte_event_eth_rx_adapter_runtime_params_get()`` respectively.
> > > > > > +The parameters that can be set/read are defined in ``struct
> > > > > rte_event_eth_rx_adapter_runtime_params``.
> > > > >
> > > > > Good.
> > > > >
> > > > > > +
> > > > > > +``rte_event_eth_rx_adapter_create()`` or
> > > > > > +``rte_event_eth_rx_adapter_create_with_params()`` configures
> > > > > > +the adapter with default value for maximum packets processed
> > > > > > +per request to
> > > > > 128.
> > > > > > +``rte_event_eth_rx_adapter_runtime_params_set()`` function
> > > > > > +allows to reconfigure maximum number of packets processed by
> > > > > > +adapter per service request. This is alternative to
> > > > > > +configuring the maximum packets processed per request by
> > > > > > +adapter by using ``rte_event_eth_rx_adapter_create_ext()``
> > > > > > +with parameter
> > > > > ``rte_event_eth_rx_adapter_conf::max_nb_rx``.
> > > > >
> > > > > This paragraph is not needed IMO. As it is specific to a driver,
> > > > > and we can keep Doxygen comment only.
> > > > >
> > > > >
> > > > > > +
> > > > > > +``rte_event_eth_rx_adapter_runtime_parmas_get()`` function
> > > > > > +retrieves the configuration parameters that are defined in
> > > > > > +``struct
> > > > > rte_event_eth_rx_adapter_runtime_params``.
> > > > >
> > > > > Good.
> > > > >
> > > > > > +
> > > > > > Getting and resetting Adapter queue stats
> > > > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > >
> > > > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > > b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > > index 34aa87379e..d8f3e750b7 100644
> > > > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > > > > > @@ -35,6 +35,8 @@
> > > > > > #define MAX_VECTOR_NS 1E9
> > > > > > #define MIN_VECTOR_NS 1E5
> > > > > >
> > > > > > +#define RXA_NB_RX_WORK_DEFAULT 128
> > > > > > +
> > > > > > #define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32
> > > > > > #define ETH_RX_ADAPTER_MEM_NAME_LEN 32
> > > > > >
> > > > > > @@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t
> > > dev_id,
> > > > > > }
> > > > > >
> > > > > > conf->event_port_id = port_id;
> > > > > > - conf->max_nb_rx = 128;
> > > > > > + conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT;
> > > > > > if (started)
> > > > > > ret = rte_event_dev_start(dev_id);
> > > > > > rx_adapter->default_cb_arg = 1; @@ -3436,6 +3438,90 @@
> > > > > > rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
> > > > > > return -EINVAL;
> > > > > > }
> > > > >
> > > > > > +
> > > > > > +int
> > > > > > +rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
> > > > > > + struct rte_event_eth_rx_adapter_runtime_params
> > > > > > +*params) {
> > > > > > + struct event_eth_rx_adapter *rxa;
> > > > > > + int ret;
> > > > > > +
> > > > > > + if (params == NULL)
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + if (rxa_memzone_lookup())
> > > > > > + return -ENOMEM;
> > > > >
> > > > > Introduce an adapter callback and move SW adapter related logic
> > > > > under callback handler.
> > > > >
> > > > >
> > > > Do you mean introducing eventdev PMD callback for HW
> implementation?
> > >
> > > Yes.
> > >
> >
> > Can this be taken care as and when the HW implementation is required?
>
> OK. As long as when case INTERNAL PORT it return -ENOSUP now.
>
> >
> > > > There are no adapter callback currently for service based SW
> > > Implementation.
> > > >
> > > > > > +
> > > > > > + rxa = rxa_id_to_adapter(id);
> > > > > > + if (rxa == NULL)
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + ret = rxa_caps_check(rxa);
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > + rte_spinlock_lock(&rxa->rx_lock);
> > > > > > + rxa->max_nb_rx = params->max_nb_rx;
> > > > > > + rte_spinlock_unlock(&rxa->rx_lock);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +int
> > > > > > +rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
> > > > > > + struct rte_event_eth_rx_adapter_runtime_params
> > > > > > +*params) {
> > > > > > + struct event_eth_rx_adapter *rxa;
> > > > > > + int ret;
> > > > > > +
> > > > > > + if (params == NULL)
> > > > > > + return -EINVAL;
> > > > >
> > > > >
> > > > > Introduce an adapter callback and move SW adapter related logic
> > > > > under callback handler.
> > > > >
> > > > >
> > > >
> > > > Same as above
> > > >
> > > > > > +
> > > > > > + if (rxa_memzone_lookup())
> > > > > > + return -ENOMEM;
> > > > > +
> > > > > > + rxa = rxa_id_to_adapter(id);
> > > > > > + if (rxa == NULL)
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + ret = rxa_caps_check(rxa);
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > + params->max_nb_rx = rxa->max_nb_rx;
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +/* RX-adapter telemetry callbacks */
> > > > > > #define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d,
> > > > > > #s,
> > > > > > stats.s)
> > > > > >
> > > > > > static int
> > > > > > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > index f4652f40e8..214ffd018c 100644
> > > > > > --- a/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > +++ b/lib/eventdev/rte_event_eth_rx_adapter.h
> > > > > > @@ -39,10 +39,21 @@
> > > > > > * - rte_event_eth_rx_adapter_queue_stats_reset()
> > > > > > * - rte_event_eth_rx_adapter_event_port_get()
> > > > > > * - rte_event_eth_rx_adapter_instance_get()
> > > > > > + * - rte_event_eth_rx_adapter_runtime_params_get()
> > > > > > + * - rte_event_eth_rx_adapter_runtime_params_set()
> > > > > > *
> > > > > > * The application creates an ethernet to event adapter using
> > > > > > * rte_event_eth_rx_adapter_create_ext() or
> > > > > rte_event_eth_rx_adapter_create()
> > > > > > * or rte_event_eth_rx_adapter_create_with_params() functions.
> > > > > > + *
> > > > > > + * rte_event_eth_rx_adapter_create() or
> > > > > > + rte_event_eth_adapter_create_with_params()
> > > > > > + * configures the adapter with default value of maximum
> > > > > > + packets processed per
> > > > > > + * iteration to RXA_NB_RX_WORK_DEFAULT(128).
> > > > > > + * rte_event_eth_rx_adapter_runtime_params_set() allows to
> > > > > > + re-configure maximum
> > > > > > + * packets processed per iteration. This is alternative to
> > > > > > + using
> > > > > > + * rte_event_eth_rx_adapter_create_ext() with parameter
> > > > > > + * rte_event_eth_rx_adapter_conf::max_nb_rx
> > > > >
> > > > > Move this to Doxygen comment against max_nb_rx
> > > > >
> > > > > > + *
> > > > > > * The adapter needs to know which ethernet rx queues to poll
> > > > > > for mbufs as
> > > > > well
> > > > > > * as event device parameters such as the event queue
> > > > > > identifier,
> > > event
> > > > > > * priority and scheduling type that the adapter should use
> > > > > > when constructing @@ -299,6 +310,19 @@ struct
> > > > > rte_event_eth_rx_adapter_params {
> > > > > > /**< flag to indicate that event buffer is separate
> > > > > > for each queue */ };
> > > > > >
> > > > > > +/**
> > > > > > + * Adapter configuration parameters */ struct
> > > > > > +rte_event_eth_rx_adapter_runtime_params {
> > > > > > + uint32_t max_nb_rx;
> > > > > > + /**< The adapter can return early if it has processed at least
> > > > > > + * max_nb_rx mbufs. This isn't treated as a
> > > > > > +requirement; batching
> > > may
> > > > > > + * cause the adapter to process more than max_nb_rx mbufs.
> > > > >
> > > > > Also tell it is valid only for INTERNAL PORT capablity is set.
> > > > >
> > > >
> > > > Do you mean, it is valid only for INTERNAL PORT capability is 'not' set?
> > >
> > > Yes.
> > >
> > > >
> > > > > > + */
> > > > > > + uint32_t rsvd[15];
> > > > > > + /**< Reserved fields for future use */
> > > > >
> > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init() to
> make
> > > > > sure rsvd is zero.
> > > > >
> > > >
> > > > The reserved fields are not used by the adapter or application.
> > > > Not sure Is it necessary to Introduce a new API to clear reserved fields.
> > >
> > > When adapter starts using new fileds(when we add new fieds in
> > > future), the old applicaiton which is not using
> > > rte_event_eth_rx_adapter_runtime_params_init() may have junk value
> > > and then adapter implementation will behave bad.
> > >
> > >
> >
> > does it mean, the application doesn't re-compile for the new DPDK?
>
> Yes. No need recompile if ABI not breaking.
>
> > When some of the reserved fields are used in the future, the application
> also may need to be recompiled along with DPDK right?
> > As the application also may need to use the newly consumed reserved
> fields?
>
> The problematic case is:
>
> Adapter implementation of 23.07(Assuming there is change params) field
> needs to work with application of 23.03.
> rte_event_eth_rx_adapter_runtime_params_init() will sove that.
>
As rte_event_eth_rx_adapter_runtime_params_init() initializes only reserved fields to zero, it may not solve the issue in this case.
The old application only tries to set/get previous valid fields and the newly used fields may still contain junk value.
If the application wants to make use of any the newly used params, the application changes are required anyway.
> >
> > > >
> > > > > > +};
> > > > > > +
> > > > > > /**
> > > > > > *
> > > > > > * Callback function invoked by the SW adapter before it
> > > > > > continues @@
> > > > > > -377,7 +401,7 @@ int
> > > > > > rte_event_eth_rx_adapter_create_ext(uint8_t
> > > > > > id,
> > > > > uint8_t dev_id,
> > > > > > * Create a new ethernet Rx event adapter with the specified
> > > identifier.
> > > > > > * This function uses an internal configuration function that
> > > > > > creates an
> > > event
> > > > > > * port. This default function reconfigures the event device
> > > > > > with an
> > > > > > - * additional event port and setups up the event port using
> > > > > > the port_config
> > > > > > + * additional event port and setup the event port using the
> > > > > > + port_config
> > > > > > * parameter passed into this function. In case the
> > > > > > application needs
> > > more
> > > > > > * control in configuration of the service, it should use the
> > > > > > * rte_event_eth_rx_adapter_create_ext() version.
> > > > > > @@ -743,6 +767,50 @@
> > > > > > rte_event_eth_rx_adapter_instance_get(uint16_t
> > > > > eth_dev_id,
> > > > > > uint16_t rx_queue_id,
> > > > > > uint8_t *rxa_inst_id);
> > > > > >
On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > >
> > > > > > > + */
> > > > > > > + uint32_t rsvd[15];
> > > > > > > + /**< Reserved fields for future use */
> > > > > >
> > > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init() to
> > make
> > > > > > sure rsvd is zero.
> > > > > >
> > > > >
> > > > > The reserved fields are not used by the adapter or application.
> > > > > Not sure Is it necessary to Introduce a new API to clear reserved fields.
> > > >
> > > > When adapter starts using new fileds(when we add new fieds in
> > > > future), the old applicaiton which is not using
> > > > rte_event_eth_rx_adapter_runtime_params_init() may have junk value
> > > > and then adapter implementation will behave bad.
> > > >
> > > >
> > >
> > > does it mean, the application doesn't re-compile for the new DPDK?
> >
> > Yes. No need recompile if ABI not breaking.
> >
> > > When some of the reserved fields are used in the future, the application
> > also may need to be recompiled along with DPDK right?
> > > As the application also may need to use the newly consumed reserved
> > fields?
> >
> > The problematic case is:
> >
> > Adapter implementation of 23.07(Assuming there is change params) field
> > needs to work with application of 23.03.
> > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> >
>
> As rte_event_eth_rx_adapter_runtime_params_init() initializes only reserved fields to zero, it may not solve the issue in this case.
rte_event_eth_rx_adapter_runtime_params_init() needs to zero all
fields, not just reserved field.
The application calling sequence is
struct my_config c;
rte_event_eth_rx_adapter_runtime_params_init(&c)
c.interseted_filed_to_be_updated = val;
Let me share an example and you can tell where is the issue
1)Assume parameter structure is 64B and for 22.03 8B are used.
2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B.
3)There is an application written based on 22.03 which using only 8B
after calling rte_event_eth_rx_adapter_runtime_params_init()
4)Assume, in 22.07 another 8B added to structure.
5)Now, the application (3) needs to run on 22.07. Since the
application is calling rte_event_eth_rx_adapter_runtime_params_init()
and 9 to 15B are zero, the implementation will not go bad.
> The old application only tries to set/get previous valid fields and the newly used fields may still contain junk value.
> If the application wants to make use of any the newly used params, the application changes are required anyway.
Yes. If application wants to make use of newly added features. No need
to change if new features are not needed for old application.
On Sat, 28 Jan 2023 16:23:45 +0530
Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > Yes. No need recompile if ABI not breaking.
> > >
> > > > When some of the reserved fields are used in the future, the application
> > > also may need to be recompiled along with DPDK right?
> > > > As the application also may need to use the newly consumed reserved
> > > fields?
> > >
> > > The problematic case is:
> > >
> > > Adapter implementation of 23.07(Assuming there is change params) field
> > > needs to work with application of 23.03.
> > > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> > >
> >
First off, reserved fields are a problematic design choice IMHO (see YAGNI).
Second. any reserved fields can not be used in future unless the
original code enforced that all reserved fields are zero.
Same is true for holes in structs which some times get reused.
You can't use a reserved field without breaking ABI unless the previous
code enforced that the field must be zero.
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Saturday, January 28, 2023 4:24 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
>
> On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
>
> > > > > >
> > > > > > > > + */
> > > > > > > > + uint32_t rsvd[15];
> > > > > > > > + /**< Reserved fields for future use */
> > > > > > >
> > > > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init() to
> > > make
> > > > > > > sure rsvd is zero.
> > > > > > >
> > > > > >
> > > > > > The reserved fields are not used by the adapter or application.
> > > > > > Not sure Is it necessary to Introduce a new API to clear reserved
> fields.
> > > > >
> > > > > When adapter starts using new fileds(when we add new fieds in
> > > > > future), the old applicaiton which is not using
> > > > > rte_event_eth_rx_adapter_runtime_params_init() may have junk
> > > > > value and then adapter implementation will behave bad.
> > > > >
> > > > >
> > > >
> > > > does it mean, the application doesn't re-compile for the new DPDK?
> > >
> > > Yes. No need recompile if ABI not breaking.
> > >
> > > > When some of the reserved fields are used in the future, the
> > > > application
> > > also may need to be recompiled along with DPDK right?
> > > > As the application also may need to use the newly consumed
> > > > reserved
> > > fields?
> > >
> > > The problematic case is:
> > >
> > > Adapter implementation of 23.07(Assuming there is change params)
> > > field needs to work with application of 23.03.
> > > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> > >
> >
> > As rte_event_eth_rx_adapter_runtime_params_init() initializes only
> reserved fields to zero, it may not solve the issue in this case.
>
> rte_event_eth_rx_adapter_runtime_params_init() needs to zero all fields,
> not just reserved field.
> The application calling sequence is
>
> struct my_config c;
> rte_event_eth_rx_adapter_runtime_params_init(&c)
> c.interseted_filed_to_be_updated = val;
>
Can it be done like
struct my_config c = {0};
c.interseted_filed_to_be_updated = val;
and update Doxygen comments to recommend above usage to reset all fields?
This way, rte_event_eth_rx_adapter_runtime_params_init() can be avoided.
> Let me share an example and you can tell where is the issue
>
> 1)Assume parameter structure is 64B and for 22.03 8B are used.
> 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B.
> 3)There is an application written based on 22.03 which using only 8B after
> calling rte_event_eth_rx_adapter_runtime_params_init()
> 4)Assume, in 22.07 another 8B added to structure.
> 5)Now, the application (3) needs to run on 22.07. Since the application is
> calling rte_event_eth_rx_adapter_runtime_params_init()
> and 9 to 15B are zero, the implementation will not go bad.
>
> > The old application only tries to set/get previous valid fields and the newly
> used fields may still contain junk value.
> > If the application wants to make use of any the newly used params, the
> application changes are required anyway.
>
> Yes. If application wants to make use of newly added features. No need to
> change if new features are not needed for old application.
On Mon, Jan 30, 2023 at 3:26 PM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Saturday, January 28, 2023 4:24 PM
> > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> > Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> > Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> >
> > On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
> > <s.v.naga.harish.k@intel.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> >
> > > > > > >
> > > > > > > > > + */
> > > > > > > > > + uint32_t rsvd[15];
> > > > > > > > > + /**< Reserved fields for future use */
> > > > > > > >
> > > > > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init() to
> > > > make
> > > > > > > > sure rsvd is zero.
> > > > > > > >
> > > > > > >
> > > > > > > The reserved fields are not used by the adapter or application.
> > > > > > > Not sure Is it necessary to Introduce a new API to clear reserved
> > fields.
> > > > > >
> > > > > > When adapter starts using new fileds(when we add new fieds in
> > > > > > future), the old applicaiton which is not using
> > > > > > rte_event_eth_rx_adapter_runtime_params_init() may have junk
> > > > > > value and then adapter implementation will behave bad.
> > > > > >
> > > > > >
> > > > >
> > > > > does it mean, the application doesn't re-compile for the new DPDK?
> > > >
> > > > Yes. No need recompile if ABI not breaking.
> > > >
> > > > > When some of the reserved fields are used in the future, the
> > > > > application
> > > > also may need to be recompiled along with DPDK right?
> > > > > As the application also may need to use the newly consumed
> > > > > reserved
> > > > fields?
> > > >
> > > > The problematic case is:
> > > >
> > > > Adapter implementation of 23.07(Assuming there is change params)
> > > > field needs to work with application of 23.03.
> > > > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> > > >
> > >
> > > As rte_event_eth_rx_adapter_runtime_params_init() initializes only
> > reserved fields to zero, it may not solve the issue in this case.
> >
> > rte_event_eth_rx_adapter_runtime_params_init() needs to zero all fields,
> > not just reserved field.
> > The application calling sequence is
> >
> > struct my_config c;
> > rte_event_eth_rx_adapter_runtime_params_init(&c)
> > c.interseted_filed_to_be_updated = val;
> >
> Can it be done like
> struct my_config c = {0};
> c.interseted_filed_to_be_updated = val;
> and update Doxygen comments to recommend above usage to reset all fields?
> This way, rte_event_eth_rx_adapter_runtime_params_init() can be avoided.
Better to have a function for documentation clarity. Similar scheme
already there
in DPDK. See rte_eth_cman_config_init()
>
> > Let me share an example and you can tell where is the issue
> >
> > 1)Assume parameter structure is 64B and for 22.03 8B are used.
> > 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B.
> > 3)There is an application written based on 22.03 which using only 8B after
> > calling rte_event_eth_rx_adapter_runtime_params_init()
> > 4)Assume, in 22.07 another 8B added to structure.
> > 5)Now, the application (3) needs to run on 22.07. Since the application is
> > calling rte_event_eth_rx_adapter_runtime_params_init()
> > and 9 to 15B are zero, the implementation will not go bad.
> >
> > > The old application only tries to set/get previous valid fields and the newly
> > used fields may still contain junk value.
> > > If the application wants to make use of any the newly used params, the
> > application changes are required anyway.
> >
> > Yes. If application wants to make use of newly added features. No need to
> > change if new features are not needed for old application.
Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, January 30, 2023 8:13 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
>
> On Mon, Jan 30, 2023 at 3:26 PM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Saturday, January 28, 2023 4:24 PM
> > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>
> > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > >
> > > On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
> > > <s.v.naga.harish.k@intel.com> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > >
> > > > > > > >
> > > > > > > > > > + */
> > > > > > > > > > + uint32_t rsvd[15];
> > > > > > > > > > + /**< Reserved fields for future use */
> > > > > > > > >
> > > > > > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > > > to
> > > > > make
> > > > > > > > > sure rsvd is zero.
> > > > > > > > >
> > > > > > > >
> > > > > > > > The reserved fields are not used by the adapter or application.
> > > > > > > > Not sure Is it necessary to Introduce a new API to clear
> > > > > > > > reserved
> > > fields.
> > > > > > >
> > > > > > > When adapter starts using new fileds(when we add new fieds
> > > > > > > in future), the old applicaiton which is not using
> > > > > > > rte_event_eth_rx_adapter_runtime_params_init() may have
> junk
> > > > > > > value and then adapter implementation will behave bad.
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > does it mean, the application doesn't re-compile for the new DPDK?
> > > > >
> > > > > Yes. No need recompile if ABI not breaking.
> > > > >
> > > > > > When some of the reserved fields are used in the future, the
> > > > > > application
> > > > > also may need to be recompiled along with DPDK right?
> > > > > > As the application also may need to use the newly consumed
> > > > > > reserved
> > > > > fields?
> > > > >
> > > > > The problematic case is:
> > > > >
> > > > > Adapter implementation of 23.07(Assuming there is change params)
> > > > > field needs to work with application of 23.03.
> > > > > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> > > > >
> > > >
> > > > As rte_event_eth_rx_adapter_runtime_params_init() initializes only
> > > reserved fields to zero, it may not solve the issue in this case.
> > >
> > > rte_event_eth_rx_adapter_runtime_params_init() needs to zero all
> > > fields, not just reserved field.
> > > The application calling sequence is
> > >
> > > struct my_config c;
> > > rte_event_eth_rx_adapter_runtime_params_init(&c)
> > > c.interseted_filed_to_be_updated = val;
> > >
> > Can it be done like
> > struct my_config c = {0};
> > c.interseted_filed_to_be_updated = val; and update Doxygen
> > comments to recommend above usage to reset all fields?
> > This way, rte_event_eth_rx_adapter_runtime_params_init() can be
> avoided.
>
> Better to have a function for documentation clarity. Similar scheme already
> there in DPDK. See rte_eth_cman_config_init()
>
>
The reference function rte_eth_cman_config_init() is resetting the params struct and initializing the required params with default values in the pmd cb.
The proposed rte_event_eth_rx_adapter_runtime_params_init () API just needs to reset the params struct. There are no pmd CBs involved.
Having an API just to reset the struct seems overkill. What do you think?
> >
> > > Let me share an example and you can tell where is the issue
> > >
> > > 1)Assume parameter structure is 64B and for 22.03 8B are used.
> > > 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B.
> > > 3)There is an application written based on 22.03 which using only 8B
> > > after calling rte_event_eth_rx_adapter_runtime_params_init()
> > > 4)Assume, in 22.07 another 8B added to structure.
> > > 5)Now, the application (3) needs to run on 22.07. Since the
> > > application is calling
> > > rte_event_eth_rx_adapter_runtime_params_init()
> > > and 9 to 15B are zero, the implementation will not go bad.
> > >
> > > > The old application only tries to set/get previous valid fields
> > > > and the newly
> > > used fields may still contain junk value.
> > > > If the application wants to make use of any the newly used params,
> > > > the
> > > application changes are required anyway.
> > >
> > > Yes. If application wants to make use of newly added features. No
> > > need to change if new features are not needed for old application.
On Thu, Feb 2, 2023 at 9:42 PM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Monday, January 30, 2023 8:13 PM
> > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> > Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> > Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> >
> > On Mon, Jan 30, 2023 at 3:26 PM Naga Harish K, S V
> > <s.v.naga.harish.k@intel.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Saturday, January 28, 2023 4:24 PM
> > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > <jay.jayatheerthan@intel.com>
> > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > > >
> > > > On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
> > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > >
> > > > > > > > >
> > > > > > > > > > > + */
> > > > > > > > > > > + uint32_t rsvd[15];
> > > > > > > > > > > + /**< Reserved fields for future use */
> > > > > > > > > >
> > > > > > > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > > > > to
> > > > > > make
> > > > > > > > > > sure rsvd is zero.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > The reserved fields are not used by the adapter or application.
> > > > > > > > > Not sure Is it necessary to Introduce a new API to clear
> > > > > > > > > reserved
> > > > fields.
> > > > > > > >
> > > > > > > > When adapter starts using new fileds(when we add new fieds
> > > > > > > > in future), the old applicaiton which is not using
> > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() may have
> > junk
> > > > > > > > value and then adapter implementation will behave bad.
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > > > does it mean, the application doesn't re-compile for the new DPDK?
> > > > > >
> > > > > > Yes. No need recompile if ABI not breaking.
> > > > > >
> > > > > > > When some of the reserved fields are used in the future, the
> > > > > > > application
> > > > > > also may need to be recompiled along with DPDK right?
> > > > > > > As the application also may need to use the newly consumed
> > > > > > > reserved
> > > > > > fields?
> > > > > >
> > > > > > The problematic case is:
> > > > > >
> > > > > > Adapter implementation of 23.07(Assuming there is change params)
> > > > > > field needs to work with application of 23.03.
> > > > > > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> > > > > >
> > > > >
> > > > > As rte_event_eth_rx_adapter_runtime_params_init() initializes only
> > > > reserved fields to zero, it may not solve the issue in this case.
> > > >
> > > > rte_event_eth_rx_adapter_runtime_params_init() needs to zero all
> > > > fields, not just reserved field.
> > > > The application calling sequence is
> > > >
> > > > struct my_config c;
> > > > rte_event_eth_rx_adapter_runtime_params_init(&c)
> > > > c.interseted_filed_to_be_updated = val;
> > > >
> > > Can it be done like
> > > struct my_config c = {0};
> > > c.interseted_filed_to_be_updated = val; and update Doxygen
> > > comments to recommend above usage to reset all fields?
> > > This way, rte_event_eth_rx_adapter_runtime_params_init() can be
> > avoided.
> >
> > Better to have a function for documentation clarity. Similar scheme already
> > there in DPDK. See rte_eth_cman_config_init()
> >
> >
>
>
> The reference function rte_eth_cman_config_init() is resetting the params struct and initializing the required params with default values in the pmd cb.
No need for PMD cb.
> The proposed rte_event_eth_rx_adapter_runtime_params_init () API just needs to reset the params struct. There are no pmd CBs involved.
> Having an API just to reset the struct seems overkill. What do you think?
It is slow path API. Keeping it as function is better. Also, it helps
the documentations of config parm in
rte_event_eth_rx_adapter_runtime_params_config()
like, This structure must be initialized with
rte_event_eth_rx_adapter_runtime_params_init() or so.
>
> > >
> > > > Let me share an example and you can tell where is the issue
> > > >
> > > > 1)Assume parameter structure is 64B and for 22.03 8B are used.
> > > > 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B.
> > > > 3)There is an application written based on 22.03 which using only 8B
> > > > after calling rte_event_eth_rx_adapter_runtime_params_init()
> > > > 4)Assume, in 22.07 another 8B added to structure.
> > > > 5)Now, the application (3) needs to run on 22.07. Since the
> > > > application is calling
> > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > and 9 to 15B are zero, the implementation will not go bad.
> > > >
> > > > > The old application only tries to set/get previous valid fields
> > > > > and the newly
> > > > used fields may still contain junk value.
> > > > > If the application wants to make use of any the newly used params,
> > > > > the
> > > > application changes are required anyway.
> > > >
> > > > Yes. If application wants to make use of newly added features. No
> > > > need to change if new features are not needed for old application.
Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Friday, February 3, 2023 3:15 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
>
> On Thu, Feb 2, 2023 at 9:42 PM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > Hi Jerin,
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Monday, January 30, 2023 8:13 PM
> > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>
> > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > >
> > > On Mon, Jan 30, 2023 at 3:26 PM Naga Harish K, S V
> > > <s.v.naga.harish.k@intel.com> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Saturday, January 28, 2023 4:24 PM
> > > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > > <jay.jayatheerthan@intel.com>
> > > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get
> > > > > APIs
> > > > >
> > > > > On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
> > > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > >
> > > > > > > > > >
> > > > > > > > > > > > + */
> > > > > > > > > > > > + uint32_t rsvd[15];
> > > > > > > > > > > > + /**< Reserved fields for future use */
> > > > > > > > > > >
> > > > > > > > > > > Introduce
> > > > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > > > > > to
> > > > > > > make
> > > > > > > > > > > sure rsvd is zero.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > The reserved fields are not used by the adapter or
> application.
> > > > > > > > > > Not sure Is it necessary to Introduce a new API to
> > > > > > > > > > clear reserved
> > > > > fields.
> > > > > > > > >
> > > > > > > > > When adapter starts using new fileds(when we add new
> > > > > > > > > fieds in future), the old applicaiton which is not using
> > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() may have
> > > junk
> > > > > > > > > value and then adapter implementation will behave bad.
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > > does it mean, the application doesn't re-compile for the new
> DPDK?
> > > > > > >
> > > > > > > Yes. No need recompile if ABI not breaking.
> > > > > > >
> > > > > > > > When some of the reserved fields are used in the future,
> > > > > > > > the application
> > > > > > > also may need to be recompiled along with DPDK right?
> > > > > > > > As the application also may need to use the newly consumed
> > > > > > > > reserved
> > > > > > > fields?
> > > > > > >
> > > > > > > The problematic case is:
> > > > > > >
> > > > > > > Adapter implementation of 23.07(Assuming there is change
> > > > > > > params) field needs to work with application of 23.03.
> > > > > > > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> > > > > > >
> > > > > >
> > > > > > As rte_event_eth_rx_adapter_runtime_params_init() initializes
> > > > > > only
> > > > > reserved fields to zero, it may not solve the issue in this case.
> > > > >
> > > > > rte_event_eth_rx_adapter_runtime_params_init() needs to zero all
> > > > > fields, not just reserved field.
> > > > > The application calling sequence is
> > > > >
> > > > > struct my_config c;
> > > > > rte_event_eth_rx_adapter_runtime_params_init(&c)
> > > > > c.interseted_filed_to_be_updated = val;
> > > > >
> > > > Can it be done like
> > > > struct my_config c = {0};
> > > > c.interseted_filed_to_be_updated = val; and update Doxygen
> > > > comments to recommend above usage to reset all fields?
> > > > This way, rte_event_eth_rx_adapter_runtime_params_init() can be
> > > avoided.
> > >
> > > Better to have a function for documentation clarity. Similar scheme
> > > already there in DPDK. See rte_eth_cman_config_init()
> > >
> > >
> >
> >
> > The reference function rte_eth_cman_config_init() is resetting the params
> struct and initializing the required params with default values in the pmd cb.
>
> No need for PMD cb.
>
> > The proposed rte_event_eth_rx_adapter_runtime_params_init () API just
> needs to reset the params struct. There are no pmd CBs involved.
> > Having an API just to reset the struct seems overkill. What do you think?
>
> It is slow path API. Keeping it as function is better. Also, it helps the
> documentations of config parm in
> rte_event_eth_rx_adapter_runtime_params_config()
> like, This structure must be initialized with
> rte_event_eth_rx_adapter_runtime_params_init() or so.
>
>
Are there any other reasons to have this API (*params_init()) other than documentation?
>
> >
> > > >
> > > > > Let me share an example and you can tell where is the issue
> > > > >
> > > > > 1)Assume parameter structure is 64B and for 22.03 8B are used.
> > > > > 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B.
> > > > > 3)There is an application written based on 22.03 which using
> > > > > only 8B after calling
> > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > 4)Assume, in 22.07 another 8B added to structure.
> > > > > 5)Now, the application (3) needs to run on 22.07. Since the
> > > > > application is calling
> > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > and 9 to 15B are zero, the implementation will not go bad.
> > > > >
> > > > > > The old application only tries to set/get previous valid
> > > > > > fields and the newly
> > > > > used fields may still contain junk value.
> > > > > > If the application wants to make use of any the newly used
> > > > > > params, the
> > > > > application changes are required anyway.
> > > > >
> > > > > Yes. If application wants to make use of newly added features.
> > > > > No need to change if new features are not needed for old application.
On Mon, Feb 6, 2023 at 11:52 AM Naga Harish K, S V
<s.v.naga.harish.k@intel.com> wrote:
>
> Hi Jerin,
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Friday, February 3, 2023 3:15 PM
> > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> > Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> > Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> >
> > On Thu, Feb 2, 2023 at 9:42 PM Naga Harish K, S V
> > <s.v.naga.harish.k@intel.com> wrote:
> > >
> > > Hi Jerin,
> > >
> > > > -----Original Message-----
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > Sent: Monday, January 30, 2023 8:13 PM
> > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > <jay.jayatheerthan@intel.com>
> > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > > >
> > > > On Mon, Jan 30, 2023 at 3:26 PM Naga Harish K, S V
> > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > Sent: Saturday, January 28, 2023 4:24 PM
> > > > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > > > <jay.jayatheerthan@intel.com>
> > > > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get
> > > > > > APIs
> > > > > >
> > > > > > On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
> > > > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > > + */
> > > > > > > > > > > > > + uint32_t rsvd[15];
> > > > > > > > > > > > > + /**< Reserved fields for future use */
> > > > > > > > > > > >
> > > > > > > > > > > > Introduce
> > > > > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > > > > > > to
> > > > > > > > make
> > > > > > > > > > > > sure rsvd is zero.
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > The reserved fields are not used by the adapter or
> > application.
> > > > > > > > > > > Not sure Is it necessary to Introduce a new API to
> > > > > > > > > > > clear reserved
> > > > > > fields.
> > > > > > > > > >
> > > > > > > > > > When adapter starts using new fileds(when we add new
> > > > > > > > > > fieds in future), the old applicaiton which is not using
> > > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() may have
> > > > junk
> > > > > > > > > > value and then adapter implementation will behave bad.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > does it mean, the application doesn't re-compile for the new
> > DPDK?
> > > > > > > >
> > > > > > > > Yes. No need recompile if ABI not breaking.
> > > > > > > >
> > > > > > > > > When some of the reserved fields are used in the future,
> > > > > > > > > the application
> > > > > > > > also may need to be recompiled along with DPDK right?
> > > > > > > > > As the application also may need to use the newly consumed
> > > > > > > > > reserved
> > > > > > > > fields?
> > > > > > > >
> > > > > > > > The problematic case is:
> > > > > > > >
> > > > > > > > Adapter implementation of 23.07(Assuming there is change
> > > > > > > > params) field needs to work with application of 23.03.
> > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() will sove that.
> > > > > > > >
> > > > > > >
> > > > > > > As rte_event_eth_rx_adapter_runtime_params_init() initializes
> > > > > > > only
> > > > > > reserved fields to zero, it may not solve the issue in this case.
> > > > > >
> > > > > > rte_event_eth_rx_adapter_runtime_params_init() needs to zero all
> > > > > > fields, not just reserved field.
> > > > > > The application calling sequence is
> > > > > >
> > > > > > struct my_config c;
> > > > > > rte_event_eth_rx_adapter_runtime_params_init(&c)
> > > > > > c.interseted_filed_to_be_updated = val;
> > > > > >
> > > > > Can it be done like
> > > > > struct my_config c = {0};
> > > > > c.interseted_filed_to_be_updated = val; and update Doxygen
> > > > > comments to recommend above usage to reset all fields?
> > > > > This way, rte_event_eth_rx_adapter_runtime_params_init() can be
> > > > avoided.
> > > >
> > > > Better to have a function for documentation clarity. Similar scheme
> > > > already there in DPDK. See rte_eth_cman_config_init()
> > > >
> > > >
> > >
> > >
> > > The reference function rte_eth_cman_config_init() is resetting the params
> > struct and initializing the required params with default values in the pmd cb.
> >
> > No need for PMD cb.
> >
> > > The proposed rte_event_eth_rx_adapter_runtime_params_init () API just
> > needs to reset the params struct. There are no pmd CBs involved.
> > > Having an API just to reset the struct seems overkill. What do you think?
> >
> > It is slow path API. Keeping it as function is better. Also, it helps the
> > documentations of config parm in
> > rte_event_eth_rx_adapter_runtime_params_config()
> > like, This structure must be initialized with
> > rte_event_eth_rx_adapter_runtime_params_init() or so.
> >
> >
>
> Are there any other reasons to have this API (*params_init()) other than documentation?
Initialization code is segregated for tracking.
>
> >
> > >
> > > > >
> > > > > > Let me share an example and you can tell where is the issue
> > > > > >
> > > > > > 1)Assume parameter structure is 64B and for 22.03 8B are used.
> > > > > > 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B.
> > > > > > 3)There is an application written based on 22.03 which using
> > > > > > only 8B after calling
> > > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > 4)Assume, in 22.07 another 8B added to structure.
> > > > > > 5)Now, the application (3) needs to run on 22.07. Since the
> > > > > > application is calling
> > > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > and 9 to 15B are zero, the implementation will not go bad.
> > > > > >
> > > > > > > The old application only tries to set/get previous valid
> > > > > > > fields and the newly
> > > > > > used fields may still contain junk value.
> > > > > > > If the application wants to make use of any the newly used
> > > > > > > params, the
> > > > > > application changes are required anyway.
> > > > > >
> > > > > > Yes. If application wants to make use of newly added features.
> > > > > > No need to change if new features are not needed for old application.
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, February 6, 2023 10:08 PM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: jerinj@marvell.com; Carrillo, Erik G <erik.g.carrillo@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; dev@dpdk.org;
> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
>
> On Mon, Feb 6, 2023 at 11:52 AM Naga Harish K, S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > Hi Jerin,
> >
> > > -----Original Message-----
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > Sent: Friday, February 3, 2023 3:15 PM
> > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > <jay.jayatheerthan@intel.com>
> > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs
> > >
> > > On Thu, Feb 2, 2023 at 9:42 PM Naga Harish K, S V
> > > <s.v.naga.harish.k@intel.com> wrote:
> > > >
> > > > Hi Jerin,
> > > >
> > > > > -----Original Message-----
> > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > Sent: Monday, January 30, 2023 8:13 PM
> > > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan, Jay
> > > > > <jay.jayatheerthan@intel.com>
> > > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get
> > > > > APIs
> > > > >
> > > > > On Mon, Jan 30, 2023 at 3:26 PM Naga Harish K, S V
> > > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > Sent: Saturday, January 28, 2023 4:24 PM
> > > > > > > To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> > > > > > > Cc: jerinj@marvell.com; Carrillo, Erik G
> > > > > > > <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
> > > > > > > <abhinandan.gujjar@intel.com>; dev@dpdk.org; Jayatheerthan,
> > > > > > > Jay <jay.jayatheerthan@intel.com>
> > > > > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params
> > > > > > > set/get APIs
> > > > > > >
> > > > > > > On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V
> > > > > > > <s.v.naga.harish.k@intel.com> wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > > > + */
> > > > > > > > > > > > > > + uint32_t rsvd[15];
> > > > > > > > > > > > > > + /**< Reserved fields for future use */
> > > > > > > > > > > > >
> > > > > > > > > > > > > Introduce
> > > > > > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > > > > > > > to
> > > > > > > > > make
> > > > > > > > > > > > > sure rsvd is zero.
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > The reserved fields are not used by the adapter or
> > > application.
> > > > > > > > > > > > Not sure Is it necessary to Introduce a new API to
> > > > > > > > > > > > clear reserved
> > > > > > > fields.
> > > > > > > > > > >
> > > > > > > > > > > When adapter starts using new fileds(when we add new
> > > > > > > > > > > fieds in future), the old applicaiton which is not
> > > > > > > > > > > using
> > > > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() may
> > > > > > > > > > > have
> > > > > junk
> > > > > > > > > > > value and then adapter implementation will behave bad.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > does it mean, the application doesn't re-compile for
> > > > > > > > > > the new
> > > DPDK?
> > > > > > > > >
> > > > > > > > > Yes. No need recompile if ABI not breaking.
> > > > > > > > >
> > > > > > > > > > When some of the reserved fields are used in the
> > > > > > > > > > future, the application
> > > > > > > > > also may need to be recompiled along with DPDK right?
> > > > > > > > > > As the application also may need to use the newly
> > > > > > > > > > consumed reserved
> > > > > > > > > fields?
> > > > > > > > >
> > > > > > > > > The problematic case is:
> > > > > > > > >
> > > > > > > > > Adapter implementation of 23.07(Assuming there is change
> > > > > > > > > params) field needs to work with application of 23.03.
> > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() will sove
> that.
> > > > > > > > >
> > > > > > > >
> > > > > > > > As rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > > initializes only
> > > > > > > reserved fields to zero, it may not solve the issue in this case.
> > > > > > >
> > > > > > > rte_event_eth_rx_adapter_runtime_params_init() needs to zero
> > > > > > > all fields, not just reserved field.
> > > > > > > The application calling sequence is
> > > > > > >
> > > > > > > struct my_config c;
> > > > > > > rte_event_eth_rx_adapter_runtime_params_init(&c)
> > > > > > > c.interseted_filed_to_be_updated = val;
> > > > > > >
> > > > > > Can it be done like
> > > > > > struct my_config c = {0};
> > > > > > c.interseted_filed_to_be_updated = val; and update
> > > > > > Doxygen comments to recommend above usage to reset all fields?
> > > > > > This way, rte_event_eth_rx_adapter_runtime_params_init() can
> > > > > > be
> > > > > avoided.
> > > > >
> > > > > Better to have a function for documentation clarity. Similar
> > > > > scheme already there in DPDK. See rte_eth_cman_config_init()
> > > > >
> > > > >
> > > >
> > > >
> > > > The reference function rte_eth_cman_config_init() is resetting the
> > > > params
> > > struct and initializing the required params with default values in the pmd
> cb.
> > >
> > > No need for PMD cb.
> > >
> > > > The proposed rte_event_eth_rx_adapter_runtime_params_init () API
> > > > just
> > > needs to reset the params struct. There are no pmd CBs involved.
> > > > Having an API just to reset the struct seems overkill. What do you
> think?
> > >
> > > It is slow path API. Keeping it as function is better. Also, it
> > > helps the documentations of config parm in
> > > rte_event_eth_rx_adapter_runtime_params_config()
> > > like, This structure must be initialized with
> > > rte_event_eth_rx_adapter_runtime_params_init() or so.
> > >
> > >
> >
> > Are there any other reasons to have this API (*params_init()) other than
> documentation?
>
> Initialization code is segregated for tracking.
>
The discussed changes are updated in the v3 patchset.
> >
> > >
> > > >
> > > > > >
> > > > > > > Let me share an example and you can tell where is the issue
> > > > > > >
> > > > > > > 1)Assume parameter structure is 64B and for 22.03 8B are used.
> > > > > > > 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all
> 64B.
> > > > > > > 3)There is an application written based on 22.03 which using
> > > > > > > only 8B after calling
> > > > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > 4)Assume, in 22.07 another 8B added to structure.
> > > > > > > 5)Now, the application (3) needs to run on 22.07. Since the
> > > > > > > application is calling
> > > > > > > rte_event_eth_rx_adapter_runtime_params_init()
> > > > > > > and 9 to 15B are zero, the implementation will not go bad.
> > > > > > >
> > > > > > > > The old application only tries to set/get previous valid
> > > > > > > > fields and the newly
> > > > > > > used fields may still contain junk value.
> > > > > > > > If the application wants to make use of any the newly used
> > > > > > > > params, the
> > > > > > > application changes are required anyway.
> > > > > > >
> > > > > > > Yes. If application wants to make use of newly added features.
> > > > > > > No need to change if new features are not needed for old
> application.
@@ -1198,6 +1198,96 @@ adapter_intrq_instance_get(void)
return TEST_SUCCESS;
}
+static int
+adapter_get_set_params(void)
+{
+ int err;
+ struct rte_event_eth_rx_adapter_runtime_params in_params;
+ struct rte_event_eth_rx_adapter_runtime_params out_params;
+ struct rte_event_eth_rx_adapter_queue_conf queue_config = {0};
+ struct rte_event ev;
+
+ ev.queue_id = 0;
+ ev.sched_type = RTE_SCHED_TYPE_ATOMIC;
+ ev.priority = 0;
+ ev.flow_id = 1;
+
+ queue_config.rx_queue_flags =
+ RTE_EVENT_ETH_RX_ADAPTER_QUEUE_FLOW_ID_VALID;
+ queue_config.ev = ev;
+ queue_config.servicing_weight = 1;
+
+ err = rte_event_eth_rx_adapter_queue_add(TEST_INST_ID,
+ TEST_ETHDEV_ID, 0,
+ &queue_config);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ /* Case 1: Get the default value of mbufs processed by Rx adapter */
+ err = rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID, &out_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ /* Case 2: Set max_nb_rx = 32 (=BATCH_SEIZE) */
+ in_params.max_nb_rx = 32;
+
+ err = rte_event_eth_rx_adapter_runtime_params_set(TEST_INST_ID, &in_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ err = rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID, &out_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+ TEST_ASSERT(in_params.max_nb_rx == out_params.max_nb_rx, "Expected %u got %u",
+ in_params.max_nb_rx, out_params.max_nb_rx);
+
+ /* Case 3: Set max_nb_rx = 192 */
+ in_params.max_nb_rx = 192;
+
+ err = rte_event_eth_rx_adapter_runtime_params_set(TEST_INST_ID, &in_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ err = rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID, &out_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+ TEST_ASSERT(in_params.max_nb_rx == out_params.max_nb_rx, "Expected %u got %u",
+ in_params.max_nb_rx, out_params.max_nb_rx);
+
+ /* Case 4: Set max_nb_rx = 256 */
+ in_params.max_nb_rx = 256;
+
+ err = rte_event_eth_rx_adapter_runtime_params_set(TEST_INST_ID, &in_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ err = rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID, &out_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+ TEST_ASSERT(in_params.max_nb_rx == out_params.max_nb_rx, "Expected %u got %u",
+ in_params.max_nb_rx, out_params.max_nb_rx);
+
+ /* Case 5: Set max_nb_rx = 30(<BATCH_SIZE) */
+ in_params.max_nb_rx = 30;
+
+ err = rte_event_eth_rx_adapter_runtime_params_set(TEST_INST_ID, &in_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ err = rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID, &out_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+ TEST_ASSERT(in_params.max_nb_rx == out_params.max_nb_rx, "Expected %u got %u",
+ in_params.max_nb_rx, out_params.max_nb_rx);
+
+ /* Case 6: Set max_nb_rx = 512 */
+ in_params.max_nb_rx = 512;
+
+ err = rte_event_eth_rx_adapter_runtime_params_set(TEST_INST_ID, &in_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ err = rte_event_eth_rx_adapter_runtime_params_get(TEST_INST_ID, &out_params);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+ TEST_ASSERT(in_params.max_nb_rx == out_params.max_nb_rx, "Expected %u got %u",
+ in_params.max_nb_rx, out_params.max_nb_rx);
+
+ err = rte_event_eth_rx_adapter_queue_del(TEST_INST_ID,
+ TEST_ETHDEV_ID, 0);
+ TEST_ASSERT(err == 0, "Expected 0 got %d", err);
+
+ return TEST_SUCCESS;
+}
+
static struct unit_test_suite event_eth_rx_tests = {
.suite_name = "rx event eth adapter test suite",
.setup = testsuite_setup,
@@ -1218,6 +1308,8 @@ static struct unit_test_suite event_eth_rx_tests = {
adapter_queue_stats_test),
TEST_CASE_ST(adapter_create, adapter_free,
adapter_pollq_instance_get),
+ TEST_CASE_ST(adapter_create, adapter_free,
+ adapter_get_set_params),
TEST_CASES_END() /**< NULL terminate unit test array */
}
};
@@ -185,6 +185,26 @@ flags for handling received packets, event queue identifier, scheduler type,
event priority, polling frequency of the receive queue and flow identifier
in struct ``rte_event_eth_rx_adapter_queue_conf``.
+Set/Get adapter runtime configuration parameters
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The runtime configuration parameters of adapter can be set/read using
+``rte_event_eth_rx_adapter_runtime_params_set()`` and
+``rte_event_eth_rx_adapter_runtime_params_get()`` respectively. The parameters that
+can be set/read are defined in ``struct rte_event_eth_rx_adapter_runtime_params``.
+
+``rte_event_eth_rx_adapter_create()`` or
+``rte_event_eth_rx_adapter_create_with_params()`` configures the adapter with
+default value for maximum packets processed per request to 128.
+``rte_event_eth_rx_adapter_runtime_params_set()`` function allows to reconfigure
+maximum number of packets processed by adapter per service request. This is
+alternative to configuring the maximum packets processed per request by adapter
+by using ``rte_event_eth_rx_adapter_create_ext()`` with parameter
+``rte_event_eth_rx_adapter_conf::max_nb_rx``.
+
+``rte_event_eth_rx_adapter_runtime_parmas_get()`` function retrieves the configuration
+parameters that are defined in ``struct rte_event_eth_rx_adapter_runtime_params``.
+
Getting and resetting Adapter queue stats
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -35,6 +35,8 @@
#define MAX_VECTOR_NS 1E9
#define MIN_VECTOR_NS 1E5
+#define RXA_NB_RX_WORK_DEFAULT 128
+
#define ETH_RX_ADAPTER_SERVICE_NAME_LEN 32
#define ETH_RX_ADAPTER_MEM_NAME_LEN 32
@@ -1554,7 +1556,7 @@ rxa_default_conf_cb(uint8_t id, uint8_t dev_id,
}
conf->event_port_id = port_id;
- conf->max_nb_rx = 128;
+ conf->max_nb_rx = RXA_NB_RX_WORK_DEFAULT;
if (started)
ret = rte_event_dev_start(dev_id);
rx_adapter->default_cb_arg = 1;
@@ -3436,6 +3438,90 @@ rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
return -EINVAL;
}
+static int
+rxa_caps_check(struct event_eth_rx_adapter *rxa)
+{
+ uint16_t eth_dev_id;
+ uint32_t caps = 0;
+ int ret;
+
+ if (!rxa->nb_queues)
+ return -EINVAL;
+
+ /* The eth_dev used is always of same type.
+ * Hence eth_dev_id is taken from first entry of poll array.
+ */
+ eth_dev_id = rxa->eth_rx_poll[0].eth_dev_id;
+ ret = rte_event_eth_rx_adapter_caps_get(rxa->eventdev_id,
+ eth_dev_id,
+ &caps);
+ if (ret) {
+ RTE_EDEV_LOG_ERR("Failed to get adapter caps edev %" PRIu8
+ "eth port %" PRIu16, rxa->eventdev_id, eth_dev_id);
+ return ret;
+ }
+
+ if (caps & RTE_EVENT_ETH_RX_ADAPTER_CAP_INTERNAL_PORT)
+ return -ENOTSUP;
+
+ return 0;
+}
+
+int
+rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
+ struct rte_event_eth_rx_adapter_runtime_params *params)
+{
+ struct event_eth_rx_adapter *rxa;
+ int ret;
+
+ if (params == NULL)
+ return -EINVAL;
+
+ if (rxa_memzone_lookup())
+ return -ENOMEM;
+
+ rxa = rxa_id_to_adapter(id);
+ if (rxa == NULL)
+ return -EINVAL;
+
+ ret = rxa_caps_check(rxa);
+ if (ret)
+ return ret;
+
+ rte_spinlock_lock(&rxa->rx_lock);
+ rxa->max_nb_rx = params->max_nb_rx;
+ rte_spinlock_unlock(&rxa->rx_lock);
+
+ return 0;
+}
+
+int
+rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
+ struct rte_event_eth_rx_adapter_runtime_params *params)
+{
+ struct event_eth_rx_adapter *rxa;
+ int ret;
+
+ if (params == NULL)
+ return -EINVAL;
+
+ if (rxa_memzone_lookup())
+ return -ENOMEM;
+
+ rxa = rxa_id_to_adapter(id);
+ if (rxa == NULL)
+ return -EINVAL;
+
+ ret = rxa_caps_check(rxa);
+ if (ret)
+ return ret;
+
+ params->max_nb_rx = rxa->max_nb_rx;
+
+ return 0;
+}
+
+/* RX-adapter telemetry callbacks */
#define RXA_ADD_DICT(stats, s) rte_tel_data_add_dict_u64(d, #s, stats.s)
static int
@@ -39,10 +39,21 @@
* - rte_event_eth_rx_adapter_queue_stats_reset()
* - rte_event_eth_rx_adapter_event_port_get()
* - rte_event_eth_rx_adapter_instance_get()
+ * - rte_event_eth_rx_adapter_runtime_params_get()
+ * - rte_event_eth_rx_adapter_runtime_params_set()
*
* The application creates an ethernet to event adapter using
* rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
* or rte_event_eth_rx_adapter_create_with_params() functions.
+ *
+ * rte_event_eth_rx_adapter_create() or rte_event_eth_adapter_create_with_params()
+ * configures the adapter with default value of maximum packets processed per
+ * iteration to RXA_NB_RX_WORK_DEFAULT(128).
+ * rte_event_eth_rx_adapter_runtime_params_set() allows to re-configure maximum
+ * packets processed per iteration. This is alternative to using
+ * rte_event_eth_rx_adapter_create_ext() with parameter
+ * rte_event_eth_rx_adapter_conf::max_nb_rx
+ *
* The adapter needs to know which ethernet rx queues to poll for mbufs as well
* as event device parameters such as the event queue identifier, event
* priority and scheduling type that the adapter should use when constructing
@@ -299,6 +310,19 @@ struct rte_event_eth_rx_adapter_params {
/**< flag to indicate that event buffer is separate for each queue */
};
+/**
+ * Adapter configuration parameters
+ */
+struct rte_event_eth_rx_adapter_runtime_params {
+ uint32_t max_nb_rx;
+ /**< The adapter can return early if it has processed at least
+ * max_nb_rx mbufs. This isn't treated as a requirement; batching may
+ * cause the adapter to process more than max_nb_rx mbufs.
+ */
+ uint32_t rsvd[15];
+ /**< Reserved fields for future use */
+};
+
/**
*
* Callback function invoked by the SW adapter before it continues
@@ -377,7 +401,7 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
* Create a new ethernet Rx event adapter with the specified identifier.
* This function uses an internal configuration function that creates an event
* port. This default function reconfigures the event device with an
- * additional event port and setups up the event port using the port_config
+ * additional event port and setup the event port using the port_config
* parameter passed into this function. In case the application needs more
* control in configuration of the service, it should use the
* rte_event_eth_rx_adapter_create_ext() version.
@@ -743,6 +767,50 @@ rte_event_eth_rx_adapter_instance_get(uint16_t eth_dev_id,
uint16_t rx_queue_id,
uint8_t *rxa_inst_id);
+/**
+ * Set the adapter runtime configuration parameters
+ *
+ * This API is to be used after adding at least one queue to the adapter
+ * and is supported only for service based adapter.
+ *
+ * @param id
+ * Adapter identifier
+ *
+ * @param params
+ * A pointer to structure of type struct rte_event_eth_rx_adapter_runtime_params
+ * with configuration parameter values.
+ *
+ * @return
+ * - 0: Success
+ * - <0: Error code on failure
+ */
+__rte_experimental
+int
+rte_event_eth_rx_adapter_runtime_params_set(uint8_t id,
+ struct rte_event_eth_rx_adapter_runtime_params *params);
+
+/**
+ * Get the adapter runtime configuration parameters
+ *
+ * This API is to be used after adding at least one queue to the adapter
+ * and is supported only for service based adapter.
+ *
+ * @param id
+ * Adapter identifier
+ *
+ * @param[out] params
+ * A pointer to structure of type struct rte_event_eth_rx_adapter_runtime_params
+ * containing valid adapter parameters when return value is 0
+ *
+ * @return
+ * - 0: Success
+ * - <0: Error code on failure
+ */
+__rte_experimental
+int
+rte_event_eth_rx_adapter_runtime_params_get(uint8_t id,
+ struct rte_event_eth_rx_adapter_runtime_params *params);
+
#ifdef __cplusplus
}
#endif
@@ -121,6 +121,8 @@ EXPERIMENTAL {
rte_event_eth_tx_adapter_queue_stop;
# added in 23.03
+ rte_event_eth_rx_adapter_runtime_params_get;
+ rte_event_eth_rx_adapter_runtime_params_set;
rte_event_timer_remaining_ticks_get;
};