diff mbox series

[v1,1/5] eventdev: rx_adapter: add support to configure event buffer size

Message ID 20210918131140.3543317-1-s.v.naga.harish.k@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series [v1,1/5] eventdev: rx_adapter: add support to configure event buffer size |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure

Commit Message

Naga Harish K, S V Sept. 18, 2021, 1:11 p.m. UTC 
  Currently Rx event buffer is static array
with a default size of 192(6*BATCH_SIZE).

``rte_event_eth_rx_adapter_create2`` api is added which takes
``struct rte_event_eth_rx_adapter_params`` to configure event
buffer size in addition other params . The event buffer is
allocated dynamically at run time aligned to BATCH_SIZE + 2*BATCH_SIZE.

Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
---
 .../prog_guide/event_ethernet_rx_adapter.rst  |  7 ++
 lib/eventdev/rte_event_eth_rx_adapter.c       | 87 +++++++++++++++++--
 lib/eventdev/rte_event_eth_rx_adapter.h       | 45 +++++++++-
 lib/eventdev/version.map                      |  2 +
 4 files changed, 133 insertions(+), 8 deletions(-)

Comments

Jerin Jacob Sept. 20, 2021, 6:20 a.m. UTC | #1 
On Sat, Sep 18, 2021 at 6:41 PM Naga Harish K S V
<s.v.naga.harish.k@intel.com> wrote:
>
> Currently Rx event buffer is static array
> with a default size of 192(6*BATCH_SIZE).
>
> ``rte_event_eth_rx_adapter_create2`` api is added which takes
> ``struct rte_event_eth_rx_adapter_params`` to configure event
> buffer size in addition other params . The event buffer is
> allocated dynamically at run time aligned to BATCH_SIZE + 2*BATCH_SIZE.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> ---
>
> +/**
> + * A structure to hold adapter config params
> + */
> +struct rte_event_eth_rx_adapter_params {
> +       uint16_t event_buf_size;
> +       /**< size of event buffer for the adapter */

See below.

> +};
> +
>  /**
>   *
>   * Callback function invoked by the SW adapter before it continues
> @@ -330,6 +339,40 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
>                                 rte_event_eth_rx_adapter_conf_cb conf_cb,
>                                 void *conf_arg);
>
> +/**
> + * Create a new ethernet Rx event adapter with the specified identifier.
> + * This function allocates Rx adapter event buffer with the size specified
> + * in rxa_params aligned to BATCH_SIZE plus (BATCH_SIZE+BATCH_SIZE) and
> + * uses an internal configuration function that creates an event port.

This function may use for adding another
rte_event_eth_rx_adapter_params:: value.
So semantics of rte_event_eth_rx_adapter_params::event_buf_size you
can document at
in that structure.  This function,  you can tell it adapter creation
varint with parameters or so
See below.

> + * This default function reconfigures the event device with an
> + * additional event port and setups up 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.
> + *
> + * @param id
> + *  The identifier of the ethernet Rx event adapter.
> + *
> + * @param dev_id
> + *  The identifier of the event device to configure.
> + *
> + * @param rxa_params
> + *  Pointer to struct rte_event_eth_rx_adapter_params containing
> + *  size to allocate rx event buffer.

Value NULL is allowed to represent the default values or so.

> + *
> + * @param port_config
> + *  Argument of type *rte_event_port_conf* that is passed to the conf_cb
> + *  function.
> + *
> + * @return
> + *   - 0: Success
> + *   - <0: Error code on failure
> + */
> +__rte_experimental
> +int rte_event_eth_rx_adapter_create2(uint8_t id, uint8_t dev_id,
> +                       struct rte_event_eth_rx_adapter_params *rxa_params,
> +                       struct rte_event_port_conf *port_config);

Couple of suggestion on API name and prototype:
- I think, we can remove 2 version and give more meaningful,name like
rte_event_eth_rx_adapter_create_with_param() or so

