diff mbox series

eventdev: configure the Rx event buffer size

Message ID 20210716170236.2297967-1-ganapati.kundapura@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series eventdev: configure the Rx event buffer size |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot success github build: passed
ci/iol-abi-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance fail Performance Testing issues

Commit Message

Ganapati Kundapura July 16, 2021, 5:02 p.m. UTC 
  As of now Rx event buffer size is static and set to 128.

This patch sets the Rx event buffer size to 192, configurable
at compile time and also errors out at run time if Rx event
buffer size is configured more than 16 bits.

Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
---
 config/rte_config.h                     |  1 +
 lib/eventdev/rte_event_eth_rx_adapter.c | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Jerin Jacob July 19, 2021, 6:43 a.m. UTC | #1 
On Fri, Jul 16, 2021 at 10:33 PM Ganapati Kundapura
<ganapati.kundapura@intel.com> wrote:
>
> As of now Rx event buffer size is static and set to 128.
>
> This patch sets the Rx event buffer size to 192, configurable
> at compile time and also errors out at run time if Rx event
> buffer size is configured more than 16 bits.
>
> Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> ---
>  config/rte_config.h                     |  1 +
>  lib/eventdev/rte_event_eth_rx_adapter.c | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/config/rte_config.h b/config/rte_config.h
> index 590903c..3d938c8 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -77,6 +77,7 @@
>  #define RTE_EVENT_ETH_INTR_RING_SIZE 1024
>  #define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
>  #define RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32
> +#define RTE_EVENT_ETH_RX_ADAPTER_BUFFER_SIZE 128

We are limiting any configuration to rte_config.h file.
Could you make it dynamic with the default value and application can
pass the value
kind of scheme?






>
>  /* rawdev defines */
>  #define RTE_RAWDEV_MAX_DEVS 64
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index 13dfb28..0fe265e 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -25,7 +25,12 @@
>
>  #define BATCH_SIZE             32
>  #define BLOCK_CNT_THRESHOLD    10
> -#define ETH_EVENT_BUFFER_SIZE  (4*BATCH_SIZE)
> +
> +#define ETH_EVENT_BUFFER_SIZE \
> +               (RTE_EVENT_ETH_RX_ADAPTER_BUFFER_SIZE + BATCH_SIZE + BATCH_SIZE)
> +
> +#define MAX_ETH_EVENT_BUFFER_SIZE (USHRT_MAX - BATCH_SIZE - BATCH_SIZE)
> +
>  #define MAX_VECTOR_SIZE                1024
>  #define MIN_VECTOR_SIZE                4
>  #define MAX_VECTOR_NS          1E9
> @@ -2165,6 +2170,13 @@ rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
>                 return -EEXIST;
>         }
>
> +       if (RTE_DIM(rx_adapter->event_enqueue_buffer.events) > USHRT_MAX) {
> +               RTE_EDEV_LOG_ERR("CONFIG_RTE_ADPTR_ETH_EVENT_BUFFER_SIZE is "
> +                                "greater than max allowed value %u",
> +                                MAX_ETH_EVENT_BUFFER_SIZE);
> +               return -EINVAL;
> +       }
> +
>         socket_id = rte_event_dev_socket_id(dev_id);
>         snprintf(mem_name, ETH_RX_ADAPTER_MEM_NAME_LEN,
>                 "rte_event_eth_rx_adapter_%d",
> --
> 2.6.4
>
Ganapati Kundapura July 19, 2021, 3:26 p.m. UTC | #2 
Hi Jerin,
   Please find my response in lined.

-----Original Message-----
From: Jerin Jacob <jerinjacobk@gmail.com> 
Sent: 19 July 2021 12:14
To: Kundapura, Ganapati <ganapati.kundapura@intel.com>
Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; dpdk-dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] eventdev: configure the Rx event buffer size

On Fri, Jul 16, 2021 at 10:33 PM Ganapati Kundapura <ganapati.kundapura@intel.com> wrote:
>
> As of now Rx event buffer size is static and set to 128.
>
> This patch sets the Rx event buffer size to 192, configurable at 
> compile time and also errors out at run time if Rx event buffer size 
> is configured more than 16 bits.
>
> Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> ---
>  config/rte_config.h                     |  1 +
>  lib/eventdev/rte_event_eth_rx_adapter.c | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/config/rte_config.h b/config/rte_config.h index 
> 590903c..3d938c8 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -77,6 +77,7 @@
>  #define RTE_EVENT_ETH_INTR_RING_SIZE 1024  #define 
> RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32  #define 
> RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32
> +#define RTE_EVENT_ETH_RX_ADAPTER_BUFFER_SIZE 128

