Thread (81 messages) 81 messages, 4 authors, 2021-10-07

Re: [dpdk-dev] [EXT] [PATCH v2 1/5] eventdev/rx_adapter: add support to configure event buffer size

From: Pavan Nikhilesh Bhagavatula <hidden>
Date: 2021-09-21 20:24:37

quoted hunk ↗ jump to hunk
Currently Rx event buffer is static array with a default size
of 192(6*BATCH_SIZE).

``rte_event_eth_rx_adapter_create_with_params`` 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 from heap after aligning the size to BATCH_SIZE and
adding 2*BATCH_SIZE. In case of NULL params argument, default event
buffer size is used.

Signed-off-by: Naga Harish K S V <redacted>
Signed-off-by: Ganapati Kundapura <redacted>

---
v2:
* Updated header file and rx adapter documentation as per review
comments.
* new api name is modified as
rte_event_eth_rx_adapter_create_with_params
 as per review comments.
* rxa_params pointer argument Value NULL is allowed to represent the
 default values

v1:
* Initial implementation with documentation and unit tests.
---
.../prog_guide/event_ethernet_rx_adapter.rst  |  7 ++
lib/eventdev/rte_event_eth_rx_adapter.c       | 94
+++++++++++++++++--
lib/eventdev/rte_event_eth_rx_adapter.h       | 40 +++++++-
lib/eventdev/version.map                      |  2 +
4 files changed, 135 insertions(+), 8 deletions(-)
diff --git a/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
b/doc/guides/prog_guide/event_ethernet_rx_adapter.rst
index 0780b6f711..dd753613bd 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_create_with_params()`` 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..df1653b497 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-
quoted
last_mask),
+				       (buf->events_size & ~buf-
quoted
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,57 @@
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 */
Why is it a multiple of 6, if its not documented please add a line here.
quoted hunk ↗ jump to hunk
+	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_create_with_params(uint8_t id, uint8_t
dev_id,
+			struct rte_event_port_conf *port_config,
+			struct rte_event_eth_rx_adapter_params
*rxa_params)
+{
+	struct rte_event_port_conf *pc;
+	int ret;
+	struct rte_event_eth_rx_adapter_params temp_params = {0};
+
+	if (port_config == NULL)
+		return -EINVAL;
+
+	/* use default values if rxa_parmas is NULL */
+	if (rxa_params == NULL) {
+		rxa_params = &temp_params;
+		rxa_params->event_buf_size =
ETH_EVENT_BUFFER_SIZE;
+	}
+
+	if (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-
quoted
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 +2328,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 +2364,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 +2737,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..a7881097b4 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_create_with_params()
 *  - 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_create_with_params() 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,16 @@ 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.
+	 * the size is aligned to BATCH_SIZE and added (2 * BATCH_SIZE)
+	 */
BATCH_SIZE is internal i.e. not exposed to application layer, can we please define
what is BATCH_SIZE here and why applications input needs to be aligned to BATCH_SIZE?
and why 2*BATCH size is required.

If this is an application driven parameter we shouldn't impose arbitrary constraints like this.
Maybe "Aligned to power of 2" would be sufficient.

OR

Extend rte_event_eth_rx_adapter_caps_get() to return optimal event_buf_size if supported.

quoted hunk ↗ jump to hunk
+};
+
/**
 *
 * Callback function invoked by the SW adapter before it continues
@@ -356,6 +367,33 @@ int
rte_event_eth_rx_adapter_create_ext(uint8_t id, uint8_t dev_id,
int rte_event_eth_rx_adapter_create(uint8_t id, uint8_t dev_id,
				struct rte_event_port_conf
*port_config);

+/**
+ * This is a variant of rte_event_eth_rx_adapter_create() with
additional
+ * adapter params specified in ``struct
rte_event_eth_rx_adapter_params``.
+ *
+ * @param id
+ *  The identifier of the ethernet Rx event adapter.
+ *
+ * @param dev_id
+ *  The identifier of the event device to configure.
+ *
+ * @param port_config
+ *  Argument of type *rte_event_port_conf* that is passed to the
conf_cb
+ *  function.
+ *
+ * @param rxa_params
+ *  Pointer to struct rte_event_eth_rx_adapter_params.
+ *  In case of NULL, default values are used.
+ *
+ * @return
+ *   - 0: Success
+ *   - <0: Error code on failure
+ */
+__rte_experimental
+int rte_event_eth_rx_adapter_create_with_params(uint8_t id,
uint8_t dev_id,
+			struct rte_event_port_conf *port_config,
+			struct rte_event_eth_rx_adapter_params
*rxa_params);
+
/**
 * Free an event adapter
 *
diff --git a/lib/eventdev/version.map b/lib/eventdev/version.map
index cd86d2d908..87586de879 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_create_with_params;

	#added in 21.05
	rte_event_vector_pool_create;
--
2.25.1
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help