[dpdk-dev] [PATCH v2 00/29] Update ENA driver to v2.1.0

[dpdk-dev] [PATCH v2 00/29] Update ENA driver to v2.1.0

[Michal Krawczyk]

Hi,

This patchset is including severeal changes, including:
* ena_com (HAL) update
* Support for upcoming device features
* Refactor of the IO path
* Documentation update about ENA usage on metal instances

v2:
* Fix comments from review

Igor Chauskin (2):
  net/ena/base: make allocation macros thread-safe
  net/ena/base: prevent allocation of 0-sized memory

Michal Krawczyk (27):
  net/ena: check if size of buffer is at least 1400B
  net/ena/base: set default hash key
  net/ena/base: rework interrupt moderation
  net/ena/base: remove extra properties strings
  net/ena/base: add accelerated LLQ mode
  net/ena/base: fix documentation of the functions
  net/ena/base: fix indentation in cq polling
  net/ena/base: add error logs when preparing Tx
  net/ena/base: use 48-bit memory addresses in ena_com
  net/ena/base: fix types for printing timestamps
  net/ena/base: fix indentation of multiple defines
  net/ena/base: update gen date and commit
  net/ena: set IO ring size to the valid value
  net/ena: refactor getting IO queues capabilities
  net/ena: add support for large LLQ headers
  net/ena: remove memory barriers before doorbells
  net/ena: add Tx drops statistic
  net/ena: disable meta caching
  net/ena: refactor Rx path
  net/ena: rework getting number of available descs
  net/ena: limit refill threshold by fixed value
  net/ena: use macros for ring idx operations
  net/ena: refactor Tx path
  net/ena: reuse 0 length Rx descriptor
  doc: add notes on ENA usage on metal instances
  net/ena: update copyright date
  net/ena: update version of the driver to v2.1.0

 doc/guides/nics/ena.rst                       |  56 +-
 drivers/net/ena/base/ena_com.c                | 362 +++----
 drivers/net/ena/base/ena_com.h                | 250 +----
 .../net/ena/base/ena_defs/ena_admin_defs.h    |  39 +-
 .../net/ena/base/ena_defs/ena_common_defs.h   |   6 +-
 drivers/net/ena/base/ena_defs/ena_gen_info.h  |   6 +-
 drivers/net/ena/base/ena_eth_com.c            |  73 +-
 drivers/net/ena/base/ena_plat_dpdk.h          |  48 +-
 drivers/net/ena/ena_ethdev.c                  | 902 +++++++++++-------
 drivers/net/ena/ena_ethdev.h                  |  48 +-
 10 files changed, 966 insertions(+), 824 deletions(-)

-- 
2.20.1

[dpdk-dev] [PATCH v2 01/29] net/ena: check if size of buffer is at least 1400B

[Michal Krawczyk]

Some of the ENA devices can't handle buffers which are smaller than a
1400B. Because of this limitation, size of the buffer is being checked
and limited during the Rx queue setup.

If it's below the allowed value, PMD won't finish it's configuration
successfully..

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
v2:
  * Remove debug printout
  * Change the way of acquiring mempool size

 drivers/net/ena/ena_ethdev.c | 10 ++++++++++
 drivers/net/ena/ena_ethdev.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 665afee4f0..292526466c 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1282,6 +1282,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 {
 	struct ena_adapter *adapter = dev->data->dev_private;
 	struct ena_ring *rxq = NULL;
+	size_t buffer_size;
 	int i;
 
 	rxq = &adapter->rx_ring[queue_idx];
@@ -1309,6 +1310,15 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+	/* ENA isn't supporting buffers smaller than 1400 bytes */
+	buffer_size = rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM;
+	if (buffer_size < ENA_RX_BUF_MIN_SIZE) {
+		PMD_DRV_LOG(ERR,
+			"Unsupported size of RX buffer: %zu (min size: %d)\n",
+			buffer_size, ENA_RX_BUF_MIN_SIZE);
+		return -EINVAL;
+	}
+
 	rxq->port_id = dev->data->port_id;
 	rxq->next_to_clean = 0;
 	rxq->next_to_use = 0;
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index af5eeea280..c1457defeb 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -20,6 +20,7 @@
 #define ENA_MIN_FRAME_LEN	64
 #define ENA_NAME_MAX_LEN	20
 #define ENA_PKT_MAX_BUFS	17
+#define ENA_RX_BUF_MIN_SIZE	1400
 
 #define ENA_MIN_MTU		128
 
-- 
2.20.1

[dpdk-dev] [PATCH v2 02/29] net/ena/base: make allocation macros thread-safe

[Michal Krawczyk]

From: Igor Chauskin <igorch@amazon.com>

Memory allocation region id could possibly be non-unique
due to non-atomic increment, causing allocation failure.

Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")
Cc: stable@dpdk.org

Signed-off-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 8 +++++---
 drivers/net/ena/ena_ethdev.c         | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index b611fb204b..192bbaefcf 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -180,7 +180,7 @@ do {                                                                   \
  * Each rte_memzone should have unique name.
  * To satisfy it, count number of allocations and add it to name.
  */
-extern uint32_t ena_alloc_cnt;
+extern rte_atomic32_t ena_alloc_cnt;
 
 #define ENA_MEM_ALLOC_COHERENT(dmadev, size, virt, phys, handle)	\
 	do {								\
@@ -188,7 +188,8 @@ extern uint32_t ena_alloc_cnt;
 		char z_name[RTE_MEMZONE_NAMESIZE];			\
 		ENA_TOUCH(dmadev); ENA_TOUCH(handle);			\
 		snprintf(z_name, sizeof(z_name),			\
-				"ena_alloc_%d", ena_alloc_cnt++);	\
+			 "ena_alloc_%d",				\
+			 rte_atomic32_add_return(&ena_alloc_cnt, 1));	\
 		mz = rte_memzone_reserve(z_name, size, SOCKET_ID_ANY,	\
 				RTE_MEMZONE_IOVA_CONTIG);		\
 		handle = mz;						\
@@ -213,7 +214,8 @@ extern uint32_t ena_alloc_cnt;
 		char z_name[RTE_MEMZONE_NAMESIZE];			\
 		ENA_TOUCH(dmadev); ENA_TOUCH(dev_node);			\
 		snprintf(z_name, sizeof(z_name),			\
-				"ena_alloc_%d", ena_alloc_cnt++);	\
+			 "ena_alloc_%d",				\
+			 rte_atomic32_add_return(&ena_alloc_cnt, 1));	\
 		mz = rte_memzone_reserve(z_name, size, node,		\
 				RTE_MEMZONE_IOVA_CONTIG);		\
 		mem_handle = mz;					\
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 292526466c..854c724e32 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -89,7 +89,7 @@ struct ena_stats {
  * Each rte_memzone should have unique name.
  * To satisfy it, count number of allocation and add it to name.
  */
-uint32_t ena_alloc_cnt;
+rte_atomic32_t ena_alloc_cnt;
 
 static const struct ena_stats ena_stats_global_strings[] = {
 	ENA_STAT_GLOBAL_ENTRY(wd_expired),
-- 
2.20.1

[dpdk-dev] [PATCH v2 03/29] net/ena/base: prevent allocation of 0-sized memory

[Michal Krawczyk]

From: Igor Chauskin <igorch@amazon.com>

rte_memzone_reserve() will reserve the biggest contiguous memzone
available if received 0 as size param.

Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")
Cc: stable@dpdk.org

Signed-off-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 29 ++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index 192bbaefcf..793ba8a957 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -184,15 +184,18 @@ extern rte_atomic32_t ena_alloc_cnt;
 
 #define ENA_MEM_ALLOC_COHERENT(dmadev, size, virt, phys, handle)	\
 	do {								\
-		const struct rte_memzone *mz;				\
-		char z_name[RTE_MEMZONE_NAMESIZE];			\
+		const struct rte_memzone *mz = NULL;			\
 		ENA_TOUCH(dmadev); ENA_TOUCH(handle);			\
-		snprintf(z_name, sizeof(z_name),			\
+		if (size > 0) {						\
+			char z_name[RTE_MEMZONE_NAMESIZE];		\
+			snprintf(z_name, sizeof(z_name),		\
 			 "ena_alloc_%d",				\
 			 rte_atomic32_add_return(&ena_alloc_cnt, 1));	\
-		mz = rte_memzone_reserve(z_name, size, SOCKET_ID_ANY,	\
-				RTE_MEMZONE_IOVA_CONTIG);		\
-		handle = mz;						\
+			mz = rte_memzone_reserve(z_name, size,		\
+					SOCKET_ID_ANY,			\
+					RTE_MEMZONE_IOVA_CONTIG);	\
+			handle = mz;					\
+		}							\
 		if (mz == NULL) {					\
 			virt = NULL;					\
 			phys = 0;					\
@@ -210,15 +213,17 @@ extern rte_atomic32_t ena_alloc_cnt;
 #define ENA_MEM_ALLOC_COHERENT_NODE(					\
 	dmadev, size, virt, phys, mem_handle, node, dev_node)		\
 	do {								\
-		const struct rte_memzone *mz;				\
-		char z_name[RTE_MEMZONE_NAMESIZE];			\
+		const struct rte_memzone *mz = NULL;			\
 		ENA_TOUCH(dmadev); ENA_TOUCH(dev_node);			\
-		snprintf(z_name, sizeof(z_name),			\
+		if (size > 0) {						\
+			char z_name[RTE_MEMZONE_NAMESIZE];		\
+			snprintf(z_name, sizeof(z_name),		\
 			 "ena_alloc_%d",				\
-			 rte_atomic32_add_return(&ena_alloc_cnt, 1));	\
-		mz = rte_memzone_reserve(z_name, size, node,		\
+			 rte_atomic32_add_return(&ena_alloc_cnt, 1));   \
+			mz = rte_memzone_reserve(z_name, size, node,	\
 				RTE_MEMZONE_IOVA_CONTIG);		\
-		mem_handle = mz;					\
+			mem_handle = mz;				\
+		}							\
 		if (mz == NULL) {					\
 			virt = NULL;					\
 			phys = 0;					\
-- 
2.20.1

[dpdk-dev] [PATCH v2 04/29] net/ena/base: set default hash key

[Michal Krawczyk]

The RSS hash key was present in the device, but it wasn't exposed to
the user. The other key still cannot be set, but now it can be accessed
if one needs to do that.

By default, the random hash key is used and it is generated only once
when requested for the first time.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
v2:
  * Remove variable declaration inside the for loop
  * Remove unlikely in condition check

 drivers/net/ena/base/ena_com.c       | 103 ++++++++++++++-------------
 drivers/net/ena/base/ena_com.h       |  30 ++++++--
 drivers/net/ena/base/ena_plat_dpdk.h |   6 ++
 drivers/net/ena/ena_ethdev.c         |  17 +++++
 4 files changed, 102 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 17b51b5a11..a5753997ed 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -1032,6 +1032,24 @@ static int ena_com_get_feature(struct ena_com_dev *ena_dev,
 				      feature_ver);
 }
 
+int ena_com_get_current_hash_function(struct ena_com_dev *ena_dev)
+{
+	return ena_dev->rss.hash_func;
+}
+
+static void ena_com_hash_key_fill_default_key(struct ena_com_dev *ena_dev)
+{
+	struct ena_admin_feature_rss_flow_hash_control *hash_key =
+		(ena_dev->rss).hash_key;
+
+	ENA_RSS_FILL_KEY(&hash_key->key, sizeof(hash_key->key));
+	/* The key is stored in the device in uint32_t array
+	 * as well as the API requires the key to be passed in this
+	 * format. Thus the size of our array should be divided by 4
+	 */
+	hash_key->keys_num = sizeof(hash_key->key) / sizeof(uint32_t);
+}
+
 static int ena_com_hash_key_allocate(struct ena_com_dev *ena_dev)
 {
 	struct ena_rss *rss = &ena_dev->rss;
@@ -1266,30 +1284,6 @@ static int ena_com_ind_tbl_convert_to_device(struct ena_com_dev *ena_dev)
 	return 0;
 }
 
-static int ena_com_ind_tbl_convert_from_device(struct ena_com_dev *ena_dev)
-{
-	u16 dev_idx_to_host_tbl[ENA_TOTAL_NUM_QUEUES] = { (u16)-1 };
-	struct ena_rss *rss = &ena_dev->rss;
-	u8 idx;
-	u16 i;
-
-	for (i = 0; i < ENA_TOTAL_NUM_QUEUES; i++)
-		dev_idx_to_host_tbl[ena_dev->io_sq_queues[i].idx] = i;
-
-	for (i = 0; i < 1 << rss->tbl_log_size; i++) {
-		if (rss->rss_ind_tbl[i].cq_idx > ENA_TOTAL_NUM_QUEUES)
-			return ENA_COM_INVAL;
-		idx = (u8)rss->rss_ind_tbl[i].cq_idx;
-
-		if (dev_idx_to_host_tbl[idx] > ENA_TOTAL_NUM_QUEUES)
-			return ENA_COM_INVAL;
-
-		rss->host_rss_ind_tbl[i] = dev_idx_to_host_tbl[idx];
-	}
-
-	return 0;
-}
-
 static int ena_com_init_interrupt_moderation_table(struct ena_com_dev *ena_dev)
 {
 	size_t size;
@@ -2381,12 +2375,14 @@ int ena_com_fill_hash_function(struct ena_com_dev *ena_dev,
 			       enum ena_admin_hash_functions func,
 			       const u8 *key, u16 key_len, u32 init_val)
 {
-	struct ena_rss *rss = &ena_dev->rss;
+	struct ena_admin_feature_rss_flow_hash_control *hash_key;
 	struct ena_admin_get_feat_resp get_resp;
-	struct ena_admin_feature_rss_flow_hash_control *hash_key =
-		rss->hash_key;
+	enum ena_admin_hash_functions old_func;
+	struct ena_rss *rss = &ena_dev->rss;
 	int rc;
 
+	hash_key = rss->hash_key;
+
 	/* Make sure size is a mult of DWs */
 	if (unlikely(key_len & 0x3))
 		return ENA_COM_INVAL;
@@ -2398,22 +2394,23 @@ int ena_com_fill_hash_function(struct ena_com_dev *ena_dev,
 	if (unlikely(rc))
 		return rc;
 
-	if (!((1 << func) & get_resp.u.flow_hash_func.supported_func)) {
+	if (!(BIT(func) & get_resp.u.flow_hash_func.supported_func)) {
 		ena_trc_err("Flow hash function %d isn't supported\n", func);
 		return ENA_COM_UNSUPPORTED;
 	}
 
 	switch (func) {
 	case ENA_ADMIN_TOEPLITZ:
-		if (key_len > sizeof(hash_key->key)) {
-			ena_trc_err("key len (%hu) is bigger than the max supported (%zu)\n",
-				    key_len, sizeof(hash_key->key));
-			return ENA_COM_INVAL;
+		if (key) {
+			if (key_len != sizeof(hash_key->key)) {
+				ena_trc_err("key len (%hu) doesn't equal the supported size (%zu)\n",
+					     key_len, sizeof(hash_key->key));
+				return ENA_COM_INVAL;
+			}
+			memcpy(hash_key->key, key, key_len);
+			rss->hash_init_val = init_val;
+			hash_key->keys_num = key_len >> 2;
 		}
-
-		memcpy(hash_key->key, key, key_len);
-		rss->hash_init_val = init_val;
-		hash_key->keys_num = key_len >> 2;
 		break;
 	case ENA_ADMIN_CRC32:
 		rss->hash_init_val = init_val;
@@ -2423,26 +2420,27 @@ int ena_com_fill_hash_function(struct ena_com_dev *ena_dev,
 		return ENA_COM_INVAL;
 	}
 
+	old_func = rss->hash_func;
 	rss->hash_func = func;
 	rc = ena_com_set_hash_function(ena_dev);
 
 	/* Restore the old function */
 	if (unlikely(rc))
-		ena_com_get_hash_function(ena_dev, NULL, NULL);
+		rss->hash_func = old_func;
 
 	return rc;
 }
 
 int ena_com_get_hash_function(struct ena_com_dev *ena_dev,
-			      enum ena_admin_hash_functions *func,
-			      u8 *key)
+			      enum ena_admin_hash_functions *func)
 {
 	struct ena_rss *rss = &ena_dev->rss;
 	struct ena_admin_get_feat_resp get_resp;
-	struct ena_admin_feature_rss_flow_hash_control *hash_key =
-		rss->hash_key;
 	int rc;
 
+	if (unlikely(!func))
+		return ENA_COM_INVAL;
+
 	rc = ena_com_get_feature_ex(ena_dev, &get_resp,
 				    ENA_ADMIN_RSS_HASH_FUNCTION,
 				    rss->hash_key_dma_addr,
@@ -2450,9 +2448,20 @@ int ena_com_get_hash_function(struct ena_com_dev *ena_dev,
 	if (unlikely(rc))
 		return rc;
 
-	rss->hash_func = get_resp.u.flow_hash_func.selected_func;
-	if (func)
-		*func = rss->hash_func;
+	/* ENA_FFS returns 1 in case the lsb is set */
+	rss->hash_func = ENA_FFS(get_resp.u.flow_hash_func.selected_func);
+	if (rss->hash_func)
+		rss->hash_func--;
+
+	*func = rss->hash_func;
+
+	return 0;
+}
+
+int ena_com_get_hash_key(struct ena_com_dev *ena_dev, u8 *key)
+{
+	struct ena_admin_feature_rss_flow_hash_control *hash_key =
+		ena_dev->rss.hash_key;
 
 	if (key)
 		memcpy(key, hash_key->key, (size_t)(hash_key->keys_num) << 2);
@@ -2714,10 +2723,6 @@ int ena_com_indirect_table_get(struct ena_com_dev *ena_dev, u32 *ind_tbl)
 	if (!ind_tbl)
 		return 0;
 
-	rc = ena_com_ind_tbl_convert_from_device(ena_dev);
-	if (unlikely(rc))
-		return rc;
-
 	for (i = 0; i < (1 << rss->tbl_log_size); i++)
 		ind_tbl[i] = rss->host_rss_ind_tbl[i];
 
@@ -2738,6 +2743,8 @@ int ena_com_rss_init(struct ena_com_dev *ena_dev, u16 indr_tbl_log_size)
 	if (unlikely(rc))
 		goto err_hash_key;
 
+	ena_com_hash_key_fill_default_key(ena_dev);
+
 	rc = ena_com_hash_ctrl_init(ena_dev);
 	if (unlikely(rc))
 		goto err_hash_ctrl;
diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h
index f2ef26c91b..dc7e0d3930 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -53,6 +53,7 @@
 #define ENA_INTR_DELAY_NEW_VALUE_WEIGHT			4
 #define ENA_INTR_MODER_LEVEL_STRIDE			1
 #define ENA_INTR_BYTE_COUNT_NOT_SUPPORTED		0xFFFFFF
+#define ENA_HASH_KEY_SIZE				40
 
 #define ENA_HW_HINTS_NO_TIMEOUT				0xFFFF
 
@@ -693,6 +694,14 @@ int ena_com_rss_init(struct ena_com_dev *ena_dev, u16 log_size);
  */
 void ena_com_rss_destroy(struct ena_com_dev *ena_dev);
 
+/* ena_com_get_current_hash_function - Get RSS hash function
+ * @ena_dev: ENA communication layer struct
+ *
+ * Return the current hash function.
+ * @return: 0 or one of the ena_admin_hash_functions values.
+ */
+int ena_com_get_current_hash_function(struct ena_com_dev *ena_dev);
+
 /* ena_com_fill_hash_function - Fill RSS hash function
  * @ena_dev: ENA communication layer struct
  * @func: The hash function (Toeplitz or crc)
@@ -724,13 +733,11 @@ int ena_com_fill_hash_function(struct ena_com_dev *ena_dev,
  */
 int ena_com_set_hash_function(struct ena_com_dev *ena_dev);
 
-/* ena_com_get_hash_function - Retrieve the hash function and the hash key
- * from the device.
+/* ena_com_get_hash_function - Retrieve the hash function from the device.
  * @ena_dev: ENA communication layer struct
  * @func: hash function
- * @key: hash key
  *
- * Retrieve the hash function and the hash key from the device.
+ * Retrieve the hash function from the device.
  *
  * @note: If the caller called ena_com_fill_hash_function but didn't flash
  * it to the device, the new configuration will be lost.
@@ -738,9 +745,20 @@ int ena_com_set_hash_function(struct ena_com_dev *ena_dev);
  * @return: 0 on Success and negative value otherwise.
  */
 int ena_com_get_hash_function(struct ena_com_dev *ena_dev,
-			      enum ena_admin_hash_functions *func,
-			      u8 *key);
+			      enum ena_admin_hash_functions *func);
 
+/* ena_com_get_hash_key - Retrieve the hash key
+ * @ena_dev: ENA communication layer struct
+ * @key: hash key
+ *
+ * Retrieve the hash key.
+ *
+ * @note: If the caller called ena_com_fill_hash_key but didn't flash
+ * it to the device, the new configuration will be lost.
+ *
+ * @return: 0 on Success and negative value otherwise.
+ */
+int ena_com_get_hash_key(struct ena_com_dev *ena_dev, u8 *key);
 /* ena_com_fill_hash_ctrl - Fill RSS hash control
  * @ena_dev: ENA communication layer struct.
  * @proto: The protocol to configure.
diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index 793ba8a957..24a831f4d4 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -301,6 +301,12 @@ extern rte_atomic32_t ena_alloc_cnt;
 
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
+#define ENA_FFS(x) ffs(x)
+
+void ena_rss_key_fill(void *key, size_t size);
+
+#define ENA_RSS_FILL_KEY(key, size) ena_rss_key_fill(key, size)
+
 #include "ena_includes.h"
 
 #endif /* DPDK_ENA_COM_ENA_PLAT_DPDK_H_ */
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 854c724e32..21c25b21b1 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -256,6 +256,23 @@ static const struct eth_dev_ops ena_dev_ops = {
 	.reta_query           = ena_rss_reta_query,
 };
 
+void ena_rss_key_fill(void *key, size_t size)
+{
+	static bool key_generated;
+	static uint8_t default_key[ENA_HASH_KEY_SIZE];
+	size_t i;
+
+	RTE_ASSERT(size <= ENA_HASH_KEY_SIZE);
+
+	if (!key_generated) {
+		for (i = 0; i < ENA_HASH_KEY_SIZE; ++i)
+			default_key[i] = rte_rand() & 0xff;
+		key_generated = true;
+	}
+
+	rte_memcpy(key, default_key, size);
+}
+
 static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
 				       struct ena_com_rx_ctx *ena_rx_ctx)
 {
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 04/29] net/ena/base: set default hash key

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> The RSS hash key was present in the device, but it wasn't exposed to
> the user. The other key still cannot be set, but now it can be accessed
> if one needs to do that.

What is 'the other key'?

> 
> By default, the random hash key is used and it is generated only once
> when requested for the first time.

It looks like this patch does
1- setting default rss hash key (generated randomly)
2- Separate 'ena_com_get_hash_function()' and 'ena_com_get_hash_key()'
  - I didn't get the motivation of this seperation, new function not called
3- Minor fixes, like 'key' check in 'ena_com_fill_hash_function()', in
'ENA_ADMIN_TOEPLITZ' case, ...
4- remove 'ena_com_ind_tbl_convert_from_device()'

What to you think seperating above ones if they are logically seperate?

> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>

<...>

> @@ -1032,6 +1032,24 @@ static int ena_com_get_feature(struct ena_com_dev *ena_dev,
>  				      feature_ver);
>  }
>  
> +int ena_com_get_current_hash_function(struct ena_com_dev *ena_dev)
> +{
> +	return ena_dev->rss.hash_func;
> +}


This function is not called, who is the intendent consumer of the function?
Commit log mentions exposing to user, is the intention this function to be
called by application, if so it shoudln't, applications doesn't call driver
fucntions directly.

<...>

> @@ -1266,30 +1284,6 @@ static int ena_com_ind_tbl_convert_to_device(struct ena_com_dev *ena_dev)
>  	return 0;
>  }
>  
> -static int ena_com_ind_tbl_convert_from_device(struct ena_com_dev *ena_dev)
> -{
> -	u16 dev_idx_to_host_tbl[ENA_TOTAL_NUM_QUEUES] = { (u16)-1 };
> -	struct ena_rss *rss = &ena_dev->rss;
> -	u8 idx;
> -	u16 i;
> -
> -	for (i = 0; i < ENA_TOTAL_NUM_QUEUES; i++)
> -		dev_idx_to_host_tbl[ena_dev->io_sq_queues[i].idx] = i;
> -
> -	for (i = 0; i < 1 << rss->tbl_log_size; i++) {
> -		if (rss->rss_ind_tbl[i].cq_idx > ENA_TOTAL_NUM_QUEUES)
> -			return ENA_COM_INVAL;
> -		idx = (u8)rss->rss_ind_tbl[i].cq_idx;
> -
> -		if (dev_idx_to_host_tbl[idx] > ENA_TOTAL_NUM_QUEUES)
> -			return ENA_COM_INVAL;
> -
> -		rss->host_rss_ind_tbl[i] = dev_idx_to_host_tbl[idx];
> -	}
> -
> -	return 0;
> -}> -

This function and where it is called seems removed, is this related to this
patch, "setting default hash key"?

<...>

>  	case ENA_ADMIN_TOEPLITZ:
> -		if (key_len > sizeof(hash_key->key)) {
> -			ena_trc_err("key len (%hu) is bigger than the max supported (%zu)\n",
> -				    key_len, sizeof(hash_key->key));
> -			return ENA_COM_INVAL;
> +		if (key) {
> +			if (key_len != sizeof(hash_key->key)) {
> +				ena_trc_err("key len (%hu) doesn't equal the supported size (%zu)\n",
> +					     key_len, sizeof(hash_key->key));
> +				return ENA_COM_INVAL;
> +			}
> +			memcpy(hash_key->key, key, key_len);
> +			rss->hash_init_val = init_val;
> +			hash_key->keys_num = key_len >> 2;

I am aware this is copy/paste, but here '>> 2' is "/ sizeof(unit2_t)", would it
be better to use that way instead of hardcoded value?

<...>

> @@ -256,6 +256,23 @@ static const struct eth_dev_ops ena_dev_ops = {
>  	.reta_query           = ena_rss_reta_query,
>  };
>  
> +void ena_rss_key_fill(void *key, size_t size)
> +{
> +	static bool key_generated;
> +	static uint8_t default_key[ENA_HASH_KEY_SIZE];

If there are multiple 'ena' devices in the platform, using 'static' variables
will cause each device use same rss key, I just want to double check this is the
intention.

Re: [dpdk-dev] [PATCH v2 04/29] net/ena/base: set default hash key

[Michał Krawczyk]

czw., 2 kwi 2020 o 14:36 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > The RSS hash key was present in the device, but it wasn't exposed to
> > the user. The other key still cannot be set, but now it can be accessed
> > if one needs to do that.
>
> What is 'the other key'?
>

I meant 'the custom key provided by the user'. I'll precise this in v3.

> >
> > By default, the random hash key is used and it is generated only once
> > when requested for the first time.
>
> It looks like this patch does
> 1- setting default rss hash key (generated randomly)
> 2- Separate 'ena_com_get_hash_function()' and 'ena_com_get_hash_key()'
>   - I didn't get the motivation of this seperation, new function not called
> 3- Minor fixes, like 'key' check in 'ena_com_fill_hash_function()', in
> 'ENA_ADMIN_TOEPLITZ' case, ...
> 4- remove 'ena_com_ind_tbl_convert_from_device()'
>
> What to you think seperating above ones if they are logically seperate?
>

Sure, I'll split this into 3 commits - I'll skip the 2nd one as
initially we were planning to add full RSS configuration support in
this release, but postponed it as for now (it's a really big patch).
So this new function (ena_com_get_hash_key()) will be used there, but
maybe as it's not yet in, we can skip this.

> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
>
> <...>
>
> > @@ -1032,6 +1032,24 @@ static int ena_com_get_feature(struct ena_com_dev *ena_dev,
> >                                     feature_ver);
> >  }
> >
> > +int ena_com_get_current_hash_function(struct ena_com_dev *ena_dev)
> > +{
> > +     return ena_dev->rss.hash_func;
> > +}
>
>
> This function is not called, who is the intendent consumer of the function?
> Commit log mentions exposing to user, is the intention this function to be
> called by application, if so it shoudln't, applications doesn't call driver
> fucntions directly.
>

The intended customer was the PMD, which could further expose it to
the application via DPDK API - but we're missing this feature in this
release, so maybe it'll be better to add it later.

> <...>
>
> > @@ -1266,30 +1284,6 @@ static int ena_com_ind_tbl_convert_to_device(struct ena_com_dev *ena_dev)
> >       return 0;
> >  }
> >
> > -static int ena_com_ind_tbl_convert_from_device(struct ena_com_dev *ena_dev)
> > -{
> > -     u16 dev_idx_to_host_tbl[ENA_TOTAL_NUM_QUEUES] = { (u16)-1 };
> > -     struct ena_rss *rss = &ena_dev->rss;
> > -     u8 idx;
> > -     u16 i;
> > -
> > -     for (i = 0; i < ENA_TOTAL_NUM_QUEUES; i++)
> > -             dev_idx_to_host_tbl[ena_dev->io_sq_queues[i].idx] = i;
> > -
> > -     for (i = 0; i < 1 << rss->tbl_log_size; i++) {
> > -             if (rss->rss_ind_tbl[i].cq_idx > ENA_TOTAL_NUM_QUEUES)
> > -                     return ENA_COM_INVAL;
> > -             idx = (u8)rss->rss_ind_tbl[i].cq_idx;
> > -
> > -             if (dev_idx_to_host_tbl[idx] > ENA_TOTAL_NUM_QUEUES)
> > -                     return ENA_COM_INVAL;
> > -
> > -             rss->host_rss_ind_tbl[i] = dev_idx_to_host_tbl[idx];
> > -     }
> > -
> > -     return 0;
> > -}> -
>
> This function and where it is called seems removed, is this related to this
> patch, "setting default hash key"?
>

Yeah - it looks like it should be a part of the separate patch.

> <...>
>
> >       case ENA_ADMIN_TOEPLITZ:
> > -             if (key_len > sizeof(hash_key->key)) {
> > -                     ena_trc_err("key len (%hu) is bigger than the max supported (%zu)\n",
> > -                                 key_len, sizeof(hash_key->key));
> > -                     return ENA_COM_INVAL;
> > +             if (key) {
> > +                     if (key_len != sizeof(hash_key->key)) {
> > +                             ena_trc_err("key len (%hu) doesn't equal the supported size (%zu)\n",
> > +                                          key_len, sizeof(hash_key->key));
> > +                             return ENA_COM_INVAL;
> > +                     }
> > +                     memcpy(hash_key->key, key, key_len);
> > +                     rss->hash_init_val = init_val;
> > +                     hash_key->keys_num = key_len >> 2;
>
> I am aware this is copy/paste, but here '>> 2' is "/ sizeof(unit2_t)", would it
> be better to use that way instead of hardcoded value?
>

Will change that in v3.

> <...>
>
> > @@ -256,6 +256,23 @@ static const struct eth_dev_ops ena_dev_ops = {
> >       .reta_query           = ena_rss_reta_query,
> >  };
> >
> > +void ena_rss_key_fill(void *key, size_t size)
> > +{
> > +     static bool key_generated;
> > +     static uint8_t default_key[ENA_HASH_KEY_SIZE];
>
> If there are multiple 'ena' devices in the platform, using 'static' variables
> will cause each device use same rss key, I just want to double check this is the
> intention.

Yes, it's acceptable behavior. The most important thing is to have
those keys random per instance, to make the DDoS attacks harder.

[dpdk-dev] [PATCH v2 05/29] net/ena/base: rework interrupt moderation

[Michal Krawczyk]

This feature allows for adaptive interrupt moderation. It's not used by
the DPDK PMD, but is a part of the newest HAL version.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.c       | 171 +++++----------------------
 drivers/net/ena/base/ena_com.h       | 154 ++----------------------
 drivers/net/ena/base/ena_plat_dpdk.h |   3 +-
 3 files changed, 42 insertions(+), 286 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index a5753997ed..cb2114acb7 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -1284,39 +1284,29 @@ static int ena_com_ind_tbl_convert_to_device(struct ena_com_dev *ena_dev)
 	return 0;
 }
 
-static int ena_com_init_interrupt_moderation_table(struct ena_com_dev *ena_dev)
-{
-	size_t size;
-
-	size = sizeof(struct ena_intr_moder_entry) * ENA_INTR_MAX_NUM_OF_LEVELS;
-
-	ena_dev->intr_moder_tbl = ENA_MEM_ALLOC(ena_dev->dmadev, size);
-	if (!ena_dev->intr_moder_tbl)
-		return ENA_COM_NO_MEM;
-
-	ena_com_config_default_interrupt_moderation_table(ena_dev);
-
-	return 0;
-}
-
 static void ena_com_update_intr_delay_resolution(struct ena_com_dev *ena_dev,
 						 u16 intr_delay_resolution)
 {
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-	unsigned int i;
+	u16 prev_intr_delay_resolution = ena_dev->intr_delay_resolution;
 
-	if (!intr_delay_resolution) {
+	if (unlikely(!intr_delay_resolution)) {
 		ena_trc_err("Illegal intr_delay_resolution provided. Going to use default 1 usec resolution\n");
-		intr_delay_resolution = 1;
+		intr_delay_resolution = ENA_DEFAULT_INTR_DELAY_RESOLUTION;
 	}
-	ena_dev->intr_delay_resolution = intr_delay_resolution;
 
 	/* update Rx */
-	for (i = 0; i < ENA_INTR_MAX_NUM_OF_LEVELS; i++)
-		intr_moder_tbl[i].intr_moder_interval /= intr_delay_resolution;
+	ena_dev->intr_moder_rx_interval =
+		ena_dev->intr_moder_rx_interval *
+		prev_intr_delay_resolution /
+		intr_delay_resolution;
 
 	/* update Tx */
-	ena_dev->intr_moder_tx_interval /= intr_delay_resolution;
+	ena_dev->intr_moder_tx_interval =
+		ena_dev->intr_moder_tx_interval *
+		prev_intr_delay_resolution /
+		intr_delay_resolution;
+
+	ena_dev->intr_delay_resolution = intr_delay_resolution;
 }
 
 /*****************************************************************************/
@@ -2892,44 +2882,35 @@ bool ena_com_interrupt_moderation_supported(struct ena_com_dev *ena_dev)
 						  ENA_ADMIN_INTERRUPT_MODERATION);
 }
 
-int ena_com_update_nonadaptive_moderation_interval_tx(struct ena_com_dev *ena_dev,
-						      u32 tx_coalesce_usecs)
+static int ena_com_update_nonadaptive_moderation_interval(u32 coalesce_usecs,
+							  u32 intr_delay_resolution,
+							  u32 *intr_moder_interval)
 {
-	if (!ena_dev->intr_delay_resolution) {
+	if (!intr_delay_resolution) {
 		ena_trc_err("Illegal interrupt delay granularity value\n");
 		return ENA_COM_FAULT;
 	}
 
-	ena_dev->intr_moder_tx_interval = tx_coalesce_usecs /
-		ena_dev->intr_delay_resolution;
+	*intr_moder_interval = coalesce_usecs / intr_delay_resolution;
 
 	return 0;
 }
 
-int ena_com_update_nonadaptive_moderation_interval_rx(struct ena_com_dev *ena_dev,
-						      u32 rx_coalesce_usecs)
-{
-	if (!ena_dev->intr_delay_resolution) {
-		ena_trc_err("Illegal interrupt delay granularity value\n");
-		return ENA_COM_FAULT;
-	}
 
-	/* We use LOWEST entry of moderation table for storing
-	 * nonadaptive interrupt coalescing values
-	 */
-	ena_dev->intr_moder_tbl[ENA_INTR_MODER_LOWEST].intr_moder_interval =
-		rx_coalesce_usecs / ena_dev->intr_delay_resolution;
-
-	return 0;
+int ena_com_update_nonadaptive_moderation_interval_tx(struct ena_com_dev *ena_dev,
+						      u32 tx_coalesce_usecs)
+{
+	return ena_com_update_nonadaptive_moderation_interval(tx_coalesce_usecs,
+							      ena_dev->intr_delay_resolution,
+							      &ena_dev->intr_moder_tx_interval);
 }
 
-void ena_com_destroy_interrupt_moderation(struct ena_com_dev *ena_dev)
+int ena_com_update_nonadaptive_moderation_interval_rx(struct ena_com_dev *ena_dev,
+						      u32 rx_coalesce_usecs)
 {
-	if (ena_dev->intr_moder_tbl)
-		ENA_MEM_FREE(ena_dev->dmadev,
-			     ena_dev->intr_moder_tbl,
-			     (sizeof(struct ena_intr_moder_entry) * ENA_INTR_MAX_NUM_OF_LEVELS));
-	ena_dev->intr_moder_tbl = NULL;
+	return ena_com_update_nonadaptive_moderation_interval(rx_coalesce_usecs,
+							      ena_dev->intr_delay_resolution,
+							      &ena_dev->intr_moder_rx_interval);
 }
 
 int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev)
@@ -2956,10 +2937,6 @@ int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev)
 		return rc;
 	}
 
-	rc = ena_com_init_interrupt_moderation_table(ena_dev);
-	if (rc)
-		goto err;
-
 	/* if moderation is supported by device we set adaptive moderation */
 	delay_resolution = get_resp.u.intr_moderation.intr_delay_resolution;
 	ena_com_update_intr_delay_resolution(ena_dev, delay_resolution);
@@ -2968,52 +2945,6 @@ int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev)
 	ena_com_disable_adaptive_moderation(ena_dev);
 
 	return 0;
-err:
-	ena_com_destroy_interrupt_moderation(ena_dev);
-	return rc;
-}
-
-void ena_com_config_default_interrupt_moderation_table(struct ena_com_dev *ena_dev)
-{
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-
-	if (!intr_moder_tbl)
-		return;
-
-	intr_moder_tbl[ENA_INTR_MODER_LOWEST].intr_moder_interval =
-		ENA_INTR_LOWEST_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_LOWEST].pkts_per_interval =
-		ENA_INTR_LOWEST_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_LOWEST].bytes_per_interval =
-		ENA_INTR_LOWEST_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_LOW].intr_moder_interval =
-		ENA_INTR_LOW_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_LOW].pkts_per_interval =
-		ENA_INTR_LOW_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_LOW].bytes_per_interval =
-		ENA_INTR_LOW_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_MID].intr_moder_interval =
-		ENA_INTR_MID_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_MID].pkts_per_interval =
-		ENA_INTR_MID_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_MID].bytes_per_interval =
-		ENA_INTR_MID_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_HIGH].intr_moder_interval =
-		ENA_INTR_HIGH_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGH].pkts_per_interval =
-		ENA_INTR_HIGH_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGH].bytes_per_interval =
-		ENA_INTR_HIGH_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_HIGHEST].intr_moder_interval =
-		ENA_INTR_HIGHEST_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGHEST].pkts_per_interval =
-		ENA_INTR_HIGHEST_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGHEST].bytes_per_interval =
-		ENA_INTR_HIGHEST_BYTES;
 }
 
 unsigned int ena_com_get_nonadaptive_moderation_interval_tx(struct ena_com_dev *ena_dev)