We are limiting any configuration to rte_config.h file.
Could you make it dynamic with the default value and application can pass the value kind of scheme?
[Ganapati] 
Making the Rx event buffer size dynamic seems to be a good idea but in case of rx adapter,
either passing event buffer size to adapter create api requires api signature change which breaks ABI
or by adding event buffer size in port_config parameter which comes from eventdev 
to adapter create function is not scalable as user can also call create_ext() with its own callback 
and parameter to callback is void * which is interpreted by user space callback function.

I think one way to do the event buffer size dynamic is to add new api to set the event buffer size.
If called, it'll set the event buffer size to the value passed otherwise rx adapter instance create api will do with 
default value. 

Let me know your opinion on this.
Jerin Jacob July 19, 2021, 4:13 p.m. UTC | #3 
On Mon, Jul 19, 2021 at 8:57 PM Kundapura, Ganapati
<ganapati.kundapura@intel.com> wrote:
>
> Hi Jerin,

HI Ganapati

>    Please find my response in lined.
>
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: 19 July 2021 12:14
> To: Kundapura, Ganapati <ganapati.kundapura@intel.com>
> Cc: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; dpdk-dev <dev@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH] eventdev: configure the Rx event buffer size
>
> On Fri, Jul 16, 2021 at 10:33 PM Ganapati Kundapura <ganapati.kundapura@intel.com> wrote:
> >
> > As of now Rx event buffer size is static and set to 128.
> >
> > This patch sets the Rx event buffer size to 192, configurable at
> > compile time and also errors out at run time if Rx event buffer size
> > is configured more than 16 bits.
> >
> > Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> > ---
> >  config/rte_config.h                     |  1 +
> >  lib/eventdev/rte_event_eth_rx_adapter.c | 14 +++++++++++++-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/config/rte_config.h b/config/rte_config.h index
> > 590903c..3d938c8 100644
> > --- a/config/rte_config.h
> > +++ b/config/rte_config.h
> > @@ -77,6 +77,7 @@
> >  #define RTE_EVENT_ETH_INTR_RING_SIZE 1024  #define
> > RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32  #define
> > RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32
> > +#define RTE_EVENT_ETH_RX_ADAPTER_BUFFER_SIZE 128
>
> We are limiting any configuration to rte_config.h file.
> Could you make it dynamic with the default value and application can pass the value kind of scheme?
> [Ganapati]
> Making the Rx event buffer size dynamic seems to be a good idea but in case of rx adapter,
> either passing event buffer size to adapter create api requires api signature change which breaks ABI
> or by adding event buffer size in port_config parameter which comes from eventdev
> to adapter create function is not scalable as user can also call create_ext() with its own callback
> and parameter to callback is void * which is interpreted by user space callback function.
>
> I think one way to do the event buffer size dynamic is to add new api to set the event buffer size.
> If called, it'll set the event buffer size to the value passed otherwise rx adapter instance create api will do with
> default value.
>
> Let me know your opinion on this.

we can break the ABI in v21.11 so create API config structure can change.
Please send depreciation notice and submit the implementation for 21.11,

>
diff mbox series

Patch

diff --git a/config/rte_config.h b/config/rte_config.h
index 590903c..3d938c8 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -77,6 +77,7 @@ 
 #define RTE_EVENT_ETH_INTR_RING_SIZE 1024
 #define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
 #define RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32
+#define RTE_EVENT_ETH_RX_ADAPTER_BUFFER_SIZE 128
 
 /* rawdev defines */
 #define RTE_RAWDEV_MAX_DEVS 64
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 13dfb28..0fe265e 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -25,7 +25,12 @@ 
 
 #define BATCH_SIZE		32
 #define BLOCK_CNT_THRESHOLD	10
-#define ETH_EVENT_BUFFER_SIZE	(4*BATCH_SIZE)
+
+#define ETH_EVENT_BUFFER_SIZE \
+		(RTE_EVENT_ETH_RX_ADAPTER_BUFFER_SIZE + BATCH_SIZE + BATCH_SIZE)
+
+#define MAX_ETH_EVENT_BUFFER_SIZE (USHRT_MAX - BATCH_SIZE - BATCH_SIZE)
+
 #define MAX_VECTOR_SIZE		1024
 #define MIN_VECTOR_SIZE		4
 #define MAX_VECTOR_NS		1E9
@@ -2165,6 +2170,13 @@  rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
 		return -EEXIST;
 	}
 
+	if (RTE_DIM(rx_adapter->event_enqueue_buffer.events) > USHRT_MAX) {
+		RTE_EDEV_LOG_ERR("CONFIG_RTE_ADPTR_ETH_EVENT_BUFFER_SIZE is "
+				 "greater than max allowed value %u",
+				 MAX_ETH_EVENT_BUFFER_SIZE);
+		return -EINVAL;
+	}
+
 	socket_id = rte_event_dev_socket_id(dev_id);
 	snprintf(mem_name, ETH_RX_ADAPTER_MEM_NAME_LEN,
 		"rte_event_eth_rx_adapter_%d",