[v6,4/4] eventdev/timer: change eventdev reconfig logic

Message ID 20230104064145.2600261-4-s.v.naga.harish.k@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series [v6,1/4] eventdev/eth_rx: change eventdev reconfig logic |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Naga Harish K, S V Jan. 4, 2023, 6:41 a.m. UTC
  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``.

With this change the application is no longer need to configure the
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(-)
  

Comments

Jerin Jacob Jan. 12, 2023, 7:06 a.m. UTC | #1
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
>
  
Naga Harish K, S V Jan. 12, 2023, 4:32 p.m. UTC | #2
Hi Jerin,

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, January 12, 2023 12:36 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 v6 4/4] eventdev/timer: change eventdev reconfig logic
> 
> 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).
> 

If the existing application is using rte_event_ <>_adapter_create() for creating
service based adapter instance and requires event_port with ``RTE_EVENT_PORT_CFG_SINGLE_LINK``
configuration, may fail at runtime.

Updated the doc/guides/rel_notes/release_23_03.rst with this info in V7 patchset.


> > 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
> >
  

Patch

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.
  *