Re: [PATCH v6 4/4] eventdev/timer: change eventdev reconfig logic

[Jerin Jacob <jerinjacobk@gmail.com>]

On Wed, Jan 4, 2023 at 12:12 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> When rte_event_timer_adapter_create() is used for creating adapter
> instance, eventdev is reconfigured with additional
> ``rte_event_dev_config::nb_event_ports`` parameter.
>
> This eventdev reconfig logic is enhanced to increment the
> ``rte_event_dev_config::nb_single_link_event_port_queues``
> parameter if the adapter event port config is of type
> ``RTE_EVENT_PORT_CFG_SINGLE_LINK``.

In general, the change is OK. Some comments,


> With this change the application is no longer need to configure the

What happens to existing application? Will it
a) Fail at runtime
b) Fail at compile time
c) Need to change application code to make existing application working
d) Change the application code to get this enhancement

This is to understand what need to be updated in
doc/guides/rel_notes/release_23_03.rst

If it is (d), Please update  doc/guides/rel_notes/release_23_03.rst to
make sure end user know this enhancement is added.
If not (d), it is kind of application breaking scenario and make it as (d).

> eventdev with ``rte_event_dev_config::nb_single_link_event_port_queues``


> parameter required for timer adapter when the adapter is created
> using above mentioned api.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> Acked-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> ---
> v2:
> * fix build error in documentation
> v3:
> * update doxygen
> v4:
> * fix programmer guide
> v5:
> * update doxygen as per review comments
> v6:
> * fix adapter cretae logic with correct event port id
> ---
> ---
>  doc/guides/prog_guide/event_timer_adapter.rst | 18 +++++++++++++++
>  lib/eventdev/rte_event_timer_adapter.c        | 23 +++++++++++--------
>  lib/eventdev/rte_event_timer_adapter.h        | 14 +++++++++++
>  3 files changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/doc/guides/prog_guide/event_timer_adapter.rst b/doc/guides/prog_guide/event_timer_adapter.rst
> index d7307a29bb..b5cd95fef1 100644
> --- a/doc/guides/prog_guide/event_timer_adapter.rst
> +++ b/doc/guides/prog_guide/event_timer_adapter.rst
> @@ -139,6 +139,24 @@ This function is passed a callback function that will be invoked if the
>  adapter needs to create an event port, giving the application the opportunity
>  to control how it is done.
>
> +Event device configuration for service based adapter
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +When rte_event_timer_adapter_create() is used for creating
> +adapter instance, ``rte_event_dev_config::nb_event_ports`` is
> +automatically incremented, and the event device is reconfigured with
> +additional event port during service initialization.
> +This event device reconfigure logic also increments the
> +``rte_event_dev_config::nb_single_link_event_port_queues``
> +parameter if the adapter event port config is of type
> +``RTE_EVENT_PORT_CFG_SINGLE_LINK``.
> +
> +Application no longer needs to account for the
> +``rte_event_dev_config::nb_event_ports`` and
> +``rte_event_dev_config::nb_single_link_event_port_queues``
> +parameters required for timer adapter in event device configuration,
> +when the adapter is created using the above-mentioned API.
> +
>  Adapter modes
>  ^^^^^^^^^^^^^
>  An event timer adapter can be configured in either periodic or non-periodic mode
> diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
> index a0f14bf861..66554f13fc 100644
> --- a/lib/eventdev/rte_event_timer_adapter.c
> +++ b/lib/eventdev/rte_event_timer_adapter.c
> @@ -88,7 +88,20 @@ default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
>                 rte_event_dev_stop(dev_id);
>
>         port_id = dev_conf.nb_event_ports;
> +       if (conf_arg != NULL)
> +               port_conf = conf_arg;
> +       else {
> +               port_conf = &def_port_conf;
> +               ret = rte_event_port_default_conf_get(dev_id, (port_id - 1),
> +                                                     port_conf);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +
>         dev_conf.nb_event_ports += 1;
> +       if (port_conf->event_port_cfg & RTE_EVENT_PORT_CFG_SINGLE_LINK)
> +               dev_conf.nb_single_link_event_port_queues += 1;
> +
>         ret = rte_event_dev_configure(dev_id, &dev_conf);
>         if (ret < 0) {
>                 EVTIM_LOG_ERR("failed to configure event dev %u\n", dev_id);
> @@ -99,16 +112,6 @@ default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t *event_port_id,
>                 return ret;
>         }
>
> -       if (conf_arg != NULL)
> -               port_conf = conf_arg;
> -       else {
> -               port_conf = &def_port_conf;
> -               ret = rte_event_port_default_conf_get(dev_id, port_id,
> -                                                     port_conf);
> -               if (ret < 0)
> -                       return ret;
> -       }
> -
>         ret = rte_event_port_setup(dev_id, port_id, port_conf);
>         if (ret < 0) {
>                 EVTIM_LOG_ERR("failed to setup event port %u on event dev %u\n",
> diff --git a/lib/eventdev/rte_event_timer_adapter.h b/lib/eventdev/rte_event_timer_adapter.h
> index cd10db19e4..e21588bede 100644
> --- a/lib/eventdev/rte_event_timer_adapter.h
> +++ b/lib/eventdev/rte_event_timer_adapter.h
> @@ -212,6 +212,20 @@ typedef int (*rte_event_timer_adapter_port_conf_cb_t)(uint16_t id,
>   *
>   * This function must be invoked first before any other function in the API.
>   *
> + * When this API is used for creating adapter instance,
> + * ``rte_event_dev_config::nb_event_ports`` is automatically incremented,
> + * and the event device is reconfigured with additional event port during
> + * service initialization. This event device reconfigure logic also increments
> + * the ``rte_event_dev_config::nb_single_link_event_port_queues``
> + * parameter if the adapter event port config is of type
> + * ``RTE_EVENT_PORT_CFG_SINGLE_LINK``.
> + *
> + * Application no longer needs to account for
> + * ``rte_event_dev_config::nb_event_ports`` and
> + * ``rte_event_dev_config::nb_single_link_event_port_queues``
> + * parameters required for Timer adapter in event device configuration
> + * when the adapter is created with this API.
> + *
>   * @param conf
>   *   The event timer adapter configuration structure.
>   *
> --
> 2.25.1
>