- Keep new parameter as last to have better compatibility i.e
rte_event_eth_rx_adapter_create_with_param(uint8_t id, uint8_t dev_id,
struct rte_event_port_conf *port_config, struct
rte_event_eth_rx_adapter_params *rxa_params)
Naga Harish K, S V Sept. 21, 2021, 1:45 p.m. UTC | #2 
Hi Jerin,
  Please see the replies inline.

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Monday, September 20, 2021 11:50 AM
> To: Naga Harish K, S V <s.v.naga.harish.k@intel.com>
> Cc: Jerin Jacob <jerinj@marvell.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>; dpdk-dev <dev@dpdk.org>; Kundapura,
> Ganapati <ganapati.kundapura@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1 1/5] eventdev: rx_adapter: add support
> to configure event buffer size
> 
> On Sat, Sep 18, 2021 at 6:41 PM Naga Harish K S V
> <s.v.naga.harish.k@intel.com> wrote:
> >
> > Currently Rx event buffer is static array with a default size of
> > 192(6*BATCH_SIZE).
> >
> > ``rte_event_eth_rx_adapter_create2`` api is added which takes ``struct
> > rte_event_eth_rx_adapter_params`` to configure event buffer size in
> > addition other params . The event buffer is allocated dynamically at
> > run time aligned to BATCH_SIZE + 2*BATCH_SIZE.
> >
> > Signed-off-by: Naga Harish K S V <s.v.naga.harish.k@intel.com>
> > Signed-off-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> > ---
> >
> > +/**
> > + * A structure to hold adapter config params  */ struct
> > +rte_event_eth_rx_adapter_params {
> > +       uint16_t event_buf_size;
> > +       /**< size of event buffer for the adapter */
> 
> See below.
> 
> > +};
> > +
> >  /**
> >   *
> >   * Callback function invoked by the SW adapter before it continues @@
> > -330,6 +339,40 @@ int rte_event_eth_rx_adapter_create_ext(uint8_t id,
> uint8_t dev_id,
> >                                 rte_event_eth_rx_adapter_conf_cb conf_cb,
> >                                 void *conf_arg);
> >
> > +/**
> > + * Create a new ethernet Rx event adapter with the specified identifier.
> > + * This function allocates Rx adapter event buffer with the size
> > +specified
> > + * in rxa_params aligned to BATCH_SIZE plus (BATCH_SIZE+BATCH_SIZE)
> > +and
> > + * uses an internal configuration function that creates an event port.
> 
> This function may use for adding another
> rte_event_eth_rx_adapter_params:: value.
> So semantics of rte_event_eth_rx_adapter_params::event_buf_size you
> can document at in that structure.  This function,  you can tell it adapter
> creation varint with parameters or so See below.
> 

The documentation is updated as as per the review comments.

> > + * This default function reconfigures the event device with an
> > + * additional event port and setups up 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.
> > + *
> > + * @param id
> > + *  The identifier of the ethernet Rx event adapter.
> > + *
> > + * @param dev_id
> > + *  The identifier of the event device to configure.
> > + *
> > + * @param rxa_params
> > + *  Pointer to struct rte_event_eth_rx_adapter_params containing
> > + *  size to allocate rx event buffer.
> 
> Value NULL is allowed to represent the default values or so.
> 

The api is updated to treat NULL pointer for adapter params with default values.

> > + *
> > + * @param port_config
> > + *  Argument of type *rte_event_port_conf* that is passed to the
> > +conf_cb
> > + *  function.
> > + *
> > + * @return
> > + *   - 0: Success
> > + *   - <0: Error code on failure
> > + */
> > +__rte_experimental
> > +int rte_event_eth_rx_adapter_create2(uint8_t id, uint8_t dev_id,
> > +                       struct rte_event_eth_rx_adapter_params *rxa_params,
> > +                       struct rte_event_port_conf *port_config);
> 
> Couple of suggestion on API name and prototype:
> - I think, we can remove 2 version and give more meaningful,name like
> rte_event_eth_rx_adapter_create_with_param() or so
> 
> - Keep new parameter as last to have better compatibility i.e
> rte_event_eth_rx_adapter_create_with_param(uint8_t id, uint8_t dev_id,
> struct rte_event_port_conf *port_config, struct
> rte_event_eth_rx_adapter_params *rxa_params)

The function name and parameters are adjusted as suggested.

Regards
Harish
diff mbox series

Patch

diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
index 0780b6f711..cbf694c66b 100644
--- a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
+++ b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
@@ -62,6 +62,13 @@  service function and needs to create an event port for it. The callback is
 expected to fill the ``struct rte_event_eth_rx_adapter_conf structure``
 passed to it.
 