@@ -3023,49 +2954,7 @@ unsigned int ena_com_get_nonadaptive_moderation_interval_tx(struct ena_com_dev *
 
 unsigned int ena_com_get_nonadaptive_moderation_interval_rx(struct ena_com_dev *ena_dev)
 {
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-
-	if (intr_moder_tbl)
-		return intr_moder_tbl[ENA_INTR_MODER_LOWEST].intr_moder_interval;
-
-	return 0;
-}
-
-void ena_com_init_intr_moderation_entry(struct ena_com_dev *ena_dev,
-					enum ena_intr_moder_level level,
-					struct ena_intr_moder_entry *entry)
-{
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-
-	if (level >= ENA_INTR_MAX_NUM_OF_LEVELS)
-		return;
-
-	intr_moder_tbl[level].intr_moder_interval = entry->intr_moder_interval;
-	if (ena_dev->intr_delay_resolution)
-		intr_moder_tbl[level].intr_moder_interval /=
-			ena_dev->intr_delay_resolution;
-	intr_moder_tbl[level].pkts_per_interval = entry->pkts_per_interval;
-
-	/* use hardcoded value until ethtool supports bytecount parameter */
-	if (entry->bytes_per_interval != ENA_INTR_BYTE_COUNT_NOT_SUPPORTED)
-		intr_moder_tbl[level].bytes_per_interval = entry->bytes_per_interval;
-}
-
-void ena_com_get_intr_moderation_entry(struct ena_com_dev *ena_dev,
-				       enum ena_intr_moder_level level,
-				       struct ena_intr_moder_entry *entry)
-{
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-
-	if (level >= ENA_INTR_MAX_NUM_OF_LEVELS)
-		return;
-
-	entry->intr_moder_interval = intr_moder_tbl[level].intr_moder_interval;
-	if (ena_dev->intr_delay_resolution)
-		entry->intr_moder_interval *= ena_dev->intr_delay_resolution;
-	entry->pkts_per_interval =
-	intr_moder_tbl[level].pkts_per_interval;
-	entry->bytes_per_interval = intr_moder_tbl[level].bytes_per_interval;
+	return ena_dev->intr_moder_rx_interval;
 }
 
 int ena_com_config_dev_mode(struct ena_com_dev *ena_dev,
diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h
index dc7e0d3930..5935d024dd 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -27,47 +27,16 @@
 /*****************************************************************************/
 /* ENA adaptive interrupt moderation settings */
 
-#define ENA_INTR_LOWEST_USECS           (0)
-#define ENA_INTR_LOWEST_PKTS            (3)
-#define ENA_INTR_LOWEST_BYTES           (2 * 1524)
-
-#define ENA_INTR_LOW_USECS              (32)
-#define ENA_INTR_LOW_PKTS               (12)
-#define ENA_INTR_LOW_BYTES              (16 * 1024)
-
-#define ENA_INTR_MID_USECS              (80)
-#define ENA_INTR_MID_PKTS               (48)
-#define ENA_INTR_MID_BYTES              (64 * 1024)
-
-#define ENA_INTR_HIGH_USECS             (128)
-#define ENA_INTR_HIGH_PKTS              (96)
-#define ENA_INTR_HIGH_BYTES             (128 * 1024)
-
-#define ENA_INTR_HIGHEST_USECS          (192)
-#define ENA_INTR_HIGHEST_PKTS           (128)
-#define ENA_INTR_HIGHEST_BYTES          (192 * 1024)
-
-#define ENA_INTR_INITIAL_TX_INTERVAL_USECS		196
-#define ENA_INTR_INITIAL_RX_INTERVAL_USECS		4
-#define ENA_INTR_DELAY_OLD_VALUE_WEIGHT			6
-#define ENA_INTR_DELAY_NEW_VALUE_WEIGHT			4
-#define ENA_INTR_MODER_LEVEL_STRIDE			1
-#define ENA_INTR_BYTE_COUNT_NOT_SUPPORTED		0xFFFFFF
+#define ENA_INTR_INITIAL_TX_INTERVAL_USECS ENA_INTR_INITIAL_TX_INTERVAL_USECS_PLAT
+#define ENA_INTR_INITIAL_RX_INTERVAL_USECS 0
+#define ENA_DEFAULT_INTR_DELAY_RESOLUTION 1
+
 #define ENA_HASH_KEY_SIZE				40
 
 #define ENA_HW_HINTS_NO_TIMEOUT				0xFFFF
 
 #define ENA_FEATURE_MAX_QUEUE_EXT_VER	1
 
-enum ena_intr_moder_level {
-	ENA_INTR_MODER_LOWEST = 0,
-	ENA_INTR_MODER_LOW,
-	ENA_INTR_MODER_MID,
-	ENA_INTR_MODER_HIGH,
-	ENA_INTR_MODER_HIGHEST,
-	ENA_INTR_MAX_NUM_OF_LEVELS,
-};
-
 struct ena_llq_configurations {
 	enum ena_admin_llq_header_location llq_header_location;
 	enum ena_admin_llq_ring_entry_size llq_ring_entry_size;
@@ -76,12 +45,6 @@ struct ena_llq_configurations {
 	u16 llq_ring_entry_size_value;
 };
 
-struct ena_intr_moder_entry {
-	unsigned int intr_moder_interval;
-	unsigned int pkts_per_interval;
-	unsigned int bytes_per_interval;
-};
-
 enum queue_direction {
 	ENA_COM_IO_QUEUE_DIRECTION_TX,
 	ENA_COM_IO_QUEUE_DIRECTION_RX
@@ -353,7 +316,13 @@ struct ena_com_dev {
 	struct ena_host_attribute host_attr;
 	bool adaptive_coalescing;
 	u16 intr_delay_resolution;
+
+	/* interrupt moderation intervals are in usec divided by
+	 * intr_delay_resolution, which is supplied by the device.
+	 */
 	u32 intr_moder_tx_interval;
+	u32 intr_moder_rx_interval;
+
 	struct ena_intr_moder_entry *intr_moder_tbl;
 
 	struct ena_com_llq_info llq_info;
@@ -938,11 +907,6 @@ int ena_com_execute_admin_command(struct ena_com_admin_queue *admin_queue,
  */
 int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev);
 
-/* ena_com_destroy_interrupt_moderation - Destroy interrupt moderation resources
- * @ena_dev: ENA communication layer struct
- */
-void ena_com_destroy_interrupt_moderation(struct ena_com_dev *ena_dev);
-
 /* ena_com_interrupt_moderation_supported - Return if interrupt moderation
  * capability is supported by the device.
  *
@@ -950,12 +914,6 @@ void ena_com_destroy_interrupt_moderation(struct ena_com_dev *ena_dev);
  */
 bool ena_com_interrupt_moderation_supported(struct ena_com_dev *ena_dev);
 
-/* ena_com_config_default_interrupt_moderation_table - Restore the interrupt
- * moderation table back to the default parameters.
- * @ena_dev: ENA communication layer struct
- */
-void ena_com_config_default_interrupt_moderation_table(struct ena_com_dev *ena_dev);
-
 /* ena_com_update_nonadaptive_moderation_interval_tx - Update the
  * non-adaptive interval in Tx direction.
  * @ena_dev: ENA communication layer struct
@@ -992,29 +950,6 @@ unsigned int ena_com_get_nonadaptive_moderation_interval_tx(struct ena_com_dev *
  */
 unsigned int ena_com_get_nonadaptive_moderation_interval_rx(struct ena_com_dev *ena_dev);
 
-/* ena_com_init_intr_moderation_entry - Update a single entry in the interrupt
- * moderation table.
- * @ena_dev: ENA communication layer struct
- * @level: Interrupt moderation table level
- * @entry: Entry value
- *
- * Update a single entry in the interrupt moderation table.
- */
-void ena_com_init_intr_moderation_entry(struct ena_com_dev *ena_dev,
-					enum ena_intr_moder_level level,
-					struct ena_intr_moder_entry *entry);
-
-/* ena_com_get_intr_moderation_entry - Init ena_intr_moder_entry.
- * @ena_dev: ENA communication layer struct
- * @level: Interrupt moderation table level
- * @entry: Entry to fill.
- *
- * Initialize the entry according to the adaptive interrupt moderation table.
- */
-void ena_com_get_intr_moderation_entry(struct ena_com_dev *ena_dev,
-				       enum ena_intr_moder_level level,
-				       struct ena_intr_moder_entry *entry);
-
 /* ena_com_config_dev_mode - Configure the placement policy of the device.
  * @ena_dev: ENA communication layer struct
  * @llq_features: LLQ feature descriptor, retrieve via
@@ -1040,75 +975,6 @@ static inline void ena_com_disable_adaptive_moderation(struct ena_com_dev *ena_d
 	ena_dev->adaptive_coalescing = false;
 }
 
-/* ena_com_calculate_interrupt_delay - Calculate new interrupt delay
- * @ena_dev: ENA communication layer struct
- * @pkts: Number of packets since the last update
- * @bytes: Number of bytes received since the last update.
- * @smoothed_interval: Returned interval
- * @moder_tbl_idx: Current table level as input update new level as return
- * value.
- */
-static inline void ena_com_calculate_interrupt_delay(struct ena_com_dev *ena_dev,
-						     unsigned int pkts,
-						     unsigned int bytes,
-						     unsigned int *smoothed_interval,
-						     unsigned int *moder_tbl_idx)
-{
-	enum ena_intr_moder_level curr_moder_idx, new_moder_idx;
-	struct ena_intr_moder_entry *curr_moder_entry;
-	struct ena_intr_moder_entry *pred_moder_entry;
-	struct ena_intr_moder_entry *new_moder_entry;
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-	unsigned int interval;
-
-	/* We apply adaptive moderation on Rx path only.
-	 * Tx uses static interrupt moderation.
-	 */
-	if (!pkts || !bytes)
-		/* Tx interrupt, or spurious interrupt,
-		 * in both cases we just use same delay values
-		 */
-		return;
-
-	curr_moder_idx = (enum ena_intr_moder_level)(*moder_tbl_idx);
-	if (unlikely(curr_moder_idx >=  ENA_INTR_MAX_NUM_OF_LEVELS)) {
-		ena_trc_err("Wrong moderation index %u\n", curr_moder_idx);
-		return;
-	}
-
-	curr_moder_entry = &intr_moder_tbl[curr_moder_idx];
-	new_moder_idx = curr_moder_idx;
-
-	if (curr_moder_idx == ENA_INTR_MODER_LOWEST) {
-		if ((pkts > curr_moder_entry->pkts_per_interval) ||
-		    (bytes > curr_moder_entry->bytes_per_interval))
-			new_moder_idx =
-				(enum ena_intr_moder_level)(curr_moder_idx + ENA_INTR_MODER_LEVEL_STRIDE);
-	} else {
-		pred_moder_entry = &intr_moder_tbl[curr_moder_idx - ENA_INTR_MODER_LEVEL_STRIDE];
-
-		if ((pkts <= pred_moder_entry->pkts_per_interval) ||
-		    (bytes <= pred_moder_entry->bytes_per_interval))
-			new_moder_idx =
-				(enum ena_intr_moder_level)(curr_moder_idx - ENA_INTR_MODER_LEVEL_STRIDE);
-		else if ((pkts > curr_moder_entry->pkts_per_interval) ||
-			 (bytes > curr_moder_entry->bytes_per_interval)) {
-			if (curr_moder_idx != ENA_INTR_MODER_HIGHEST)
-				new_moder_idx =
-					(enum ena_intr_moder_level)(curr_moder_idx + ENA_INTR_MODER_LEVEL_STRIDE);
-		}
-	}
-	new_moder_entry = &intr_moder_tbl[new_moder_idx];
-
-	interval = new_moder_entry->intr_moder_interval;
-	*smoothed_interval = (
-		(interval * ENA_INTR_DELAY_NEW_VALUE_WEIGHT +
-		ENA_INTR_DELAY_OLD_VALUE_WEIGHT * (*smoothed_interval)) + 5) /
-		10;
-
-	*moder_tbl_idx = new_moder_idx;
-}
-
 /* ena_com_update_intr_reg - Prepare interrupt register
  * @intr_reg: interrupt register to update.
  * @rx_delay_interval: Rx interval in usecs
diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index 24a831f4d4..2989df8f7e 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -307,6 +307,7 @@ void ena_rss_key_fill(void *key, size_t size);
 
 #define ENA_RSS_FILL_KEY(key, size) ena_rss_key_fill(key, size)
 
-#include "ena_includes.h"
+#define ENA_INTR_INITIAL_TX_INTERVAL_USECS_PLAT 0
 
+#include "ena_includes.h"
 #endif /* DPDK_ENA_COM_ENA_PLAT_DPDK_H_ */
-- 
2.20.1

[dpdk-dev] [PATCH v2 06/29] net/ena/base: remove extra properties strings

[Michal Krawczyk]

This buffer was never used by the ENA PMD. It could be used for
debugging, but it's presence is redundant now.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.c | 56 ----------------------------------
 drivers/net/ena/base/ena_com.h | 33 --------------------
 2 files changed, 89 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index cb2114acb7..19815db9ad 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -1910,62 +1910,6 @@ int ena_com_get_link_params(struct ena_com_dev *ena_dev,
 	return ena_com_get_feature(ena_dev, resp, ENA_ADMIN_LINK_CONFIG, 0);
 }
 
-int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev)
-{
-	struct ena_admin_get_feat_resp resp;
-	struct ena_extra_properties_strings *extra_properties_strings =
-			&ena_dev->extra_properties_strings;
-	u32 rc;
-	extra_properties_strings->size = ENA_ADMIN_EXTRA_PROPERTIES_COUNT *
-		ENA_ADMIN_EXTRA_PROPERTIES_STRING_LEN;
-
-	ENA_MEM_ALLOC_COHERENT(ena_dev->dmadev,
-			       extra_properties_strings->size,
-			       extra_properties_strings->virt_addr,
-			       extra_properties_strings->dma_addr,
-			       extra_properties_strings->dma_handle);
-	if (unlikely(!extra_properties_strings->virt_addr)) {
-		ena_trc_err("Failed to allocate extra properties strings\n");
-		return 0;
-	}
-
-	rc = ena_com_get_feature_ex(ena_dev, &resp,
-				    ENA_ADMIN_EXTRA_PROPERTIES_STRINGS,
-				    extra_properties_strings->dma_addr,
-				    extra_properties_strings->size, 0);
-	if (rc) {
-		ena_trc_dbg("Failed to get extra properties strings\n");
-		goto err;
-	}
-
-	return resp.u.extra_properties_strings.count;
-err:
-	ena_com_delete_extra_properties_strings(ena_dev);
-	return 0;
-}
-
-void ena_com_delete_extra_properties_strings(struct ena_com_dev *ena_dev)
-{
-	struct ena_extra_properties_strings *extra_properties_strings =
-				&ena_dev->extra_properties_strings;
-
-	if (extra_properties_strings->virt_addr) {
-		ENA_MEM_FREE_COHERENT(ena_dev->dmadev,
-				      extra_properties_strings->size,
-				      extra_properties_strings->virt_addr,
-				      extra_properties_strings->dma_addr,
-				      extra_properties_strings->dma_handle);
-		extra_properties_strings->virt_addr = NULL;
-	}
-}
-
-int ena_com_get_extra_properties_flags(struct ena_com_dev *ena_dev,
-				       struct ena_admin_get_feat_resp *resp)
-{
-	return ena_com_get_feature(ena_dev, resp,
-				   ENA_ADMIN_EXTRA_PROPERTIES_FLAGS, 0);
-}
-
 int ena_com_get_dev_attr_feat(struct ena_com_dev *ena_dev,
 			      struct ena_com_dev_get_features_ctx *get_feat_ctx)
 {
diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h
index 5935d024dd..0e34a13fde 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -284,13 +284,6 @@ struct ena_host_attribute {
 	ena_mem_handle_t host_info_dma_handle;
 };
 
-struct ena_extra_properties_strings {
-	u8 *virt_addr;
-	dma_addr_t dma_addr;
-	ena_mem_handle_t dma_handle;
-	u32 size;
-};
-
 /* Each ena_dev is a PCI function. */
 struct ena_com_dev {
 	struct ena_com_admin_queue admin_queue;
@@ -326,7 +319,6 @@ struct ena_com_dev {
 	struct ena_intr_moder_entry *intr_moder_tbl;
 
 	struct ena_com_llq_info llq_info;
-	struct ena_extra_properties_strings extra_properties_strings;
 };
 
 struct ena_com_dev_get_features_ctx {
@@ -564,31 +556,6 @@ int ena_com_validate_version(struct ena_com_dev *ena_dev);
 int ena_com_get_link_params(struct ena_com_dev *ena_dev,
 			    struct ena_admin_get_feat_resp *resp);
 
-/* ena_com_extra_properties_strings_init - Initialize the extra properties strings buffer.
- * @ena_dev: ENA communication layer struct
- *
- * Initialize the extra properties strings buffer.
- */
-int ena_com_extra_properties_strings_init(struct ena_com_dev *ena_dev);
-
-/* ena_com_delete_extra_properties_strings - Free the extra properties strings buffer.
- * @ena_dev: ENA communication layer struct
- *
- * Free the allocated extra properties strings buffer.
- */
-void ena_com_delete_extra_properties_strings(struct ena_com_dev *ena_dev);
-
-/* ena_com_get_extra_properties_flags - Retrieve extra properties flags.
- * @ena_dev: ENA communication layer struct
- * @resp: Extra properties flags.
- *
- * Retrieve the extra properties flags.
- *
- * @return - 0 on Success negative value otherwise.
- */
-int ena_com_get_extra_properties_flags(struct ena_com_dev *ena_dev,
-				       struct ena_admin_get_feat_resp *resp);
-
 /* ena_com_get_dma_width - Retrieve physical dma address width the device
  * supports.
  * @ena_dev: ENA communication layer struct
-- 
2.20.1

[dpdk-dev] [PATCH v2 07/29] net/ena/base: add accelerated LLQ mode

[Michal Krawczyk]

In order to use the accelerated LLQ, the driver must limit the Tx burst
and be aware that the device has the meta caching disabled. In that
situation, the meta descriptor must be valid on each Tx packet.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.c                | 20 +++++++-
 drivers/net/ena/base/ena_com.h                |  3 ++
 .../net/ena/base/ena_defs/ena_admin_defs.h    | 39 +++++++++++++--
 drivers/net/ena/base/ena_eth_com.c            | 49 +++++++++++++------
 4 files changed, 91 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 19815db9ad..d15b7f22dc 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -378,6 +378,8 @@ static int ena_com_init_io_sq(struct ena_com_dev *ena_dev,
 		       0x0, io_sq->llq_info.desc_list_entry_size);
 		io_sq->llq_buf_ctrl.descs_left_in_line =
 			io_sq->llq_info.descs_num_before_header;
+		io_sq->disable_meta_caching =
+			io_sq->llq_info.disable_meta_caching;
 
 		if (io_sq->llq_info.max_entries_in_tx_burst > 0)
 			io_sq->entries_in_tx_burst_left =
@@ -595,6 +597,14 @@ static int ena_com_set_llq(struct ena_com_dev *ena_dev)
 	cmd.u.llq.desc_num_before_header_enabled = llq_info->descs_num_before_header;
 	cmd.u.llq.descriptors_stride_ctrl_enabled = llq_info->desc_stride_ctrl;
 
+	if (llq_info->disable_meta_caching)
+		cmd.u.llq.accel_mode.u.set.enabled_flags |=
+			BIT(ENA_ADMIN_DISABLE_META_CACHING);
+
+	if (llq_info->max_entries_in_tx_burst)
+		cmd.u.llq.accel_mode.u.set.enabled_flags |=
+			BIT(ENA_ADMIN_LIMIT_TX_BURST);
+
 	ret = ena_com_execute_admin_command(admin_queue,
 					    (struct ena_admin_aq_entry *)&cmd,
 					    sizeof(cmd),
@@ -714,9 +724,15 @@ static int ena_com_config_llq_info(struct ena_com_dev *ena_dev,
 			    supported_feat,
 			    llq_info->descs_num_before_header);
 	}
+	/* Check for accelerated queue supported */
+	llq_info->disable_meta_caching =
+		llq_features->accel_mode.u.get.supported_flags &
+		BIT(ENA_ADMIN_DISABLE_META_CACHING);
 
-	llq_info->max_entries_in_tx_burst =
-		(u16)(llq_features->max_tx_burst_size /	llq_default_cfg->llq_ring_entry_size_value);
+	if (llq_features->accel_mode.u.get.supported_flags & BIT(ENA_ADMIN_LIMIT_TX_BURST))
+		llq_info->max_entries_in_tx_burst =
+			llq_features->accel_mode.u.get.max_tx_burst_size /
+			llq_default_cfg->llq_ring_entry_size_value;
 
 	rc = ena_com_set_llq(ena_dev);
 	if (rc)
diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h
index 0e34a13fde..9f2c6ea8ee 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -82,6 +82,7 @@ struct ena_com_llq_info {
 	u16 descs_num_before_header;
 	u16 descs_per_entry;
 	u16 max_entries_in_tx_burst;
+	bool disable_meta_caching;
 };
 
 struct ena_com_io_cq {
@@ -146,6 +147,8 @@ struct ena_com_io_sq {
 	enum queue_direction direction;
 	enum ena_admin_placement_policy_type mem_queue_type;
 
+	bool disable_meta_caching;
+
 	u32 msix_vector;
 	struct ena_com_tx_meta cached_tx_meta;
 	struct ena_com_llq_info llq_info;
diff --git a/drivers/net/ena/base/ena_defs/ena_admin_defs.h b/drivers/net/ena/base/ena_defs/ena_admin_defs.h
index fb4d4d03f0..020dc78e26 100644
--- a/drivers/net/ena/base/ena_defs/ena_admin_defs.h
+++ b/drivers/net/ena/base/ena_defs/ena_admin_defs.h
@@ -469,6 +469,36 @@ enum ena_admin_llq_stride_ctrl {
 	ENA_ADMIN_MULTIPLE_DESCS_PER_ENTRY          = 2,
 };
 
+enum ena_admin_accel_mode_feat {
+	ENA_ADMIN_DISABLE_META_CACHING              = 0,
+	ENA_ADMIN_LIMIT_TX_BURST                    = 1,
+};
+
+struct ena_admin_accel_mode_get {
+	/* bit field of enum ena_admin_accel_mode_feat */
+	uint16_t supported_flags;
+
+	/* maximum burst size between two doorbells. The size is in bytes */
+	uint16_t max_tx_burst_size;
+};
+
+struct ena_admin_accel_mode_set {
+	/* bit field of enum ena_admin_accel_mode_feat */
+	uint16_t enabled_flags;
+
+	uint16_t reserved;
+};
+
+struct ena_admin_accel_mode_req {
+	union {
+		uint32_t raw[2];
+
+		struct ena_admin_accel_mode_get get;
+
+		struct ena_admin_accel_mode_set set;
+	} u;
+};
+
 struct ena_admin_feature_llq_desc {
 	uint32_t max_llq_num;
 
@@ -514,10 +544,13 @@ struct ena_admin_feature_llq_desc {
 	/* the stride control the driver selected to use */
 	uint16_t descriptors_stride_ctrl_enabled;
 
-	/* Maximum size in bytes taken by llq entries in a single tx burst.
-	 * Set to 0 when there is no such limit.
+	/* reserved */
+	uint32_t reserved1;
+
+	/* accelerated low latency queues requirment. driver needs to
+	 * support those requirments in order to use accelerated llq
 	 */
-	uint32_t max_tx_burst_size;
+	struct ena_admin_accel_mode_req accel_mode;
 };
 
 struct ena_admin_queue_ext_feature_fields {
diff --git a/drivers/net/ena/base/ena_eth_com.c b/drivers/net/ena/base/ena_eth_com.c
index d4d44226df..aabc294fb7 100644
--- a/drivers/net/ena/base/ena_eth_com.c
+++ b/drivers/net/ena/base/ena_eth_com.c
@@ -258,11 +258,10 @@ static u16 ena_com_cdesc_rx_pkt_get(struct ena_com_io_cq *io_cq,
 	return count;
 }
 
-static int ena_com_create_and_store_tx_meta_desc(struct ena_com_io_sq *io_sq,
-							struct ena_com_tx_ctx *ena_tx_ctx)
+static int ena_com_create_meta(struct ena_com_io_sq *io_sq,
+			       struct ena_com_tx_meta *ena_meta)
 {
 	struct ena_eth_io_tx_meta_desc *meta_desc = NULL;
-	struct ena_com_tx_meta *ena_meta = &ena_tx_ctx->ena_meta;
 
 	meta_desc = get_sq_desc(io_sq);
 	memset(meta_desc, 0x0, sizeof(struct ena_eth_io_tx_meta_desc));
@@ -282,12 +281,13 @@ static int ena_com_create_and_store_tx_meta_desc(struct ena_com_io_sq *io_sq,
 
 	/* Extended meta desc */
 	meta_desc->len_ctrl |= ENA_ETH_IO_TX_META_DESC_ETH_META_TYPE_MASK;
-	meta_desc->len_ctrl |= ENA_ETH_IO_TX_META_DESC_META_STORE_MASK;
 	meta_desc->len_ctrl |= (io_sq->phase <<
 		ENA_ETH_IO_TX_META_DESC_PHASE_SHIFT) &
 		ENA_ETH_IO_TX_META_DESC_PHASE_MASK;
 
 	meta_desc->len_ctrl |= ENA_ETH_IO_TX_META_DESC_FIRST_MASK;
+	meta_desc->len_ctrl |= ENA_ETH_IO_TX_META_DESC_META_STORE_MASK;
+
 	meta_desc->word2 |= ena_meta->l3_hdr_len &
 		ENA_ETH_IO_TX_META_DESC_L3_HDR_LEN_MASK;
 	meta_desc->word2 |= (ena_meta->l3_hdr_offset <<
@@ -298,13 +298,34 @@ static int ena_com_create_and_store_tx_meta_desc(struct ena_com_io_sq *io_sq,
 		ENA_ETH_IO_TX_META_DESC_L4_HDR_LEN_IN_WORDS_SHIFT) &
 		ENA_ETH_IO_TX_META_DESC_L4_HDR_LEN_IN_WORDS_MASK;
 
-	meta_desc->len_ctrl |= ENA_ETH_IO_TX_META_DESC_META_STORE_MASK;
+	return ena_com_sq_update_tail(io_sq);
+}
 
-	/* Cached the meta desc */
-	memcpy(&io_sq->cached_tx_meta, ena_meta,
-	       sizeof(struct ena_com_tx_meta));
+static int ena_com_create_and_store_tx_meta_desc(struct ena_com_io_sq *io_sq,
+						 struct ena_com_tx_ctx *ena_tx_ctx,
+						 bool *have_meta)
+{
+	struct ena_com_tx_meta *ena_meta = &ena_tx_ctx->ena_meta;
 
-	return ena_com_sq_update_tail(io_sq);
+	/* When disable meta caching is set, don't bother to save the meta and
+	 * compare it to the stored version, just create the meta
+	 */
+	if (io_sq->disable_meta_caching) {
+		if (unlikely(!ena_tx_ctx->meta_valid))
+			return ENA_COM_INVAL;
+
+		*have_meta = true;
+		return ena_com_create_meta(io_sq, ena_meta);
+	} else if (ena_com_meta_desc_changed(io_sq, ena_tx_ctx)) {
+		*have_meta = true;
+		/* Cache the meta desc */
+		memcpy(&io_sq->cached_tx_meta, ena_meta,
+		       sizeof(struct ena_com_tx_meta));
+		return ena_com_create_meta(io_sq, ena_meta);
+	} else {
+		*have_meta = false;
+		return ENA_COM_OK;
+	}
 }
 
 static void ena_com_rx_set_flags(struct ena_com_rx_ctx *ena_rx_ctx,
@@ -380,12 +401,10 @@ int ena_com_prepare_tx(struct ena_com_io_sq *io_sq,
 	if (unlikely(rc))
 		return rc;
 
-	have_meta = ena_tx_ctx->meta_valid && ena_com_meta_desc_changed(io_sq,
-			ena_tx_ctx);
-	if (have_meta) {
-		rc = ena_com_create_and_store_tx_meta_desc(io_sq, ena_tx_ctx);
-		if (unlikely(rc))
-			return rc;
+	rc = ena_com_create_and_store_tx_meta_desc(io_sq, ena_tx_ctx, &have_meta);
+	if (unlikely(rc)) {
+		ena_trc_err("failed to create and store tx meta desc\n");
+		return rc;
 	}
 
 	/* If the caller doesn't want to send packets */
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 07/29] net/ena/base: add accelerated LLQ mode

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> In order to use the accelerated LLQ, the driver must limit the Tx burst

I would be nice to show LLQ means "Low-latency Queue"

> and be aware that the device has the meta caching disabled. In that
> situation, the meta descriptor must be valid on each Tx packet.
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>

<...>

Re: [dpdk-dev] [PATCH v2 07/29] net/ena/base: add accelerated LLQ mode

[Michał Krawczyk]

czw., 2 kwi 2020 o 14:41 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > In order to use the accelerated LLQ, the driver must limit the Tx burst
>
> I would be nice to show LLQ means "Low-latency Queue"
>

Ok - will do that in v3.

> > and be aware that the device has the meta caching disabled. In that
> > situation, the meta descriptor must be valid on each Tx packet.
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
>
> <...>
>

[dpdk-dev] [PATCH v2 08/29] net/ena/base: fix documentation of the functions

[Michal Krawczyk]

The documentation format was aligned and few typos were fixed.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h
index 9f2c6ea8ee..9b87fa94e4 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -370,7 +370,7 @@ extern "C" {
  */
 int ena_com_mmio_reg_read_request_init(struct ena_com_dev *ena_dev);
 
-/* ena_com_set_mmio_read_mode - Enable/disable the mmio reg read mechanism
+/* ena_com_set_mmio_read_mode - Enable/disable the indirect mmio reg read mechanism
  * @ena_dev: ENA communication layer struct
  * @readless_supported: readless mode (enable/disable)
  */
@@ -504,7 +504,7 @@ void ena_com_set_admin_auto_polling_mode(struct ena_com_dev *ena_dev,
 /* ena_com_admin_q_comp_intr_handler - admin queue interrupt handler
  * @ena_dev: ENA communication layer struct
  *
- * This method go over the admin completion queue and wake up all the pending
+ * This method goes over the admin completion queue and wakes up all the pending
  * threads that wait on the commands wait event.
  *
  * @note: Should be called after MSI-X interrupt.
@@ -514,7 +514,7 @@ void ena_com_admin_q_comp_intr_handler(struct ena_com_dev *ena_dev);
 /* ena_com_aenq_intr_handler - AENQ interrupt handler
  * @ena_dev: ENA communication layer struct
  *
- * This method go over the async event notification queue and call the proper
+ * This method goes over the async event notification queue and calls the proper
  * aenq handler.
  */
 void ena_com_aenq_intr_handler(struct ena_com_dev *dev, void *data);
@@ -531,14 +531,14 @@ void ena_com_abort_admin_commands(struct ena_com_dev *ena_dev);
 /* ena_com_wait_for_abort_completion - Wait for admin commands abort.
  * @ena_dev: ENA communication layer struct
  *
- * This method wait until all the outstanding admin commands will be completed.
+ * This method waits until all the outstanding admin commands are completed.
  */
 void ena_com_wait_for_abort_completion(struct ena_com_dev *ena_dev);
 
 /* ena_com_validate_version - Validate the device parameters
  * @ena_dev: ENA communication layer struct
  *
- * This method validate the device parameters are the same as the saved
+ * This method verifies the device parameters are the same as the saved
  * parameters in ena_dev.
  * This method is useful after device reset, to validate the device mac address
  * and the device offloads are the same as before the reset.
@@ -732,7 +732,7 @@ int ena_com_set_hash_ctrl(struct ena_com_dev *ena_dev);
  *
  * Retrieve the hash control from the device.
  *
- * @note, If the caller called ena_com_fill_hash_ctrl but didn't flash
+ * @note: If the caller called ena_com_fill_hash_ctrl but didn't flash
  * it to the device, the new configuration will be lost.
  *
  * @return: 0 on Success and negative value otherwise.
@@ -784,7 +784,7 @@ int ena_com_indirect_table_set(struct ena_com_dev *ena_dev);
  *
  * Retrieve the RSS indirection table from the device.
  *
- * @note: If the caller called ena_com_indirect_table_fill_entry but didn't flash
+ * @note: If the caller called ena_com_indirect_table_fill_entry but didn't flush
  * it to the device, the new configuration will be lost.
  *
  * @return: 0 on Success and negative value otherwise.
@@ -810,14 +810,14 @@ int ena_com_allocate_debug_area(struct ena_com_dev *ena_dev,
 /* ena_com_delete_debug_area - Free the debug area resources.
  * @ena_dev: ENA communication layer struct
  *
- * Free the allocate debug area.
+ * Free the allocated debug area.
  */
 void ena_com_delete_debug_area(struct ena_com_dev *ena_dev);
 
 /* ena_com_delete_host_info - Free the host info resources.
  * @ena_dev: ENA communication layer struct
  *
- * Free the allocate host info.
+ * Free the allocated host info.
  */
 void ena_com_delete_host_info(struct ena_com_dev *ena_dev);
 
@@ -858,9 +858,9 @@ int ena_com_destroy_io_cq(struct ena_com_dev *ena_dev,
  * @cmd_completion: command completion return value.
  * @cmd_comp_size: command completion size.
 
- * Submit an admin command and then wait until the device will return a
+ * Submit an admin command and then wait until the device returns a
  * completion.
- * The completion will be copyed into cmd_comp.
+ * The completion will be copied into cmd_comp.
  *
  * @return - 0 on success, negative value on failure.
  */
@@ -949,7 +949,7 @@ static inline void ena_com_disable_adaptive_moderation(struct ena_com_dev *ena_d
  * @intr_reg: interrupt register to update.
  * @rx_delay_interval: Rx interval in usecs
  * @tx_delay_interval: Tx interval in usecs
- * @unmask: unask enable/disable
+ * @unmask: unmask enable/disable
  *
  * Prepare interrupt update register with the supplied parameters.
  */
-- 
2.20.1

[dpdk-dev] [PATCH v2 09/29] net/ena/base: fix indentation in cq polling

[Michal Krawczyk]

The spaces instead of tabs were used for the indent.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index d15b7f22dc..7c1d0aef20 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -532,11 +532,11 @@ static int ena_com_wait_and_process_admin_cq_polling(struct ena_comp_ctx *comp_c
 	timeout = ENA_GET_SYSTEM_TIMEOUT(admin_queue->completion_timeout);
 
 	while (1) {
-                ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags);
-                ena_com_handle_admin_completion(admin_queue);
-                ENA_SPINLOCK_UNLOCK(admin_queue->q_lock, flags);
+		ENA_SPINLOCK_LOCK(admin_queue->q_lock, flags);
+		ena_com_handle_admin_completion(admin_queue);
+		ENA_SPINLOCK_UNLOCK(admin_queue->q_lock, flags);
 
-                if (comp_ctx->status != ENA_CMD_SUBMITTED)
+		if (comp_ctx->status != ENA_CMD_SUBMITTED)
 			break;
 
 		if (ENA_TIME_EXPIRE(timeout)) {
-- 
2.20.1

[dpdk-dev] [PATCH v2 10/29] net/ena/base: add error logs when preparing Tx

[Michal Krawczyk]

To make the debugging easier, the error logs were added in the Tx path.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_eth_com.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ena/base/ena_eth_com.c b/drivers/net/ena/base/ena_eth_com.c
index aabc294fb7..747450fec5 100644
--- a/drivers/net/ena/base/ena_eth_com.c
+++ b/drivers/net/ena/base/ena_eth_com.c
@@ -148,8 +148,10 @@ static int ena_com_close_bounce_buffer(struct ena_com_io_sq *io_sq)
 	if (pkt_ctrl->idx) {
 		rc = ena_com_write_bounce_buffer_to_dev(io_sq,
 							pkt_ctrl->curr_bounce_buf);
-		if (unlikely(rc))
+		if (unlikely(rc)) {
+			ena_trc_err("failed to write bounce buffer to device\n");
 			return rc;
+		}
 
 		pkt_ctrl->curr_bounce_buf =
 			ena_com_get_next_bounce_buffer(&io_sq->bounce_buf_ctrl);
@@ -179,8 +181,10 @@ static int ena_com_sq_update_llq_tail(struct ena_com_io_sq *io_sq)
 	if (!pkt_ctrl->descs_left_in_line) {
 		rc = ena_com_write_bounce_buffer_to_dev(io_sq,
 							pkt_ctrl->curr_bounce_buf);
-		if (unlikely(rc))
+		if (unlikely(rc)) {
+			ena_trc_err("failed to write bounce buffer to device\n");
 			return rc;
+		}
 
 		pkt_ctrl->curr_bounce_buf =
 			ena_com_get_next_bounce_buffer(&io_sq->bounce_buf_ctrl);
@@ -394,8 +398,10 @@ int ena_com_prepare_tx(struct ena_com_io_sq *io_sq,
 	}
 
 	if (unlikely(io_sq->mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV
-		     && !buffer_to_push))
+		     && !buffer_to_push)) {
+		ena_trc_err("push header wasn't provided on LLQ mode\n");
 		return ENA_COM_INVAL;
+	}
 
 	rc = ena_com_write_header_to_bounce(io_sq, buffer_to_push, header_len);
 	if (unlikely(rc))
@@ -410,6 +416,8 @@ int ena_com_prepare_tx(struct ena_com_io_sq *io_sq,
 	/* If the caller doesn't want to send packets */
 	if (unlikely(!num_bufs && !header_len)) {
 		rc = ena_com_close_bounce_buffer(io_sq);
+		if (rc)
+			ena_trc_err("failed to write buffers to LLQ\n");
 		*nb_hw_desc = io_sq->tail - start_tail;
 		return rc;
 	}
@@ -469,8 +477,10 @@ int ena_com_prepare_tx(struct ena_com_io_sq *io_sq,
 		/* The first desc share the same desc as the header */
 		if (likely(i != 0)) {
 			rc = ena_com_sq_update_tail(io_sq);
-			if (unlikely(rc))
+			if (unlikely(rc)) {
+				ena_trc_err("failed to update sq tail\n");
 				return rc;
+			}
 
 			desc = get_sq_desc(io_sq);
 			if (unlikely(!desc))
@@ -499,10 +509,14 @@ int ena_com_prepare_tx(struct ena_com_io_sq *io_sq,
 	desc->len_ctrl |= ENA_ETH_IO_TX_DESC_LAST_MASK;
 
 	rc = ena_com_sq_update_tail(io_sq);
-	if (unlikely(rc))
+	if (unlikely(rc)) {
+		ena_trc_err("failed to update sq tail of the last descriptor\n");
 		return rc;
+	}
 
 	rc = ena_com_close_bounce_buffer(io_sq);
+	if (rc)
+		ena_trc_err("failed when closing bounce buffer\n");
 
 	*nb_hw_desc = io_sq->tail - start_tail;
 	return rc;
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 10/29] net/ena/base: add error logs when preparing Tx

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> To make the debugging easier, the error logs were added in the Tx path.
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> ---
>  drivers/net/ena/base/ena_eth_com.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ena/base/ena_eth_com.c b/drivers/net/ena/base/ena_eth_com.c
> index aabc294fb7..747450fec5 100644
> --- a/drivers/net/ena/base/ena_eth_com.c
> +++ b/drivers/net/ena/base/ena_eth_com.c
> @@ -148,8 +148,10 @@ static int ena_com_close_bounce_buffer(struct ena_com_io_sq *io_sq)
>  	if (pkt_ctrl->idx) {
>  		rc = ena_com_write_bounce_buffer_to_dev(io_sq,
>  							pkt_ctrl->curr_bounce_buf);
> -		if (unlikely(rc))
> +		if (unlikely(rc)) {
> +			ena_trc_err("failed to write bounce buffer to device\n");

Is 'ena_trc_err()' for datapath?

DPDK has two types of logs,
- for control path, dynamic logging, whose logging level can be dynamically changed
- for data path, a compile time controlled logging, this is to prevent
performance loss in data path. For ena these flags are
CONFIG_RTE_LIBRTE_ENA_DEBUG_RX=n
CONFIG_RTE_LIBRTE_ENA_DEBUG_TX=n
CONFIG_RTE_LIBRTE_ENA_DEBUG_TX_FREE=n


I can see ena also has 'RTE_LIBRTE_ENA_COM_DEBUG' compile to flag, which
controls above 'ena_trc_err' variants. If this is control path, what do you
think removing the compile time flag and convert these 'COM' logs to dynamic
logging?

Re: [dpdk-dev] [PATCH v2 10/29] net/ena/base: add error logs when preparing Tx

[Michał Krawczyk]

czw., 2 kwi 2020 o 14:53 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > To make the debugging easier, the error logs were added in the Tx path.
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> > ---
> >  drivers/net/ena/base/ena_eth_com.c | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ena/base/ena_eth_com.c b/drivers/net/ena/base/ena_eth_com.c
> > index aabc294fb7..747450fec5 100644
> > --- a/drivers/net/ena/base/ena_eth_com.c
> > +++ b/drivers/net/ena/base/ena_eth_com.c
> > @@ -148,8 +148,10 @@ static int ena_com_close_bounce_buffer(struct ena_com_io_sq *io_sq)
> >       if (pkt_ctrl->idx) {
> >               rc = ena_com_write_bounce_buffer_to_dev(io_sq,
> >                                                       pkt_ctrl->curr_bounce_buf);
> > -             if (unlikely(rc))
> > +             if (unlikely(rc)) {
> > +                     ena_trc_err("failed to write bounce buffer to device\n");
>
> Is 'ena_trc_err()' for datapath?
>

ena_trc_err() is being used in both control and datapath.

> DPDK has two types of logs,
> - for control path, dynamic logging, whose logging level can be dynamically changed
> - for data path, a compile time controlled logging, this is to prevent
> performance loss in data path. For ena these flags are
> CONFIG_RTE_LIBRTE_ENA_DEBUG_RX=n
> CONFIG_RTE_LIBRTE_ENA_DEBUG_TX=n
> CONFIG_RTE_LIBRTE_ENA_DEBUG_TX_FREE=n
>
>
> I can see ena also has 'RTE_LIBRTE_ENA_COM_DEBUG' compile to flag, which
> controls above 'ena_trc_err' variants. If this is control path, what do you
> think removing the compile time flag and convert these 'COM' logs to dynamic
> logging?

I agree with you that it would be great if we would use dynamic
logging for control path - I must talk with engineers responsible for
ena_com to raise that issue and, hopefully, it'll be possible to
adjust that in the future.

I suggest to postpone that kind of change for the next release, as
making ena_com changes is a bit time consuming.

[dpdk-dev] [PATCH v2 11/29] net/ena/base: use 48-bit memory addresses in ena_com

[Michal Krawczyk]

ENA device is using 48-bit memory for IO. because of that, the upper
limit had to be updated.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.c                  | 2 +-
 drivers/net/ena/base/ena_defs/ena_common_defs.h | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 7c1d0aef20..b7749209b3 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -73,7 +73,7 @@ static int ena_com_mem_addr_set(struct ena_com_dev *ena_dev,
 	}
 
 	ena_addr->mem_addr_low = lower_32_bits(addr);
-	ena_addr->mem_addr_high = upper_32_bits(addr);
+	ena_addr->mem_addr_high = (u16)upper_32_bits(addr);
 
 	return 0;
 }
diff --git a/drivers/net/ena/base/ena_defs/ena_common_defs.h b/drivers/net/ena/base/ena_defs/ena_common_defs.h
index 1818c29a87..349474d265 100644
--- a/drivers/net/ena/base/ena_defs/ena_common_defs.h
+++ b/drivers/net/ena/base/ena_defs/ena_common_defs.h
@@ -9,10 +9,14 @@
 #define ENA_COMMON_SPEC_VERSION_MAJOR        2
 #define ENA_COMMON_SPEC_VERSION_MINOR        0
 
+/* ENA operates with 48-bit memory addresses. ena_mem_addr_t */
 struct ena_common_mem_addr {
 	uint32_t mem_addr_low;
 
-	uint32_t mem_addr_high;
+	uint16_t mem_addr_high;
+
+	/* MBZ */
+	uint16_t reserved16;
 };
 
 #endif /* _ENA_COMMON_H_ */
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 11/29] net/ena/base: use 48-bit memory addresses in ena_com

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> ENA device is using 48-bit memory for IO. because of that, the upper
> limit had to be updated.

What is the impact of this change, and what is the reason of the change? Can you
please explain in the commit log?

> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> ---
>  drivers/net/ena/base/ena_com.c                  | 2 +-
>  drivers/net/ena/base/ena_defs/ena_common_defs.h | 6 +++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
> index 7c1d0aef20..b7749209b3 100644
> --- a/drivers/net/ena/base/ena_com.c
> +++ b/drivers/net/ena/base/ena_com.c
> @@ -73,7 +73,7 @@ static int ena_com_mem_addr_set(struct ena_com_dev *ena_dev,
>  	}
>  
>  	ena_addr->mem_addr_low = lower_32_bits(addr);
> -	ena_addr->mem_addr_high = upper_32_bits(addr);
> +	ena_addr->mem_addr_high = (u16)upper_32_bits(addr);
>  
>  	return 0;
>  }
> diff --git a/drivers/net/ena/base/ena_defs/ena_common_defs.h b/drivers/net/ena/base/ena_defs/ena_common_defs.h
> index 1818c29a87..349474d265 100644
> --- a/drivers/net/ena/base/ena_defs/ena_common_defs.h
> +++ b/drivers/net/ena/base/ena_defs/ena_common_defs.h
> @@ -9,10 +9,14 @@
>  #define ENA_COMMON_SPEC_VERSION_MAJOR        2
>  #define ENA_COMMON_SPEC_VERSION_MINOR        0
>  
> +/* ENA operates with 48-bit memory addresses. ena_mem_addr_t */
>  struct ena_common_mem_addr {
>  	uint32_t mem_addr_low;
>  
> -	uint32_t mem_addr_high;
> +	uint16_t mem_addr_high;
> +
> +	/* MBZ */
> +	uint16_t reserved16;
>  };
>  
>  #endif /* _ENA_COMMON_H_ */
> 

Re: [dpdk-dev] [PATCH v2 11/29] net/ena/base: use 48-bit memory addresses in ena_com

[Michał Krawczyk]

czw., 2 kwi 2020 o 14:55 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > ENA device is using 48-bit memory for IO. because of that, the upper
> > limit had to be updated.
>
> What is the impact of this change, and what is the reason of the change? Can you
> please explain in the commit log?
>

It's just a cosmetic change in our case and to align it with what the
device is in fact doing. Structure 'ena_common_mem_addr' could be
misleading - it was defining 64 bits for address, while the device in
fact could read up to 48 bits.

This value (address) is being verified before being set in
'ena_com_mem_addr_set()' function, so the functionality remains still
the same.

I'll update the commit log in v3.

[dpdk-dev] [PATCH v2 12/29] net/ena/base: fix types for printing timestamps

[Michal Krawczyk]

Because ena_com is being used by multiple platforms which are using
different C versions, PRIu64 cannot be used directly and must be defined
in the platform file.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.c       | 2 +-
 drivers/net/ena/base/ena_plat_dpdk.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index b7749209b3..45f1647699 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -2068,7 +2068,7 @@ void ena_com_aenq_intr_handler(struct ena_com_dev *dev, void *data)
 		timestamp = (u64)aenq_common->timestamp_low |
 			((u64)aenq_common->timestamp_high << 32);
 		ENA_TOUCH(timestamp); /* In case debug is disabled */
-		ena_trc_dbg("AENQ! Group[%x] Syndrom[%x] timestamp: [%"PRIu64"]\n",
+		ena_trc_dbg("AENQ! Group[%x] Syndrom[%x] timestamp: [%" ENA_PRIu64 "s]\n",
 			    aenq_common->group,
 			    aenq_common->syndrom,
 			    timestamp);
diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index 2989df8f7e..c69426a062 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -309,5 +309,7 @@ void ena_rss_key_fill(void *key, size_t size);
 
 #define ENA_INTR_INITIAL_TX_INTERVAL_USECS_PLAT 0
 
+#define ENA_PRIu64 PRIu64
+
 #include "ena_includes.h"
 #endif /* DPDK_ENA_COM_ENA_PLAT_DPDK_H_ */
-- 
2.20.1

[dpdk-dev] [PATCH v2 13/29] net/ena/base: fix indentation of multiple defines

[Michal Krawczyk]

As the alignemnt of the defines wasn't valid, it was removed at all, so
instead of using multiple spaces or tabs, the single space after define
name is being used.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_com.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h
index 9b87fa94e4..274c0cd9f5 100644
--- a/drivers/net/ena/base/ena_com.h
+++ b/drivers/net/ena/base/ena_com.h
@@ -8,9 +8,9 @@
 
 #include "ena_plat.h"
 
-#define ENA_MAX_NUM_IO_QUEUES		128U
+#define ENA_MAX_NUM_IO_QUEUES 128U
 /* We need to queues for each IO (on for Tx and one for Rx) */
-#define ENA_TOTAL_NUM_QUEUES		(2 * (ENA_MAX_NUM_IO_QUEUES))
+#define ENA_TOTAL_NUM_QUEUES (2 * (ENA_MAX_NUM_IO_QUEUES))
 
 #define ENA_MAX_HANDLERS 256
 
@@ -33,9 +33,9 @@
 
 #define ENA_HASH_KEY_SIZE				40
 
-#define ENA_HW_HINTS_NO_TIMEOUT				0xFFFF
+#define ENA_HW_HINTS_NO_TIMEOUT 0xFFFF
 
-#define ENA_FEATURE_MAX_QUEUE_EXT_VER	1
+#define ENA_FEATURE_MAX_QUEUE_EXT_VER 1
 
 struct ena_llq_configurations {
 	enum ena_admin_llq_header_location llq_header_location;
-- 
2.20.1

[dpdk-dev] [PATCH v2 14/29] net/ena/base: update gen date and commit

[Michal Krawczyk]

The current ena_com version was generated on 25.09.2019.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_defs/ena_gen_info.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ena/base/ena_defs/ena_gen_info.h b/drivers/net/ena/base/ena_defs/ena_gen_info.h
index 019b1fdb79..f486e9fe6e 100644
--- a/drivers/net/ena/base/ena_defs/ena_gen_info.h
+++ b/drivers/net/ena/base/ena_defs/ena_gen_info.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
+ * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
-#define	ENA_GEN_DATE	"Wed Mar 20 10:40:42 STD 2019"
-#define	ENA_GEN_COMMIT	"1476830"
+#define	ENA_GEN_DATE	"Wed Sep 25 11:32:57 UTC 2019"
+#define	ENA_GEN_COMMIT	"952697a9e0d3"
-- 
2.20.1

[dpdk-dev] [PATCH v2 15/29] net/ena: set IO ring size to the valid value

[Michal Krawczyk]

IO rings were configured with the maximum allowed size for the Tx/Rx
rings. However, the application could decide to create smaller rings.

This patch is using value stored in the ring instead of the value from
the adapter which is indicating the maximum allowed value.

Fixes: df238f84c0a2 ("net/ena: recreate HW IO rings on start and stop")
Cc: stable@dpdk.org

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 21c25b21b1..13a016227c 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1099,16 +1099,15 @@ static int ena_create_io_queue(struct ena_ring *ring)
 		ena_qid = ENA_IO_TXQ_IDX(ring->id);
 		ctx.direction = ENA_COM_IO_QUEUE_DIRECTION_TX;
 		ctx.mem_queue_type = ena_dev->tx_mem_queue_type;
-		ctx.queue_size = adapter->tx_ring_size;
 		for (i = 0; i < ring->ring_size; i++)
 			ring->empty_tx_reqs[i] = i;
 	} else {
 		ena_qid = ENA_IO_RXQ_IDX(ring->id);
 		ctx.direction = ENA_COM_IO_QUEUE_DIRECTION_RX;
-		ctx.queue_size = adapter->rx_ring_size;
 		for (i = 0; i < ring->ring_size; i++)
 			ring->empty_rx_reqs[i] = i;
 	}
+	ctx.queue_size = ring->ring_size;
 	ctx.qid = ena_qid;
 	ctx.msix_vector = -1; /* interrupts not used */
 	ctx.numa_node = ring->numa_socket_id;
-- 
2.20.1

[dpdk-dev] [PATCH v2 16/29] net/ena: refactor getting IO queues capabilities

[Michal Krawczyk]

Reading values from the device is about the maximum capabilities of the
device. Because of that, the names of the fields storing those values,
functions and temporary variables, should be more descriptive in order
to improve self documentation fo the code.

In connection with this, the way of getting maximum queue size could be
simplified - no hardcoded values are needed, as the device is going to
send it's capabilities anyway.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 101 ++++++++++++++++-------------------
 drivers/net/ena/ena_ethdev.h |  11 ++--
 2 files changed, 52 insertions(+), 60 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 13a016227c..d5f700093f 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -82,9 +82,6 @@ struct ena_stats {
 #define ENA_STAT_GLOBAL_ENTRY(stat) \
 	ENA_STAT_ENTRY(stat, dev)
 
-#define ENA_MAX_RING_SIZE_RX 8192
-#define ENA_MAX_RING_SIZE_TX 1024
-
 /*
  * Each rte_memzone should have unique name.
  * To satisfy it, count number of allocation and add it to name.
@@ -845,29 +842,26 @@ static int ena_check_valid_conf(struct ena_adapter *adapter)
 }
 
 static int
-ena_calc_queue_size(struct ena_calc_queue_size_ctx *ctx)
+ena_calc_io_queue_size(struct ena_calc_queue_size_ctx *ctx)
 {
 	struct ena_admin_feature_llq_desc *llq = &ctx->get_feat_ctx->llq;
 	struct ena_com_dev *ena_dev = ctx->ena_dev;
-	uint32_t tx_queue_size = ENA_MAX_RING_SIZE_TX;
-	uint32_t rx_queue_size = ENA_MAX_RING_SIZE_RX;
+	uint32_t max_tx_queue_size;
+	uint32_t max_rx_queue_size;
 
 	if (ena_dev->supported_features & BIT(ENA_ADMIN_MAX_QUEUES_EXT)) {
 		struct ena_admin_queue_ext_feature_fields *max_queue_ext =
 			&ctx->get_feat_ctx->max_queue_ext.max_queue_ext;
-		rx_queue_size = RTE_MIN(rx_queue_size,
-			max_queue_ext->max_rx_cq_depth);
-		rx_queue_size = RTE_MIN(rx_queue_size,
+		max_rx_queue_size = RTE_MIN(max_queue_ext->max_rx_cq_depth,
 			max_queue_ext->max_rx_sq_depth);
-		tx_queue_size = RTE_MIN(tx_queue_size,
-			max_queue_ext->max_tx_cq_depth);
+		max_tx_queue_size = max_queue_ext->max_tx_cq_depth;
 
 		if (ena_dev->tx_mem_queue_type ==
 		    ENA_ADMIN_PLACEMENT_POLICY_DEV) {
-			tx_queue_size = RTE_MIN(tx_queue_size,
+			max_tx_queue_size = RTE_MIN(max_tx_queue_size,
 				llq->max_llq_depth);
 		} else {
-			tx_queue_size = RTE_MIN(tx_queue_size,
+			max_tx_queue_size = RTE_MIN(max_tx_queue_size,
 				max_queue_ext->max_tx_sq_depth);
 		}
 
@@ -878,39 +872,36 @@ ena_calc_queue_size(struct ena_calc_queue_size_ctx *ctx)
 	} else {
 		struct ena_admin_queue_feature_desc *max_queues =
 			&ctx->get_feat_ctx->max_queues;
-		rx_queue_size = RTE_MIN(rx_queue_size,
-			max_queues->max_cq_depth);
-		rx_queue_size = RTE_MIN(rx_queue_size,
+		max_rx_queue_size = RTE_MIN(max_queues->max_cq_depth,
 			max_queues->max_sq_depth);
-		tx_queue_size = RTE_MIN(tx_queue_size,
-			max_queues->max_cq_depth);
+		max_tx_queue_size = max_queues->max_cq_depth;
 
 		if (ena_dev->tx_mem_queue_type ==
 		    ENA_ADMIN_PLACEMENT_POLICY_DEV) {
-			tx_queue_size = RTE_MIN(tx_queue_size,
+			max_tx_queue_size = RTE_MIN(max_tx_queue_size,
 				llq->max_llq_depth);
 		} else {
-			tx_queue_size = RTE_MIN(tx_queue_size,
+			max_tx_queue_size = RTE_MIN(max_tx_queue_size,
 				max_queues->max_sq_depth);
 		}
 
 		ctx->max_rx_sgl_size = RTE_MIN(ENA_PKT_MAX_BUFS,
-			max_queues->max_packet_tx_descs);
-		ctx->max_tx_sgl_size = RTE_MIN(ENA_PKT_MAX_BUFS,
 			max_queues->max_packet_rx_descs);
+		ctx->max_tx_sgl_size = RTE_MIN(ENA_PKT_MAX_BUFS,
+			max_queues->max_packet_tx_descs);
 	}
 
 	/* Round down to the nearest power of 2 */
-	rx_queue_size = rte_align32prevpow2(rx_queue_size);
-	tx_queue_size = rte_align32prevpow2(tx_queue_size);
+	max_rx_queue_size = rte_align32prevpow2(max_rx_queue_size);
+	max_tx_queue_size = rte_align32prevpow2(max_tx_queue_size);
 
-	if (unlikely(rx_queue_size == 0 || tx_queue_size == 0)) {
+	if (unlikely(max_rx_queue_size == 0 || max_tx_queue_size == 0)) {
 		PMD_INIT_LOG(ERR, "Invalid queue size");
 		return -EFAULT;
 	}
 
-	ctx->rx_queue_size = rx_queue_size;
-	ctx->tx_queue_size = tx_queue_size;
+	ctx->max_tx_queue_size = max_tx_queue_size;
+	ctx->max_rx_queue_size = max_rx_queue_size;
 
 	return 0;
 }
@@ -1230,15 +1221,15 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
-	if (nb_desc > adapter->tx_ring_size) {
+	if (nb_desc > adapter->max_tx_ring_size) {
 		PMD_DRV_LOG(ERR,
 			"Unsupported size of TX queue (max size: %d)\n",
-			adapter->tx_ring_size);
+			adapter->max_tx_ring_size);
 		return -EINVAL;
 	}
 
 	if (nb_desc == RTE_ETH_DEV_FALLBACK_TX_RINGSIZE)
-		nb_desc = adapter->tx_ring_size;
+		nb_desc = adapter->max_tx_ring_size;
 
 	txq->port_id = dev->data->port_id;
 	txq->next_to_clean = 0;
@@ -1310,7 +1301,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 	}
 
 	if (nb_desc == RTE_ETH_DEV_FALLBACK_RX_RINGSIZE)
-		nb_desc = adapter->rx_ring_size;
+		nb_desc = adapter->max_rx_ring_size;
 
 	if (!rte_is_power_of_2(nb_desc)) {
 		PMD_DRV_LOG(ERR,
@@ -1319,10 +1310,10 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
-	if (nb_desc > adapter->rx_ring_size) {
+	if (nb_desc > adapter->max_rx_ring_size) {
 		PMD_DRV_LOG(ERR,
 			"Unsupported size of RX queue (max size: %d)\n",
-			adapter->rx_ring_size);
+			adapter->max_rx_ring_size);
 		return -EINVAL;
 	}
 
@@ -1654,10 +1645,10 @@ ena_set_queues_placement_policy(struct ena_adapter *adapter,
 	return 0;
 }
 
-static int ena_calc_io_queue_num(struct ena_com_dev *ena_dev,
-				 struct ena_com_dev_get_features_ctx *get_feat_ctx)
+static uint32_t ena_calc_max_io_queue_num(struct ena_com_dev *ena_dev,
+	struct ena_com_dev_get_features_ctx *get_feat_ctx)
 {
-	uint32_t io_tx_sq_num, io_tx_cq_num, io_rx_num, io_queue_num;
+	uint32_t io_tx_sq_num, io_tx_cq_num, io_rx_num, max_num_io_queues;
 
 	/* Regular queues capabilities */
 	if (ena_dev->supported_features & BIT(ENA_ADMIN_MAX_QUEUES_EXT)) {
@@ -1679,16 +1670,16 @@ static int ena_calc_io_queue_num(struct ena_com_dev *ena_dev,
 	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV)
 		io_tx_sq_num = get_feat_ctx->llq.max_llq_num;
 
-	io_queue_num = RTE_MIN(ENA_MAX_NUM_IO_QUEUES, io_rx_num);
-	io_queue_num = RTE_MIN(io_queue_num, io_tx_sq_num);
-	io_queue_num = RTE_MIN(io_queue_num, io_tx_cq_num);
+	max_num_io_queues = RTE_MIN(ENA_MAX_NUM_IO_QUEUES, io_rx_num);
+	max_num_io_queues = RTE_MIN(max_num_io_queues, io_tx_sq_num);
+	max_num_io_queues = RTE_MIN(max_num_io_queues, io_tx_cq_num);
 
-	if (unlikely(io_queue_num == 0)) {
+	if (unlikely(max_num_io_queues == 0)) {
 		PMD_DRV_LOG(ERR, "Number of IO queues should not be 0\n");
 		return -EFAULT;
 	}
 
-	return io_queue_num;
+	return max_num_io_queues;
 }
 
 static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
@@ -1701,6 +1692,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	struct ena_com_dev_get_features_ctx get_feat_ctx;
 	struct ena_llq_configurations llq_config;
 	const char *queue_type_str;
+	uint32_t max_num_io_queues;
 	int rc;
 
 	static int adapters_found;
@@ -1772,20 +1764,19 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 
 	calc_queue_ctx.ena_dev = ena_dev;
 	calc_queue_ctx.get_feat_ctx = &get_feat_ctx;
-	adapter->num_queues = ena_calc_io_queue_num(ena_dev,
-						    &get_feat_ctx);
 
-	rc = ena_calc_queue_size(&calc_queue_ctx);
-	if (unlikely((rc != 0) || (adapter->num_queues <= 0))) {
+	max_num_io_queues = ena_calc_max_io_queue_num(ena_dev, &get_feat_ctx);
+	rc = ena_calc_io_queue_size(&calc_queue_ctx);
+	if (unlikely((rc != 0) || (max_num_io_queues == 0))) {
 		rc = -EFAULT;
 		goto err_device_destroy;
 	}
 
-	adapter->tx_ring_size = calc_queue_ctx.tx_queue_size;
-	adapter->rx_ring_size = calc_queue_ctx.rx_queue_size;
-
+	adapter->max_tx_ring_size = calc_queue_ctx.max_tx_queue_size;
+	adapter->max_rx_ring_size = calc_queue_ctx.max_rx_queue_size;
 	adapter->max_tx_sgl_size = calc_queue_ctx.max_tx_sgl_size;
 	adapter->max_rx_sgl_size = calc_queue_ctx.max_rx_sgl_size;
+	adapter->max_num_io_queues = max_num_io_queues;
 
 	/* prepare ring structures */
 	ena_init_rings(adapter);
@@ -1904,9 +1895,9 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
 
 static void ena_init_rings(struct ena_adapter *adapter)
 {
-	int i;
+	size_t i;
 
-	for (i = 0; i < adapter->num_queues; i++) {
+	for (i = 0; i < adapter->max_num_io_queues; i++) {
 		struct ena_ring *ring = &adapter->tx_ring[i];
 
 		ring->configured = 0;
@@ -1918,7 +1909,7 @@ static void ena_init_rings(struct ena_adapter *adapter)
 		ring->sgl_size = adapter->max_tx_sgl_size;
 	}
 
-	for (i = 0; i < adapter->num_queues; i++) {
+	for (i = 0; i < adapter->max_num_io_queues; i++) {
 		struct ena_ring *ring = &adapter->rx_ring[i];
 
 		ring->configured = 0;
@@ -1982,21 +1973,21 @@ static int ena_infos_get(struct rte_eth_dev *dev,
 	dev_info->max_rx_pktlen  = adapter->max_mtu;
 	dev_info->max_mac_addrs = 1;
 
-	dev_info->max_rx_queues = adapter->num_queues;
-	dev_info->max_tx_queues = adapter->num_queues;
+	dev_info->max_rx_queues = adapter->max_num_io_queues;
+	dev_info->max_tx_queues = adapter->max_num_io_queues;
 	dev_info->reta_size = ENA_RX_RSS_TABLE_SIZE;
 
 	adapter->tx_supported_offloads = tx_feat;
 	adapter->rx_supported_offloads = rx_feat;
 
-	dev_info->rx_desc_lim.nb_max = adapter->rx_ring_size;
+	dev_info->rx_desc_lim.nb_max = adapter->max_rx_ring_size;
 	dev_info->rx_desc_lim.nb_min = ENA_MIN_RING_DESC;
 	dev_info->rx_desc_lim.nb_seg_max = RTE_MIN(ENA_PKT_MAX_BUFS,
 					adapter->max_rx_sgl_size);
 	dev_info->rx_desc_lim.nb_mtu_seg_max = RTE_MIN(ENA_PKT_MAX_BUFS,
 					adapter->max_rx_sgl_size);
 
-	dev_info->tx_desc_lim.nb_max = adapter->tx_ring_size;
+	dev_info->tx_desc_lim.nb_max = adapter->max_tx_ring_size;
 	dev_info->tx_desc_lim.nb_min = ENA_MIN_RING_DESC;
 	dev_info->tx_desc_lim.nb_seg_max = RTE_MIN(ENA_PKT_MAX_BUFS,
 					adapter->max_tx_sgl_size);
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index c1457defeb..99d1fba64d 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -21,6 +21,7 @@
 #define ENA_NAME_MAX_LEN	20
 #define ENA_PKT_MAX_BUFS	17
 #define ENA_RX_BUF_MIN_SIZE	1400
+#define ENA_DEFAULT_RING_SIZE	1024
 
 #define ENA_MIN_MTU		128
 
@@ -46,8 +47,8 @@ struct ena_tx_buffer {
 struct ena_calc_queue_size_ctx {
 	struct ena_com_dev_get_features_ctx *get_feat_ctx;
 	struct ena_com_dev *ena_dev;
-	u16 rx_queue_size;
-	u16 tx_queue_size;
+	u32 max_rx_queue_size;
+	u32 max_tx_queue_size;
 	u16 max_tx_sgl_size;
 	u16 max_rx_sgl_size;
 };
@@ -159,15 +160,15 @@ struct ena_adapter {
 
 	/* TX */
 	struct ena_ring tx_ring[ENA_MAX_NUM_QUEUES] __rte_cache_aligned;
-	int tx_ring_size;
+	u32 max_tx_ring_size;
 	u16 max_tx_sgl_size;
 
 	/* RX */
 	struct ena_ring rx_ring[ENA_MAX_NUM_QUEUES] __rte_cache_aligned;
-	int rx_ring_size;
+	u32 max_rx_ring_size;
 	u16 max_rx_sgl_size;
 
-	u16 num_queues;
+	u32 max_num_io_queues;
 	u16 max_mtu;
 	struct ena_offloads offloads;
 
-- 
2.20.1

[dpdk-dev] [PATCH v2 17/29] net/ena: add support for large LLQ headers

[Michal Krawczyk]

If the device supports larger LLQ (Low Latency Queue) headers, the user
can activate them by enabling CONFIG_RTE_LIBRTE_ENA_LARGE_LLQ_HEADERS
flag in the configuration file.

If the device isn't supporting this feature, the default value will be
used.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
v2:
  * Use devargs instead of compilation options

 doc/guides/nics/ena.rst      |   8 +++
 drivers/net/ena/ena_ethdev.c | 110 +++++++++++++++++++++++++++++++++--
 drivers/net/ena/ena_ethdev.h |   2 +
 3 files changed, 114 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/ena.rst b/doc/guides/nics/ena.rst
index bbf27f235a..d5081e91e5 100644
--- a/doc/guides/nics/ena.rst
+++ b/doc/guides/nics/ena.rst
@@ -95,6 +95,14 @@ Configuration information
    * **CONFIG_RTE_LIBRTE_ENA_COM_DEBUG** (default n): Enables or disables debug
      logging of low level tx/rx logic in ena_com(base) within the ENA PMD driver.
 
+**Runtime Configuration Parameters**
+
+   * **large_llq_hdr** (default 0)
+
+     Enables or disables usage of large LLQ headers. This option will have
+     effect only if the device also supports large LLQ headers. Otherwise, the
+     default value will be used.
+
 **ENA Configuration Parameters**
 
    * **Number of Queues**
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index d5f700093f..f23efbe464 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -13,6 +13,7 @@
 #include <rte_errno.h>
 #include <rte_version.h>
 #include <rte_net.h>
+#include <rte_kvargs.h>
 
 #include "ena_ethdev.h"
 #include "ena_logs.h"
@@ -82,6 +83,9 @@ struct ena_stats {
 #define ENA_STAT_GLOBAL_ENTRY(stat) \
 	ENA_STAT_ENTRY(stat, dev)
 
+/* Device arguments */
+#define ENA_DEVARG_LARGE_LLQ_HDR "large_llq_hdr"
+
 /*
  * Each rte_memzone should have unique name.
  * To satisfy it, count number of allocation and add it to name.
@@ -231,6 +235,11 @@ static int ena_xstats_get_by_id(struct rte_eth_dev *dev,
 				const uint64_t *ids,
 				uint64_t *values,
 				unsigned int n);
+static int ena_process_bool_devarg(const char *key,
+				   const char *value,
+				   void *opaque);
+static int ena_parse_devargs(struct ena_adapter *adapter,
+			     struct rte_devargs *devargs);
 
 static const struct eth_dev_ops ena_dev_ops = {
 	.dev_configure        = ena_dev_configure,
@@ -842,7 +851,8 @@ static int ena_check_valid_conf(struct ena_adapter *adapter)
 }
 
 static int
-ena_calc_io_queue_size(struct ena_calc_queue_size_ctx *ctx)
+ena_calc_io_queue_size(struct ena_calc_queue_size_ctx *ctx,
+		       bool use_large_llq_hdr)
 {
 	struct ena_admin_feature_llq_desc *llq = &ctx->get_feat_ctx->llq;
 	struct ena_com_dev *ena_dev = ctx->ena_dev;
@@ -895,6 +905,21 @@ ena_calc_io_queue_size(struct ena_calc_queue_size_ctx *ctx)
 	max_rx_queue_size = rte_align32prevpow2(max_rx_queue_size);
 	max_tx_queue_size = rte_align32prevpow2(max_tx_queue_size);
 
+	if (use_large_llq_hdr) {
+		if ((llq->entry_size_ctrl_supported &
+		     ENA_ADMIN_LIST_ENTRY_SIZE_256B) &&
+		    (ena_dev->tx_mem_queue_type ==
+		     ENA_ADMIN_PLACEMENT_POLICY_DEV)) {
+			max_tx_queue_size /= 2;
+			PMD_INIT_LOG(INFO,
+				"Forcing large headers and decreasing maximum TX queue size to %d\n",
+				max_tx_queue_size);
+		} else {
+			PMD_INIT_LOG(ERR,
+				"Forcing large headers failed: LLQ is disabled or device does not support large headers\n");
+		}
+	}
+
 	if (unlikely(max_rx_queue_size == 0 || max_tx_queue_size == 0)) {
 		PMD_INIT_LOG(ERR, "Invalid queue size");
 		return -EFAULT;
@@ -1594,14 +1619,25 @@ static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer,
 }
 
 static inline void
-set_default_llq_configurations(struct ena_llq_configurations *llq_config)
+set_default_llq_configurations(struct ena_llq_configurations *llq_config,
+			       struct ena_admin_feature_llq_desc *llq,
+			       bool use_large_llq_hdr)
 {
 	llq_config->llq_header_location = ENA_ADMIN_INLINE_HEADER;
-	llq_config->llq_ring_entry_size = ENA_ADMIN_LIST_ENTRY_SIZE_128B;
 	llq_config->llq_stride_ctrl = ENA_ADMIN_MULTIPLE_DESCS_PER_ENTRY;
 	llq_config->llq_num_decs_before_header =
 		ENA_ADMIN_LLQ_NUM_DESCS_BEFORE_HEADER_2;
-	llq_config->llq_ring_entry_size_value = 128;
+
+	if (use_large_llq_hdr &&
+	    (llq->entry_size_ctrl_supported & ENA_ADMIN_LIST_ENTRY_SIZE_256B)) {
+		llq_config->llq_ring_entry_size =
+			ENA_ADMIN_LIST_ENTRY_SIZE_256B;
+		llq_config->llq_ring_entry_size_value = 256;
+	} else {
+		llq_config->llq_ring_entry_size =
+			ENA_ADMIN_LIST_ENTRY_SIZE_128B;
+		llq_config->llq_ring_entry_size_value = 128;
+	}
 }
 
 static int
@@ -1740,6 +1776,12 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	snprintf(adapter->name, ENA_NAME_MAX_LEN, "ena_%d",
 		 adapter->id_number);
 
+	rc = ena_parse_devargs(adapter, pci_dev->device.devargs);
+	if (rc != 0) {
+		PMD_INIT_LOG(CRIT, "Failed to parse devargs\n");
+		goto err;
+	}
+
 	/* device specific initialization routine */
 	rc = ena_device_init(ena_dev, &get_feat_ctx, &wd_state);
 	if (rc) {
@@ -1748,7 +1790,8 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	}
 	adapter->wd_state = wd_state;
 
-	set_default_llq_configurations(&llq_config);
+	set_default_llq_configurations(&llq_config, &get_feat_ctx.llq,
+		adapter->use_large_llq_hdr);
 	rc = ena_set_queues_placement_policy(adapter, ena_dev,
 					     &get_feat_ctx.llq, &llq_config);
 	if (unlikely(rc)) {
@@ -1766,7 +1809,8 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	calc_queue_ctx.get_feat_ctx = &get_feat_ctx;
 
 	max_num_io_queues = ena_calc_max_io_queue_num(ena_dev, &get_feat_ctx);
-	rc = ena_calc_io_queue_size(&calc_queue_ctx);
+	rc = ena_calc_io_queue_size(&calc_queue_ctx,
+		adapter->use_large_llq_hdr);
 	if (unlikely((rc != 0) || (max_num_io_queues == 0))) {
 		rc = -EFAULT;
 		goto err_device_destroy;
@@ -2582,6 +2626,59 @@ static int ena_xstats_get_by_id(struct rte_eth_dev *dev,
 	return valid;
 }
 
+static int ena_process_bool_devarg(const char *key,
+				   const char *value,
+				   void *opaque)
+{
+	struct ena_adapter *adapter = opaque;
+	bool bool_value;
+
+	/* Parse the value. */
+	if (strcmp(value, "1") == 0) {
+		bool_value = true;
+	} else if (strcmp(value, "0") == 0) {
+		bool_value = false;
+	} else {
+		PMD_INIT_LOG(ERR,
+			"Invalid value: '%s' for key '%s'. Accepted: '0' or '1'\n",
+			value, key);
+		return -EINVAL;
+	}
+
+	/* Now, assign it to the proper adapter field. */
+	if (strcmp(key, ENA_DEVARG_LARGE_LLQ_HDR) == 0)
+		adapter->use_large_llq_hdr = bool_value;
+
+	return 0;
+}
+
+static int ena_parse_devargs(struct ena_adapter *adapter,
+			     struct rte_devargs *devargs)
+{
+	static const char * const allowed_args[] = {
+		ENA_DEVARG_LARGE_LLQ_HDR,
+	};
+	struct rte_kvargs *kvlist;
+	int rc;
+
+	if (devargs == NULL)
+		return 0;
+
+	kvlist = rte_kvargs_parse(devargs->args, allowed_args);
+	if (kvlist == NULL) {
+		PMD_INIT_LOG(ERR, "Invalid device arguments: %s\n",
+			devargs->args);
+		return -EINVAL;
+	}
+
+	rc = rte_kvargs_process(kvlist, ENA_DEVARG_LARGE_LLQ_HDR,
+		ena_process_bool_devarg, adapter);
+
+	rte_kvargs_free(kvlist);
+
+	return rc;
+}
+
 /*********************************************************************
  *  PMD configuration
  *********************************************************************/
@@ -2608,6 +2705,7 @@ static struct rte_pci_driver rte_ena_pmd = {
 RTE_PMD_REGISTER_PCI(net_ena, rte_ena_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_ena, pci_id_ena_map);
 RTE_PMD_REGISTER_KMOD_DEP(net_ena, "* igb_uio | uio_pci_generic | vfio-pci");
+RTE_PMD_REGISTER_PARAM_STRING(net_ena, ENA_DEVARG_LARGE_LLQ_HDR "=<0|1>");
 
 RTE_INIT(ena_init_log)
 {
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 99d1fba64d..0e82c894f4 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -200,6 +200,8 @@ struct ena_adapter {
 	bool trigger_reset;
 
 	bool wd_state;
+
+	bool use_large_llq_hdr;
 };
 
 #endif /* _ENA_ETHDEV_H_ */
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 17/29] net/ena: add support for large LLQ headers

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> If the device supports larger LLQ (Low Latency Queue) headers, the user
> can activate them by enabling CONFIG_RTE_LIBRTE_ENA_LARGE_LLQ_HEADERS
> flag in the configuration file.
> 
> If the device isn't supporting this feature, the default value will be
> used.
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> ---
> v2:
>   * Use devargs instead of compilation options
> 
>  doc/guides/nics/ena.rst      |   8 +++
>  drivers/net/ena/ena_ethdev.c | 110 +++++++++++++++++++++++++++++++++--
>  drivers/net/ena/ena_ethdev.h |   2 +
>  3 files changed, 114 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/guides/nics/ena.rst b/doc/guides/nics/ena.rst
> index bbf27f235a..d5081e91e5 100644
> --- a/doc/guides/nics/ena.rst
> +++ b/doc/guides/nics/ena.rst
> @@ -95,6 +95,14 @@ Configuration information
>     * **CONFIG_RTE_LIBRTE_ENA_COM_DEBUG** (default n): Enables or disables debug
>       logging of low level tx/rx logic in ena_com(base) within the ENA PMD driver.
>  
> +**Runtime Configuration Parameters**
> +
> +   * **large_llq_hdr** (default 0)
> +
> +     Enables or disables usage of large LLQ headers. This option will have
> +     effect only if the device also supports large LLQ headers. Otherwise, the
> +     default value will be used.

Again, better to describe what 'LLQ' is, and you may want to describe what is
the usage/benefit of enabling large LLQ headers, up to you.

Re: [dpdk-dev] [PATCH v2 17/29] net/ena: add support for large LLQ headers

[Michał Krawczyk]

czw., 2 kwi 2020 o 15:02 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > If the device supports larger LLQ (Low Latency Queue) headers, the user
> > can activate them by enabling CONFIG_RTE_LIBRTE_ENA_LARGE_LLQ_HEADERS
> > flag in the configuration file.
> >
> > If the device isn't supporting this feature, the default value will be
> > used.
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> > ---
> > v2:
> >   * Use devargs instead of compilation options
> >
> >  doc/guides/nics/ena.rst      |   8 +++
> >  drivers/net/ena/ena_ethdev.c | 110 +++++++++++++++++++++++++++++++++--
> >  drivers/net/ena/ena_ethdev.h |   2 +
> >  3 files changed, 114 insertions(+), 6 deletions(-)
> >
> > diff --git a/doc/guides/nics/ena.rst b/doc/guides/nics/ena.rst
> > index bbf27f235a..d5081e91e5 100644
> > --- a/doc/guides/nics/ena.rst
> > +++ b/doc/guides/nics/ena.rst
> > @@ -95,6 +95,14 @@ Configuration information
> >     * **CONFIG_RTE_LIBRTE_ENA_COM_DEBUG** (default n): Enables or disables debug
> >       logging of low level tx/rx logic in ena_com(base) within the ENA PMD driver.
> >
> > +**Runtime Configuration Parameters**
> > +
> > +   * **large_llq_hdr** (default 0)
> > +
> > +     Enables or disables usage of large LLQ headers. This option will have
> > +     effect only if the device also supports large LLQ headers. Otherwise, the
> > +     default value will be used.
>
> Again, better to describe what 'LLQ' is, and you may want to describe what is
> the usage/benefit of enabling large LLQ headers, up to you.

Sure, I will do that in v3. Moreover, the commit log needs an update,
as it's currently describing static configuration, while we're using
devargs now (introduced in v2).

[dpdk-dev] [PATCH v2 18/29] net/ena: remove memory barriers before doorbells

[Michal Krawczyk]

The doorbell code is already issuing the doorbell by using rte_write.
Because of that, there is no need to do that before calling the
function.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index f23efbe464..eb97c80660 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1462,12 +1462,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 
 	/* When we submitted free recources to device... */
 	if (likely(i > 0)) {
-		/* ...let HW know that it can fill buffers with data
-		 *
-		 * Add memory barrier to make sure the desc were written before
-		 * issue a doorbell
-		 */
-		rte_wmb();
+		/* ...let HW know that it can fill buffers with data. */
 		ena_com_write_sq_doorbell(rxq->ena_com_io_sq);
 
 		rxq->next_to_use = next_to_use;
@@ -2405,7 +2400,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			PMD_DRV_LOG(DEBUG, "llq tx max burst size of queue %d"
 				" achieved, writing doorbell to send burst\n",
 				tx_ring->id);
-			rte_wmb();
 			ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
 		}
 
@@ -2428,7 +2422,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	/* If there are ready packets to be xmitted... */
 	if (sent_idx > 0) {
 		/* ...let HW do its best :-) */
-		rte_wmb();
 		ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
 		tx_ring->tx_stats.doorbells++;
 		tx_ring->next_to_use = next_to_use;
-- 
2.20.1

[dpdk-dev] [PATCH v2 19/29] net/ena: add Tx drops statistic

[Michal Krawczyk]

ENA device can report in the AENQ handler amount of Tx packets that were
dropped and not sent.

This statistic is showing global value for the device and because
rte_eth_stats is missing field that could indicate this value (it
isn't the Tx error), it is being presented as a extended statistic.

As the current design of extended statistics prevents tx_drops from
being an atomic variable and both tx_drops and rx_drops are only updated
from the AENQ handler, both were set as non-atomic for the alignment.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 11 ++++++++---
 drivers/net/ena/ena_ethdev.h |  8 +++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index eb97c80660..b7cdc5711c 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -96,6 +96,7 @@ static const struct ena_stats ena_stats_global_strings[] = {
 	ENA_STAT_GLOBAL_ENTRY(wd_expired),
 	ENA_STAT_GLOBAL_ENTRY(dev_start),
 	ENA_STAT_GLOBAL_ENTRY(dev_stop),
+	ENA_STAT_GLOBAL_ENTRY(tx_drops),
 };
 
 static const struct ena_stats ena_stats_tx_strings[] = {
@@ -938,7 +939,7 @@ static void ena_stats_restart(struct rte_eth_dev *dev)
 	rte_atomic64_init(&adapter->drv_stats->ierrors);
 	rte_atomic64_init(&adapter->drv_stats->oerrors);
 	rte_atomic64_init(&adapter->drv_stats->rx_nombuf);
-	rte_atomic64_init(&adapter->drv_stats->rx_drops);
+	adapter->drv_stats->rx_drops = 0;
 }
 
 static int ena_stats_get(struct rte_eth_dev *dev,
@@ -972,7 +973,7 @@ static int ena_stats_get(struct rte_eth_dev *dev,
 					ena_stats.tx_bytes_low);
 
 	/* Driver related stats */
-	stats->imissed = rte_atomic64_read(&adapter->drv_stats->rx_drops);
+	stats->imissed = adapter->drv_stats->rx_drops;
 	stats->ierrors = rte_atomic64_read(&adapter->drv_stats->ierrors);
 	stats->oerrors = rte_atomic64_read(&adapter->drv_stats->oerrors);
 	stats->rx_nombuf = rte_atomic64_read(&adapter->drv_stats->rx_nombuf);
@@ -2785,12 +2786,16 @@ static void ena_keep_alive(void *adapter_data,
 	struct ena_adapter *adapter = adapter_data;
 	struct ena_admin_aenq_keep_alive_desc *desc;
 	uint64_t rx_drops;
+	uint64_t tx_drops;
 
 	adapter->timestamp_wd = rte_get_timer_cycles();
 
 	desc = (struct ena_admin_aenq_keep_alive_desc *)aenq_e;
 	rx_drops = ((uint64_t)desc->rx_drops_high << 32) | desc->rx_drops_low;
-	rte_atomic64_set(&adapter->drv_stats->rx_drops, rx_drops);
+	tx_drops = ((uint64_t)desc->tx_drops_high << 32) | desc->tx_drops_low;
+
+	adapter->drv_stats->rx_drops = rx_drops;
+	adapter->dev_stats.tx_drops = tx_drops;
 }
 
 /**
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 0e82c894f4..9558ee072f 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -134,13 +134,19 @@ struct ena_driver_stats {
 	rte_atomic64_t ierrors;
 	rte_atomic64_t oerrors;
 	rte_atomic64_t rx_nombuf;
-	rte_atomic64_t rx_drops;
+	u64 rx_drops;
 };
 
 struct ena_stats_dev {
 	u64 wd_expired;
 	u64 dev_start;
 	u64 dev_stop;
+	/*
+	 * Tx drops cannot be reported as the driver statistic, because DPDK
+	 * rte_eth_stats structure isn't providing appropriate field for that.
+	 * As a workaround it is being published as an extended statistic.
+	 */
+	u64 tx_drops;
 };
 
 struct ena_offloads {
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 19/29] net/ena: add Tx drops statistic

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> ENA device can report in the AENQ handler amount of Tx packets that were
> dropped and not sent.
> 
> This statistic is showing global value for the device and because
> rte_eth_stats is missing field that could indicate this value (it
> isn't the Tx error), it is being presented as a extended statistic.

What is the reason of Tx drop if it is not error?

> 
> As the current design of extended statistics prevents tx_drops from
> being an atomic variable and both tx_drops and rx_drops are only updated
> from the AENQ handler, both were set as non-atomic for the alignment.
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>

<...>

Re: [dpdk-dev] [PATCH v2 19/29] net/ena: add Tx drops statistic

[Michał Krawczyk]

czw., 2 kwi 2020 o 15:05 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > ENA device can report in the AENQ handler amount of Tx packets that were
> > dropped and not sent.
> >
> > This statistic is showing global value for the device and because
> > rte_eth_stats is missing field that could indicate this value (it
> > isn't the Tx error), it is being presented as a extended statistic.
>
> What is the reason of Tx drop if it is not error?
>

It's connected with the device architecture (I'm sorry - I don't know
the details).

> >
> > As the current design of extended statistics prevents tx_drops from
> > being an atomic variable and both tx_drops and rx_drops are only updated
> > from the AENQ handler, both were set as non-atomic for the alignment.
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
>
> <...>
>

[dpdk-dev] [PATCH v2 20/29] net/ena: disable meta caching

[Michal Krawczyk]

In the LLQ mode, the device can indicate that meta data descriptor
caching is disabled. In that case the driver should send valid meta
descriptor on every Tx packet.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 28 ++++++++++++++++++++++------
 drivers/net/ena/ena_ethdev.h |  2 ++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index b7cdc5711c..64c91531a7 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -191,7 +191,8 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 static uint16_t eth_ena_recv_pkts(void *rx_queue,
 				  struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
 static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count);
-static void ena_init_rings(struct ena_adapter *adapter);
+static void ena_init_rings(struct ena_adapter *adapter,
+			   bool disable_meta_caching);
 static int ena_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
 static int ena_start(struct rte_eth_dev *dev);
 static void ena_stop(struct rte_eth_dev *dev);
@@ -313,7 +314,8 @@ static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
 
 static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 				       struct ena_com_tx_ctx *ena_tx_ctx,
-				       uint64_t queue_offloads)
+				       uint64_t queue_offloads,
+				       bool disable_meta_caching)
 {
 	struct ena_com_tx_meta *ena_meta = &ena_tx_ctx->ena_meta;
 
@@ -363,6 +365,9 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 		ena_meta->l3_hdr_len = mbuf->l3_len;
 		ena_meta->l3_hdr_offset = mbuf->l2_len;
 
+		ena_tx_ctx->meta_valid = true;
+	} else if (disable_meta_caching) {
+		memset(ena_meta, 0, sizeof(*ena_meta));
 		ena_tx_ctx->meta_valid = true;
 	} else {
 		ena_tx_ctx->meta_valid = false;
@@ -1726,8 +1731,8 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	const char *queue_type_str;
 	uint32_t max_num_io_queues;
 	int rc;
-
 	static int adapters_found;
+	bool disable_meta_caching;
 	bool wd_state;
 
 	eth_dev->dev_ops = &ena_dev_ops;
@@ -1818,8 +1823,16 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	adapter->max_rx_sgl_size = calc_queue_ctx.max_rx_sgl_size;
 	adapter->max_num_io_queues = max_num_io_queues;
 
+	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV) {
+		disable_meta_caching =
+			!!(get_feat_ctx.llq.accel_mode.u.get.supported_flags &
+			BIT(ENA_ADMIN_DISABLE_META_CACHING));
+	} else {
+		disable_meta_caching = false;
+	}
+
 	/* prepare ring structures */
-	ena_init_rings(adapter);
+	ena_init_rings(adapter, disable_meta_caching);
 
 	ena_config_debug_area(adapter);
 
@@ -1933,7 +1946,8 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
 	return 0;
 }
 
-static void ena_init_rings(struct ena_adapter *adapter)
+static void ena_init_rings(struct ena_adapter *adapter,
+			   bool disable_meta_caching)
 {
 	size_t i;
 
@@ -1947,6 +1961,7 @@ static void ena_init_rings(struct ena_adapter *adapter)
 		ring->tx_mem_queue_type = adapter->ena_dev.tx_mem_queue_type;
 		ring->tx_max_header_size = adapter->ena_dev.tx_max_header_size;
 		ring->sgl_size = adapter->max_tx_sgl_size;
+		ring->disable_meta_caching = disable_meta_caching;
 	}
 
 	for (i = 0; i < adapter->max_num_io_queues; i++) {
@@ -2359,7 +2374,8 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		} /* there's no else as we take advantage of memset zeroing */
 
 		/* Set TX offloads flags, if applicable */
-		ena_tx_mbuf_prepare(mbuf, &ena_tx_ctx, tx_ring->offloads);
+		ena_tx_mbuf_prepare(mbuf, &ena_tx_ctx, tx_ring->offloads,
+			tx_ring->disable_meta_caching);
 
 		rte_prefetch0(tx_pkts[(sent_idx + 4) & ring_mask]);
 
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 9558ee072f..99191ac3e7 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -113,6 +113,8 @@ struct ena_ring {
 	uint64_t offloads;
 	u16 sgl_size;
 
+	bool disable_meta_caching;
+
 	union {
 		struct ena_stats_rx rx_stats;
 		struct ena_stats_tx tx_stats;
-- 
2.20.1

[dpdk-dev] [PATCH v2 21/29] net/ena: refactor Rx path

[Michal Krawczyk]

* Split main Rx function into multiple ones - the body of the main
  was very big and further there were 2 nested loops, which were
  making the code hard to read
* Rework how the Rx mbuf chains are being created - Instead of having
  while loop which has conditional check if it's first segment, handle
  this segment outside the loop and if more fragments are existing,
  process them inside.
* Initialize Rx mbuf using simple function - it's the common thing for
  the 1st and next segments.
* Create structure for Rx buffer to align it with Tx path, other ENA
  drivers and to make the variable name more descriptive - on DPDK, Rx
  buffer must hold only mbuf, so initially array of mbufs was used as
  the buffers. However, it was misleading, as it was named
  "rx_buffer_info". To make it more clear, the structure holding mbuf
  pointer was added and now there is possibility to expand it in the
  future without reworking the driver.
* Remove redundant variables and conditional checks.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 182 ++++++++++++++++++++++-------------
 drivers/net/ena/ena_ethdev.h |   8 +-
 2 files changed, 124 insertions(+), 66 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 64c91531a7..1a9c28122b 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -188,6 +188,12 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 			      uint16_t nb_desc, unsigned int socket_id,
 			      const struct rte_eth_rxconf *rx_conf,
 			      struct rte_mempool *mp);
+static inline void ena_init_rx_mbuf(struct rte_mbuf *mbuf, uint16_t len);
+static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
+				    struct ena_com_rx_buf_info *ena_bufs,
+				    uint32_t descs,
+				    uint16_t *next_to_clean,
+				    uint8_t offset);
 static uint16_t eth_ena_recv_pkts(void *rx_queue,
 				  struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
 static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count);
@@ -749,11 +755,13 @@ static void ena_rx_queue_release_bufs(struct ena_ring *ring)
 {
 	unsigned int i;
 
-	for (i = 0; i < ring->ring_size; ++i)
-		if (ring->rx_buffer_info[i]) {
-			rte_mbuf_raw_free(ring->rx_buffer_info[i]);
-			ring->rx_buffer_info[i] = NULL;
+	for (i = 0; i < ring->ring_size; ++i) {
+		struct ena_rx_buffer *rx_info = &ring->rx_buffer_info[i];
+		if (rx_info->mbuf) {
+			rte_mbuf_raw_free(rx_info->mbuf);
+			rx_info->mbuf = NULL;
 		}
+	}
 }
 
 static void ena_tx_queue_release_bufs(struct ena_ring *ring)
@@ -1365,8 +1373,8 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->mb_pool = mp;
 
 	rxq->rx_buffer_info = rte_zmalloc("rxq->buffer_info",
-					  sizeof(struct rte_mbuf *) * nb_desc,
-					  RTE_CACHE_LINE_SIZE);
+		sizeof(struct ena_rx_buffer) * nb_desc,
+		RTE_CACHE_LINE_SIZE);
 	if (!rxq->rx_buffer_info) {
 		PMD_DRV_LOG(ERR, "failed to alloc mem for rx buffer info\n");
 		return -ENOMEM;
@@ -1434,15 +1442,17 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		uint16_t next_to_use_masked = next_to_use & ring_mask;
 		struct rte_mbuf *mbuf = mbufs[i];
 		struct ena_com_buf ebuf;
+		struct ena_rx_buffer *rx_info;
 
 		if (likely((i + 4) < count))
 			rte_prefetch0(mbufs[i + 4]);
 
 		req_id = rxq->empty_rx_reqs[next_to_use_masked];
 		rc = validate_rx_req_id(rxq, req_id);
-		if (unlikely(rc < 0))
+		if (unlikely(rc))
 			break;
-		rxq->rx_buffer_info[req_id] = mbuf;
+
+		rx_info = &rxq->rx_buffer_info[req_id];
 
 		/* prepare physical address for DMA transaction */
 		ebuf.paddr = mbuf->buf_iova + RTE_PKTMBUF_HEADROOM;
@@ -1452,9 +1462,9 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 						&ebuf, req_id);
 		if (unlikely(rc)) {
 			PMD_DRV_LOG(WARNING, "failed adding rx desc\n");
-			rxq->rx_buffer_info[req_id] = NULL;
 			break;
 		}
+		rx_info->mbuf = mbuf;
 		next_to_use++;
 	}
 
@@ -2052,6 +2062,83 @@ static int ena_infos_get(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static inline void ena_init_rx_mbuf(struct rte_mbuf *mbuf, uint16_t len)
+{
+	mbuf->data_len = len;
+	mbuf->data_off = RTE_PKTMBUF_HEADROOM;
+	mbuf->refcnt = 1;
+	mbuf->next = NULL;
+}
+
+static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
+				    struct ena_com_rx_buf_info *ena_bufs,
+				    uint32_t descs,
+				    uint16_t *next_to_clean,
+				    uint8_t offset)
+{
+	struct rte_mbuf *mbuf;
+	struct rte_mbuf *mbuf_head;
+	struct ena_rx_buffer *rx_info;
+	unsigned int ring_mask = rx_ring->ring_size - 1;
+	uint16_t ntc, len, req_id, buf = 0;
+
+	if (unlikely(descs == 0))
+		return NULL;
+
+	ntc = *next_to_clean;
+
+	len = ena_bufs[buf].len;
+	req_id = ena_bufs[buf].req_id;
+	if (unlikely(validate_rx_req_id(rx_ring, req_id)))
+		return NULL;
+
+	rx_info = &rx_ring->rx_buffer_info[req_id];
+
+	mbuf = rx_info->mbuf;
+	RTE_ASSERT(mbuf != NULL);
+
+	ena_init_rx_mbuf(mbuf, len);
+
+	/* Fill the mbuf head with the data specific for 1st segment. */
+	mbuf_head = mbuf;
+	mbuf_head->nb_segs = descs;
+	mbuf_head->port = rx_ring->port_id;
+	mbuf_head->pkt_len = len;
+	mbuf_head->data_off += offset;
+
+	rx_info->mbuf = NULL;
+	rx_ring->empty_rx_reqs[ntc & ring_mask] = req_id;
+	++ntc;
+
+	while (--descs) {
+		++buf;
+		len = ena_bufs[buf].len;
+		req_id = ena_bufs[buf].req_id;
+		if (unlikely(validate_rx_req_id(rx_ring, req_id))) {
+			rte_mbuf_raw_free(mbuf_head);
+			return NULL;
+		}
+
+		rx_info = &rx_ring->rx_buffer_info[req_id];
+		RTE_ASSERT(rx_info->mbuf != NULL);
+
+		/* Create an mbuf chain. */
+		mbuf->next = rx_info->mbuf;
+		mbuf = mbuf->next;
+
+		ena_init_rx_mbuf(mbuf, len);
+		mbuf_head->pkt_len += len;
+
+		rx_info->mbuf = NULL;
+		rx_ring->empty_rx_reqs[ntc & ring_mask] = req_id;
+		++ntc;
+	}
+
+	*next_to_clean = ntc;
+
+	return mbuf_head;
+}
+
 static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				  uint16_t nb_pkts)
 {
@@ -2060,16 +2147,10 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	unsigned int ring_mask = ring_size - 1;
 	uint16_t next_to_clean = rx_ring->next_to_clean;
 	uint16_t desc_in_use = 0;
-	uint16_t req_id;
-	unsigned int recv_idx = 0;
-	struct rte_mbuf *mbuf = NULL;
-	struct rte_mbuf *mbuf_head = NULL;
-	struct rte_mbuf *mbuf_prev = NULL;
-	struct rte_mbuf **rx_buff_info = rx_ring->rx_buffer_info;
-	unsigned int completed;
-
+	struct rte_mbuf *mbuf;
+	uint16_t completed;
 	struct ena_com_rx_ctx ena_rx_ctx;
-	int rc = 0;
+	int i, rc = 0;
 
 	/* Check adapter state */
 	if (unlikely(rx_ring->adapter->state != ENA_ADAPTER_STATE_RUNNING)) {
@@ -2083,8 +2164,6 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		nb_pkts = desc_in_use;
 
 	for (completed = 0; completed < nb_pkts; completed++) {
-		int segments = 0;
-
 		ena_rx_ctx.max_bufs = rx_ring->sgl_size;
 		ena_rx_ctx.ena_bufs = rx_ring->ena_bufs;
 		ena_rx_ctx.descs = 0;
@@ -2102,63 +2181,36 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			return 0;
 		}
 
-		if (unlikely(ena_rx_ctx.descs == 0))
-			break;
-
-		while (segments < ena_rx_ctx.descs) {
-			req_id = ena_rx_ctx.ena_bufs[segments].req_id;
-			rc = validate_rx_req_id(rx_ring, req_id);
-			if (unlikely(rc)) {
-				if (segments != 0)
-					rte_mbuf_raw_free(mbuf_head);
-				break;
-			}
-
-			mbuf = rx_buff_info[req_id];
-			rx_buff_info[req_id] = NULL;
-			mbuf->data_len = ena_rx_ctx.ena_bufs[segments].len;
-			mbuf->data_off = RTE_PKTMBUF_HEADROOM;
-			mbuf->refcnt = 1;
-			mbuf->next = NULL;
-			if (unlikely(segments == 0)) {
-				mbuf->nb_segs = ena_rx_ctx.descs;
-				mbuf->port = rx_ring->port_id;
-				mbuf->pkt_len = 0;
-				mbuf->data_off += ena_rx_ctx.pkt_offset;
-				mbuf_head = mbuf;
-			} else {
-				/* for multi-segment pkts create mbuf chain */
-				mbuf_prev->next = mbuf;
+		mbuf = ena_rx_mbuf(rx_ring,
+			ena_rx_ctx.ena_bufs,
+			ena_rx_ctx.descs,
+			&next_to_clean,
+			ena_rx_ctx.pkt_offset);
+		if (unlikely(mbuf == NULL)) {
+			for (i = 0; i < ena_rx_ctx.descs; ++i) {
+				rx_ring->empty_rx_reqs[next_to_clean & ring_mask] =
+					rx_ring->ena_bufs[i].req_id;
+				++next_to_clean;
 			}
-			mbuf_head->pkt_len += mbuf->data_len;
-
-			mbuf_prev = mbuf;
-			rx_ring->empty_rx_reqs[next_to_clean & ring_mask] =
-				req_id;
-			segments++;
-			next_to_clean++;
-		}
-		if (unlikely(rc))
 			break;
+		}
 
 		/* fill mbuf attributes if any */
-		ena_rx_mbuf_prepare(mbuf_head, &ena_rx_ctx);
+		ena_rx_mbuf_prepare(mbuf, &ena_rx_ctx);
 
-		if (unlikely(mbuf_head->ol_flags &
+		if (unlikely(mbuf->ol_flags &
 				(PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD))) {
 			rte_atomic64_inc(&rx_ring->adapter->drv_stats->ierrors);
 			++rx_ring->rx_stats.bad_csum;
 		}
 
-		mbuf_head->hash.rss = ena_rx_ctx.hash;
+		mbuf->hash.rss = ena_rx_ctx.hash;
 
-		/* pass to DPDK application head mbuf */
-		rx_pkts[recv_idx] = mbuf_head;
-		recv_idx++;
-		rx_ring->rx_stats.bytes += mbuf_head->pkt_len;
+		rx_pkts[completed] = mbuf;
+		rx_ring->rx_stats.bytes += mbuf->pkt_len;
 	}
 
-	rx_ring->rx_stats.cnt += recv_idx;
+	rx_ring->rx_stats.cnt += completed;
 	rx_ring->next_to_clean = next_to_clean;
 
 	desc_in_use = desc_in_use - completed + 1;
@@ -2168,7 +2220,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		ena_populate_rx_queue(rx_ring, ring_size - desc_in_use);
 	}
 
-	return recv_idx;
+	return completed;
 }
 
 static uint16_t
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 99191ac3e7..9df34136da 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -44,6 +44,12 @@ struct ena_tx_buffer {
 	struct ena_com_buf bufs[ENA_PKT_MAX_BUFS];
 };
 
+/* Rx buffer holds only pointer to the mbuf - may be expanded in the future */
+struct ena_rx_buffer {
+	struct rte_mbuf *mbuf;
+	struct ena_com_buf ena_buf;
+};
+
 struct ena_calc_queue_size_ctx {
 	struct ena_com_dev_get_features_ctx *get_feat_ctx;
 	struct ena_com_dev *ena_dev;
@@ -89,7 +95,7 @@ struct ena_ring {
 
 	union {
 		struct ena_tx_buffer *tx_buffer_info; /* contex of tx packet */
-		struct rte_mbuf **rx_buffer_info; /* contex of rx packet */
+		struct ena_rx_buffer *rx_buffer_info; /* contex of rx packet */
 	};
 	struct rte_mbuf **rx_refill_buffer;
 	unsigned int ring_size; /* number of tx/rx_buffer_info's entries */
-- 
2.20.1

[dpdk-dev] [PATCH v2 22/29] net/ena: rework getting number of available descs

[Michal Krawczyk]

ena_com API should be preferred for getting number of used/available
descriptors unless extra calculation needs to be performed.

Some helper variables were added for storing values that are later
reused. Moreover, for limiting the value of sent/received packets to
the number of available descriptors, the RTE_MIN is used instead of
if function, which was doing similar thing but was less descriptive.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 1a9c28122b..a6375e71c5 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1426,7 +1426,8 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 	if (unlikely(!count))
 		return 0;
 
-	in_use = rxq->next_to_use - rxq->next_to_clean;
+	in_use = ring_size - ena_com_free_q_entries(rxq->ena_com_io_sq) - 1;
+
 	ena_assert_msg(((in_use + count) < ring_size), "bad ring state\n");
 
 	/* get resources for incoming packets */
@@ -2145,8 +2146,9 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	struct ena_ring *rx_ring = (struct ena_ring *)(rx_queue);
 	unsigned int ring_size = rx_ring->ring_size;
 	unsigned int ring_mask = ring_size - 1;
+	unsigned int refill_required;
 	uint16_t next_to_clean = rx_ring->next_to_clean;
-	uint16_t desc_in_use = 0;
+	uint16_t descs_in_use;
 	struct rte_mbuf *mbuf;
 	uint16_t completed;
 	struct ena_com_rx_ctx ena_rx_ctx;
@@ -2159,9 +2161,9 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		return 0;
 	}
 
-	desc_in_use = rx_ring->next_to_use - next_to_clean;
-	if (unlikely(nb_pkts > desc_in_use))
-		nb_pkts = desc_in_use;
+	descs_in_use = ring_size -
+		ena_com_free_q_entries(rx_ring->ena_com_io_sq) - 1;
+	nb_pkts = RTE_MIN(descs_in_use, nb_pkts);
 
 	for (completed = 0; completed < nb_pkts; completed++) {
 		ena_rx_ctx.max_bufs = rx_ring->sgl_size;
@@ -2213,11 +2215,11 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rx_ring->rx_stats.cnt += completed;
 	rx_ring->next_to_clean = next_to_clean;
 
-	desc_in_use = desc_in_use - completed + 1;
+	refill_required = ena_com_free_q_entries(rx_ring->ena_com_io_sq);
 	/* Burst refill to save doorbells, memory barriers, const interval */
-	if (ring_size - desc_in_use > ENA_RING_DESCS_RATIO(ring_size)) {
+	if (refill_required > ENA_RING_DESCS_RATIO(ring_size)) {
 		ena_com_update_dev_comp_head(rx_ring->ena_com_io_cq);
-		ena_populate_rx_queue(rx_ring, ring_size - desc_in_use);
+		ena_populate_rx_queue(rx_ring, refill_required);
 	}
 
 	return completed;
@@ -2360,7 +2362,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	struct ena_tx_buffer *tx_info;
 	struct ena_com_buf *ebuf;
 	uint16_t rc, req_id, total_tx_descs = 0;
-	uint16_t sent_idx = 0, empty_tx_reqs;
+	uint16_t sent_idx = 0;
 	uint16_t push_len = 0;
 	uint16_t delta = 0;
 	int nb_hw_desc;
@@ -2373,9 +2375,8 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		return 0;
 	}
 
-	empty_tx_reqs = ring_size - (next_to_use - next_to_clean);
-	if (nb_pkts > empty_tx_reqs)
-		nb_pkts = empty_tx_reqs;
+	nb_pkts = RTE_MIN(ena_com_free_q_entries(tx_ring->ena_com_io_sq),
+		nb_pkts);
 
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		mbuf = tx_pkts[sent_idx];
-- 
2.20.1

[dpdk-dev] [PATCH v2 23/29] net/ena: limit refill threshold by fixed value

[Michal Krawczyk]

Divider used for both Tx and Rx cleanup/refill threshold can cause too
big delay in case of the really big rings - for example if the 8k Rx
ring will be used, the refill won't trigger unless 1024 threshold will
be reached. It will also cause driver to try to allocate that much
descriptors.

Limiting it by fixed value - 256 in that case, would limit maximum
time spent in repopulate function.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 27 ++++++++++++++-------------
 drivers/net/ena/ena_ethdev.h | 10 ++++++++++
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index a6375e71c5..fc14f1cb32 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -35,14 +35,6 @@
 /*reverse version of ENA_IO_RXQ_IDX*/
 #define ENA_IO_RXQ_IDX_REV(q)	((q - 1) / 2)
 
-/* While processing submitted and completed descriptors (rx and tx path
- * respectively) in a loop it is desired to:
- *  - perform batch submissions while populating sumbissmion queue
- *  - avoid blocking transmission of other packets during cleanup phase
- * Hence the utilization ratio of 1/8 of a queue size.
- */
-#define ENA_RING_DESCS_RATIO(ring_size)	(ring_size / 8)
-
 #define __MERGE_64B_H_L(h, l) (((uint64_t)h << 32) | l)
 #define TEST_BIT(val, bit_shift) (val & (1UL << bit_shift))
 
@@ -2146,7 +2138,8 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	struct ena_ring *rx_ring = (struct ena_ring *)(rx_queue);
 	unsigned int ring_size = rx_ring->ring_size;
 	unsigned int ring_mask = ring_size - 1;
-	unsigned int refill_required;
+	unsigned int free_queue_entries;
+	unsigned int refill_threshold;
 	uint16_t next_to_clean = rx_ring->next_to_clean;
 	uint16_t descs_in_use;
 	struct rte_mbuf *mbuf;
@@ -2215,11 +2208,15 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rx_ring->rx_stats.cnt += completed;
 	rx_ring->next_to_clean = next_to_clean;
 
-	refill_required = ena_com_free_q_entries(rx_ring->ena_com_io_sq);
+	free_queue_entries = ena_com_free_q_entries(rx_ring->ena_com_io_sq);
+	refill_threshold =
+		RTE_MIN(ring_size / ENA_REFILL_THRESH_DIVIDER,
+		(unsigned int)ENA_REFILL_THRESH_PACKET);
+
 	/* Burst refill to save doorbells, memory barriers, const interval */
-	if (refill_required > ENA_RING_DESCS_RATIO(ring_size)) {
+	if (free_queue_entries > refill_threshold) {
 		ena_com_update_dev_comp_head(rx_ring->ena_com_io_cq);
-		ena_populate_rx_queue(rx_ring, refill_required);
+		ena_populate_rx_queue(rx_ring, free_queue_entries);
 	}
 
 	return completed;
@@ -2358,6 +2355,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	uint16_t seg_len;
 	unsigned int ring_size = tx_ring->ring_size;
 	unsigned int ring_mask = ring_size - 1;
+	unsigned int cleanup_budget;
 	struct ena_com_tx_ctx ena_tx_ctx;
 	struct ena_tx_buffer *tx_info;
 	struct ena_com_buf *ebuf;
@@ -2515,9 +2513,12 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		/* Put back descriptor to the ring for reuse */
 		tx_ring->empty_tx_reqs[next_to_clean & ring_mask] = req_id;
 		next_to_clean++;
+		cleanup_budget =
+			RTE_MIN(ring_size / ENA_REFILL_THRESH_DIVIDER,
+			(unsigned int)ENA_REFILL_THRESH_PACKET);
 
 		/* If too many descs to clean, leave it for another run */
-		if (unlikely(total_tx_descs > ENA_RING_DESCS_RATIO(ring_size)))
+		if (unlikely(total_tx_descs > cleanup_budget))
 			break;
 	}
 	tx_ring->tx_stats.available_desc =
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 9df34136da..77c405fbdd 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -30,6 +30,16 @@
 #define ENA_WD_TIMEOUT_SEC	3
 #define ENA_DEVICE_KALIVE_TIMEOUT (ENA_WD_TIMEOUT_SEC * rte_get_timer_hz())
 
+/* While processing submitted and completed descriptors (rx and tx path
+ * respectively) in a loop it is desired to:
+ *  - perform batch submissions while populating sumbissmion queue
+ *  - avoid blocking transmission of other packets during cleanup phase
+ * Hence the utilization ratio of 1/8 of a queue size or max value if the size
+ * of the ring is very big - like 8k Rx rings.
+ */
+#define ENA_REFILL_THRESH_DIVIDER      8
+#define ENA_REFILL_THRESH_PACKET       256
+
 struct ena_adapter;
 
 enum ena_ring_type {
-- 
2.20.1

[dpdk-dev] [PATCH v2 24/29] net/ena: use macros for ring idx operations

[Michal Krawczyk]

To improve code readability, abstraction was added for operating on IO
rings indexes.

Driver was defining local variable for ring mask in each function that
needed to operate on the ring indexes. Now it is being stored in the
ring as this value won't change unless size of the ring will change and
macros for advancing indexes using the mask has been added.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 53 ++++++++++++++++++------------------
 drivers/net/ena/ena_ethdev.h |  4 +++
 2 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index fc14f1cb32..3517c3f97d 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1266,6 +1266,7 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 	txq->next_to_clean = 0;
 	txq->next_to_use = 0;
 	txq->ring_size = nb_desc;
+	txq->size_mask = nb_desc - 1;
 	txq->numa_socket_id = socket_id;
 
 	txq->tx_buffer_info = rte_zmalloc("txq->tx_buffer_info",
@@ -1361,6 +1362,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 	rxq->next_to_clean = 0;
 	rxq->next_to_use = 0;
 	rxq->ring_size = nb_desc;
+	rxq->size_mask = nb_desc - 1;
 	rxq->numa_socket_id = socket_id;
 	rxq->mb_pool = mp;
 
@@ -1409,8 +1411,6 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 {
 	unsigned int i;
 	int rc;
-	uint16_t ring_size = rxq->ring_size;
-	uint16_t ring_mask = ring_size - 1;
 	uint16_t next_to_use = rxq->next_to_use;
 	uint16_t in_use, req_id;
 	struct rte_mbuf **mbufs = rxq->rx_refill_buffer;
@@ -1418,9 +1418,10 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 	if (unlikely(!count))
 		return 0;
 
-	in_use = ring_size - ena_com_free_q_entries(rxq->ena_com_io_sq) - 1;
-
-	ena_assert_msg(((in_use + count) < ring_size), "bad ring state\n");
+	in_use = rxq->ring_size - 1 -
+		ena_com_free_q_entries(rxq->ena_com_io_sq);
+	ena_assert_msg(((in_use + count) < rxq->ring_size),
+		"bad ring state\n");
 
 	/* get resources for incoming packets */
 	rc = rte_mempool_get_bulk(rxq->mb_pool, (void **)mbufs, count);
@@ -1432,7 +1433,6 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 	}
 
 	for (i = 0; i < count; i++) {
-		uint16_t next_to_use_masked = next_to_use & ring_mask;
 		struct rte_mbuf *mbuf = mbufs[i];
 		struct ena_com_buf ebuf;
 		struct ena_rx_buffer *rx_info;
@@ -1440,7 +1440,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		if (likely((i + 4) < count))
 			rte_prefetch0(mbufs[i + 4]);
 
-		req_id = rxq->empty_rx_reqs[next_to_use_masked];
+		req_id = rxq->empty_rx_reqs[next_to_use];
 		rc = validate_rx_req_id(rxq, req_id);
 		if (unlikely(rc))
 			break;
@@ -1458,7 +1458,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 			break;
 		}
 		rx_info->mbuf = mbuf;
-		next_to_use++;
+		next_to_use = ENA_IDX_NEXT_MASKED(next_to_use, rxq->size_mask);
 	}
 
 	if (unlikely(i < count)) {
@@ -2072,7 +2072,6 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
 	struct rte_mbuf *mbuf;
 	struct rte_mbuf *mbuf_head;
 	struct ena_rx_buffer *rx_info;
-	unsigned int ring_mask = rx_ring->ring_size - 1;
 	uint16_t ntc, len, req_id, buf = 0;
 
 	if (unlikely(descs == 0))
@@ -2100,8 +2099,8 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
 	mbuf_head->data_off += offset;
 
 	rx_info->mbuf = NULL;
-	rx_ring->empty_rx_reqs[ntc & ring_mask] = req_id;
-	++ntc;
+	rx_ring->empty_rx_reqs[ntc] = req_id;
+	ntc = ENA_IDX_NEXT_MASKED(ntc, rx_ring->size_mask);
 
 	while (--descs) {
 		++buf;
@@ -2123,8 +2122,8 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
 		mbuf_head->pkt_len += len;
 
 		rx_info->mbuf = NULL;
-		rx_ring->empty_rx_reqs[ntc & ring_mask] = req_id;
-		++ntc;
+		rx_ring->empty_rx_reqs[ntc] = req_id;
+		ntc = ENA_IDX_NEXT_MASKED(ntc, rx_ring->size_mask);
 	}
 
 	*next_to_clean = ntc;
@@ -2136,8 +2135,6 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				  uint16_t nb_pkts)
 {
 	struct ena_ring *rx_ring = (struct ena_ring *)(rx_queue);
-	unsigned int ring_size = rx_ring->ring_size;
-	unsigned int ring_mask = ring_size - 1;
 	unsigned int free_queue_entries;
 	unsigned int refill_threshold;
 	uint16_t next_to_clean = rx_ring->next_to_clean;
@@ -2154,7 +2151,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		return 0;
 	}
 
-	descs_in_use = ring_size -
+	descs_in_use = rx_ring->ring_size -
 		ena_com_free_q_entries(rx_ring->ena_com_io_sq) - 1;
 	nb_pkts = RTE_MIN(descs_in_use, nb_pkts);
 
@@ -2183,9 +2180,10 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			ena_rx_ctx.pkt_offset);
 		if (unlikely(mbuf == NULL)) {
 			for (i = 0; i < ena_rx_ctx.descs; ++i) {
-				rx_ring->empty_rx_reqs[next_to_clean & ring_mask] =
+				rx_ring->empty_rx_reqs[next_to_clean] =
 					rx_ring->ena_bufs[i].req_id;
-				++next_to_clean;
+				next_to_clean = ENA_IDX_NEXT_MASKED(
+					next_to_clean, rx_ring->size_mask);
 			}
 			break;
 		}
@@ -2210,7 +2208,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 
 	free_queue_entries = ena_com_free_q_entries(rx_ring->ena_com_io_sq);
 	refill_threshold =
-		RTE_MIN(ring_size / ENA_REFILL_THRESH_DIVIDER,
+		RTE_MIN(rx_ring->ring_size / ENA_REFILL_THRESH_DIVIDER,
 		(unsigned int)ENA_REFILL_THRESH_PACKET);
 
 	/* Burst refill to save doorbells, memory barriers, const interval */
@@ -2353,8 +2351,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	uint16_t next_to_clean = tx_ring->next_to_clean;
 	struct rte_mbuf *mbuf;
 	uint16_t seg_len;
-	unsigned int ring_size = tx_ring->ring_size;
-	unsigned int ring_mask = ring_size - 1;
 	unsigned int cleanup_budget;
 	struct ena_com_tx_ctx ena_tx_ctx;
 	struct ena_tx_buffer *tx_info;
@@ -2384,7 +2380,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		if (unlikely(rc))
 			break;
 
-		req_id = tx_ring->empty_tx_reqs[next_to_use & ring_mask];
+		req_id = tx_ring->empty_tx_reqs[next_to_use];
 		tx_info = &tx_ring->tx_buffer_info[req_id];
 		tx_info->mbuf = mbuf;
 		tx_info->num_of_bufs = 0;
@@ -2428,7 +2424,8 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		ena_tx_mbuf_prepare(mbuf, &ena_tx_ctx, tx_ring->offloads,
 			tx_ring->disable_meta_caching);
 
-		rte_prefetch0(tx_pkts[(sent_idx + 4) & ring_mask]);
+		rte_prefetch0(tx_pkts[ENA_IDX_ADD_MASKED(
+			sent_idx, 4, tx_ring->size_mask)]);
 
 		/* Process first segment taking into
 		 * consideration pushed header
@@ -2480,7 +2477,8 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		}
 		tx_info->tx_descs = nb_hw_desc;
 
-		next_to_use++;
+		next_to_use = ENA_IDX_NEXT_MASKED(next_to_use,
+			tx_ring->size_mask);
 		tx_ring->tx_stats.cnt++;
 		tx_ring->tx_stats.bytes += total_length;
 	}
@@ -2511,10 +2509,11 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		tx_info->mbuf = NULL;
 
 		/* Put back descriptor to the ring for reuse */
-		tx_ring->empty_tx_reqs[next_to_clean & ring_mask] = req_id;
-		next_to_clean++;
+		tx_ring->empty_tx_reqs[next_to_clean] = req_id;
+		next_to_clean = ENA_IDX_NEXT_MASKED(next_to_clean,
+			tx_ring->size_mask);
 		cleanup_budget =
-			RTE_MIN(ring_size / ENA_REFILL_THRESH_DIVIDER,
+			RTE_MIN(tx_ring->ring_size / ENA_REFILL_THRESH_DIVIDER,
 			(unsigned int)ENA_REFILL_THRESH_PACKET);
 
 		/* If too many descs to clean, leave it for another run */
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 77c405fbdd..77d40d5a1e 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -40,6 +40,9 @@
 #define ENA_REFILL_THRESH_DIVIDER      8
 #define ENA_REFILL_THRESH_PACKET       256
 
+#define ENA_IDX_NEXT_MASKED(idx, mask) (((idx) + 1) & (mask))
+#define ENA_IDX_ADD_MASKED(idx, n, mask) (((idx) + (n)) & (mask))
+
 struct ena_adapter;
 
 enum ena_ring_type {
@@ -109,6 +112,7 @@ struct ena_ring {
 	};
 	struct rte_mbuf **rx_refill_buffer;
 	unsigned int ring_size; /* number of tx/rx_buffer_info's entries */
+	unsigned int size_mask;
 
 	struct ena_com_io_cq *ena_com_io_cq;
 	struct ena_com_io_sq *ena_com_io_sq;
-- 
2.20.1

[dpdk-dev] [PATCH v2 25/29] net/ena: refactor Tx path

[Michal Krawczyk]

The original Tx function was very long and was containing both cleanup
and the sending sections. Because of that it was having a lot of local
variables, big indentation and was hard to read.

This function was split into 2 sections:
  * Sending - which is responsible for preparing the mbuf, mapping it
    to the device descriptors and finally, sending packet to the HW
  * Cleanup - which is releasing packets sent by the HW. Loop which was
    releasing packets was reworked a bit, to make intention more visible
    and aligned with other parts of the driver.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
v2:
  * Fix compilation error on icc by adding braces around 0

 drivers/net/ena/ena_ethdev.c | 323 +++++++++++++++++++----------------
 1 file changed, 179 insertions(+), 144 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 3517c3f97d..601b4cb831 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -169,6 +169,13 @@ static int ena_device_init(struct ena_com_dev *ena_dev,
 			   struct ena_com_dev_get_features_ctx *get_feat_ctx,
 			   bool *wd_state);
 static int ena_dev_configure(struct rte_eth_dev *dev);
+static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
+	struct ena_tx_buffer *tx_info,
+	struct rte_mbuf *mbuf,
+	void **push_header,
+	uint16_t *header_len);
+static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf);
+static void ena_tx_cleanup(struct ena_ring *tx_ring);
 static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 				  uint16_t nb_pkts);
 static uint16_t eth_ena_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
@@ -2343,193 +2350,221 @@ static int ena_check_and_linearize_mbuf(struct ena_ring *tx_ring,
 	return rc;
 }
 
-static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
-				  uint16_t nb_pkts)
+static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
+	struct ena_tx_buffer *tx_info,
+	struct rte_mbuf *mbuf,
+	void **push_header,
+	uint16_t *header_len)
 {
-	struct ena_ring *tx_ring = (struct ena_ring *)(tx_queue);
-	uint16_t next_to_use = tx_ring->next_to_use;
-	uint16_t next_to_clean = tx_ring->next_to_clean;
-	struct rte_mbuf *mbuf;
-	uint16_t seg_len;
-	unsigned int cleanup_budget;
-	struct ena_com_tx_ctx ena_tx_ctx;
-	struct ena_tx_buffer *tx_info;
-	struct ena_com_buf *ebuf;
-	uint16_t rc, req_id, total_tx_descs = 0;
-	uint16_t sent_idx = 0;
-	uint16_t push_len = 0;
-	uint16_t delta = 0;
-	int nb_hw_desc;
-	uint32_t total_length;
-
-	/* Check adapter state */
-	if (unlikely(tx_ring->adapter->state != ENA_ADAPTER_STATE_RUNNING)) {
-		PMD_DRV_LOG(ALERT,
-			"Trying to xmit pkts while device is NOT running\n");
-		return 0;
-	}
+	struct ena_com_buf *ena_buf;
+	uint16_t delta, seg_len, push_len;
 
-	nb_pkts = RTE_MIN(ena_com_free_q_entries(tx_ring->ena_com_io_sq),
-		nb_pkts);
+	delta = 0;
+	seg_len = mbuf->data_len;
 
-	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
-		mbuf = tx_pkts[sent_idx];
-		total_length = 0;
+	tx_info->mbuf = mbuf;
+	ena_buf = tx_info->bufs;
 
-		rc = ena_check_and_linearize_mbuf(tx_ring, mbuf);
-		if (unlikely(rc))
-			break;
+	if (tx_ring->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV) {
+		/*
+		 * Tx header might be (and will be in most cases) smaller than
+		 * tx_max_header_size. But it's not an issue to send more data
+		 * to the device, than actually needed if the mbuf size is
+		 * greater than tx_max_header_size.
+		 */
+		push_len = RTE_MIN(mbuf->pkt_len, tx_ring->tx_max_header_size);
+		*header_len = push_len;
 
-		req_id = tx_ring->empty_tx_reqs[next_to_use];
-		tx_info = &tx_ring->tx_buffer_info[req_id];
-		tx_info->mbuf = mbuf;
-		tx_info->num_of_bufs = 0;
-		ebuf = tx_info->bufs;
+		if (likely(push_len <= seg_len)) {
+			/* If the push header is in the single segment, then
+			 * just point it to the 1st mbuf data.
+			 */
+			*push_header = rte_pktmbuf_mtod(mbuf, uint8_t *);
+		} else {
+			/* If the push header lays in the several segments, copy
+			 * it to the intermediate buffer.
+			 */
+			rte_pktmbuf_read(mbuf, 0, push_len,
+				tx_ring->push_buf_intermediate_buf);
+			*push_header = tx_ring->push_buf_intermediate_buf;
+			delta = push_len - seg_len;
+		}
+	} else {
+		*push_header = NULL;
+		*header_len = 0;
+		push_len = 0;
+	}
 
-		/* Prepare TX context */
-		memset(&ena_tx_ctx, 0x0, sizeof(struct ena_com_tx_ctx));
-		memset(&ena_tx_ctx.ena_meta, 0x0,
-		       sizeof(struct ena_com_tx_meta));
-		ena_tx_ctx.ena_bufs = ebuf;
-		ena_tx_ctx.req_id = req_id;
+	/* Process first segment taking into consideration pushed header */
+	if (seg_len > push_len) {
+		ena_buf->paddr = mbuf->buf_iova +
+				mbuf->data_off +
+				push_len;
+		ena_buf->len = seg_len - push_len;
+		ena_buf++;
+		tx_info->num_of_bufs++;
+	}
 
-		delta = 0;
+	while ((mbuf = mbuf->next) != NULL) {
 		seg_len = mbuf->data_len;
 
-		if (tx_ring->tx_mem_queue_type ==
-				ENA_ADMIN_PLACEMENT_POLICY_DEV) {
-			push_len = RTE_MIN(mbuf->pkt_len,
-					   tx_ring->tx_max_header_size);
-			ena_tx_ctx.header_len = push_len;
-
-			if (likely(push_len <= seg_len)) {
-				/* If the push header is in the single segment,
-				 * then just point it to the 1st mbuf data.
-				 */
-				ena_tx_ctx.push_header =
-					rte_pktmbuf_mtod(mbuf, uint8_t *);
-			} else {
-				/* If the push header lays in the several
-				 * segments, copy it to the intermediate buffer.
-				 */
-				rte_pktmbuf_read(mbuf, 0, push_len,
-					tx_ring->push_buf_intermediate_buf);
-				ena_tx_ctx.push_header =
-					tx_ring->push_buf_intermediate_buf;
-				delta = push_len - seg_len;
-			}
-		} /* there's no else as we take advantage of memset zeroing */
+		/* Skip mbufs if whole data is pushed as a header */
+		if (unlikely(delta > seg_len)) {
+			delta -= seg_len;
+			continue;
+		}
 
-		/* Set TX offloads flags, if applicable */
-		ena_tx_mbuf_prepare(mbuf, &ena_tx_ctx, tx_ring->offloads,
-			tx_ring->disable_meta_caching);
+		ena_buf->paddr = mbuf->buf_iova + mbuf->data_off + delta;
+		ena_buf->len = seg_len - delta;
+		ena_buf++;
+		tx_info->num_of_bufs++;
 
-		rte_prefetch0(tx_pkts[ENA_IDX_ADD_MASKED(
-			sent_idx, 4, tx_ring->size_mask)]);
+		delta = 0;
+	}
+}
 
-		/* Process first segment taking into
-		 * consideration pushed header
-		 */
-		if (seg_len > push_len) {
-			ebuf->paddr = mbuf->buf_iova +
-				      mbuf->data_off +
-				      push_len;
-			ebuf->len = seg_len - push_len;
-			ebuf++;
-			tx_info->num_of_bufs++;
-		}
-		total_length += mbuf->data_len;
+static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
+{
+	struct ena_tx_buffer *tx_info;
+	struct ena_com_tx_ctx ena_tx_ctx = { { 0 } };
+	uint16_t next_to_use;
+	uint16_t header_len;
+	uint16_t req_id;
+	void *push_header;
+	int nb_hw_desc;
+	int rc;
 
-		while ((mbuf = mbuf->next) != NULL) {
-			seg_len = mbuf->data_len;
+	rc = ena_check_and_linearize_mbuf(tx_ring, mbuf);
+	if (unlikely(rc))
+		return rc;
 
-			/* Skip mbufs if whole data is pushed as a header */
-			if (unlikely(delta > seg_len)) {
-				delta -= seg_len;
-				continue;
-			}
+	next_to_use = tx_ring->next_to_use;
 
-			ebuf->paddr = mbuf->buf_iova + mbuf->data_off + delta;
-			ebuf->len = seg_len - delta;
-			total_length += ebuf->len;
-			ebuf++;
-			tx_info->num_of_bufs++;
+	req_id = tx_ring->empty_tx_reqs[next_to_use];
+	tx_info = &tx_ring->tx_buffer_info[req_id];
+	tx_info->num_of_bufs = 0;
 
-			delta = 0;
-		}
+	ena_tx_map_mbuf(tx_ring, tx_info, mbuf, &push_header, &header_len);
 
-		ena_tx_ctx.num_bufs = tx_info->num_of_bufs;
+	ena_tx_ctx.ena_bufs = tx_info->bufs;
+	ena_tx_ctx.push_header = push_header;
+	ena_tx_ctx.num_bufs = tx_info->num_of_bufs;
+	ena_tx_ctx.req_id = req_id;
+	ena_tx_ctx.header_len = header_len;
 
-		if (ena_com_is_doorbell_needed(tx_ring->ena_com_io_sq,
-					       &ena_tx_ctx)) {
-			PMD_DRV_LOG(DEBUG, "llq tx max burst size of queue %d"
-				" achieved, writing doorbell to send burst\n",
-				tx_ring->id);
-			ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
-		}
-
-		/* prepare the packet's descriptors to dma engine */
-		rc = ena_com_prepare_tx(tx_ring->ena_com_io_sq,
-					&ena_tx_ctx, &nb_hw_desc);
-		if (unlikely(rc)) {
-			++tx_ring->tx_stats.prepare_ctx_err;
-			break;
-		}
-		tx_info->tx_descs = nb_hw_desc;
+	/* Set Tx offloads flags, if applicable */
+	ena_tx_mbuf_prepare(mbuf, &ena_tx_ctx, tx_ring->offloads,
+		tx_ring->disable_meta_caching);
 
-		next_to_use = ENA_IDX_NEXT_MASKED(next_to_use,
-			tx_ring->size_mask);
-		tx_ring->tx_stats.cnt++;
-		tx_ring->tx_stats.bytes += total_length;
+	if (unlikely(ena_com_is_doorbell_needed(tx_ring->ena_com_io_sq,
+			&ena_tx_ctx))) {
+		PMD_DRV_LOG(DEBUG,
+			"llq tx max burst size of queue %d achieved, writing doorbell to send burst\n",
+			tx_ring->id);
+		ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
 	}
-	tx_ring->tx_stats.available_desc =
-		ena_com_free_q_entries(tx_ring->ena_com_io_sq);
 
-	/* If there are ready packets to be xmitted... */
-	if (sent_idx > 0) {
-		/* ...let HW do its best :-) */
-		ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
-		tx_ring->tx_stats.doorbells++;
-		tx_ring->next_to_use = next_to_use;
+	/* prepare the packet's descriptors to dma engine */
+	rc = ena_com_prepare_tx(tx_ring->ena_com_io_sq,	&ena_tx_ctx,
+		&nb_hw_desc);
+	if (unlikely(rc)) {
+		++tx_ring->tx_stats.prepare_ctx_err;
+		return rc;
 	}
 
-	/* Clear complete packets  */
-	while (ena_com_tx_comp_req_id_get(tx_ring->ena_com_io_cq, &req_id) >= 0) {
-		rc = validate_tx_req_id(tx_ring, req_id);
-		if (rc)
+	tx_info->tx_descs = nb_hw_desc;
+
+	tx_ring->tx_stats.cnt++;
+	tx_ring->tx_stats.bytes += mbuf->pkt_len;
+
+	tx_ring->next_to_use = ENA_IDX_NEXT_MASKED(next_to_use,
+		tx_ring->size_mask);
+
+	return 0;
+}
+
+static void ena_tx_cleanup(struct ena_ring *tx_ring)
+{
+	unsigned int cleanup_budget;
+	unsigned int total_tx_descs = 0;
+	uint16_t next_to_clean = tx_ring->next_to_clean;
+
+	cleanup_budget = RTE_MIN(tx_ring->ring_size / ENA_REFILL_THRESH_DIVIDER,
+		(unsigned int)ENA_REFILL_THRESH_PACKET);
+
+	while (likely(total_tx_descs < cleanup_budget)) {
+		struct rte_mbuf *mbuf;
+		struct ena_tx_buffer *tx_info;
+		uint16_t req_id;
+
+		if (ena_com_tx_comp_req_id_get(tx_ring->ena_com_io_cq, &req_id) != 0)
+			break;
+
+		if (unlikely(validate_tx_req_id(tx_ring, req_id) != 0))
 			break;
 
 		/* Get Tx info & store how many descs were processed  */
 		tx_info = &tx_ring->tx_buffer_info[req_id];
-		total_tx_descs += tx_info->tx_descs;
 
-		/* Free whole mbuf chain  */
 		mbuf = tx_info->mbuf;
 		rte_pktmbuf_free(mbuf);
+
 		tx_info->mbuf = NULL;
+		tx_ring->empty_tx_reqs[next_to_clean] = req_id;
+
+		total_tx_descs += tx_info->tx_descs;
 
 		/* Put back descriptor to the ring for reuse */
-		tx_ring->empty_tx_reqs[next_to_clean] = req_id;
 		next_to_clean = ENA_IDX_NEXT_MASKED(next_to_clean,
 			tx_ring->size_mask);
-		cleanup_budget =
-			RTE_MIN(tx_ring->ring_size / ENA_REFILL_THRESH_DIVIDER,
-			(unsigned int)ENA_REFILL_THRESH_PACKET);
-
-		/* If too many descs to clean, leave it for another run */
-		if (unlikely(total_tx_descs > cleanup_budget))
-			break;
 	}
-	tx_ring->tx_stats.available_desc =
-		ena_com_free_q_entries(tx_ring->ena_com_io_sq);
 
-	if (total_tx_descs > 0) {
+	if (likely(total_tx_descs > 0)) {
 		/* acknowledge completion of sent packets */
 		tx_ring->next_to_clean = next_to_clean;
 		ena_com_comp_ack(tx_ring->ena_com_io_sq, total_tx_descs);
 		ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
 	}
+}
+
+static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
+				  uint16_t nb_pkts)
+{
+	struct ena_ring *tx_ring = (struct ena_ring *)(tx_queue);
+	uint16_t sent_idx = 0;
+
+	/* Check adapter state */
+	if (unlikely(tx_ring->adapter->state != ENA_ADAPTER_STATE_RUNNING)) {
+		PMD_DRV_LOG(ALERT,
+			"Trying to xmit pkts while device is NOT running\n");
+		return 0;
+	}
+
+	nb_pkts = RTE_MIN(ena_com_free_q_entries(tx_ring->ena_com_io_sq),
+		nb_pkts);
+
+	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
+		if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx]))
+			break;
 
+		rte_prefetch0(tx_pkts[ENA_IDX_ADD_MASKED(sent_idx, 4,
+			tx_ring->size_mask)]);
+	}
+
+	tx_ring->tx_stats.available_desc =
+		ena_com_free_q_entries(tx_ring->ena_com_io_sq);
+
+	/* If there are ready packets to be xmitted... */
+	if (sent_idx > 0) {
+		/* ...let HW do its best :-) */
+		ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
+		tx_ring->tx_stats.doorbells++;
+	}
+
+	ena_tx_cleanup(tx_ring);
+
+	tx_ring->tx_stats.available_desc =
+		ena_com_free_q_entries(tx_ring->ena_com_io_sq);
 	tx_ring->tx_stats.tx_poll++;
 
 	return sent_idx;
-- 
2.20.1

[dpdk-dev] [PATCH v2 26/29] net/ena: reuse 0 length Rx descriptor

[Michal Krawczyk]

Some ENA devices can pass to the driver descriptor with length 0. To
avoid extra allocation, the descriptor can be reused by simply putting
it back to the device.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
v2:
  * Compare rc (error code) to 0 value explicitely

 drivers/net/ena/ena_ethdev.c | 75 ++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 601b4cb831..34f910c9c4 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -195,6 +195,8 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
 				    uint8_t offset);
 static uint16_t eth_ena_recv_pkts(void *rx_queue,
 				  struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+static int ena_add_single_rx_desc(struct ena_com_io_sq *io_sq,
+				  struct rte_mbuf *mbuf, uint16_t id);
 static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count);
 static void ena_init_rings(struct ena_adapter *adapter,
 			   bool disable_meta_caching);
@@ -1414,6 +1416,24 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int ena_add_single_rx_desc(struct ena_com_io_sq *io_sq,
+				  struct rte_mbuf *mbuf, uint16_t id)
+{
+	struct ena_com_buf ebuf;
+	int rc;
+
+	/* prepare physical address for DMA transaction */
+	ebuf.paddr = mbuf->buf_iova + RTE_PKTMBUF_HEADROOM;
+	ebuf.len = mbuf->buf_len - RTE_PKTMBUF_HEADROOM;
+
+	/* pass resource to device */
+	rc = ena_com_add_single_rx_desc(io_sq, &ebuf, id);
+	if (unlikely(rc != 0))
+		PMD_DRV_LOG(WARNING, "failed adding rx desc\n");
+
+	return rc;
+}
+
 static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 {
 	unsigned int i;
@@ -1441,7 +1461,6 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 
 	for (i = 0; i < count; i++) {
 		struct rte_mbuf *mbuf = mbufs[i];
-		struct ena_com_buf ebuf;
 		struct ena_rx_buffer *rx_info;
 
 		if (likely((i + 4) < count))
@@ -1454,16 +1473,10 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 
 		rx_info = &rxq->rx_buffer_info[req_id];
 
-		/* prepare physical address for DMA transaction */
-		ebuf.paddr = mbuf->buf_iova + RTE_PKTMBUF_HEADROOM;
-		ebuf.len = mbuf->buf_len - RTE_PKTMBUF_HEADROOM;
-		/* pass resource to device */
-		rc = ena_com_add_single_rx_desc(rxq->ena_com_io_sq,
-						&ebuf, req_id);
-		if (unlikely(rc)) {
-			PMD_DRV_LOG(WARNING, "failed adding rx desc\n");
+		rc = ena_add_single_rx_desc(rxq->ena_com_io_sq, mbuf, req_id);
+		if (unlikely(rc != 0))
 			break;
-		}
+
 		rx_info->mbuf = mbuf;
 		next_to_use = ENA_IDX_NEXT_MASKED(next_to_use, rxq->size_mask);
 	}
@@ -2079,6 +2092,7 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
 	struct rte_mbuf *mbuf;
 	struct rte_mbuf *mbuf_head;
 	struct ena_rx_buffer *rx_info;
+	int rc;
 	uint16_t ntc, len, req_id, buf = 0;
 
 	if (unlikely(descs == 0))
@@ -2121,13 +2135,44 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring,
 		rx_info = &rx_ring->rx_buffer_info[req_id];
 		RTE_ASSERT(rx_info->mbuf != NULL);
 
-		/* Create an mbuf chain. */
-		mbuf->next = rx_info->mbuf;
-		mbuf = mbuf->next;
+		if (unlikely(len == 0)) {
+			/*
+			 * Some devices can pass descriptor with the length 0.
+			 * To avoid confusion, the PMD is simply putting the
+			 * descriptor back, as it was never used. We'll avoid
+			 * mbuf allocation that way.
+			 */
+			rc = ena_add_single_rx_desc(rx_ring->ena_com_io_sq,
+				rx_info->mbuf, req_id);
+			if (unlikely(rc != 0)) {
+				/* Free the mbuf in case of an error. */
+				rte_mbuf_raw_free(rx_info->mbuf);
+			} else {
+				/*
+				 * If there was no error, just exit the loop as
+				 * 0 length descriptor is always the last one.
+				 */
+				break;
+			}
+		} else {
+			/* Create an mbuf chain. */
+			mbuf->next = rx_info->mbuf;
+			mbuf = mbuf->next;
 
-		ena_init_rx_mbuf(mbuf, len);
-		mbuf_head->pkt_len += len;
+			ena_init_rx_mbuf(mbuf, len);
+			mbuf_head->pkt_len += len;
+		}
 
+		/*
+		 * Mark the descriptor as depleted and perform necessary
+		 * cleanup.
+		 * This code will execute in two cases:
+		 *  1. Descriptor len was greater than 0 - normal situation.
+		 *  2. Descriptor len was 0 and we failed to add the descriptor
+		 *     to the device. In that situation, we should try to add
+		 *     the mbuf again in the populate routine and mark the
+		 *     descriptor as used up by the device.
+		 */
 		rx_info->mbuf = NULL;
 		rx_ring->empty_rx_reqs[ntc] = req_id;
 		ntc = ENA_IDX_NEXT_MASKED(ntc, rx_ring->size_mask);
-- 
2.20.1

[dpdk-dev] [PATCH v2 27/29] doc: add notes on ENA usage on metal instances

[Michal Krawczyk]

As AWS metal instances are supporting IOMMU, the usage of igb_uio or
vfio-pci can lead to a problems (when to use which module), especially
that the vfio-pci isn't supporting SMMU on arm64.

To clear up the problem of using those modules in various setup
conditions (with or without IOMMU) on metal instances, more detailed
explanation was added.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 doc/guides/nics/ena.rst | 48 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/doc/guides/nics/ena.rst b/doc/guides/nics/ena.rst
index d5081e91e5..57749eb736 100644
--- a/doc/guides/nics/ena.rst
+++ b/doc/guides/nics/ena.rst
@@ -187,11 +187,55 @@ Prerequisites
 
      echo 1 > /sys/module/vfio/parameters/enable_unsafe_noiommu_mode
 
-#. Bind the intended ENA device to ``vfio-pci`` or ``igb_uio`` module.
+   To use ``noiommu`` mode, the ``vfio-pci`` must be built with flag
+   ``CONFIG_VFIO_NOIOMMU``.
 
+#. Bind the intended ENA device to ``vfio-pci`` or ``igb_uio`` module.
 
 At this point the system should be ready to run DPDK applications. Once the
-application runs to completion, the ENA can be detached from igb_uio if necessary.
+application runs to completion, the ENA can be detached from attached module if
+necessary.
+
+**Note about usage on \*.metal instances**
+
+On AWS, the metal instances are supporting IOMMU for both arm64 and x86_64
+hosts.
+
+* x86_64 (e.g. c5.metal, i3.metal):
+   IOMMU should be disabled by default. In that situation, the ``igb_uio`` can
+   be used as it is but ``vfio-pci`` should be working in no-IOMMU mode (please
+   see above).
+
+   When IOMMU is enabled, ``igb_uio`` cannot be used as it's not supporting this
+   feature, while ``vfio-pci`` should work without any changes.
+   To enable IOMMU on those hosts, please update ``GRUB_CMDLINE_LINUX`` in file
+   ``/etc/default/grub`` with the below extra boot arguments::
+
+    iommu=1 intel_iommu=on
+
+   Then, make the changes live by executing as a root::
+
+    # grub2-mkconfig > /boot/grub2/grub.cfg
+
+   Finally, reboot should result in IOMMU being enabled.
+
+* arm64 (a1.metal):
+   IOMMU should be enabled by default. Unfortunately, ``vfio-pci`` isn't
+   supporting SMMU, which is implementation of IOMMU for arm64 architecture and
+   ``igb_uio`` isn't supporting IOMMU at all, so to use DPDK with ENA on those
+   hosts, one must disable IOMMU. This can be done by updating
+   ``GRUB_CMDLINE_LINUX`` in file ``/etc/default/grub`` with the extra boot
+   argument::
+
+    iommu.passthrough=1
+
+   Then, make the changes live by executing as a root::
+
+    # grub2-mkconfig > /boot/grub2/grub.cfg
+
+   Finally, reboot should result in IOMMU being disabled.
+   Without IOMMU, ``igb_uio`` can be used as it is but ``vfio-pci`` should be
+   working in no-IOMMU mode (please see above).
 
 Usage example
 -------------
-- 
2.20.1

[dpdk-dev] [PATCH v2 28/29] net/ena: update copyright date

[Michal Krawczyk]

The v2.1.0 making changes in the ena_ethdev files were done this year,
so the copyright notice should be updated.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 2 +-
 drivers/net/ena/ena_ethdev.c         | 2 +-
 drivers/net/ena/ena_ethdev.h         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index c69426a062..595967e6e3 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
+ * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 34f910c9c4..53798b8af4 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
+ * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 77d40d5a1e..6e24a4e582 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
+ * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
  * All rights reserved.
  */
 
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 28/29] net/ena: update copyright date

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> The v2.1.0 making changes in the ena_ethdev files were done this year,
> so the copyright notice should be updated.
> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> ---
>  drivers/net/ena/base/ena_plat_dpdk.h | 2 +-
>  drivers/net/ena/ena_ethdev.c         | 2 +-
>  drivers/net/ena/ena_ethdev.h         | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
> index c69426a062..595967e6e3 100644
> --- a/drivers/net/ena/base/ena_plat_dpdk.h
> +++ b/drivers/net/ena/base/ena_plat_dpdk.h
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
> + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
>   * All rights reserved.
>   */
>  
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 34f910c9c4..53798b8af4 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
> + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
>   * All rights reserved.
>   */
>  
> diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
> index 77d40d5a1e..6e24a4e582 100644
> --- a/drivers/net/ena/ena_ethdev.h
> +++ b/drivers/net/ena/ena_ethdev.h
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
> + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
>   * All rights reserved.
>   */
>  
> 

These files seems updated in this series already, instead of making separate
patch to update copyright date, can you please update copyright date in the
patch that updates those files?

Re: [dpdk-dev] [PATCH v2 28/29] net/ena: update copyright date

[Michał Krawczyk]

czw., 2 kwi 2020 o 17:51 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > The v2.1.0 making changes in the ena_ethdev files were done this year,
> > so the copyright notice should be updated.
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> > ---
> >  drivers/net/ena/base/ena_plat_dpdk.h | 2 +-
> >  drivers/net/ena/ena_ethdev.c         | 2 +-
> >  drivers/net/ena/ena_ethdev.h         | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
> > index c69426a062..595967e6e3 100644
> > --- a/drivers/net/ena/base/ena_plat_dpdk.h
> > +++ b/drivers/net/ena/base/ena_plat_dpdk.h
> > @@ -1,5 +1,5 @@
> >  /* SPDX-License-Identifier: BSD-3-Clause
> > - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
> > + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
> >   * All rights reserved.
> >   */
> >
> > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> > index 34f910c9c4..53798b8af4 100644
> > --- a/drivers/net/ena/ena_ethdev.c
> > +++ b/drivers/net/ena/ena_ethdev.c
> > @@ -1,5 +1,5 @@
> >  /* SPDX-License-Identifier: BSD-3-Clause
> > - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
> > + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
> >   * All rights reserved.
> >   */
> >
> > diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
> > index 77d40d5a1e..6e24a4e582 100644
> > --- a/drivers/net/ena/ena_ethdev.h
> > +++ b/drivers/net/ena/ena_ethdev.h
> > @@ -1,5 +1,5 @@
> >  /* SPDX-License-Identifier: BSD-3-Clause
> > - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates.
> > + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates.
> >   * All rights reserved.
> >   */
> >
> >
>
> These files seems updated in this series already, instead of making separate
> patch to update copyright date, can you please update copyright date in the
> patch that updates those files?

Sure - I'll do that in v3.

[dpdk-dev] [PATCH v2 29/29] net/ena: update version of the driver to v2.1.0

[Michal Krawczyk]

The v2.1.0 is refactoring Tx and Rx paths, including few bug fixes and
is also adding a new features which are going to be available with the
newest hardware:
* Accelerated LLQ mode
  * Disabling meta descriptor caching is required for that
* Handling Rx descriptor which length is 0
* Tx drops are now being reported as extended statistic
* Support LLQ headers higher than 96B

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
Reviewed-by: Igor Chauskin <igorch@amazon.com>
Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
---
 drivers/net/ena/ena_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 53798b8af4..b467489cc2 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -27,8 +27,8 @@
 #include <ena_eth_io_defs.h>
 
 #define DRV_MODULE_VER_MAJOR	2
-#define DRV_MODULE_VER_MINOR	0
-#define DRV_MODULE_VER_SUBMINOR	3
+#define DRV_MODULE_VER_MINOR	1
+#define DRV_MODULE_VER_SUBMINOR	0
 
 #define ENA_IO_TXQ_IDX(q)	(2 * (q))
 #define ENA_IO_RXQ_IDX(q)	(2 * (q) + 1)
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 29/29] net/ena: update version of the driver to v2.1.0

[Ferruh Yigit]

On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> The v2.1.0 is refactoring Tx and Rx paths, including few bug fixes and
> is also adding a new features which are going to be available with the
> newest hardware:
> * Accelerated LLQ mode
>   * Disabling meta descriptor caching is required for that
> * Handling Rx descriptor which length is 0
> * Tx drops are now being reported as extended statistic
> * Support LLQ headers higher than 96B

Can you put above list into the release notes
(doc/guides/rel_notes/release_20_05.rst), not as a separate patch but in each
patch that adds above mentioned feature update release notes too to document it..

Thanks,
ferruh

> 
> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 53798b8af4..b467489cc2 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -27,8 +27,8 @@
>  #include <ena_eth_io_defs.h>
>  
>  #define DRV_MODULE_VER_MAJOR	2
> -#define DRV_MODULE_VER_MINOR	0
> -#define DRV_MODULE_VER_SUBMINOR	3
> +#define DRV_MODULE_VER_MINOR	1
> +#define DRV_MODULE_VER_SUBMINOR	0
>  
>  #define ENA_IO_TXQ_IDX(q)	(2 * (q))
>  #define ENA_IO_RXQ_IDX(q)	(2 * (q) + 1)
> 

Re: [dpdk-dev] [PATCH v2 29/29] net/ena: update version of the driver to v2.1.0

[Michał Krawczyk]

czw., 2 kwi 2020 o 17:53 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 4/1/2020 3:21 PM, Michal Krawczyk wrote:
> > The v2.1.0 is refactoring Tx and Rx paths, including few bug fixes and
> > is also adding a new features which are going to be available with the
> > newest hardware:
> > * Accelerated LLQ mode
> >   * Disabling meta descriptor caching is required for that
> > * Handling Rx descriptor which length is 0
> > * Tx drops are now being reported as extended statistic
> > * Support LLQ headers higher than 96B
>
> Can you put above list into the release notes
> (doc/guides/rel_notes/release_20_05.rst), not as a separate patch but in each
> patch that adds above mentioned feature update release notes too to document it..
>

Sure - I'll do that this way in v3. Thanks a lot for the review!

Thanks,
Michal

> Thanks,
> ferruh
>
> >
> > Signed-off-by: Michal Krawczyk <mk@semihalf.com>
> > Reviewed-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> > ---
> >  drivers/net/ena/ena_ethdev.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> > index 53798b8af4..b467489cc2 100644
> > --- a/drivers/net/ena/ena_ethdev.c
> > +++ b/drivers/net/ena/ena_ethdev.c
> > @@ -27,8 +27,8 @@
> >  #include <ena_eth_io_defs.h>
> >
> >  #define DRV_MODULE_VER_MAJOR 2
> > -#define DRV_MODULE_VER_MINOR 0
> > -#define DRV_MODULE_VER_SUBMINOR      3
> > +#define DRV_MODULE_VER_MINOR 1
> > +#define DRV_MODULE_VER_SUBMINOR      0
> >
> >  #define ENA_IO_TXQ_IDX(q)    (2 * (q))
> >  #define ENA_IO_RXQ_IDX(q)    (2 * (q) + 1)
> >
>