+If the application desires to control the event buffer size, it can use the
+``rte_event_eth_rx_adapter_create2()`` api. The event buffer size is
+specified using ``struct rte_event_eth_rx_adapter_params::event_buf_size``.
+The function is passed the event device to be associated with the adapter
+and port configuration for the adapter to setup an event port if the
+adapter needs to use a service function.
+
 Adding Rx Queues to the Adapter Instance
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index f2dc69503d..f567a83223 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -82,7 +82,9 @@  struct rte_eth_event_enqueue_buffer {
 	/* Count of events in this buffer */
 	uint16_t count;
 	/* Array of events in this buffer */
-	struct rte_event events[ETH_EVENT_BUFFER_SIZE];
+	struct rte_event *events;
+	/* size of event buffer */
+	uint16_t events_size;
 	/* Event enqueue happens from head */
 	uint16_t head;
 	/* New packets from rte_eth_rx_burst is enqued from tail */
@@ -919,7 +921,7 @@  rxa_buffer_mbufs(struct rte_event_eth_rx_adapter *rx_adapter,
 		dropped = 0;
 		nb_cb = dev_info->cb_fn(eth_dev_id, rx_queue_id,
 				       buf->last |
-				       (RTE_DIM(buf->events) & ~buf->last_mask),
+				       (buf->events_size & ~buf->last_mask),
 				       buf->count >= BATCH_SIZE ?
 						buf->count - BATCH_SIZE : 0,
 				       &buf->events[buf->tail],
@@ -945,7 +947,7 @@  rxa_pkt_buf_available(struct rte_eth_event_enqueue_buffer *buf)
 	uint32_t nb_req = buf->tail + BATCH_SIZE;
 
 	if (!buf->last) {
-		if (nb_req <= RTE_DIM(buf->events))
+		if (nb_req <= buf->events_size)
 			return true;
 
 		if (buf->head >= BATCH_SIZE) {
@@ -2164,12 +2166,15 @@  rxa_ctrl(uint8_t id, int start)
 	return 0;
 }
 
-int
-rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
-				rte_event_eth_rx_adapter_conf_cb conf_cb,
-				void *conf_arg)
+static int
+rxa_create(uint8_t id, uint8_t dev_id,
+	   struct rte_event_eth_rx_adapter_params *rxa_params,
+	   rte_event_eth_rx_adapter_conf_cb conf_cb,
+	   void *conf_arg)
 {
 	struct rte_event_eth_rx_adapter *rx_adapter;
+	struct rte_eth_event_enqueue_buffer *buf;
+	struct rte_event *events;
 	int ret;
 	int socket_id;
 	uint16_t i;
@@ -2184,6 +2189,7 @@  rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
 
 	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
 	RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
+
 	if (conf_cb == NULL)
 		return -EINVAL;
 
@@ -2231,11 +2237,30 @@  rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
 		rte_free(rx_adapter);
 		return -ENOMEM;
 	}
+
 	rte_spinlock_init(&rx_adapter->rx_lock);
+
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++)
 		rx_adapter->eth_devices[i].dev = &rte_eth_devices[i];
 
+	/* Rx adapter event buffer allocation */
+	buf = &rx_adapter->event_enqueue_buffer;
+	buf->events_size = RTE_ALIGN(rxa_params->event_buf_size, BATCH_SIZE);
+
+	events = rte_zmalloc_socket(rx_adapter->mem_name,
+				    buf->events_size * sizeof(*events),
+				    0, socket_id);
+	if (events == NULL) {
+		RTE_EDEV_LOG_ERR("Failed to allocate mem for event buffer\n");
+		rte_free(rx_adapter->eth_devices);
+		rte_free(rx_adapter);
+		return -ENOMEM;
+	}
+
+	rx_adapter->event_enqueue_buffer.events = events;
+
 	event_eth_rx_adapter[id] = rx_adapter;
+
 	if (conf_cb == rxa_default_conf_cb)
 		rx_adapter->default_cb_arg = 1;
 	rte_eventdev_trace_eth_rx_adapter_create(id, dev_id, conf_cb,
@@ -2243,6 +2268,50 @@  rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
 	return 0;
 }
 
+int
+rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
+				rte_event_eth_rx_adapter_conf_cb conf_cb,
+				void *conf_arg)
+{
+	struct rte_event_eth_rx_adapter_params rxa_params;
+
+	/* Event buffer with default size = 6*BATCH_SIZE */
+	rxa_params.event_buf_size = ETH_EVENT_BUFFER_SIZE;
+	return rxa_create(id, dev_id, &rxa_params, conf_cb, conf_arg);
+}
+
+int
+rte_event_eth_rx_adapter_create2(uint8_t id, uint8_t dev_id,
+			struct rte_event_eth_rx_adapter_params *rxa_params,
+			struct rte_event_port_conf *port_config)
+{
+	struct rte_event_port_conf *pc;
+	int ret;
+
+	if (port_config == NULL)
+		return -EINVAL;
+
+	if (rxa_params == NULL || rxa_params->event_buf_size == 0)
+		return -EINVAL;
+
+	pc = rte_malloc(NULL, sizeof(*pc), 0);
+	if (pc == NULL)
+		return -ENOMEM;
+
+	*pc = *port_config;
+
+	/* event buff size aligned to BATCH_SIZE + 2*BATCH_SIZE */
+	rxa_params->event_buf_size = RTE_ALIGN(rxa_params->event_buf_size,
+					       BATCH_SIZE);
+	rxa_params->event_buf_size += BATCH_SIZE + BATCH_SIZE;
+
+	ret = rxa_create(id, dev_id, rxa_params, rxa_default_conf_cb, pc);
+	if (ret)
+		rte_free(pc);
+
+	return ret;
+}
+
 int
 rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
 		struct rte_event_port_conf *port_config)
@@ -2252,12 +2321,14 @@  rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
 
 	if (port_config == NULL)
 		return -EINVAL;
+
 	RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, -EINVAL);
 
 	pc = rte_malloc(NULL, sizeof(*pc), 0);
 	if (pc == NULL)
 		return -ENOMEM;
 	*pc = *port_config;
+
 	ret = rte_event_eth_rx_adapter_create_ext(id, dev_id,
 					rxa_default_conf_cb,
 					pc);
@@ -2286,6 +2357,7 @@  rte_event_eth_rx_adapter_free(uint8_t id)
 	if (rx_adapter->default_cb_arg)
 		rte_free(rx_adapter->conf_arg);
 	rte_free(rx_adapter->eth_devices);
+	rte_free(rx_adapter->event_enqueue_buffer.events);
 	rte_free(rx_adapter);
 	event_eth_rx_adapter[id] = NULL;
 
@@ -2658,6 +2730,7 @@  rte_event_eth_rx_adapter_stats_get(uint8_t id,
 
 	stats->rx_packets += dev_stats_sum.rx_packets;
 	stats->rx_enq_count += dev_stats_sum.rx_enq_count;
+
 	return 0;
 }
 
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.h b/lib/eventdev/rte_event_eth_rx_adapter.h
index 3f8b362295..a1b5e0ed37 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.h
+++ b/lib/eventdev/rte_event_eth_rx_adapter.h
@@ -26,6 +26,7 @@ 
  * The ethernet Rx event adapter's functions are:
  *  - rte_event_eth_rx_adapter_create_ext()
  *  - rte_event_eth_rx_adapter_create()
+ *  - rte_event_eth_rx_adapter_create2()
  *  - rte_event_eth_rx_adapter_free()
  *  - rte_event_eth_rx_adapter_queue_add()
  *  - rte_event_eth_rx_adapter_queue_del()
@@ -36,7 +37,7 @@ 
  *
  * The application creates an ethernet to event adapter using
  * rte_event_eth_rx_adapter_create_ext() or rte_event_eth_rx_adapter_create()
- * functions.
+ * or rte_event_eth_rx_adapter_create2() functions.
  * 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
@@ -256,6 +257,14 @@  struct rte_event_eth_rx_adapter_vector_limits {
 	 */
 };
 
+/**
+ * A structure to hold adapter config params
+ */
+struct rte_event_eth_rx_adapter_params {
+	uint16_t event_buf_size;
+	/**< size of event buffer for the adapter */
+};
+
 /**
  *
  * Callback function invoked by the SW adapter before it continues
@@ -330,6 +339,40 @@  int rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
 				rte_event_eth_rx_adapter_conf_cb conf_cb,
 				void *conf_arg);
 
+/**
+ * Create a new ethernet Rx event adapter with the specified identifier.
+ * This function allocates Rx adapter event buffer with the size specified
+ * in rxa_params aligned to BATCH_SIZE plus (BATCH_SIZE+BATCH_SIZE) and
+ * 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
+ * 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.
+ *
+ * @param id
+ *  The identifier of the ethernet Rx event adapter.
+ *
+ * @param dev_id
+ *  The identifier of the event device to configure.
+ *
+ * @param rxa_params
+ *  Pointer to struct rte_event_eth_rx_adapter_params containing
+ *  size to allocate rx event buffer.
+ *
+ * @param port_config
+ *  Argument of type *rte_event_port_conf* that is passed to the conf_cb
+ *  function.
+ *
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure
+ */
+__rte_experimental
+int rte_event_eth_rx_adapter_create2(uint8_t id, uint8_t dev_id,
+			struct rte_event_eth_rx_adapter_params *rxa_params,
+			struct rte_event_port_conf *port_config);
+
 /**
  * Create a new ethernet Rx event adapter with the specified identifier.
  * This function uses an internal configuration function that creates an event
diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
index cd86d2d908..868d352eb3 100644
--- a/lib/eventdev/version.map
+++ b/lib/eventdev/version.map
@@ -138,6 +138,8 @@  EXPERIMENTAL {
 	__rte_eventdev_trace_port_setup;
 	# added in 20.11
 	rte_event_pmd_pci_probe_named;
+	# added in 21.11
+	rte_event_eth_rx_adapter_create2;
 
 	#added in 21.05
 	rte_event_vector_pool_create;