net/ena: v2.6.0 driver update

PATCH Superseded Michal Krawczyk 2 participants 77 messages Feb 22, 2022 – Feb 23, 2022 Network drivers

lore

Share & tools

lore

Michal Krawczyk

author

2022-02-22 16:06 UTC #0 lore raw

cover net/ena: v2.6.0 driver update

Hi,

this set contains new ENA features:

* New xstats.
* Reconfigurable link status event.
* Usage of the optimized memcpy on arm/arm64.
* Better MP support.
* Reconfigurable Tx completion timeout value using devarg.

Beside that, this patchset contains multiple fixes, minor improvements, new DPDK API support and extra error checks.

Dawid Gorecki (4):
net/ena: expose Tx cleanup function
net/ena: check if reset was already triggered net/ena: extend debug prints for invalid req ID resets net/ena: don't initialize LLQ when membar isn't exposed

Michal Krawczyk (15):
net/ena: remove linearization function net/ena: add assertion on Tx info mbuf net/ena: remove unused enumeration
net/ena: remove unused offloads variables net/ena: add extra Rx checksum related xstats net/ena: make LSC configurable
net/ena: skip timer if the reset is triggered net/ena: perform Tx cleanup before sending pkts net/ena/base: use optimized memcpy version also on Arm net/ena/base: make IO memzone unique per port net/ena: add API for probing xstat names by ID net/ena: make Tx completion timeout configurable net/ena: fix meta-desc DF flag setup
net/ena: don't indicate bad csum for L4 csum error net/ena: update version to 2.6.0

Stanislaw Kardach (2):
net/ena: proxy AQ calls to primary process
net/ena: enable stats get function for MP mode

 doc/guides/nics/ena.rst                |   9 +
 doc/guides/nics/features/ena.ini       |   2 +
 doc/guides/rel_notes/release_22_03.rst |  14 +
 drivers/net/ena/base/ena_plat_dpdk.h   |  69 +-
 drivers/net/ena/ena_ethdev.c           | 916 ++++++++++++++++++++-----
 drivers/net/ena/ena_ethdev.h           |  28 +-
 drivers/net/ena/ena_rss.c              |  10 +-
 7 files changed, 800 insertions(+), 248 deletions(-)

--
2.25.1

Michal Krawczyk

author

2022-02-22 16:06 UTC #1 lore raw

PATCH 1/21 net/ena: remove linearization function

The linearization of the mbuf isn't common practice for the PMD, as it can expose it's capabilities to the upper layer using rte_eth_dev_info_get().

Moreover, the rte_eth_tx_prepare() function should also verify if the number of segments inside the mbuf isn't too high.

Because of those 2 circumstances, it may be safer to avoid modifying mbuf on PMD's Tx side and remove linearization at all. Instead, add verification of the number of segments to the eth_ena_prep_pkts().

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 74 +++++++++---------------------------
 drivers/net/ena/ena_ethdev.h |  2 -
 2 files changed, 19 insertions(+), 57 deletions(-)

drivers/net/ena/ena_ethdev.c +19 ~~-55~~

index db2b5ec8e7..deaee30664 100644
@@ -93,8 +93,6 @@ static const struct ena_stats ena_stats_tx_strings[] = {
 	ENA_STAT_TX_ENTRY(cnt),
 	ENA_STAT_TX_ENTRY(bytes),
 	ENA_STAT_TX_ENTRY(prepare_ctx_err),
-	ENA_STAT_TX_ENTRY(linearize),
-	ENA_STAT_TX_ENTRY(linearize_failed),
 	ENA_STAT_TX_ENTRY(tx_poll),
 	ENA_STAT_TX_ENTRY(doorbells),
 	ENA_STAT_TX_ENTRY(bad_req_id),

@@ -2408,6 +2406,17 @@ eth_ena_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			return i;
 		}
 
+		if (unlikely(m->nb_segs >= tx_ring->sgl_size &&
+		    !(tx_ring->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV &&
+		      m->nb_segs == tx_ring->sgl_size &&
+		      m->data_len < tx_ring->tx_max_header_size))) {
+			PMD_TX_LOG(DEBUG,
+				"mbuf[%" PRIu32 "] has too many segments: %" PRIu16 "\n",
+				i, m->nb_segs);
+			rte_errno = EINVAL;
+			return i;
+		}
+
 #ifdef RTE_LIBRTE_ETHDEV_DEBUG
 		/* Check if requested offload is also enabled for the queue */
 		if ((ol_flags & RTE_MBUF_F_TX_IP_CKSUM &&

@@ -2554,56 +2563,6 @@ static void ena_update_hints(struct ena_adapter *adapter,
 	}
 }
 
-static int ena_check_space_and_linearize_mbuf(struct ena_ring *tx_ring,
-					      struct rte_mbuf *mbuf)
-{
-	struct ena_com_dev *ena_dev;
-	int num_segments, header_len, rc;
-
-	ena_dev = &tx_ring->adapter->ena_dev;
-	num_segments = mbuf->nb_segs;
-	header_len = mbuf->data_len;
-
-	if (likely(num_segments < tx_ring->sgl_size))
-		goto checkspace;
-
-	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV &&
-	    (num_segments == tx_ring->sgl_size) &&
-	    (header_len < tx_ring->tx_max_header_size))
-		goto checkspace;
-
-	/* Checking for space for 2 additional metadata descriptors due to
-	 * possible header split and metadata descriptor. Linearization will
-	 * be needed so we reduce the segments number from num_segments to 1
-	 */
-	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq, 3)) {
-		PMD_TX_LOG(DEBUG, "Not enough space in the Tx queue\n");
-		return ENA_COM_NO_MEM;
-	}
-	++tx_ring->tx_stats.linearize;
-	rc = rte_pktmbuf_linearize(mbuf);
-	if (unlikely(rc)) {
-		PMD_TX_LOG(WARNING, "Mbuf linearize failed\n");
-		rte_atomic64_inc(&tx_ring->adapter->drv_stats->ierrors);
-		++tx_ring->tx_stats.linearize_failed;
-		return rc;
-	}
-
-	return 0;
-
-checkspace:
-	/* Checking for space for 2 additional metadata descriptors due to
-	 * possible header split and metadata descriptor
-	 */
-	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq,
-					  num_segments + 2)) {
-		PMD_TX_LOG(DEBUG, "Not enough space in the Tx queue\n");
-		return ENA_COM_NO_MEM;
-	}
-
-	return 0;
-}
-
 static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
 	struct ena_tx_buffer *tx_info,
 	struct rte_mbuf *mbuf,

@@ -2688,9 +2647,14 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	int nb_hw_desc;
 	int rc;
 
-	rc = ena_check_space_and_linearize_mbuf(tx_ring, mbuf);
-	if (unlikely(rc))
-		return rc;
+	/* Checking for space for 2 additional metadata descriptors due to
+	 * possible header split and metadata descriptor
+	 */
+	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq,
+					  mbuf->nb_segs + 2)) {
+		PMD_DRV_LOG(DEBUG, "Not enough space in the tx queue\n");
+		return ENA_COM_NO_MEM;
+	}
 
 	next_to_use = tx_ring->next_to_use;

drivers/net/ena/ena_ethdev.h +0 -2

index f99e4f3984..f47ba3fb02 100644
@@ -103,8 +103,6 @@ struct ena_stats_tx {
 	u64 cnt;
 	u64 bytes;
 	u64 prepare_ctx_err;
-	u64 linearize;
-	u64 linearize_failed;
 	u64 tx_poll;
 	u64 doorbells;
 	u64 bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #2 lore raw

PATCH 2/21 net/ena: add assertion on Tx info mbuf

In reply to Michal Krawczyk

To make sure there is no outstanding mbuf in the reused Tx queue (due to unproper cleanup, or some invalid logic on Tx path), the assertion was added on the Tx path.

As it's being compiled out in the release version, it won't affect the IO path performance.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ena/ena_ethdev.c +1 -0

index deaee30664..34fb43cb00 100644
@@ -2661,6 +2661,7 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	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;
+	RTE_ASSERT(tx_info->mbuf == NULL);
 
 	ena_tx_map_mbuf(tx_ring, tx_info, mbuf, &push_header, &header_len);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #3 lore raw

PATCH 3/21 net/ena: remove unused enumeration

In reply to Michal Krawczyk

The enumeration seems to be leftover from porting the Linux driver to the DPDK. It was used nowhere and refers to the ethtool which is not present in the DPDK.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 5 -----
 1 file changed, 5 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -5

index 34fb43cb00..2de2dcf12f 100644
@@ -38,11 +38,6 @@
 
 #define ENA_PTYPE_HAS_HASH	(RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP)
 
-enum ethtool_stringset {
-	ETH_SS_TEST             = 0,
-	ETH_SS_STATS,
-};
-
 struct ena_stats {
 	char name[ETH_GSTRING_LEN];
 	int stat_offset;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #4 lore raw

PATCH 4/21 net/ena: remove unused offloads variables

In reply to Michal Krawczyk

Those variables are being set, but never read. As they seem to be leftover from the old offloads API and don't have any purpose right now, they are simply being removed.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 ---
 drivers/net/ena/ena_ethdev.h | 5 -----
 2 files changed, 8 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index 2de2dcf12f..de5fdb8f1d 100644
@@ -2015,9 +2015,6 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
 	 */
 	adapter->tx_cleanup_stall_delay = adapter->missing_tx_completion_to / 2;
 
-	adapter->tx_selected_offloads = dev->data->dev_conf.txmode.offloads;
-	adapter->rx_selected_offloads = dev->data->dev_conf.rxmode.offloads;
-
 	return 0;
 }

drivers/net/ena/ena_ethdev.h +0 -5

index f47ba3fb02..4dfa604d51 100644
@@ -278,11 +278,6 @@ struct ena_adapter {
 	struct ena_driver_stats *drv_stats;
 	enum ena_adapter_state state;
 
-	uint64_t tx_supported_offloads;
-	uint64_t tx_selected_offloads;
-	uint64_t rx_supported_offloads;
-	uint64_t rx_selected_offloads;
-
 	bool link_status;
 
 	enum ena_regs_reset_reason_types reset_reason;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #5 lore raw

PATCH 5/21 net/ena: add extra Rx checksum related xstats

In reply to Michal Krawczyk

* Split 'bad_csum' Rx statistic into 'l3_csum_bad' and 'l4_csum_bad' to be able to check which checksum was not calculated properly.
* Add l4_csum_good statistic, which shows how many times L4 Rx checksum was properly offloaded.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |  8 +++++++
 drivers/net/ena/ena_ethdev.c           | 32 ++++++++++++++++----------
 drivers/net/ena/ena_ethdev.h           |  4 +++-
 3 files changed, 31 insertions(+), 13 deletions(-)

doc/guides/rel_notes/release_22_03.rst +8 -0

index 41923f50e6..6b69763f85 100644
@@ -104,6 +104,14 @@ New Features
   * Added support for libxdp >=v1.2.2.
   * Re-enabled secondary process support. RX/TX is not supported.
 
+* **Updated Amazon ENA PMD.**
+
+  Updated the Amazon ENA PMD. The new driver version (v2.6.0) introduced
+  bug fixes and improvements, including:
+
+  * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
+    ``l4_csum_good``.
+
 * **Updated Cisco enic driver.**
 
   * Added rte_flow support for matching GENEVE packets.

drivers/net/ena/ena_ethdev.c +20 ~~-12~~

index de5fdb8f1d..d534b93749 100644
@@ -99,7 +99,9 @@ static const struct ena_stats ena_stats_rx_strings[] = {
 	ENA_STAT_RX_ENTRY(cnt),
 	ENA_STAT_RX_ENTRY(bytes),
 	ENA_STAT_RX_ENTRY(refill_partial),
-	ENA_STAT_RX_ENTRY(bad_csum),
+	ENA_STAT_RX_ENTRY(l3_csum_bad),
+	ENA_STAT_RX_ENTRY(l4_csum_bad),
+	ENA_STAT_RX_ENTRY(l4_csum_good),
 	ENA_STAT_RX_ENTRY(mbuf_alloc_fail),
 	ENA_STAT_RX_ENTRY(bad_desc_num),
 	ENA_STAT_RX_ENTRY(bad_req_id),

@@ -273,10 +275,12 @@ static const struct eth_dev_ops ena_dev_ops = {
 	.rss_hash_conf_get    = ena_rss_hash_conf_get,
 };
 
-static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
+static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
+				       struct rte_mbuf *mbuf,
 				       struct ena_com_rx_ctx *ena_rx_ctx,
 				       bool fill_hash)
 {
+	struct ena_stats_rx *rx_stats = &rx_ring->rx_stats;
 	uint64_t ol_flags = 0;
 	uint32_t packet_type = 0;

@@ -287,21 +291,27 @@ static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
 
 	if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4) {
 		packet_type |= RTE_PTYPE_L3_IPV4;
-		if (unlikely(ena_rx_ctx->l3_csum_err))
+		if (unlikely(ena_rx_ctx->l3_csum_err)) {
+			++rx_stats->l3_csum_bad;
 			ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
-		else
+		} else {
 			ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+		}
 	} else if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV6) {
 		packet_type |= RTE_PTYPE_L3_IPV6;
 	}
 
-	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag)
+	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag) {
 		ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
-	else
-		if (unlikely(ena_rx_ctx->l4_csum_err))
+	} else {
+		if (unlikely(ena_rx_ctx->l4_csum_err)) {
+			++rx_stats->l4_csum_bad;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
-		else
+		} else {
+			++rx_stats->l4_csum_good;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
+		}
+	}
 
 	if (fill_hash &&
 	    likely((packet_type & ENA_PTYPE_HAS_HASH) && !ena_rx_ctx->frag)) {

@@ -2336,13 +2346,11 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		}
 
 		/* fill mbuf attributes if any */
-		ena_rx_mbuf_prepare(mbuf, &ena_rx_ctx, fill_hash);
+		ena_rx_mbuf_prepare(rx_ring, mbuf, &ena_rx_ctx, fill_hash);
 
 		if (unlikely(mbuf->ol_flags &
-				(RTE_MBUF_F_RX_IP_CKSUM_BAD | RTE_MBUF_F_RX_L4_CKSUM_BAD))) {
+				(RTE_MBUF_F_RX_IP_CKSUM_BAD | RTE_MBUF_F_RX_L4_CKSUM_BAD)))
 			rte_atomic64_inc(&rx_ring->adapter->drv_stats->ierrors);
-			++rx_ring->rx_stats.bad_csum;
-		}
 
 		rx_pkts[completed] = mbuf;
 		rx_ring->rx_stats.bytes += mbuf->pkt_len;

drivers/net/ena/ena_ethdev.h +3 -1

index 4dfa604d51..42c47c9455 100644
@@ -114,7 +114,9 @@ struct ena_stats_rx {
 	u64 cnt;
 	u64 bytes;
 	u64 refill_partial;
-	u64 bad_csum;
+	u64 l3_csum_bad;
+	u64 l4_csum_bad;
+	u64 l4_csum_good;
 	u64 mbuf_alloc_fail;
 	u64 bad_desc_num;
 	u64 bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #6 lore raw

PATCH 6/21 net/ena: make LSC configurable

In reply to Michal Krawczyk

ENA uses AENQ for notification about various events, like LSC, keep alive etc. By default it was enabling all AENQ that were supported by both the driver and the device. As a result the LSC was always processed even if the application turned it off explicitly.

As the DPDK provides application with the possibility to configure the LSC, ENA should respect that. AENQ groups are now being updated upon configure step, thus LSC can be activated or disabled between ENA PMD reconfigurations. Moreover, the LSC capability for the device is being determined dynamically.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 66 +++++++++++++++++++-------
 drivers/net/ena/ena_ethdev.h           |  5 +-
 3 files changed, 53 insertions(+), 19 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 6b69763f85..f803402138 100644
@@ -111,6 +111,7 @@ New Features
 
   * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
     ``l4_csum_good``.
+  * Added support for the link status configuration.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +49 ~~-17~~

134 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +3 -2

index 42c47c9455..f660b6a7cb 100644
@@ -291,9 +291,10 @@ struct ena_adapter {
 	struct ena_stats_dev dev_stats;
 	struct ena_stats_eni eni_stats;
 
-	bool trigger_reset;
+	uint32_t all_aenq_groups;
+	uint32_t active_aenq_groups;
 
-	bool wd_state;
+	bool trigger_reset;
 
 	bool use_large_llq_hdr;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #7 lore raw

PATCH 7/21 net/ena: skip timer if the reset is triggered

In reply to Michal Krawczyk

Some user applications may not support PMD reset handling. If they will support timer service it could cause a situation, when information about the reset trigger is being showed every time the timer service is being called.

Timer service is now being skipped if the reset was already triggered.

Fixes d9b8b106bf9d ("net/ena: add watchdog and keep alive AENQ handler")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/net/ena/ena_ethdev.c +3 -0

index a2793f13cd..4b82372155 100644
@@ -1624,6 +1624,9 @@ static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer,
 	struct rte_eth_dev *dev = arg;
 	struct ena_adapter *adapter = dev->data->dev_private;
 
+	if (unlikely(adapter->trigger_reset))
+		return;
+
 	check_for_missing_keep_alive(adapter);
 	check_for_admin_com_state(adapter);
 	check_for_tx_completions(adapter);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #8 lore raw

PATCH 8/21 net/ena: perform Tx cleanup before sending pkts

In reply to Michal Krawczyk

To increase likehood that current burst will fit in the HW rings, perform Tx cleanup before pushing packets to the HW. It may increase latency a bit for sparse bursts, but the Tx flow now should be more smooth.

It's also common order in the Tx burst function for other PMDs.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/net/ena/ena_ethdev.c +4 -6

index 4b82372155..ed3dd162ba 100644
@@ -2776,6 +2776,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	}
 #endif
 
+	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
+	if (available_desc < tx_ring->tx_free_thresh)
+		ena_tx_cleanup(tx_ring);
+
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx]))
 			break;

@@ -2784,9 +2788,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			tx_ring->size_mask)]);
 	}
 
-	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
-	tx_ring->tx_stats.available_desc = available_desc;
-
 	/* If there are ready packets to be xmitted... */
 	if (likely(tx_ring->pkts_without_db)) {
 		/* ...let HW do its best :-) */

@@ -2795,9 +2796,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		tx_ring->pkts_without_db = false;
 	}
 
-	if (available_desc < tx_ring->tx_free_thresh)
-		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++;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:07 UTC #9 lore raw

PATCH 9/21 net/ena/base: use optimized memcpy version also on Arm

In reply to Michal Krawczyk

As the default behavior for arm64 is to alias rte_memcpy as memcpy, ENA cannot redefine memcpy as rte_memcpy as it would cause nested declaration.

To make it possible to use optimized memcpy in the ena_com layer on Arm, the driver now redefines memcpy when it is beneficial:
* For arm64 only when the flag RTE_ARCH_ARM64_MEMCPY was defined
* For arm only when the flag RTE_ARCH_ARM_NEON_MEMCPY was defined

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst | 1 +
 drivers/net/ena/base/ena_plat_dpdk.h   | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index f803402138..9c7e246348 100644
@@ -112,6 +112,7 @@ New Features
   * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
     ``l4_csum_good``.
   * Added support for the link status configuration.
+  * Added optimized memcpy support for the ARM platforms.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/base/ena_plat_dpdk.h +5 -2

index 4e7f52881a..41db883c63 100644
@@ -66,8 +66,11 @@ typedef uint64_t dma_addr_t;
 #define ENA_UDELAY(x) rte_delay_us_block(x)
 
 #define ENA_TOUCH(x) ((void)(x))
-/* Avoid nested declaration on arm64, as it may define rte_memcpy as memcpy. */
-#if defined(RTE_ARCH_X86)
+/* Redefine memcpy with caution: rte_memcpy can be simply aliased to memcpy, so
+ * make the redefinition only if it's safe (and beneficial) to do so.
+ */
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64_MEMCPY) || \
+	defined(RTE_ARCH_ARM_NEON_MEMCPY)
 #undef memcpy
 #define memcpy rte_memcpy
 #endif

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #10 lore raw

PATCH 10/21 net/ena: proxy AQ calls to primary process

In reply to Michal Krawczyk

From: Stanislaw Kardach <[email protected]>

Due to how the ena_com compatibility layer is written, all AQ commands triggering functions use stack to save results of AQ and then copy them to user given function.
Therefore to keep the compatibility layer common, introduce ENA_PROXY macro. It either calls the wrapped function directly (in primary process) or proxies it to the primary via DPDK IPC mechanism. Since all proxied calls are taken under a lock share the result data through shared memory (in struct ena_adapter) to work around 256B IPC parameter size limit.

New proxy calls can be added by
1. Adding a new message type at the end of enum ena_mp_req
2. Adding new message arguments to the struct ena_mp_body if needed
3. Defining proxy request descriptor with ENA_PROXY_DESC. Its arguments include handlers for request preparation and response processing. Any of those may be empty (aside of marking arguments as used).
4. Adding request handling logic to ena_mp_primary_handle()
5. Replacing proxied function calls with ENA_PROXY(adapter, <func>, ...)

Signed-off-by Stanislaw Kardach <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |   1 +
 drivers/net/ena/ena_ethdev.c           | 384 ++++++++++++++++++++++++-
 drivers/net/ena/ena_ethdev.h           |   9 +
 drivers/net/ena/ena_rss.c              |  10 +-
 4 files changed, 384 insertions(+), 20 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 9c7e246348..92f36fa8ab 100644
@@ -113,6 +113,7 @@ New Features
     ``l4_csum_good``.
   * Added support for the link status configuration.
   * Added optimized memcpy support for the ARM platforms.
+  * Added ENA admin queue support for the MP applications.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +369 ~~-15~~

484 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +9 -0

index f660b6a7cb..01cf0ef5db 100644
@@ -29,6 +29,9 @@
 #define ENA_RX_BUF_MIN_SIZE	1400
 #define ENA_DEFAULT_RING_SIZE	1024
 
+#define ENA_RX_RSS_TABLE_LOG_SIZE	7
+#define ENA_RX_RSS_TABLE_SIZE		(1 << ENA_RX_RSS_TABLE_LOG_SIZE)
+
 #define ENA_MIN_MTU		128
 
 #define ENA_MMIO_DISABLE_REG_READ	BIT(0)

@@ -290,6 +293,9 @@ struct ena_adapter {
 
 	struct ena_stats_dev dev_stats;
 	struct ena_stats_eni eni_stats;
+	struct ena_admin_basic_stats basic_stats;
+
+	u32 indirect_table[ENA_RX_RSS_TABLE_SIZE];
 
 	uint32_t all_aenq_groups;
 	uint32_t active_aenq_groups;

@@ -304,6 +310,9 @@ struct ena_adapter {
 	uint64_t tx_cleanup_stall_delay;
 };
 
+int ena_mp_indirect_table_set(struct ena_adapter *adapter);
+int ena_mp_indirect_table_get(struct ena_adapter *adapter,
+			      uint32_t *indirect_table);
 int ena_rss_reta_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_reta_entry64 *reta_conf,
 			uint16_t reta_size);

drivers/net/ena/ena_rss.c +5 -5

index be4007e3f3..b6c4f76e38 100644
@@ -89,6 +89,8 @@ int ena_rss_reta_update(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+	/* Prevent RETA table structure update races */
+	rte_spinlock_lock(&adapter->admin_lock);
 	for (i = 0 ; i < reta_size ; i++) {
 		/* Each reta_conf is for 64 entries.
 		 * To support 128 we use 2 conf of 64.

@@ -109,8 +111,7 @@ int ena_rss_reta_update(struct rte_eth_dev *dev,
 		}
 	}
 
-	rte_spinlock_lock(&adapter->admin_lock);
-	rc = ena_com_indirect_table_set(ena_dev);
+	rc = ena_mp_indirect_table_set(adapter);
 	rte_spinlock_unlock(&adapter->admin_lock);
 	if (unlikely(rc != 0)) {
 		PMD_DRV_LOG(ERR, "Cannot set the indirection table\n");

@@ -128,9 +129,8 @@ int ena_rss_reta_query(struct rte_eth_dev *dev,
 		       struct rte_eth_rss_reta_entry64 *reta_conf,
 		       uint16_t reta_size)
 {
-	uint32_t indirect_table[ENA_RX_RSS_TABLE_SIZE] = { 0 };
+	uint32_t indirect_table[ENA_RX_RSS_TABLE_SIZE];
 	struct ena_adapter *adapter = dev->data->dev_private;
-	struct ena_com_dev *ena_dev = &adapter->ena_dev;
 	int rc;
 	int i;
 	int reta_conf_idx;

@@ -146,7 +146,7 @@ int ena_rss_reta_query(struct rte_eth_dev *dev,
 	}
 
 	rte_spinlock_lock(&adapter->admin_lock);
-	rc = ena_com_indirect_table_get(ena_dev, indirect_table);
+	rc = ena_mp_indirect_table_get(adapter, indirect_table);
 	rte_spinlock_unlock(&adapter->admin_lock);
 	if (unlikely(rc != 0)) {
 		PMD_DRV_LOG(ERR, "Cannot get indirection table\n");

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #11 lore raw

PATCH 11/21 net/ena: enable stats get function for MP mode

In reply to Michal Krawczyk

From: Stanislaw Kardach <[email protected]>

Since statistic gathering is now proxied safely to primary process, it can be enabled in secondary processes.

Signed-off-by Stanislaw Kardach <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index bb532923b6..ff6f27e4ad 100644
@@ -1068,9 +1068,6 @@ static int ena_stats_get(struct rte_eth_dev *dev,
 	int i;
 	int max_rings_stats;
 
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -ENOTSUP;
-
 	memset(&ena_stats, 0, sizeof(ena_stats));
 
 	rte_spinlock_lock(&adapter->admin_lock);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #12 lore raw

PATCH 12/21 net/ena/base: make IO memzone unique per port

In reply to Michal Krawczyk

Originally, the ena_com memzone counter was shared by ports, which caused the memzones to be harder to indentify and could potentially lead to race and beucase of that the counter had to be atomic.

This atomic counter was global variable and it couldn't work in the multiprocess implementation.

The memzone is now being identified by the local to port memzone counter and the port ID - both of those information can be found in the shared data, so it can be probed easily.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/features/ena.ini     |  1 +
 drivers/net/ena/base/ena_plat_dpdk.h | 62 ++++++++--------------------
 drivers/net/ena/ena_ethdev.c         | 52 ++++++++++++++++++++++-
 drivers/net/ena/ena_ethdev.h         |  2 +
 4 files changed, 71 insertions(+), 46 deletions(-)

doc/guides/nics/features/ena.ini +1 -0

index 4db1db11f4..55690aaf5a 100644
@@ -17,6 +17,7 @@ L3 checksum offload  = Y
 L4 checksum offload  = Y
 Basic stats          = Y
 Extended stats       = Y
+Multiprocess aware   = Y
 Linux                = Y
 ARMv8                = Y
 x86-32               = Y

drivers/net/ena/base/ena_plat_dpdk.h +18 ~~-44~~

84 lines not on this page · raw message

drivers/net/ena/ena_ethdev.c +50 -2

64 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +2 -0

index 01cf0ef5db..ca3e5ed691 100644
@@ -308,6 +308,8 @@ struct ena_adapter {
 	uint64_t missing_tx_completion_to;
 	uint64_t missing_tx_completion_budget;
 	uint64_t tx_cleanup_stall_delay;
+
+	uint64_t memzone_cnt;
 };
 
 int ena_mp_indirect_table_set(struct ena_adapter *adapter);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #13 lore raw

PATCH 13/21 net/ena: expose Tx cleanup function

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

ENA driver did not allow applications to call tx_cleanup. Freeing Tx mbufs was always done by the driver and it was not possible to manually request the driver to free mbufs.

Modify ena_tx_cleanup function to accept maximum number of packets to free and return number of packets that was freed.

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/features/ena.ini       |  1 +
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 27 ++++++++++++++++++--------
 3 files changed, 21 insertions(+), 8 deletions(-)

doc/guides/nics/features/ena.ini +1 -0

index 55690aaf5a..59c1ae85fa 100644
@@ -7,6 +7,7 @@
 Link status          = Y
 Link status event    = Y
 Rx interrupt         = Y
+Free Tx mbuf on demand = Y
 MTU update           = Y
 Scattered Rx         = Y
 TSO                  = Y

doc/guides/rel_notes/release_22_03.rst +1 -0

index 92f36fa8ab..91324dae18 100644
@@ -114,6 +114,7 @@ New Features
   * Added support for the link status configuration.
   * Added optimized memcpy support for the ARM platforms.
   * Added ENA admin queue support for the MP applications.
+  * Added free Tx mbuf on demand feature support.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +19 -8

index 8f30718f2c..eecdd447dd 100644
@@ -170,7 +170,7 @@ static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
 	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 int ena_tx_cleanup(void *txp, uint32_t free_pkt_cnt);
 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,

@@ -276,6 +276,7 @@ static const struct eth_dev_ops ena_dev_ops = {
 	.rx_queue_intr_disable = ena_rx_queue_intr_disable,
 	.rss_hash_update      = ena_rss_hash_update,
 	.rss_hash_conf_get    = ena_rss_hash_conf_get,
+	.tx_done_cleanup      = ena_tx_cleanup,
 };
 
 /*********************************************************************

@@ -2990,16 +2991,22 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	return 0;
 }
 
-static void ena_tx_cleanup(struct ena_ring *tx_ring)
+static int ena_tx_cleanup(void *txp, uint32_t free_pkt_cnt)
 {
+	struct ena_ring *tx_ring = (struct ena_ring *)txp;
 	unsigned int total_tx_descs = 0;
+	unsigned int total_tx_pkts = 0;
 	uint16_t cleanup_budget;
 	uint16_t next_to_clean = tx_ring->next_to_clean;
 
-	/* Attempt to release all Tx descriptors (ring_size - 1 -> size_mask) */
-	cleanup_budget = tx_ring->size_mask;
+	/*
+	 * If free_pkt_cnt is equal to 0, it means that the user requested
+	 * full cleanup, so attempt to release all Tx descriptors
+	 * (ring_size - 1 -> size_mask)
+	 */
+	cleanup_budget = (free_pkt_cnt == 0) ? tx_ring->size_mask : free_pkt_cnt;
 
-	while (likely(total_tx_descs < cleanup_budget)) {
+	while (likely(total_tx_pkts < cleanup_budget)) {
 		struct rte_mbuf *mbuf;
 		struct ena_tx_buffer *tx_info;
 		uint16_t req_id;

@@ -3021,6 +3028,7 @@ static void ena_tx_cleanup(struct ena_ring *tx_ring)
 		tx_ring->empty_tx_reqs[next_to_clean] = req_id;
 
 		total_tx_descs += tx_info->tx_descs;
+		total_tx_pkts++;
 
 		/* Put back descriptor to the ring for reuse */
 		next_to_clean = ENA_IDX_NEXT_MASKED(next_to_clean,

@@ -3034,8 +3042,11 @@ static void ena_tx_cleanup(struct ena_ring *tx_ring)
 		ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
 	}
 
-	/* Notify completion handler that the cleanup was just called */
-	tx_ring->last_cleanup_ticks = rte_get_timer_cycles();
+	/* Notify completion handler that full cleanup was performed */
+	if (free_pkt_cnt == 0 || total_tx_pkts < cleanup_budget)
+		tx_ring->last_cleanup_ticks = rte_get_timer_cycles();
+
+	return total_tx_pkts;
 }
 
 static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,

@@ -3056,7 +3067,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
 	if (available_desc < tx_ring->tx_free_thresh)
-		ena_tx_cleanup(tx_ring);
+		ena_tx_cleanup((void *)tx_ring, 0);
 
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx]))

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #14 lore raw

PATCH 14/21 net/ena: add API for probing xstat names by ID

In reply to Michal Krawczyk

ENA was only supporting retrieval of all the xstats name and wasn't implementing the eth_xstats_get_names_by_id API.

As this API may be more efficient than retrieving all the names, it tries to avoid excessive string copying.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |   1 +
 drivers/net/ena/ena_ethdev.c           | 130 ++++++++++++++++++++-----
 2 files changed, 108 insertions(+), 23 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 91324dae18..1b87b9c174 100644
@@ -115,6 +115,7 @@ New Features
   * Added optimized memcpy support for the ARM platforms.
   * Added ENA admin queue support for the MP applications.
   * Added free Tx mbuf on demand feature support.
+  * Added ``rte_eth_xstats_get_names_by_id`` API support.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +107 ~~-23~~

148 lines not on this page · raw message

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #15 lore raw

PATCH 15/21 net/ena: check if reset was already triggered

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

When triggering the reset, no check was performed to see if the reset was already triggered. This could result in original reset reaon being overwritten. Add ena_trigger_reset helper function, which checks if the reset was triggered and only sets the reset reason if the reset wasn't triggered yet. Replace all occurences of manually setting the reset with ena_trigger_reset call.

Fixes 2081d5e2e92d ("net/ena: add reset routine")

Cc [email protected]

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

drivers/net/ena/ena_ethdev.c +18 ~~-14~~

index 0766f613b0..d413111b08 100644
@@ -539,6 +539,15 @@ ENA_PROXY_DESC(ena_com_indirect_table_get, ENA_MP_IND_TBL_GET,
 }),
 	struct ena_com_dev *ena_dev, u32 *ind_tbl);
 
+static inline void ena_trigger_reset(struct ena_adapter *adapter,
+				     enum ena_regs_reset_reason_types reason)
+{
+	if (likely(!adapter->trigger_reset)) {
+		adapter->reset_reason = reason;
+		adapter->trigger_reset = true;
+	}
+}
+
 static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
 				       struct rte_mbuf *mbuf,
 				       struct ena_com_rx_ctx *ena_rx_ctx,

@@ -666,8 +675,7 @@ static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id)
 
 	/* Trigger device reset */
 	++tx_ring->tx_stats.bad_req_id;
-	tx_ring->adapter->reset_reason = ENA_REGS_RESET_INV_TX_REQ_ID;
-	tx_ring->adapter->trigger_reset	= true;
+	ena_trigger_reset(tx_ring->adapter, ENA_REGS_RESET_INV_TX_REQ_ID);
 	return -EFAULT;
 }

@@ -1783,8 +1791,7 @@ static void check_for_missing_keep_alive(struct ena_adapter *adapter)
 	if (unlikely((rte_get_timer_cycles() - adapter->timestamp_wd) >=
 	    adapter->keep_alive_timeout)) {
 		PMD_DRV_LOG(ERR, "Keep alive timeout\n");
-		adapter->reset_reason = ENA_REGS_RESET_KEEP_ALIVE_TO;
-		adapter->trigger_reset = true;
+		ena_trigger_reset(adapter, ENA_REGS_RESET_KEEP_ALIVE_TO);
 		++adapter->dev_stats.wd_expired;
 	}
 }

@@ -1794,8 +1801,7 @@ static void check_for_admin_com_state(struct ena_adapter *adapter)
 {
 	if (unlikely(!ena_com_get_admin_running_state(&adapter->ena_dev))) {
 		PMD_DRV_LOG(ERR, "ENA admin queue is not in running state\n");
-		adapter->reset_reason = ENA_REGS_RESET_ADMIN_TO;
-		adapter->trigger_reset = true;
+		ena_trigger_reset(adapter, ENA_REGS_RESET_ADMIN_TO);
 	}
 }

@@ -2606,14 +2612,13 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				rc);
 			if (rc == ENA_COM_NO_SPACE) {
 				++rx_ring->rx_stats.bad_desc_num;
-				rx_ring->adapter->reset_reason =
-					ENA_REGS_RESET_TOO_MANY_RX_DESCS;
+				ena_trigger_reset(rx_ring->adapter,
+					ENA_REGS_RESET_TOO_MANY_RX_DESCS);
 			} else {
 				++rx_ring->rx_stats.bad_req_id;
-				rx_ring->adapter->reset_reason =
-					ENA_REGS_RESET_INV_RX_REQ_ID;
+				ena_trigger_reset(rx_ring->adapter,
+					ENA_REGS_RESET_INV_RX_REQ_ID);
 			}
-			rx_ring->adapter->trigger_reset = true;
 			return 0;
 		}

@@ -2978,9 +2983,8 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	if (unlikely(rc)) {
 		PMD_DRV_LOG(ERR, "Failed to prepare Tx buffers, rc: %d\n", rc);
 		++tx_ring->tx_stats.prepare_ctx_err;
-		tx_ring->adapter->reset_reason =
-		    ENA_REGS_RESET_DRIVER_INVALID_STATE;
-		tx_ring->adapter->trigger_reset = true;
+		ena_trigger_reset(tx_ring->adapter,
+			ENA_REGS_RESET_DRIVER_INVALID_STATE);
 		return rc;
 	}

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #16 lore raw

PATCH 16/21 net/ena: make Tx completion timeout configurable

In reply to Michal Krawczyk

The default missing Tx completion timeout was set to 5 seconds. In order to provide users with the interface to control this timeout to adjust it with the application's watchdog, the device argument for controlling this value was added.

The parameter is called 'miss_txc_to' and can be modified using the devargs interface:

./app -a <bdf>,miss_txc_to=UINT_NUMBER

This parameter accepts values from 0 to 60 and indicates number of seconds after which the Tx packet will be considered as missing.

HW hints for the Tx completions timeout were removed to do not overwrite parameter from the user. Also specyfing default Tx completion timeout value was moved from the configuration to init phase in order to simplify default value assignment.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/ena.rst                |  9 ++++
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 66 ++++++++++++++++++++------
 drivers/net/ena/ena_ethdev.h           |  1 +
 4 files changed, 62 insertions(+), 15 deletions(-)

doc/guides/nics/ena.rst +9 -0

index 85c790e80f..3d780acac9 100644
@@ -87,6 +87,15 @@ Configuration information
      effect only if the device also supports large LLQ headers. Otherwise, the
      default value will be used.
 
+   * **miss_txc_to** (default 5)
+
+     Number of seconds after which the Tx packet will be considered missing.
+     If the missing packets number will exceed dynamically calculated threshold,
+     the driver will trigger the device reset which should be handled by the
+     application. Checking for missing Tx completions happens in the driver's
+     timer service. Setting this parameter to 0 disables this feature. Maximum
+     allowed value is 60 seconds.
+
 **ENA Configuration Parameters**
 
    * **Number of Queues**

doc/guides/rel_notes/release_22_03.rst +1 -0

index 1b87b9c174..d0d2824371 100644
@@ -116,6 +116,7 @@ New Features
   * Added ENA admin queue support for the MP applications.
   * Added free Tx mbuf on demand feature support.
   * Added ``rte_eth_xstats_get_names_by_id`` API support.
+  * Added ``miss_txc_to`` device argument for setting the Tx completion timeout.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +51 ~~-15~~

109 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +1 -0

index ca3e5ed691..c0094b03ee 100644
@@ -40,6 +40,7 @@
 #define ENA_DEVICE_KALIVE_TIMEOUT (ENA_WD_TIMEOUT_SEC * rte_get_timer_hz())
 
 #define ENA_TX_TIMEOUT			(5 * rte_get_timer_hz())
+#define ENA_MAX_TX_TIMEOUT_SECONDS	60
 #define ENA_MONITORED_TX_QUEUES		3
 #define ENA_DEFAULT_MISSING_COMP	256U

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:08 UTC #17 lore raw

PATCH 17/21 net/ena: fix meta-desc DF flag setup

In reply to Michal Krawczyk

Whenever Tx checksum offload is being used, the meta descriptor content is taken into consideration. Setting DF field properly in the meta descriptor may have huge impact on the performance both for the IPv4 and IPv6 packets.

The requirements for the df field are as below:
* No offload used - value doesn't matter
* IPv4 - 0 or 1, depending on the DF flag in the IPv4 header
* IPv6 - 1

Setting DF to 0 causes the packet to enter the slow-path in the HW and as a result can noticeable impact the performance.

Moreover, as 'true' may not always be mapped to 1 depending on it's definition for the given platform/compiler, for safety DF field is being set explicitly to 1.

Fixes 1173fca25af9 ("ena: add polling-mode driver")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +3 -1

index 4bb2b195b1..02b4144388 100644
@@ -624,6 +624,8 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 
 		if (mbuf->ol_flags & RTE_MBUF_F_TX_IPV6) {
 			ena_tx_ctx->l3_proto = ENA_ETH_IO_L3_PROTO_IPV6;
+			/* For the IPv6 packets, DF always needs to be true. */
+			ena_tx_ctx->df = 1;
 		} else {
 			ena_tx_ctx->l3_proto = ENA_ETH_IO_L3_PROTO_IPV4;

@@ -631,7 +633,7 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 			if (mbuf->packet_type &
 				(RTE_PTYPE_L4_NONFRAG
 				 | RTE_PTYPE_INNER_L4_NONFRAG))
-				ena_tx_ctx->df = true;
+				ena_tx_ctx->df = 1;
 		}
 
 		/* check if L4 checksum is needed */

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:09 UTC #18 lore raw

PATCH 18/21 net/ena: extend debug prints for invalid req ID resets

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

Add information about port id, queue id and req_id to error logs in validate_tx_req_id.

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/net/ena/ena_ethdev.c +4 -2

index 02b4144388..f4be79d240 100644
@@ -675,9 +675,11 @@ static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id)
 	}
 
 	if (tx_info)
-		PMD_TX_LOG(ERR, "tx_info doesn't have valid mbuf\n");
+		PMD_TX_LOG(ERR, "tx_info doesn't have valid mbuf. queue %d:%d req_id %u\n",
+			tx_ring->port_id, tx_ring->id, req_id);
 	else
-		PMD_TX_LOG(ERR, "Invalid req_id: %hu\n", req_id);
+		PMD_TX_LOG(ERR, "Invalid req_id: %hu in queue %d:%d\n",
+			req_id, tx_ring->port_id, tx_ring->id);
 
 	/* Trigger device reset */
 	++tx_ring->tx_stats.bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:09 UTC #19 lore raw

PATCH 19/21 net/ena: don't initialize LLQ when membar isn't exposed

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

The ena_com_config_dev_mode() performs many calculations related to LLQ and then performs an admin queue call to configure LLQ in the device.

All of the operations performed by ena_com_config_dev_mode() are unnecessary if membar hasn't been found. Move the dev_mem_base check before ena_com_config_dev_mode() call. This prevents the unnecessary operations from being performed.

Fixes 2fca2a98c0d1 ("net/ena: support LLQv2")

Cc [email protected]

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/net/ena/ena_ethdev.c +7 -7

index f4be79d240..9f29cf04ac 100644
@@ -1953,6 +1953,13 @@ ena_set_queues_placement_policy(struct ena_adapter *adapter,
 		return 0;
 	}
 
+	if (adapter->dev_mem_base == NULL) {
+		PMD_DRV_LOG(ERR,
+			"LLQ is advertised as supported, but device doesn't expose mem bar\n");
+		ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
+		return 0;
+	}
+
 	rc = ena_com_config_dev_mode(ena_dev, llq, llq_default_configurations);
 	if (unlikely(rc)) {
 		PMD_INIT_LOG(WARNING,

@@ -1965,13 +1972,6 @@ ena_set_queues_placement_policy(struct ena_adapter *adapter,
 	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_HOST)
 		return 0;
 
-	if (!adapter->dev_mem_base) {
-		PMD_DRV_LOG(ERR,
-			"Unable to access LLQ BAR resource. Fallback to host mode policy.\n");
-		ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
-		return 0;
-	}
-
 	ena_dev->mem_bar = adapter->dev_mem_base;
 
 	return 0;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:09 UTC #20 lore raw

PATCH 20/21 net/ena: don't indicate bad csum for L4 csum error

In reply to Michal Krawczyk

Some HW may invalidly set checksum error bit for the valid L4 checksum. To avoid drop of the packets in that situation, do not indicate bad checksum for L4 Rx csum offloads. Instead, set it as unknown, so the application will re-verify this value.

The statistics counters will still work as previously.

Fixes 05817057faba ("net/ena: fix indication of bad L4 Rx checksums")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +7 -1

index 9f29cf04ac..0861206ea4 100644
@@ -583,7 +583,13 @@ static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
 	} else {
 		if (unlikely(ena_rx_ctx->l4_csum_err)) {
 			++rx_stats->l4_csum_bad;
-			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+			/*
+			 * For the L4 Rx checksum offload the HW may indicate
+			 * bad checksum although it's valid. Because of that,
+			 * we're setting the UNKNOWN flag to let the app
+			 * re-verify the checksum.
+			 */
+			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
 		} else {
 			++rx_stats->l4_csum_good;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 16:09 UTC #21 lore raw

PATCH 21/21 net/ena: update version to 2.6.0

In reply to Michal Krawczyk

This release contains multiple bug fixes and improvements, including
- Removal of the linearization function from the xmit Tx path. The DPDK assumes checking for the mbuf segments number in the Tx prepare function.
- Extra logs, statistics, checks...
- Cleanup of the unused variables and definitions.
- Configurable Link Status event.
- Improvements for the timer service and the reset.
- Usage of the optimized memcpy on ARM.
- MP awareness improvements - extra API support for the secondary processes (like reading basic statistics).
- Support of the xstats API to get xstat names by ID.
- Configurable Tx completions timeout.
- Proper setting of the meta-descriptor's DF flag.

Signed-off-by Michal Krawczyk <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +1 -1

index 0861206ea4..53b2983d27 100644
@@ -21,7 +21,7 @@
 #include <ena_eth_io_defs.h>
 
 #define DRV_MODULE_VER_MAJOR	2
-#define DRV_MODULE_VER_MINOR	5
+#define DRV_MODULE_VER_MINOR	6
 #define DRV_MODULE_VER_SUBMINOR	0
 
 #define __MERGE_64B_H_L(h, l) (((uint64_t)h << 32) | l)

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #22 lore raw

v2 cover net/ena: v2.6.0 driver update

In reply to Michal Krawczyk

Hi,

this set contains new ENA features:

* New xstats.
* Reconfigurable link status event.
* Usage of the optimized memcpy on arm/arm64.
* Better MP support.
* Reconfigurable Tx completion timeout value using devarg.

Beside that, this patchset contains multiple fixes, minor improvements, new DPDK API support and extra error checks.

v2:
* Fix reviewer's email (s/[email protected]/[email protected]/).
* Fix commit messages typos found by the checkpatch.

Stanislaw Kardach (2):
net/ena: proxy AQ calls to primary process
net/ena: enable stats get function for MP mode

 doc/guides/nics/ena.rst                |   9 +
 doc/guides/nics/features/ena.ini       |   2 +
 doc/guides/rel_notes/release_22_03.rst |  14 +
 drivers/net/ena/base/ena_plat_dpdk.h   |  69 +-
 drivers/net/ena/ena_ethdev.c           | 916 ++++++++++++++++++++-----
 drivers/net/ena/ena_ethdev.h           |  28 +-
 drivers/net/ena/ena_rss.c              |  10 +-
 7 files changed, 800 insertions(+), 248 deletions(-)

--
2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #23 lore raw

PATCH v2 1/21 net/ena: remove linearization function

The linearization of the mbuf isn't common practice for the PMD, as it can expose it's capabilities to the upper layer using rte_eth_dev_info_get().

Moreover, the rte_eth_tx_prepare() function should also verify if the number of segments inside the mbuf isn't too high.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 74 +++++++++---------------------------
 drivers/net/ena/ena_ethdev.h |  2 -
 2 files changed, 19 insertions(+), 57 deletions(-)

drivers/net/ena/ena_ethdev.c +19 ~~-55~~

index db2b5ec8e7..deaee30664 100644
@@ -93,8 +93,6 @@ static const struct ena_stats ena_stats_tx_strings[] = {
 	ENA_STAT_TX_ENTRY(cnt),
 	ENA_STAT_TX_ENTRY(bytes),
 	ENA_STAT_TX_ENTRY(prepare_ctx_err),
-	ENA_STAT_TX_ENTRY(linearize),
-	ENA_STAT_TX_ENTRY(linearize_failed),
 	ENA_STAT_TX_ENTRY(tx_poll),
 	ENA_STAT_TX_ENTRY(doorbells),
 	ENA_STAT_TX_ENTRY(bad_req_id),

@@ -2408,6 +2406,17 @@ eth_ena_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			return i;
 		}
 
+		if (unlikely(m->nb_segs >= tx_ring->sgl_size &&
+		    !(tx_ring->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV &&
+		      m->nb_segs == tx_ring->sgl_size &&
+		      m->data_len < tx_ring->tx_max_header_size))) {
+			PMD_TX_LOG(DEBUG,
+				"mbuf[%" PRIu32 "] has too many segments: %" PRIu16 "\n",
+				i, m->nb_segs);
+			rte_errno = EINVAL;
+			return i;
+		}
+
 #ifdef RTE_LIBRTE_ETHDEV_DEBUG
 		/* Check if requested offload is also enabled for the queue */
 		if ((ol_flags & RTE_MBUF_F_TX_IP_CKSUM &&

@@ -2554,56 +2563,6 @@ static void ena_update_hints(struct ena_adapter *adapter,
 	}
 }
 
-static int ena_check_space_and_linearize_mbuf(struct ena_ring *tx_ring,
-					      struct rte_mbuf *mbuf)
-{
-	struct ena_com_dev *ena_dev;
-	int num_segments, header_len, rc;
-
-	ena_dev = &tx_ring->adapter->ena_dev;
-	num_segments = mbuf->nb_segs;
-	header_len = mbuf->data_len;
-
-	if (likely(num_segments < tx_ring->sgl_size))
-		goto checkspace;
-
-	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV &&
-	    (num_segments == tx_ring->sgl_size) &&
-	    (header_len < tx_ring->tx_max_header_size))
-		goto checkspace;
-
-	/* Checking for space for 2 additional metadata descriptors due to
-	 * possible header split and metadata descriptor. Linearization will
-	 * be needed so we reduce the segments number from num_segments to 1
-	 */
-	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq, 3)) {
-		PMD_TX_LOG(DEBUG, "Not enough space in the Tx queue\n");
-		return ENA_COM_NO_MEM;
-	}
-	++tx_ring->tx_stats.linearize;
-	rc = rte_pktmbuf_linearize(mbuf);
-	if (unlikely(rc)) {
-		PMD_TX_LOG(WARNING, "Mbuf linearize failed\n");
-		rte_atomic64_inc(&tx_ring->adapter->drv_stats->ierrors);
-		++tx_ring->tx_stats.linearize_failed;
-		return rc;
-	}
-
-	return 0;
-
-checkspace:
-	/* Checking for space for 2 additional metadata descriptors due to
-	 * possible header split and metadata descriptor
-	 */
-	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq,
-					  num_segments + 2)) {
-		PMD_TX_LOG(DEBUG, "Not enough space in the Tx queue\n");
-		return ENA_COM_NO_MEM;
-	}
-
-	return 0;
-}
-
 static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
 	struct ena_tx_buffer *tx_info,
 	struct rte_mbuf *mbuf,

@@ -2688,9 +2647,14 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	int nb_hw_desc;
 	int rc;
 
-	rc = ena_check_space_and_linearize_mbuf(tx_ring, mbuf);
-	if (unlikely(rc))
-		return rc;
+	/* Checking for space for 2 additional metadata descriptors due to
+	 * possible header split and metadata descriptor
+	 */
+	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq,
+					  mbuf->nb_segs + 2)) {
+		PMD_DRV_LOG(DEBUG, "Not enough space in the tx queue\n");
+		return ENA_COM_NO_MEM;
+	}
 
 	next_to_use = tx_ring->next_to_use;

drivers/net/ena/ena_ethdev.h +0 -2

index f99e4f3984..f47ba3fb02 100644
@@ -103,8 +103,6 @@ struct ena_stats_tx {
 	u64 cnt;
 	u64 bytes;
 	u64 prepare_ctx_err;
-	u64 linearize;
-	u64 linearize_failed;
 	u64 tx_poll;
 	u64 doorbells;
 	u64 bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #24 lore raw

PATCH v2 2/21 net/ena: add assertion on Tx info mbuf

In reply to Michal Krawczyk

To make sure there is no outstanding mbuf in the reused Tx queue (due to unproper cleanup, or some invalid logic on Tx path), the assertion was added on the Tx path.

As it's being compiled out in the release version, it won't affect the IO path performance.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ena/ena_ethdev.c +1 -0

index deaee30664..34fb43cb00 100644
@@ -2661,6 +2661,7 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	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;
+	RTE_ASSERT(tx_info->mbuf == NULL);
 
 	ena_tx_map_mbuf(tx_ring, tx_info, mbuf, &push_header, &header_len);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #25 lore raw

PATCH v2 3/21 net/ena: remove unused enumeration

In reply to Michal Krawczyk

The enumeration seems to be leftover from porting the Linux driver to the DPDK. It was used nowhere and refers to the ethtool which is not present in the DPDK.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 5 -----
 1 file changed, 5 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -5

index 34fb43cb00..2de2dcf12f 100644
@@ -38,11 +38,6 @@
 
 #define ENA_PTYPE_HAS_HASH	(RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP)
 
-enum ethtool_stringset {
-	ETH_SS_TEST             = 0,
-	ETH_SS_STATS,
-};
-
 struct ena_stats {
 	char name[ETH_GSTRING_LEN];
 	int stat_offset;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #26 lore raw

PATCH v2 4/21 net/ena: remove unused offloads variables

In reply to Michal Krawczyk

Those variables are being set, but never read. As they seem to be leftover from the old offloads API and don't have any purpose right now, they are simply being removed.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 ---
 drivers/net/ena/ena_ethdev.h | 5 -----
 2 files changed, 8 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index 2de2dcf12f..de5fdb8f1d 100644
@@ -2015,9 +2015,6 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
 	 */
 	adapter->tx_cleanup_stall_delay = adapter->missing_tx_completion_to / 2;
 
-	adapter->tx_selected_offloads = dev->data->dev_conf.txmode.offloads;
-	adapter->rx_selected_offloads = dev->data->dev_conf.rxmode.offloads;
-
 	return 0;
 }

drivers/net/ena/ena_ethdev.h +0 -5

index f47ba3fb02..4dfa604d51 100644
@@ -278,11 +278,6 @@ struct ena_adapter {
 	struct ena_driver_stats *drv_stats;
 	enum ena_adapter_state state;
 
-	uint64_t tx_supported_offloads;
-	uint64_t tx_selected_offloads;
-	uint64_t rx_supported_offloads;
-	uint64_t rx_selected_offloads;
-
 	bool link_status;
 
 	enum ena_regs_reset_reason_types reset_reason;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #27 lore raw

PATCH v2 5/21 net/ena: add extra Rx checksum related xstats

In reply to Michal Krawczyk

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |  8 +++++++
 drivers/net/ena/ena_ethdev.c           | 32 ++++++++++++++++----------
 drivers/net/ena/ena_ethdev.h           |  4 +++-
 3 files changed, 31 insertions(+), 13 deletions(-)

doc/guides/rel_notes/release_22_03.rst +8 -0

index 41923f50e6..6b69763f85 100644
@@ -104,6 +104,14 @@ New Features
   * Added support for libxdp >=v1.2.2.
   * Re-enabled secondary process support. RX/TX is not supported.
 
+* **Updated Amazon ENA PMD.**
+
+  Updated the Amazon ENA PMD. The new driver version (v2.6.0) introduced
+  bug fixes and improvements, including:
+
+  * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
+    ``l4_csum_good``.
+
 * **Updated Cisco enic driver.**
 
   * Added rte_flow support for matching GENEVE packets.

drivers/net/ena/ena_ethdev.c +20 ~~-12~~

index de5fdb8f1d..d534b93749 100644
@@ -99,7 +99,9 @@ static const struct ena_stats ena_stats_rx_strings[] = {
 	ENA_STAT_RX_ENTRY(cnt),
 	ENA_STAT_RX_ENTRY(bytes),
 	ENA_STAT_RX_ENTRY(refill_partial),
-	ENA_STAT_RX_ENTRY(bad_csum),
+	ENA_STAT_RX_ENTRY(l3_csum_bad),
+	ENA_STAT_RX_ENTRY(l4_csum_bad),
+	ENA_STAT_RX_ENTRY(l4_csum_good),
 	ENA_STAT_RX_ENTRY(mbuf_alloc_fail),
 	ENA_STAT_RX_ENTRY(bad_desc_num),
 	ENA_STAT_RX_ENTRY(bad_req_id),

@@ -273,10 +275,12 @@ static const struct eth_dev_ops ena_dev_ops = {
 	.rss_hash_conf_get    = ena_rss_hash_conf_get,
 };
 
-static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
+static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
+				       struct rte_mbuf *mbuf,
 				       struct ena_com_rx_ctx *ena_rx_ctx,
 				       bool fill_hash)
 {
+	struct ena_stats_rx *rx_stats = &rx_ring->rx_stats;
 	uint64_t ol_flags = 0;
 	uint32_t packet_type = 0;

@@ -287,21 +291,27 @@ static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
 
 	if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4) {
 		packet_type |= RTE_PTYPE_L3_IPV4;
-		if (unlikely(ena_rx_ctx->l3_csum_err))
+		if (unlikely(ena_rx_ctx->l3_csum_err)) {
+			++rx_stats->l3_csum_bad;
 			ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
-		else
+		} else {
 			ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+		}
 	} else if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV6) {
 		packet_type |= RTE_PTYPE_L3_IPV6;
 	}
 
-	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag)
+	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag) {
 		ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
-	else
-		if (unlikely(ena_rx_ctx->l4_csum_err))
+	} else {
+		if (unlikely(ena_rx_ctx->l4_csum_err)) {
+			++rx_stats->l4_csum_bad;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
-		else
+		} else {
+			++rx_stats->l4_csum_good;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
+		}
+	}
 
 	if (fill_hash &&
 	    likely((packet_type & ENA_PTYPE_HAS_HASH) && !ena_rx_ctx->frag)) {

@@ -2336,13 +2346,11 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		}
 
 		/* fill mbuf attributes if any */
-		ena_rx_mbuf_prepare(mbuf, &ena_rx_ctx, fill_hash);
+		ena_rx_mbuf_prepare(rx_ring, mbuf, &ena_rx_ctx, fill_hash);
 
 		if (unlikely(mbuf->ol_flags &
-				(RTE_MBUF_F_RX_IP_CKSUM_BAD | RTE_MBUF_F_RX_L4_CKSUM_BAD))) {
+				(RTE_MBUF_F_RX_IP_CKSUM_BAD | RTE_MBUF_F_RX_L4_CKSUM_BAD)))
 			rte_atomic64_inc(&rx_ring->adapter->drv_stats->ierrors);
-			++rx_ring->rx_stats.bad_csum;
-		}
 
 		rx_pkts[completed] = mbuf;
 		rx_ring->rx_stats.bytes += mbuf->pkt_len;

drivers/net/ena/ena_ethdev.h +3 -1

index 4dfa604d51..42c47c9455 100644
@@ -114,7 +114,9 @@ struct ena_stats_rx {
 	u64 cnt;
 	u64 bytes;
 	u64 refill_partial;
-	u64 bad_csum;
+	u64 l3_csum_bad;
+	u64 l4_csum_bad;
+	u64 l4_csum_good;
 	u64 mbuf_alloc_fail;
 	u64 bad_desc_num;
 	u64 bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #28 lore raw

PATCH v2 6/21 net/ena: make LSC configurable

In reply to Michal Krawczyk

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 66 +++++++++++++++++++-------
 drivers/net/ena/ena_ethdev.h           |  5 +-
 3 files changed, 53 insertions(+), 19 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 6b69763f85..f803402138 100644
@@ -111,6 +111,7 @@ New Features
 
   * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
     ``l4_csum_good``.
+  * Added support for the link status configuration.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +49 ~~-17~~

134 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +3 -2

index 42c47c9455..f660b6a7cb 100644
@@ -291,9 +291,10 @@ struct ena_adapter {
 	struct ena_stats_dev dev_stats;
 	struct ena_stats_eni eni_stats;
 
-	bool trigger_reset;
+	uint32_t all_aenq_groups;
+	uint32_t active_aenq_groups;
 
-	bool wd_state;
+	bool trigger_reset;
 
 	bool use_large_llq_hdr;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:12 UTC #29 lore raw

PATCH v2 7/21 net/ena: skip timer if the reset is triggered

In reply to Michal Krawczyk

Timer service is now being skipped if the reset was already triggered.

Fixes d9b8b106bf9d ("net/ena: add watchdog and keep alive AENQ handler")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/net/ena/ena_ethdev.c +3 -0

index a2793f13cd..4b82372155 100644
@@ -1624,6 +1624,9 @@ static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer,
 	struct rte_eth_dev *dev = arg;
 	struct ena_adapter *adapter = dev->data->dev_private;
 
+	if (unlikely(adapter->trigger_reset))
+		return;
+
 	check_for_missing_keep_alive(adapter);
 	check_for_admin_com_state(adapter);
 	check_for_tx_completions(adapter);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #30 lore raw

PATCH v2 8/21 net/ena: perform Tx cleanup before sending pkts

In reply to Michal Krawczyk

It's also common order in the Tx burst function for other PMDs.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/net/ena/ena_ethdev.c +4 -6

index 4b82372155..ed3dd162ba 100644
@@ -2776,6 +2776,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	}
 #endif
 
+	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
+	if (available_desc < tx_ring->tx_free_thresh)
+		ena_tx_cleanup(tx_ring);
+
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx]))
 			break;

@@ -2784,9 +2788,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			tx_ring->size_mask)]);
 	}
 
-	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
-	tx_ring->tx_stats.available_desc = available_desc;
-
 	/* If there are ready packets to be xmitted... */
 	if (likely(tx_ring->pkts_without_db)) {
 		/* ...let HW do its best :-) */

@@ -2795,9 +2796,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		tx_ring->pkts_without_db = false;
 	}
 
-	if (available_desc < tx_ring->tx_free_thresh)
-		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++;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #31 lore raw

PATCH v2 9/21 net/ena/base: use optimized memcpy version also on Arm

In reply to Michal Krawczyk

As the default behavior for arm64 is to alias rte_memcpy as memcpy, ENA cannot redefine memcpy as rte_memcpy as it would cause nested declaration.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst | 1 +
 drivers/net/ena/base/ena_plat_dpdk.h   | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index f803402138..9c7e246348 100644
@@ -112,6 +112,7 @@ New Features
   * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
     ``l4_csum_good``.
   * Added support for the link status configuration.
+  * Added optimized memcpy support for the ARM platforms.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/base/ena_plat_dpdk.h +5 -2

index 4e7f52881a..41db883c63 100644
@@ -66,8 +66,11 @@ typedef uint64_t dma_addr_t;
 #define ENA_UDELAY(x) rte_delay_us_block(x)
 
 #define ENA_TOUCH(x) ((void)(x))
-/* Avoid nested declaration on arm64, as it may define rte_memcpy as memcpy. */
-#if defined(RTE_ARCH_X86)
+/* Redefine memcpy with caution: rte_memcpy can be simply aliased to memcpy, so
+ * make the redefinition only if it's safe (and beneficial) to do so.
+ */
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64_MEMCPY) || \
+	defined(RTE_ARCH_ARM_NEON_MEMCPY)
 #undef memcpy
 #define memcpy rte_memcpy
 #endif

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #32 lore raw

PATCH v2 10/21 net/ena: proxy AQ calls to primary process

In reply to Michal Krawczyk

From: Stanislaw Kardach <[email protected]>

Signed-off-by Stanislaw Kardach <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |   1 +
 drivers/net/ena/ena_ethdev.c           | 384 ++++++++++++++++++++++++-
 drivers/net/ena/ena_ethdev.h           |   9 +
 drivers/net/ena/ena_rss.c              |  10 +-
 4 files changed, 384 insertions(+), 20 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 9c7e246348..92f36fa8ab 100644
@@ -113,6 +113,7 @@ New Features
     ``l4_csum_good``.
   * Added support for the link status configuration.
   * Added optimized memcpy support for the ARM platforms.
+  * Added ENA admin queue support for the MP applications.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +369 ~~-15~~

484 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +9 -0

index f660b6a7cb..01cf0ef5db 100644
@@ -29,6 +29,9 @@
 #define ENA_RX_BUF_MIN_SIZE	1400
 #define ENA_DEFAULT_RING_SIZE	1024
 
+#define ENA_RX_RSS_TABLE_LOG_SIZE	7
+#define ENA_RX_RSS_TABLE_SIZE		(1 << ENA_RX_RSS_TABLE_LOG_SIZE)
+
 #define ENA_MIN_MTU		128
 
 #define ENA_MMIO_DISABLE_REG_READ	BIT(0)

@@ -290,6 +293,9 @@ struct ena_adapter {
 
 	struct ena_stats_dev dev_stats;
 	struct ena_stats_eni eni_stats;
+	struct ena_admin_basic_stats basic_stats;
+
+	u32 indirect_table[ENA_RX_RSS_TABLE_SIZE];
 
 	uint32_t all_aenq_groups;
 	uint32_t active_aenq_groups;

@@ -304,6 +310,9 @@ struct ena_adapter {
 	uint64_t tx_cleanup_stall_delay;
 };
 
+int ena_mp_indirect_table_set(struct ena_adapter *adapter);
+int ena_mp_indirect_table_get(struct ena_adapter *adapter,
+			      uint32_t *indirect_table);
 int ena_rss_reta_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_reta_entry64 *reta_conf,
 			uint16_t reta_size);

drivers/net/ena/ena_rss.c +5 -5

index be4007e3f3..b6c4f76e38 100644
@@ -89,6 +89,8 @@ int ena_rss_reta_update(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+	/* Prevent RETA table structure update races */
+	rte_spinlock_lock(&adapter->admin_lock);
 	for (i = 0 ; i < reta_size ; i++) {
 		/* Each reta_conf is for 64 entries.
 		 * To support 128 we use 2 conf of 64.

@@ -109,8 +111,7 @@ int ena_rss_reta_update(struct rte_eth_dev *dev,
 		}
 	}
 
-	rte_spinlock_lock(&adapter->admin_lock);
-	rc = ena_com_indirect_table_set(ena_dev);
+	rc = ena_mp_indirect_table_set(adapter);
 	rte_spinlock_unlock(&adapter->admin_lock);
 	if (unlikely(rc != 0)) {
 		PMD_DRV_LOG(ERR, "Cannot set the indirection table\n");

@@ -128,9 +129,8 @@ int ena_rss_reta_query(struct rte_eth_dev *dev,
 		       struct rte_eth_rss_reta_entry64 *reta_conf,
 		       uint16_t reta_size)
 {
-	uint32_t indirect_table[ENA_RX_RSS_TABLE_SIZE] = { 0 };
+	uint32_t indirect_table[ENA_RX_RSS_TABLE_SIZE];
 	struct ena_adapter *adapter = dev->data->dev_private;
-	struct ena_com_dev *ena_dev = &adapter->ena_dev;
 	int rc;
 	int i;
 	int reta_conf_idx;

@@ -146,7 +146,7 @@ int ena_rss_reta_query(struct rte_eth_dev *dev,
 	}
 
 	rte_spinlock_lock(&adapter->admin_lock);
-	rc = ena_com_indirect_table_get(ena_dev, indirect_table);
+	rc = ena_mp_indirect_table_get(adapter, indirect_table);
 	rte_spinlock_unlock(&adapter->admin_lock);
 	if (unlikely(rc != 0)) {
 		PMD_DRV_LOG(ERR, "Cannot get indirection table\n");

Signature · 1 line

2.25.1

Ferruh Yigit

2022-02-22 22:24 UTC #33 lore raw

Re: [PATCH v2 10/21] net/ena: proxy AQ calls to primary process

Quoted 25 lines · Michal Krawczyk

From: Stanislaw Kardach <[email protected]>
Due to how the ena_com compatibility layer is written, all AQ commands triggering functions use stack to save results of AQ and then copy them to user given function.
Therefore to keep the compatibility layer common, introduce ENA_PROXY macro. It either calls the wrapped function directly (in primary process) or proxies it to the primary via DPDK IPC mechanism. Since all proxied calls are taken under a lock share the result data through shared memory (in struct ena_adapter) to work around 256B IPC parameter size limit.
New proxy calls can be added by
1. Adding a new message type at the end of enum ena_mp_req
2. Adding new message arguments to the struct ena_mp_body if needed
3. Defining proxy request descriptor with ENA_PROXY_DESC. Its arguments include handlers for request preparation and response processing. Any of those may be empty (aside of marking arguments as used).
4. Adding request handling logic to ena_mp_primary_handle()
5. Replacing proxied function calls with ENA_PROXY(adapter, <func>, ...)
Signed-off-by Stanislaw Kardach <[email protected]>
Reviewed-by Michal Krawczyk <[email protected]>
Reviewed-by Dawid Gorecki <[email protected]>
Reviewed-by Shai Brandes <[email protected]>

<...>

Quoted 11 lines

+ */

drivers/net/ena/ena_ethdev.c +10 -0

+#define ENA_PROXY(a, f, ...)						\
+({									\
+	struct ena_adapter *_a = (a);					\
+	struct timespec ts = { .tv_sec = ENA_MP_REQ_TMO };		\
+	struct ena_mp_body *req, *rsp;					\
+	struct rte_mp_reply mp_rep;					\
+	struct rte_mp_msg mp_req;					\
+	int ret;							\
+									\
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {		\

drivers/net/ena/ena_ethdev.c static const struct eth_dev_ops ena_dev_ops = { · line 434

Quoted 10 lines

drivers/net/ena/ena_ethdev.c +10 -0

+		ret = f(__VA_ARGS__);					\
+	} else {							\
+		/* Prepare and send request */				\
+		req = (struct ena_mp_body *)&mp_req.param;		\
+		mp_msg_init(&mp_req, mp_type_ ## f, _a->edev_data->port_id); \
+		mp_prep_ ## f(_a, req, ## __VA_ARGS__);			\
+									\
+		ret = rte_mp_request_sync(&mp_req, &mp_rep, &ts);	\
+		if (likely(!ret)) {					\
+			RTE_ASSERT(mp_rsp.nb_received == 1);		\

'mp_rsp' is not defined, I assume intention is to use 'mp_rep'.

Build error can be reproduced by enabling RTE_ASSERT (by setting 'RTE_ENABLE_ASSERT').

Quoted 24 lines

drivers/net/ena/ena_ethdev.c +23 -0

+			rsp = (struct ena_mp_body *)&mp_rep.msgs[0].param; \
+			ret = rsp->result;				\
+			if (ret == 0) {					\
+				mp_proc_##f(_a, rsp, ## __VA_ARGS__);	\
+			} else {					\
+				PMD_DRV_LOG(ERR,			\
+					    "%s returned error: %d\n",	\
+					    mp_name_ ## f, rsp->result);\
+			}						\
+			free(mp_rep.msgs);				\
+		} else if (rte_errno == ENOTSUP) {			\
+			PMD_DRV_LOG(ERR,				\
+				    "No IPC, can't proxy to primary\n");\
+			ret = -rte_errno;				\
+		} else {						\
+			PMD_DRV_LOG(ERR, "Request %s failed: %s\n",	\
+				    mp_name_ ## f,			\
+				    rte_strerror(rte_errno));		\
+			ret = -ECOMM;					\
+		}							\
+	}								\
+	ret;								\
+})

<...>

Ferruh Yigit

2022-02-23 00:51 UTC #34 lore raw

Re: [PATCH v2 10/21] net/ena: proxy AQ calls to primary process

Quoted 53 lines · Ferruh Yigit

Quoted 25 lines · Michal Krawczyk
From: Stanislaw Kardach <[email protected]>
Due to how the ena_com compatibility layer is written, all AQ commands triggering functions use stack to save results of AQ and then copy them to user given function.
Therefore to keep the compatibility layer common, introduce ENA_PROXY macro. It either calls the wrapped function directly (in primary process) or proxies it to the primary via DPDK IPC mechanism. Since all proxied calls are taken under a lock share the result data through shared memory (in struct ena_adapter) to work around 256B IPC parameter size limit.
New proxy calls can be added by
1. Adding a new message type at the end of enum ena_mp_req
2. Adding new message arguments to the struct ena_mp_body if needed
3. Defining proxy request descriptor with ENA_PROXY_DESC. Its arguments include handlers for request preparation and response processing. Any of those may be empty (aside of marking arguments as used).
4. Adding request handling logic to ena_mp_primary_handle()
5. Replacing proxied function calls with ENA_PROXY(adapter, <func>, ...)
Signed-off-by Stanislaw Kardach <[email protected]>
Reviewed-by Michal Krawczyk <[email protected]>
Reviewed-by Dawid Gorecki <[email protected]>
Reviewed-by Shai Brandes <[email protected]>
<...>
Quoted 11 lines
+ */
drivers/net/ena/ena_ethdev.c +10 -0
+#define ENA_PROXY(a, f, ...)                        \
+({                                    \
+    struct ena_adapter *_a = (a);                    \
+    struct timespec ts = { .tv_sec = ENA_MP_REQ_TMO };        \
+    struct ena_mp_body *req, *rsp;                    \
+    struct rte_mp_reply mp_rep;                    \
+    struct rte_mp_msg mp_req;                    \
+    int ret;                            \
+                                    \
+    if (rte_eal_process_type() == RTE_PROC_PRIMARY) {        \
drivers/net/ena/ena_ethdev.c static const struct eth_dev_ops ena_dev_ops = { · line 434
Quoted 10 lines
drivers/net/ena/ena_ethdev.c +10 -0
+        ret = f(__VA_ARGS__);                    \
+    } else {                            \
+        /* Prepare and send request */                \
+        req = (struct ena_mp_body *)&mp_req.param;        \
+        mp_msg_init(&mp_req, mp_type_ ## f, _a->edev_data->port_id); \
+        mp_prep_ ## f(_a, req, ## __VA_ARGS__);            \
+                                    \
+        ret = rte_mp_request_sync(&mp_req, &mp_rep, &ts);    \
+        if (likely(!ret)) {                    \
+            RTE_ASSERT(mp_rsp.nb_received == 1);        \
'mp_rsp' is not defined, I assume intention is to use 'mp_rep'.

Quoted 22 lines

Build error can be reproduced by enabling RTE_ASSERT (by setting 'RTE_ENABLE_ASSERT').
Quoted 19 lines
+            rsp = (struct ena_mp_body *)&mp_rep.msgs[0].param; \
+            ret = rsp->result;                \
+            if (ret == 0) {                    \
+                mp_proc_##f(_a, rsp, ## __VA_ARGS__);    \
+            } else {                    \
+                PMD_DRV_LOG(ERR,            \
+                        "%s returned error: %d\n",    \
+                        mp_name_ ## f, rsp->result);\
+            }                        \
+            free(mp_rep.msgs);                \
+        } else if (rte_errno == ENOTSUP) {            \
+            PMD_DRV_LOG(ERR,                \
+                    "No IPC, can't proxy to primary\n");\
+            ret = -rte_errno;                \
+        } else {                        \
+            PMD_DRV_LOG(ERR, "Request %s failed: %s\n",    \
+                    mp_name_ ## f,            \
+                    rte_strerror(rte_errno));        \
+            ret = -ECOMM;                    \

Also 'ECOMM' seems only defined for Linux, so it breaks the FreeBSD build:
http://mails.dpdk.org/archives/test-report/2022-February/263131.html

Quoted 6 lines

Quoted 5 lines
+        }                            \
+    }                                \
+    ret;                                \
+})
<...>

Michal Krawczyk

author

2022-02-22 18:13 UTC #35 lore raw

PATCH v2 11/21 net/ena: enable stats get function for MP mode

In reply to Michal Krawczyk

From: Stanislaw Kardach <[email protected]>

Since statistic gathering is now proxied safely to primary process, it can be enabled in secondary processes.

Signed-off-by Stanislaw Kardach <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index bb532923b6..ff6f27e4ad 100644
@@ -1068,9 +1068,6 @@ static int ena_stats_get(struct rte_eth_dev *dev,
 	int i;
 	int max_rings_stats;
 
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -ENOTSUP;
-
 	memset(&ena_stats, 0, sizeof(ena_stats));
 
 	rte_spinlock_lock(&adapter->admin_lock);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #36 lore raw

PATCH v2 12/21 net/ena/base: make IO memzone unique per port

In reply to Michal Krawczyk

Originally, the ena_com memzone counter was shared by ports, which caused the memzones to be harder to identify and could potentially lead to race and because of that the counter had to be atomic.

This atomic counter was global variable and it couldn't work in the multiprocess implementation.

The memzone is now being identified by the local to port memzone counter and the port ID - both of those information can be found in the shared data, so it can be probed easily.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/features/ena.ini     |  1 +
 drivers/net/ena/base/ena_plat_dpdk.h | 62 ++++++++--------------------
 drivers/net/ena/ena_ethdev.c         | 52 ++++++++++++++++++++++-
 drivers/net/ena/ena_ethdev.h         |  2 +
 4 files changed, 71 insertions(+), 46 deletions(-)

doc/guides/nics/features/ena.ini +1 -0

index 4db1db11f4..55690aaf5a 100644
@@ -17,6 +17,7 @@ L3 checksum offload  = Y
 L4 checksum offload  = Y
 Basic stats          = Y
 Extended stats       = Y
+Multiprocess aware   = Y
 Linux                = Y
 ARMv8                = Y
 x86-32               = Y

drivers/net/ena/base/ena_plat_dpdk.h +18 ~~-44~~

84 lines not on this page · raw message

drivers/net/ena/ena_ethdev.c +50 -2

64 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +2 -0

index 01cf0ef5db..ca3e5ed691 100644
@@ -308,6 +308,8 @@ struct ena_adapter {
 	uint64_t missing_tx_completion_to;
 	uint64_t missing_tx_completion_budget;
 	uint64_t tx_cleanup_stall_delay;
+
+	uint64_t memzone_cnt;
 };
 
 int ena_mp_indirect_table_set(struct ena_adapter *adapter);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #37 lore raw

PATCH v2 13/21 net/ena: expose Tx cleanup function

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

ENA driver did not allow applications to call tx_cleanup. Freeing Tx mbufs was always done by the driver and it was not possible to manually request the driver to free mbufs.

Modify ena_tx_cleanup function to accept maximum number of packets to free and return number of packets that was freed.

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/features/ena.ini       |  1 +
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 27 ++++++++++++++++++--------
 3 files changed, 21 insertions(+), 8 deletions(-)

doc/guides/nics/features/ena.ini +1 -0

index 55690aaf5a..59c1ae85fa 100644
@@ -7,6 +7,7 @@
 Link status          = Y
 Link status event    = Y
 Rx interrupt         = Y
+Free Tx mbuf on demand = Y
 MTU update           = Y
 Scattered Rx         = Y
 TSO                  = Y

doc/guides/rel_notes/release_22_03.rst +1 -0

index 92f36fa8ab..91324dae18 100644
@@ -114,6 +114,7 @@ New Features
   * Added support for the link status configuration.
   * Added optimized memcpy support for the ARM platforms.
   * Added ENA admin queue support for the MP applications.
+  * Added free Tx mbuf on demand feature support.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +19 -8

index 8f30718f2c..eecdd447dd 100644
@@ -170,7 +170,7 @@ static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
 	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 int ena_tx_cleanup(void *txp, uint32_t free_pkt_cnt);
 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,

@@ -276,6 +276,7 @@ static const struct eth_dev_ops ena_dev_ops = {
 	.rx_queue_intr_disable = ena_rx_queue_intr_disable,
 	.rss_hash_update      = ena_rss_hash_update,
 	.rss_hash_conf_get    = ena_rss_hash_conf_get,
+	.tx_done_cleanup      = ena_tx_cleanup,
 };
 
 /*********************************************************************

@@ -2990,16 +2991,22 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	return 0;
 }
 
-static void ena_tx_cleanup(struct ena_ring *tx_ring)
+static int ena_tx_cleanup(void *txp, uint32_t free_pkt_cnt)
 {
+	struct ena_ring *tx_ring = (struct ena_ring *)txp;
 	unsigned int total_tx_descs = 0;
+	unsigned int total_tx_pkts = 0;
 	uint16_t cleanup_budget;
 	uint16_t next_to_clean = tx_ring->next_to_clean;
 
-	/* Attempt to release all Tx descriptors (ring_size - 1 -> size_mask) */
-	cleanup_budget = tx_ring->size_mask;
+	/*
+	 * If free_pkt_cnt is equal to 0, it means that the user requested
+	 * full cleanup, so attempt to release all Tx descriptors
+	 * (ring_size - 1 -> size_mask)
+	 */
+	cleanup_budget = (free_pkt_cnt == 0) ? tx_ring->size_mask : free_pkt_cnt;
 
-	while (likely(total_tx_descs < cleanup_budget)) {
+	while (likely(total_tx_pkts < cleanup_budget)) {
 		struct rte_mbuf *mbuf;
 		struct ena_tx_buffer *tx_info;
 		uint16_t req_id;

@@ -3021,6 +3028,7 @@ static void ena_tx_cleanup(struct ena_ring *tx_ring)
 		tx_ring->empty_tx_reqs[next_to_clean] = req_id;
 
 		total_tx_descs += tx_info->tx_descs;
+		total_tx_pkts++;
 
 		/* Put back descriptor to the ring for reuse */
 		next_to_clean = ENA_IDX_NEXT_MASKED(next_to_clean,

@@ -3034,8 +3042,11 @@ static void ena_tx_cleanup(struct ena_ring *tx_ring)
 		ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
 	}
 
-	/* Notify completion handler that the cleanup was just called */
-	tx_ring->last_cleanup_ticks = rte_get_timer_cycles();
+	/* Notify completion handler that full cleanup was performed */
+	if (free_pkt_cnt == 0 || total_tx_pkts < cleanup_budget)
+		tx_ring->last_cleanup_ticks = rte_get_timer_cycles();
+
+	return total_tx_pkts;
 }
 
 static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,

@@ -3056,7 +3067,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
 	if (available_desc < tx_ring->tx_free_thresh)
-		ena_tx_cleanup(tx_ring);
+		ena_tx_cleanup((void *)tx_ring, 0);
 
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx]))

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #38 lore raw

PATCH v2 14/21 net/ena: add API for probing xstat names by ID

In reply to Michal Krawczyk

ENA was only supporting retrieval of all the xstats name and wasn't implementing the eth_xstats_get_names_by_id API.

As this API may be more efficient than retrieving all the names, it tries to avoid excessive string copying.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |   1 +
 drivers/net/ena/ena_ethdev.c           | 130 ++++++++++++++++++++-----
 2 files changed, 108 insertions(+), 23 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 91324dae18..1b87b9c174 100644
@@ -115,6 +115,7 @@ New Features
   * Added optimized memcpy support for the ARM platforms.
   * Added ENA admin queue support for the MP applications.
   * Added free Tx mbuf on demand feature support.
+  * Added ``rte_eth_xstats_get_names_by_id`` API support.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +107 ~~-23~~

148 lines not on this page · raw message

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #39 lore raw

PATCH v2 15/21 net/ena: check if reset was already triggered

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

When triggering the reset, no check was performed to see if the reset was already triggered. This could result in original reset reason being overwritten. Add ena_trigger_reset helper function, which checks if the reset was triggered and only sets the reset reason if the reset wasn't triggered yet. Replace all occurrences of manually setting the reset with ena_trigger_reset call.

Fixes 2081d5e2e92d ("net/ena: add reset routine")

Cc [email protected]

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

drivers/net/ena/ena_ethdev.c +18 ~~-14~~

index 0766f613b0..d413111b08 100644
@@ -539,6 +539,15 @@ ENA_PROXY_DESC(ena_com_indirect_table_get, ENA_MP_IND_TBL_GET,
 }),
 	struct ena_com_dev *ena_dev, u32 *ind_tbl);
 
+static inline void ena_trigger_reset(struct ena_adapter *adapter,
+				     enum ena_regs_reset_reason_types reason)
+{
+	if (likely(!adapter->trigger_reset)) {
+		adapter->reset_reason = reason;
+		adapter->trigger_reset = true;
+	}
+}
+
 static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
 				       struct rte_mbuf *mbuf,
 				       struct ena_com_rx_ctx *ena_rx_ctx,

@@ -666,8 +675,7 @@ static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id)
 
 	/* Trigger device reset */
 	++tx_ring->tx_stats.bad_req_id;
-	tx_ring->adapter->reset_reason = ENA_REGS_RESET_INV_TX_REQ_ID;
-	tx_ring->adapter->trigger_reset	= true;
+	ena_trigger_reset(tx_ring->adapter, ENA_REGS_RESET_INV_TX_REQ_ID);
 	return -EFAULT;
 }

@@ -1783,8 +1791,7 @@ static void check_for_missing_keep_alive(struct ena_adapter *adapter)
 	if (unlikely((rte_get_timer_cycles() - adapter->timestamp_wd) >=
 	    adapter->keep_alive_timeout)) {
 		PMD_DRV_LOG(ERR, "Keep alive timeout\n");
-		adapter->reset_reason = ENA_REGS_RESET_KEEP_ALIVE_TO;
-		adapter->trigger_reset = true;
+		ena_trigger_reset(adapter, ENA_REGS_RESET_KEEP_ALIVE_TO);
 		++adapter->dev_stats.wd_expired;
 	}
 }

@@ -1794,8 +1801,7 @@ static void check_for_admin_com_state(struct ena_adapter *adapter)
 {
 	if (unlikely(!ena_com_get_admin_running_state(&adapter->ena_dev))) {
 		PMD_DRV_LOG(ERR, "ENA admin queue is not in running state\n");
-		adapter->reset_reason = ENA_REGS_RESET_ADMIN_TO;
-		adapter->trigger_reset = true;
+		ena_trigger_reset(adapter, ENA_REGS_RESET_ADMIN_TO);
 	}
 }

@@ -2606,14 +2612,13 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				rc);
 			if (rc == ENA_COM_NO_SPACE) {
 				++rx_ring->rx_stats.bad_desc_num;
-				rx_ring->adapter->reset_reason =
-					ENA_REGS_RESET_TOO_MANY_RX_DESCS;
+				ena_trigger_reset(rx_ring->adapter,
+					ENA_REGS_RESET_TOO_MANY_RX_DESCS);
 			} else {
 				++rx_ring->rx_stats.bad_req_id;
-				rx_ring->adapter->reset_reason =
-					ENA_REGS_RESET_INV_RX_REQ_ID;
+				ena_trigger_reset(rx_ring->adapter,
+					ENA_REGS_RESET_INV_RX_REQ_ID);
 			}
-			rx_ring->adapter->trigger_reset = true;
 			return 0;
 		}

@@ -2978,9 +2983,8 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	if (unlikely(rc)) {
 		PMD_DRV_LOG(ERR, "Failed to prepare Tx buffers, rc: %d\n", rc);
 		++tx_ring->tx_stats.prepare_ctx_err;
-		tx_ring->adapter->reset_reason =
-		    ENA_REGS_RESET_DRIVER_INVALID_STATE;
-		tx_ring->adapter->trigger_reset = true;
+		ena_trigger_reset(tx_ring->adapter,
+			ENA_REGS_RESET_DRIVER_INVALID_STATE);
 		return rc;
 	}

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:13 UTC #40 lore raw

PATCH v2 16/21 net/ena: make Tx completion timeout configurable

In reply to Michal Krawczyk

The parameter is called 'miss_txc_to' and can be modified using the devargs interface:

./app -a <bdf>,miss_txc_to=UINT_NUMBER

This parameter accepts values from 0 to 60 and indicates number of seconds after which the Tx packet will be considered as missing.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/ena.rst                |  9 ++++
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 66 ++++++++++++++++++++------
 drivers/net/ena/ena_ethdev.h           |  1 +
 4 files changed, 62 insertions(+), 15 deletions(-)

doc/guides/nics/ena.rst +9 -0

index 85c790e80f..3d780acac9 100644
@@ -87,6 +87,15 @@ Configuration information
      effect only if the device also supports large LLQ headers. Otherwise, the
      default value will be used.
 
+   * **miss_txc_to** (default 5)
+
+     Number of seconds after which the Tx packet will be considered missing.
+     If the missing packets number will exceed dynamically calculated threshold,
+     the driver will trigger the device reset which should be handled by the
+     application. Checking for missing Tx completions happens in the driver's
+     timer service. Setting this parameter to 0 disables this feature. Maximum
+     allowed value is 60 seconds.
+
 **ENA Configuration Parameters**
 
    * **Number of Queues**

doc/guides/rel_notes/release_22_03.rst +1 -0

index 1b87b9c174..d0d2824371 100644
@@ -116,6 +116,7 @@ New Features
   * Added ENA admin queue support for the MP applications.
   * Added free Tx mbuf on demand feature support.
   * Added ``rte_eth_xstats_get_names_by_id`` API support.
+  * Added ``miss_txc_to`` device argument for setting the Tx completion timeout.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +51 ~~-15~~

109 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +1 -0

index ca3e5ed691..c0094b03ee 100644
@@ -40,6 +40,7 @@
 #define ENA_DEVICE_KALIVE_TIMEOUT (ENA_WD_TIMEOUT_SEC * rte_get_timer_hz())
 
 #define ENA_TX_TIMEOUT			(5 * rte_get_timer_hz())
+#define ENA_MAX_TX_TIMEOUT_SECONDS	60
 #define ENA_MONITORED_TX_QUEUES		3
 #define ENA_DEFAULT_MISSING_COMP	256U

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:14 UTC #41 lore raw

PATCH v2 17/21 net/ena: fix meta-desc DF flag setup

In reply to Michal Krawczyk

The requirements for the df field are as below:
* No offload used - value doesn't matter
* IPv4 - 0 or 1, depending on the DF flag in the IPv4 header
* IPv6 - 1

Setting DF to 0 causes the packet to enter the slow-path in the HW and as a result can noticeable impact the performance.

Moreover, as 'true' may not always be mapped to 1 depending on it's definition for the given platform/compiler, for safety DF field is being set explicitly to 1.

Fixes 1173fca25af9 ("ena: add polling-mode driver")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +3 -1

index 4bb2b195b1..02b4144388 100644
@@ -624,6 +624,8 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 
 		if (mbuf->ol_flags & RTE_MBUF_F_TX_IPV6) {
 			ena_tx_ctx->l3_proto = ENA_ETH_IO_L3_PROTO_IPV6;
+			/* For the IPv6 packets, DF always needs to be true. */
+			ena_tx_ctx->df = 1;
 		} else {
 			ena_tx_ctx->l3_proto = ENA_ETH_IO_L3_PROTO_IPV4;

@@ -631,7 +633,7 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 			if (mbuf->packet_type &
 				(RTE_PTYPE_L4_NONFRAG
 				 | RTE_PTYPE_INNER_L4_NONFRAG))
-				ena_tx_ctx->df = true;
+				ena_tx_ctx->df = 1;
 		}
 
 		/* check if L4 checksum is needed */

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:14 UTC #42 lore raw

PATCH v2 18/21 net/ena: extend debug prints for invalid req ID resets

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

Add information about port id, queue id and req_id to error logs in validate_tx_req_id.

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/net/ena/ena_ethdev.c +4 -2

index 02b4144388..f4be79d240 100644
@@ -675,9 +675,11 @@ static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id)
 	}
 
 	if (tx_info)
-		PMD_TX_LOG(ERR, "tx_info doesn't have valid mbuf\n");
+		PMD_TX_LOG(ERR, "tx_info doesn't have valid mbuf. queue %d:%d req_id %u\n",
+			tx_ring->port_id, tx_ring->id, req_id);
 	else
-		PMD_TX_LOG(ERR, "Invalid req_id: %hu\n", req_id);
+		PMD_TX_LOG(ERR, "Invalid req_id: %hu in queue %d:%d\n",
+			req_id, tx_ring->port_id, tx_ring->id);
 
 	/* Trigger device reset */
 	++tx_ring->tx_stats.bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:14 UTC #43 lore raw

PATCH v2 19/21 net/ena: don't initialize LLQ when membar isn't exposed

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

The ena_com_config_dev_mode() performs many calculations related to LLQ and then performs an admin queue call to configure LLQ in the device.

Fixes 2fca2a98c0d1 ("net/ena: support LLQv2")

Cc [email protected]

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/net/ena/ena_ethdev.c +7 -7

index f4be79d240..9f29cf04ac 100644
@@ -1953,6 +1953,13 @@ ena_set_queues_placement_policy(struct ena_adapter *adapter,
 		return 0;
 	}
 
+	if (adapter->dev_mem_base == NULL) {
+		PMD_DRV_LOG(ERR,
+			"LLQ is advertised as supported, but device doesn't expose mem bar\n");
+		ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
+		return 0;
+	}
+
 	rc = ena_com_config_dev_mode(ena_dev, llq, llq_default_configurations);
 	if (unlikely(rc)) {
 		PMD_INIT_LOG(WARNING,

@@ -1965,13 +1972,6 @@ ena_set_queues_placement_policy(struct ena_adapter *adapter,
 	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_HOST)
 		return 0;
 
-	if (!adapter->dev_mem_base) {
-		PMD_DRV_LOG(ERR,
-			"Unable to access LLQ BAR resource. Fallback to host mode policy.\n");
-		ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
-		return 0;
-	}
-
 	ena_dev->mem_bar = adapter->dev_mem_base;
 
 	return 0;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:14 UTC #44 lore raw

PATCH v2 20/21 net/ena: don't indicate bad csum for L4 csum error

In reply to Michal Krawczyk

The statistics counters will still work as previously.

Fixes 05817057faba ("net/ena: fix indication of bad L4 Rx checksums")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +7 -1

index 9f29cf04ac..0861206ea4 100644
@@ -583,7 +583,13 @@ static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
 	} else {
 		if (unlikely(ena_rx_ctx->l4_csum_err)) {
 			++rx_stats->l4_csum_bad;
-			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+			/*
+			 * For the L4 Rx checksum offload the HW may indicate
+			 * bad checksum although it's valid. Because of that,
+			 * we're setting the UNKNOWN flag to let the app
+			 * re-verify the checksum.
+			 */
+			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
 		} else {
 			++rx_stats->l4_csum_good;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-22 18:14 UTC #45 lore raw

PATCH v2 21/21 net/ena: update version to 2.6.0

In reply to Michal Krawczyk

Signed-off-by Michal Krawczyk <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +1 -1

index 0861206ea4..53b2983d27 100644
@@ -21,7 +21,7 @@
 #include <ena_eth_io_defs.h>
 
 #define DRV_MODULE_VER_MAJOR	2
-#define DRV_MODULE_VER_MINOR	5
+#define DRV_MODULE_VER_MINOR	6
 #define DRV_MODULE_VER_SUBMINOR	0
 
 #define __MERGE_64B_H_L(h, l) (((uint64_t)h << 32) | l)

Signature · 1 line

2.25.1

Ferruh Yigit

2022-02-22 22:21 UTC #46 lore raw

In reply to Michal Krawczyk

Quoted 40 lines · Michal Krawczyk

Hi,
this set contains new ENA features:
* New xstats.
* Reconfigurable link status event.
* Usage of the optimized memcpy on arm/arm64.
* Better MP support.
* Reconfigurable Tx completion timeout value using devarg.
Beside that, this patchset contains multiple fixes, minor improvements, new DPDK API support and extra error checks.
v2:
* Fix reviewer's email (s/[email protected]/[email protected]/).
* Fix commit messages typos found by the checkpatch.
Dawid Gorecki (4):
net/ena: expose Tx cleanup function
net/ena: check if reset was already triggered net/ena: extend debug prints for invalid req ID resets net/ena: don't initialize LLQ when membar isn't exposed
Michal Krawczyk (15):
net/ena: remove linearization function net/ena: add assertion on Tx info mbuf net/ena: remove unused enumeration
net/ena: remove unused offloads variables net/ena: add extra Rx checksum related xstats net/ena: make LSC configurable
net/ena: skip timer if the reset is triggered net/ena: perform Tx cleanup before sending pkts net/ena/base: use optimized memcpy version also on Arm net/ena/base: make IO memzone unique per port net/ena: add API for probing xstat names by ID net/ena: make Tx completion timeout configurable net/ena: fix meta-desc DF flag setup
net/ena: don't indicate bad csum for L4 csum error net/ena: update version to 2.6.0

Quoted 3 lines

Stanislaw Kardach (2):
net/ena: proxy AQ calls to primary process
net/ena: enable stats get function for MP mode

Hi Michal, Marcin, Shai, Evgeny, Igor,

We are 3-4 days away from -rc2 and received this biggish set, I wish it was sent a little earlier to give more time for it.

At least can you please send the roadmap next time so we can know what will come, what to expect instead of surprise.

At first glance there is a build error on the set, I will comment on the relevant patch.

Thanks,
ferruh

Michał Krawczyk

author

2022-02-23 10:07 UTC #47 lore raw

wt., 22 lut 2022 o 23:21 Ferruh Yigit <[email protected]> napisał(a):

Quoted 46 lines · Michal Krawczyk

Quoted 40 lines
Hi,
this set contains new ENA features:
* New xstats.
* Reconfigurable link status event.
* Usage of the optimized memcpy on arm/arm64.
* Better MP support.
* Reconfigurable Tx completion timeout value using devarg.
Beside that, this patchset contains multiple fixes, minor improvements, new DPDK API support and extra error checks.
v2:
* Fix reviewer's email (s/[email protected]/[email protected]/).
* Fix commit messages typos found by the checkpatch.
Dawid Gorecki (4):
net/ena: expose Tx cleanup function
net/ena: check if reset was already triggered net/ena: extend debug prints for invalid req ID resets net/ena: don't initialize LLQ when membar isn't exposed
Michal Krawczyk (15):
net/ena: remove linearization function net/ena: add assertion on Tx info mbuf net/ena: remove unused enumeration
net/ena: remove unused offloads variables net/ena: add extra Rx checksum related xstats net/ena: make LSC configurable
net/ena: skip timer if the reset is triggered net/ena: perform Tx cleanup before sending pkts net/ena/base: use optimized memcpy version also on Arm net/ena/base: make IO memzone unique per port net/ena: add API for probing xstat names by ID net/ena: make Tx completion timeout configurable net/ena: fix meta-desc DF flag setup
net/ena: don't indicate bad csum for L4 csum error net/ena: update version to 2.6.0
Quoted 3 lines
Stanislaw Kardach (2):
net/ena: proxy AQ calls to primary process
net/ena: enable stats get function for MP mode

Quoted 8 lines

Hi Michal, Marcin, Shai, Evgeny, Igor,
We are 3-4 days away from -rc2 and received this biggish set, I wish it was sent a little earlier to give more time for it.
At least can you please send the roadmap next time so we can know what will come, what to expect instead of surprise.

Hi Ferruh,

I'm sorry for that, we had those patches enqueued internally for a longer period and finally managed to perform the internal review.

We'll work on better communication from our side toward the next releases and choose the best way to inform you about the upcoming changes and internal deadlines, similar to what the other vendors are doing.

Quoted 3 lines

At first glance there is a build error on the set, I will comment on the relevant patch.

Thanks for taking a look at those patches, I really appreciate that. I'll work on relevant fixes and send v3 ASAP.

Thanks,
Michal

Quoted 3 lines

Thanks,
ferruh

Michal Krawczyk

author

2022-02-23 12:20 UTC #48 lore raw

v3 cover net/ena: v2.6.0 driver update

In reply to Michal Krawczyk

Hi,

this set contains new ENA features:

* New xstats.
* Reconfigurable link status event.
* Usage of the optimized memcpy on arm/arm64.
* Better MP support.
* Reconfigurable Tx completion timeout value using devarg.

Beside that, this patchset contains multiple fixes, minor improvements, new DPDK API support and extra error checks.

v3:
* Fix build errors of the AQ proxy patch:
- for the FreeBSD, by changing ECOMM error to EIO,
- with assertions enabled, by fixing variable name.

v2:
* Fix reviewer's email (s/[email protected]/[email protected]/).
* Fix commit messages typos found by the checkpatch.

Stanislaw Kardach (2):
net/ena: proxy AQ calls to primary process
net/ena: enable stats get function for MP mode

 doc/guides/nics/ena.rst                |   9 +
 doc/guides/nics/features/ena.ini       |   2 +
 doc/guides/rel_notes/release_22_03.rst |  14 +
 drivers/net/ena/base/ena_plat_dpdk.h   |  69 +-
 drivers/net/ena/ena_ethdev.c           | 916 ++++++++++++++++++++-----
 drivers/net/ena/ena_ethdev.h           |  28 +-
 drivers/net/ena/ena_rss.c              |  10 +-
 7 files changed, 800 insertions(+), 248 deletions(-)

--
2.25.1

Michal Krawczyk

author

2022-02-23 12:20 UTC #49 lore raw

PATCH v3 1/21 net/ena: remove linearization function

The linearization of the mbuf isn't common practice for the PMD, as it can expose it's capabilities to the upper layer using rte_eth_dev_info_get().

Moreover, the rte_eth_tx_prepare() function should also verify if the number of segments inside the mbuf isn't too high.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 74 +++++++++---------------------------
 drivers/net/ena/ena_ethdev.h |  2 -
 2 files changed, 19 insertions(+), 57 deletions(-)

drivers/net/ena/ena_ethdev.c +19 ~~-55~~

index db2b5ec8e7..deaee30664 100644
@@ -93,8 +93,6 @@ static const struct ena_stats ena_stats_tx_strings[] = {
 	ENA_STAT_TX_ENTRY(cnt),
 	ENA_STAT_TX_ENTRY(bytes),
 	ENA_STAT_TX_ENTRY(prepare_ctx_err),
-	ENA_STAT_TX_ENTRY(linearize),
-	ENA_STAT_TX_ENTRY(linearize_failed),
 	ENA_STAT_TX_ENTRY(tx_poll),
 	ENA_STAT_TX_ENTRY(doorbells),
 	ENA_STAT_TX_ENTRY(bad_req_id),

@@ -2408,6 +2406,17 @@ eth_ena_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			return i;
 		}
 
+		if (unlikely(m->nb_segs >= tx_ring->sgl_size &&
+		    !(tx_ring->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV &&
+		      m->nb_segs == tx_ring->sgl_size &&
+		      m->data_len < tx_ring->tx_max_header_size))) {
+			PMD_TX_LOG(DEBUG,
+				"mbuf[%" PRIu32 "] has too many segments: %" PRIu16 "\n",
+				i, m->nb_segs);
+			rte_errno = EINVAL;
+			return i;
+		}
+
 #ifdef RTE_LIBRTE_ETHDEV_DEBUG
 		/* Check if requested offload is also enabled for the queue */
 		if ((ol_flags & RTE_MBUF_F_TX_IP_CKSUM &&

@@ -2554,56 +2563,6 @@ static void ena_update_hints(struct ena_adapter *adapter,
 	}
 }
 
-static int ena_check_space_and_linearize_mbuf(struct ena_ring *tx_ring,
-					      struct rte_mbuf *mbuf)
-{
-	struct ena_com_dev *ena_dev;
-	int num_segments, header_len, rc;
-
-	ena_dev = &tx_ring->adapter->ena_dev;
-	num_segments = mbuf->nb_segs;
-	header_len = mbuf->data_len;
-
-	if (likely(num_segments < tx_ring->sgl_size))
-		goto checkspace;
-
-	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV &&
-	    (num_segments == tx_ring->sgl_size) &&
-	    (header_len < tx_ring->tx_max_header_size))
-		goto checkspace;
-
-	/* Checking for space for 2 additional metadata descriptors due to
-	 * possible header split and metadata descriptor. Linearization will
-	 * be needed so we reduce the segments number from num_segments to 1
-	 */
-	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq, 3)) {
-		PMD_TX_LOG(DEBUG, "Not enough space in the Tx queue\n");
-		return ENA_COM_NO_MEM;
-	}
-	++tx_ring->tx_stats.linearize;
-	rc = rte_pktmbuf_linearize(mbuf);
-	if (unlikely(rc)) {
-		PMD_TX_LOG(WARNING, "Mbuf linearize failed\n");
-		rte_atomic64_inc(&tx_ring->adapter->drv_stats->ierrors);
-		++tx_ring->tx_stats.linearize_failed;
-		return rc;
-	}
-
-	return 0;
-
-checkspace:
-	/* Checking for space for 2 additional metadata descriptors due to
-	 * possible header split and metadata descriptor
-	 */
-	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq,
-					  num_segments + 2)) {
-		PMD_TX_LOG(DEBUG, "Not enough space in the Tx queue\n");
-		return ENA_COM_NO_MEM;
-	}
-
-	return 0;
-}
-
 static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
 	struct ena_tx_buffer *tx_info,
 	struct rte_mbuf *mbuf,

@@ -2688,9 +2647,14 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	int nb_hw_desc;
 	int rc;
 
-	rc = ena_check_space_and_linearize_mbuf(tx_ring, mbuf);
-	if (unlikely(rc))
-		return rc;
+	/* Checking for space for 2 additional metadata descriptors due to
+	 * possible header split and metadata descriptor
+	 */
+	if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq,
+					  mbuf->nb_segs + 2)) {
+		PMD_DRV_LOG(DEBUG, "Not enough space in the tx queue\n");
+		return ENA_COM_NO_MEM;
+	}
 
 	next_to_use = tx_ring->next_to_use;

drivers/net/ena/ena_ethdev.h +0 -2

index f99e4f3984..f47ba3fb02 100644
@@ -103,8 +103,6 @@ struct ena_stats_tx {
 	u64 cnt;
 	u64 bytes;
 	u64 prepare_ctx_err;
-	u64 linearize;
-	u64 linearize_failed;
 	u64 tx_poll;
 	u64 doorbells;
 	u64 bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:20 UTC #50 lore raw

PATCH v3 2/21 net/ena: add assertion on Tx info mbuf

In reply to Michal Krawczyk

To make sure there is no outstanding mbuf in the reused Tx queue (due to unproper cleanup, or some invalid logic on Tx path), the assertion was added on the Tx path.

As it's being compiled out in the release version, it won't affect the IO path performance.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ena/ena_ethdev.c +1 -0

index deaee30664..34fb43cb00 100644
@@ -2661,6 +2661,7 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	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;
+	RTE_ASSERT(tx_info->mbuf == NULL);
 
 	ena_tx_map_mbuf(tx_ring, tx_info, mbuf, &push_header, &header_len);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:20 UTC #51 lore raw

PATCH v3 3/21 net/ena: remove unused enumeration

In reply to Michal Krawczyk

The enumeration seems to be leftover from porting the Linux driver to the DPDK. It was used nowhere and refers to the ethtool which is not present in the DPDK.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 5 -----
 1 file changed, 5 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -5

index 34fb43cb00..2de2dcf12f 100644
@@ -38,11 +38,6 @@
 
 #define ENA_PTYPE_HAS_HASH	(RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP)
 
-enum ethtool_stringset {
-	ETH_SS_TEST             = 0,
-	ETH_SS_STATS,
-};
-
 struct ena_stats {
 	char name[ETH_GSTRING_LEN];
 	int stat_offset;

Signature · 1 line

2.25.1

Ferruh Yigit

2022-02-23 17:26 UTC #52 lore raw

Re: [PATCH v3 03/21] net/ena: remove unused enumeration

Quoted 4 lines · Michal Krawczyk

The enumeration seems to be leftover from porting the Linux driver to the DPDK. It was used nowhere and refers to the ethtool which is not present in the DPDK.

Fixes 372c1af5ed8f ("net/ena: add dedicated memory area for extra device info")

Cc [email protected]

Quoted 25 lines

Signed-off-by Michal Krawczyk <[email protected]>
Reviewed-by Artur Rojek <[email protected]>
Reviewed-by Dawid Gorecki <[email protected]>
Reviewed-by Igor Chauskin <[email protected]>
Reviewed-by Shai Brandes <[email protected]>
---
  drivers/net/ena/ena_ethdev.c | 5 -----
  1 file changed, 5 deletions(-)
drivers/net/ena/ena_ethdev.c +0 -5
index 34fb43cb00..2de2dcf12f 100644
@@ -38,11 +38,6 @@
 
  #define ENA_PTYPE_HAS_HASH	(RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP)
 
-enum ethtool_stringset {
-	ETH_SS_TEST             = 0,
-	ETH_SS_STATS,
-};
-
  struct ena_stats {
  	char name[ETH_GSTRING_LEN];
  	int stat_offset;

Michal Krawczyk

author

2022-02-23 12:20 UTC #53 lore raw

PATCH v3 4/21 net/ena: remove unused offloads variables

In reply to Michal Krawczyk

Those variables are being set, but never read. As they seem to be leftover from the old offloads API and don't have any purpose right now, they are simply being removed.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 ---
 drivers/net/ena/ena_ethdev.h | 5 -----
 2 files changed, 8 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index 2de2dcf12f..de5fdb8f1d 100644
@@ -2015,9 +2015,6 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
 	 */
 	adapter->tx_cleanup_stall_delay = adapter->missing_tx_completion_to / 2;
 
-	adapter->tx_selected_offloads = dev->data->dev_conf.txmode.offloads;
-	adapter->rx_selected_offloads = dev->data->dev_conf.rxmode.offloads;
-
 	return 0;
 }

drivers/net/ena/ena_ethdev.h +0 -5

index f47ba3fb02..4dfa604d51 100644
@@ -278,11 +278,6 @@ struct ena_adapter {
 	struct ena_driver_stats *drv_stats;
 	enum ena_adapter_state state;
 
-	uint64_t tx_supported_offloads;
-	uint64_t tx_selected_offloads;
-	uint64_t rx_supported_offloads;
-	uint64_t rx_selected_offloads;
-
 	bool link_status;
 
 	enum ena_regs_reset_reason_types reset_reason;

Signature · 1 line

2.25.1

Ferruh Yigit

2022-02-23 17:36 UTC #54 lore raw

Re: [PATCH v3 04/21] net/ena: remove unused offloads variables

Quoted 4 lines · Michal Krawczyk

Those variables are being set, but never read. As they seem to be leftover from the old offloads API and don't have any purpose right now, they are simply being removed.

It can be good to add fixes tag, both for
- document in which commit old offload flags because useless
- backport the change to LTS, I can see the changes don't do much but backporting them can help to reduce future conflicts for future fixes (cc'ed LTS maintainers for comment)

If agreed please send the fixes line, I can amend it in next-net.

Quoted 40 lines

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
  drivers/net/ena/ena_ethdev.c | 3 ---
  drivers/net/ena/ena_ethdev.h | 5 -----
  2 files changed, 8 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index 2de2dcf12f..de5fdb8f1d 100644
@@ -2015,9 +2015,6 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
  	 */
  	adapter->tx_cleanup_stall_delay = adapter->missing_tx_completion_to / 2;
 
-	adapter->tx_selected_offloads = dev->data->dev_conf.txmode.offloads;
-	adapter->rx_selected_offloads = dev->data->dev_conf.rxmode.offloads;
-
  	return 0;
  }

drivers/net/ena/ena_ethdev.h +0 -5

index f47ba3fb02..4dfa604d51 100644
@@ -278,11 +278,6 @@ struct ena_adapter {
  	struct ena_driver_stats *drv_stats;
  	enum ena_adapter_state state;
 
-	uint64_t tx_supported_offloads;
-	uint64_t tx_selected_offloads;
-	uint64_t rx_supported_offloads;
-	uint64_t rx_selected_offloads;
-
  	bool link_status;
 
  	enum ena_regs_reset_reason_types reset_reason;

Michał Krawczyk

author

2022-02-23 17:47 UTC #55 lore raw

Re: [PATCH v3 04/21] net/ena: remove unused offloads variables

śr., 23 lut 2022 o 18:25 Ferruh Yigit <[email protected]> napisał(a):

Quoted 7 lines · Michal Krawczyk

Quoted 4 lines
Those variables are being set, but never read. As they seem to be leftover from the old offloads API and don't have any purpose right now, they are simply being removed.

Quoted 6 lines

It can be good to add fixes tag, both for
- document in which commit old offload flags because useless
- backport the change to LTS, I can see the changes don't do much but backporting them can help to reduce future conflicts for future fixes (cc'ed LTS maintainers for comment)

Good note - I'm ok with adding the fixline. Thanks!

Quoted 1 line

If agreed please send the fixes line, I can amend it in next-net.

Fixes a4996bd89c42 ("ethdev: new Rx/Tx offloads API")

Cc [email protected]

Quoted 43 lines

Quoted 41 lines

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Artur Rojek <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Igor Chauskin <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
  drivers/net/ena/ena_ethdev.c | 3 ---
  drivers/net/ena/ena_ethdev.h | 5 -----
  2 files changed, 8 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index 2de2dcf12f..de5fdb8f1d 100644
@@ -2015,9 +2015,6 @@ static int ena_dev_configure(struct rte_eth_dev *dev)
       */
      adapter->tx_cleanup_stall_delay = adapter->missing_tx_completion_to / 2;
 
-     adapter->tx_selected_offloads = dev->data->dev_conf.txmode.offloads;
-     adapter->rx_selected_offloads = dev->data->dev_conf.rxmode.offloads;
-
      return 0;
  }

drivers/net/ena/ena_ethdev.h +0 -5

index f47ba3fb02..4dfa604d51 100644
@@ -278,11 +278,6 @@ struct ena_adapter {
      struct ena_driver_stats *drv_stats;
      enum ena_adapter_state state;
 
-     uint64_t tx_supported_offloads;
-     uint64_t tx_selected_offloads;
-     uint64_t rx_supported_offloads;
-     uint64_t rx_selected_offloads;
-
      bool link_status;
 
      enum ena_regs_reset_reason_types reset_reason;

Ferruh Yigit

2022-02-23 18:12 UTC #56 lore raw

Re: [PATCH v3 04/21] net/ena: remove unused offloads variables

Quoted 15 lines · Michał Krawczyk

śr., 23 lut 2022 o 18:25 Ferruh Yigit <[email protected]> napisał(a):
Quoted 7 lines · Michal Krawczyk
Quoted 4 lines
Those variables are being set, but never read. As they seem to be leftover from the old offloads API and don't have any purpose right now, they are simply being removed.
Quoted 6 lines
It can be good to add fixes tag, both for
- document in which commit old offload flags because useless
- backport the change to LTS, I can see the changes don't do much but backporting them can help to reduce future conflicts for future fixes (cc'ed LTS maintainers for comment)

Quoted 7 lines

Good note - I'm ok with adding the fixline. Thanks!
Quoted 1 line
If agreed please send the fixes line, I can amend it in next-net.
Fixes a4996bd89c42 ("ethdev: new Rx/Tx offloads API")
Cc [email protected]

done, thanks.

Michal Krawczyk

author

2022-02-23 12:20 UTC #57 lore raw

PATCH v3 5/21 net/ena: add extra Rx checksum related xstats

In reply to Michal Krawczyk

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |  8 +++++++
 drivers/net/ena/ena_ethdev.c           | 32 ++++++++++++++++----------
 drivers/net/ena/ena_ethdev.h           |  4 +++-
 3 files changed, 31 insertions(+), 13 deletions(-)

doc/guides/rel_notes/release_22_03.rst +8 -0

index 74965ebd56..f6b881b8b4 100644
@@ -104,6 +104,14 @@ New Features
   * Added support for libxdp >=v1.2.2.
   * Re-enabled secondary process support. RX/TX is not supported.
 
+* **Updated Amazon ENA PMD.**
+
+  Updated the Amazon ENA PMD. The new driver version (v2.6.0) introduced
+  bug fixes and improvements, including:
+
+  * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
+    ``l4_csum_good``.
+
 * **Updated Cisco enic driver.**
 
   * Added rte_flow support for matching GENEVE packets.

drivers/net/ena/ena_ethdev.c +20 ~~-12~~

index de5fdb8f1d..d534b93749 100644
@@ -99,7 +99,9 @@ static const struct ena_stats ena_stats_rx_strings[] = {
 	ENA_STAT_RX_ENTRY(cnt),
 	ENA_STAT_RX_ENTRY(bytes),
 	ENA_STAT_RX_ENTRY(refill_partial),
-	ENA_STAT_RX_ENTRY(bad_csum),
+	ENA_STAT_RX_ENTRY(l3_csum_bad),
+	ENA_STAT_RX_ENTRY(l4_csum_bad),
+	ENA_STAT_RX_ENTRY(l4_csum_good),
 	ENA_STAT_RX_ENTRY(mbuf_alloc_fail),
 	ENA_STAT_RX_ENTRY(bad_desc_num),
 	ENA_STAT_RX_ENTRY(bad_req_id),

@@ -273,10 +275,12 @@ static const struct eth_dev_ops ena_dev_ops = {
 	.rss_hash_conf_get    = ena_rss_hash_conf_get,
 };
 
-static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
+static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
+				       struct rte_mbuf *mbuf,
 				       struct ena_com_rx_ctx *ena_rx_ctx,
 				       bool fill_hash)
 {
+	struct ena_stats_rx *rx_stats = &rx_ring->rx_stats;
 	uint64_t ol_flags = 0;
 	uint32_t packet_type = 0;

@@ -287,21 +291,27 @@ static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf,
 
 	if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV4) {
 		packet_type |= RTE_PTYPE_L3_IPV4;
-		if (unlikely(ena_rx_ctx->l3_csum_err))
+		if (unlikely(ena_rx_ctx->l3_csum_err)) {
+			++rx_stats->l3_csum_bad;
 			ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_BAD;
-		else
+		} else {
 			ol_flags |= RTE_MBUF_F_RX_IP_CKSUM_GOOD;
+		}
 	} else if (ena_rx_ctx->l3_proto == ENA_ETH_IO_L3_PROTO_IPV6) {
 		packet_type |= RTE_PTYPE_L3_IPV6;
 	}
 
-	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag)
+	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag) {
 		ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
-	else
-		if (unlikely(ena_rx_ctx->l4_csum_err))
+	} else {
+		if (unlikely(ena_rx_ctx->l4_csum_err)) {
+			++rx_stats->l4_csum_bad;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
-		else
+		} else {
+			++rx_stats->l4_csum_good;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
+		}
+	}
 
 	if (fill_hash &&
 	    likely((packet_type & ENA_PTYPE_HAS_HASH) && !ena_rx_ctx->frag)) {

@@ -2336,13 +2346,11 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		}
 
 		/* fill mbuf attributes if any */
-		ena_rx_mbuf_prepare(mbuf, &ena_rx_ctx, fill_hash);
+		ena_rx_mbuf_prepare(rx_ring, mbuf, &ena_rx_ctx, fill_hash);
 
 		if (unlikely(mbuf->ol_flags &
-				(RTE_MBUF_F_RX_IP_CKSUM_BAD | RTE_MBUF_F_RX_L4_CKSUM_BAD))) {
+				(RTE_MBUF_F_RX_IP_CKSUM_BAD | RTE_MBUF_F_RX_L4_CKSUM_BAD)))
 			rte_atomic64_inc(&rx_ring->adapter->drv_stats->ierrors);
-			++rx_ring->rx_stats.bad_csum;
-		}
 
 		rx_pkts[completed] = mbuf;
 		rx_ring->rx_stats.bytes += mbuf->pkt_len;

drivers/net/ena/ena_ethdev.h +3 -1

index 4dfa604d51..42c47c9455 100644
@@ -114,7 +114,9 @@ struct ena_stats_rx {
 	u64 cnt;
 	u64 bytes;
 	u64 refill_partial;
-	u64 bad_csum;
+	u64 l3_csum_bad;
+	u64 l4_csum_bad;
+	u64 l4_csum_good;
 	u64 mbuf_alloc_fail;
 	u64 bad_desc_num;
 	u64 bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #58 lore raw

PATCH v3 6/21 net/ena: make LSC configurable

In reply to Michal Krawczyk

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 66 +++++++++++++++++++-------
 drivers/net/ena/ena_ethdev.h           |  5 +-
 3 files changed, 53 insertions(+), 19 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index f6b881b8b4..c8e38d4c70 100644
@@ -111,6 +111,7 @@ New Features
 
   * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
     ``l4_csum_good``.
+  * Added support for the link status configuration.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +49 ~~-17~~

134 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +3 -2

index 42c47c9455..f660b6a7cb 100644
@@ -291,9 +291,10 @@ struct ena_adapter {
 	struct ena_stats_dev dev_stats;
 	struct ena_stats_eni eni_stats;
 
-	bool trigger_reset;
+	uint32_t all_aenq_groups;
+	uint32_t active_aenq_groups;
 
-	bool wd_state;
+	bool trigger_reset;
 
 	bool use_large_llq_hdr;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #59 lore raw

PATCH v3 7/21 net/ena: skip timer if the reset is triggered

In reply to Michal Krawczyk

Timer service is now being skipped if the reset was already triggered.

Fixes d9b8b106bf9d ("net/ena: add watchdog and keep alive AENQ handler")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/net/ena/ena_ethdev.c +3 -0

index a2793f13cd..4b82372155 100644
@@ -1624,6 +1624,9 @@ static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer,
 	struct rte_eth_dev *dev = arg;
 	struct ena_adapter *adapter = dev->data->dev_private;
 
+	if (unlikely(adapter->trigger_reset))
+		return;
+
 	check_for_missing_keep_alive(adapter);
 	check_for_admin_com_state(adapter);
 	check_for_tx_completions(adapter);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #60 lore raw

PATCH v3 8/21 net/ena: perform Tx cleanup before sending pkts

In reply to Michal Krawczyk

It's also common order in the Tx burst function for other PMDs.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/net/ena/ena_ethdev.c +4 -6

index 4b82372155..ed3dd162ba 100644
@@ -2776,6 +2776,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	}
 #endif
 
+	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
+	if (available_desc < tx_ring->tx_free_thresh)
+		ena_tx_cleanup(tx_ring);
+
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx]))
 			break;

@@ -2784,9 +2788,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			tx_ring->size_mask)]);
 	}
 
-	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
-	tx_ring->tx_stats.available_desc = available_desc;
-
 	/* If there are ready packets to be xmitted... */
 	if (likely(tx_ring->pkts_without_db)) {
 		/* ...let HW do its best :-) */

@@ -2795,9 +2796,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		tx_ring->pkts_without_db = false;
 	}
 
-	if (available_desc < tx_ring->tx_free_thresh)
-		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++;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #61 lore raw

PATCH v3 9/21 net/ena/base: use optimized memcpy version also on Arm

In reply to Michal Krawczyk

As the default behavior for arm64 is to alias rte_memcpy as memcpy, ENA cannot redefine memcpy as rte_memcpy as it would cause nested declaration.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst | 1 +
 drivers/net/ena/base/ena_plat_dpdk.h   | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index c8e38d4c70..92490afd60 100644
@@ -112,6 +112,7 @@ New Features
   * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
     ``l4_csum_good``.
   * Added support for the link status configuration.
+  * Added optimized memcpy support for the ARM platforms.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/base/ena_plat_dpdk.h +5 -2

index 4e7f52881a..41db883c63 100644
@@ -66,8 +66,11 @@ typedef uint64_t dma_addr_t;
 #define ENA_UDELAY(x) rte_delay_us_block(x)
 
 #define ENA_TOUCH(x) ((void)(x))
-/* Avoid nested declaration on arm64, as it may define rte_memcpy as memcpy. */
-#if defined(RTE_ARCH_X86)
+/* Redefine memcpy with caution: rte_memcpy can be simply aliased to memcpy, so
+ * make the redefinition only if it's safe (and beneficial) to do so.
+ */
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64_MEMCPY) || \
+	defined(RTE_ARCH_ARM_NEON_MEMCPY)
 #undef memcpy
 #define memcpy rte_memcpy
 #endif

Signature · 1 line

2.25.1

Ferruh Yigit

2022-02-23 17:32 UTC #62 lore raw

Re: [PATCH v3 09/21] net/ena/base: use optimized memcpy version also on Arm

Quoted 5 lines · Michal Krawczyk

As the default behavior for arm64 is to alias rte_memcpy as memcpy, ENA cannot redefine memcpy as rte_memcpy as it would cause nested declaration.
To make it possible to use optimized memcpy in the ena_com layer on Arm,

Out of curiosity, do you have any performance measurements for the optimized memcpy usage?

Quoted 32 lines

the driver now redefines memcpy when it is beneficial:
* For arm64 only when the flag RTE_ARCH_ARM64_MEMCPY was defined
* For arm only when the flag RTE_ARCH_ARM_NEON_MEMCPY was defined
Signed-off-by Michal Krawczyk <[email protected]>
Reviewed-by Dawid Gorecki <[email protected]>
Reviewed-by Shai Brandes <[email protected]>
---
  doc/guides/rel_notes/release_22_03.rst | 1 +
  drivers/net/ena/base/ena_plat_dpdk.h   | 7 +++++--
  2 files changed, 6 insertions(+), 2 deletions(-)
doc/guides/rel_notes/release_22_03.rst +1 -0
index c8e38d4c70..92490afd60 100644
@@ -112,6 +112,7 @@ New Features
    * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
      ``l4_csum_good``.
    * Added support for the link status configuration.
+  * Added optimized memcpy support for the ARM platforms.
 
  * **Updated Cisco enic driver.**
 
drivers/net/ena/base/ena_plat_dpdk.h +0 -0
index 4e7f52881a..41db883c63 100644
@@ -66,8 +66,11 @@ typedef uint64_t dma_addr_t;
  #define ENA_UDELAY(x) rte_delay_us_block(x)
 
  #define ENA_TOUCH(x) ((void)(x))

drivers/net/ena/base/ena_plat_dpdk.h typedef uint64_t dma_addr_t; · line 76

Quoted 10 lines

drivers/net/ena/base/ena_plat_dpdk.h +5 -2

-/* Avoid nested declaration on arm64, as it may define rte_memcpy as memcpy. */
-#if defined(RTE_ARCH_X86)
+/* Redefine memcpy with caution: rte_memcpy can be simply aliased to memcpy, so
+ * make the redefinition only if it's safe (and beneficial) to do so.
+ */
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64_MEMCPY) || \
+	defined(RTE_ARCH_ARM_NEON_MEMCPY)
  #undef memcpy
  #define memcpy rte_memcpy
  #endif

I can see there is 'ena_plat_dpdk.h', which seems like an osdep header,

it is possible to use 'ena_memcpy' in the code and in the 'ena_plat_dpdk.h' define it as:
#define ena_memcpy rte_memcpy

This is just for your information if it helps, usage is up to you.

Michał Krawczyk

author

2022-02-23 17:40 UTC #63 lore raw

Re: [PATCH v3 09/21] net/ena/base: use optimized memcpy version also on Arm

śr., 23 lut 2022 o 18:26 Ferruh Yigit <[email protected]> napisał(a):

Quoted 11 lines · Michal Krawczyk

Quoted 5 lines
As the default behavior for arm64 is to alias rte_memcpy as memcpy, ENA cannot redefine memcpy as rte_memcpy as it would cause nested declaration.
To make it possible to use optimized memcpy in the ena_com layer on Arm,
Out of curiosity, do you have any performance measurements for the optimized memcpy usage?

Unfortunately no - on AWS it's hard to see noticeable performance improvement due to limitations in terms of the PPS number that can be processed at maximum. The basic packet generator applications are mostly idle in that situation.

Quoted 45 lines

Quoted 32 lines
the driver now redefines memcpy when it is beneficial:
* For arm64 only when the flag RTE_ARCH_ARM64_MEMCPY was defined
* For arm only when the flag RTE_ARCH_ARM_NEON_MEMCPY was defined
Signed-off-by Michal Krawczyk <[email protected]>
Reviewed-by Dawid Gorecki <[email protected]>
Reviewed-by Shai Brandes <[email protected]>
---
  doc/guides/rel_notes/release_22_03.rst | 1 +
  drivers/net/ena/base/ena_plat_dpdk.h   | 7 +++++--
  2 files changed, 6 insertions(+), 2 deletions(-)
doc/guides/rel_notes/release_22_03.rst +1 -0
index c8e38d4c70..92490afd60 100644
@@ -112,6 +112,7 @@ New Features
    * Added new checksum related xstats: ``l3_csum_bad``, ``l4_csum_bad`` and
      ``l4_csum_good``.
    * Added support for the link status configuration.
+  * Added optimized memcpy support for the ARM platforms.
 
  * **Updated Cisco enic driver.**
 
drivers/net/ena/base/ena_plat_dpdk.h +0 -0
index 4e7f52881a..41db883c63 100644
@@ -66,8 +66,11 @@ typedef uint64_t dma_addr_t;
  #define ENA_UDELAY(x) rte_delay_us_block(x)
 
  #define ENA_TOUCH(x) ((void)(x))
drivers/net/ena/base/ena_plat_dpdk.h typedef uint64_t dma_addr_t; · line 76
Quoted 10 lines
drivers/net/ena/base/ena_plat_dpdk.h +5 -2
-/* Avoid nested declaration on arm64, as it may define rte_memcpy as memcpy. */
-#if defined(RTE_ARCH_X86)
+/* Redefine memcpy with caution: rte_memcpy can be simply aliased to memcpy, so
+ * make the redefinition only if it's safe (and beneficial) to do so.
+ */
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64_MEMCPY) || \
+     defined(RTE_ARCH_ARM_NEON_MEMCPY)
  #undef memcpy
  #define memcpy rte_memcpy
  #endif
I can see there is 'ena_plat_dpdk.h', which seems like an osdep header,

Quoted 6 lines

it is possible to use 'ena_memcpy' in the code and in the 'ena_plat_dpdk.h' define it as:
#define ena_memcpy rte_memcpy
This is just for your information if it helps, usage is up to you.

Thanks for the input! It looks like a robust solution, but unfortunately we can't modify other ena_com files which use raw memcpy calls.The preferred approach is to do some work-arounds in the osdep headers instead of modifying the common files.

Michal Krawczyk

author

2022-02-23 12:21 UTC #64 lore raw

PATCH v3 10/21 net/ena: proxy AQ calls to primary process

In reply to Michal Krawczyk

From: Stanislaw Kardach <[email protected]>

Signed-off-by Stanislaw Kardach <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
v3:
* Fix FreeBSD build error, by changing ECOMM error to EIO.
* Fix build errors with assertions enabled, by fixing the variable name.

 doc/guides/rel_notes/release_22_03.rst |   1 +
 drivers/net/ena/ena_ethdev.c           | 384 ++++++++++++++++++++++++-
 drivers/net/ena/ena_ethdev.h           |   9 +
 drivers/net/ena/ena_rss.c              |  10 +-
 4 files changed, 384 insertions(+), 20 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 92490afd60..5c2521e870 100644
@@ -113,6 +113,7 @@ New Features
     ``l4_csum_good``.
   * Added support for the link status configuration.
   * Added optimized memcpy support for the ARM platforms.
+  * Added ENA admin queue support for the MP applications.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +369 ~~-15~~

484 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +9 -0

index f660b6a7cb..01cf0ef5db 100644
@@ -29,6 +29,9 @@
 #define ENA_RX_BUF_MIN_SIZE	1400
 #define ENA_DEFAULT_RING_SIZE	1024
 
+#define ENA_RX_RSS_TABLE_LOG_SIZE	7
+#define ENA_RX_RSS_TABLE_SIZE		(1 << ENA_RX_RSS_TABLE_LOG_SIZE)
+
 #define ENA_MIN_MTU		128
 
 #define ENA_MMIO_DISABLE_REG_READ	BIT(0)

@@ -290,6 +293,9 @@ struct ena_adapter {
 
 	struct ena_stats_dev dev_stats;
 	struct ena_stats_eni eni_stats;
+	struct ena_admin_basic_stats basic_stats;
+
+	u32 indirect_table[ENA_RX_RSS_TABLE_SIZE];
 
 	uint32_t all_aenq_groups;
 	uint32_t active_aenq_groups;

@@ -304,6 +310,9 @@ struct ena_adapter {
 	uint64_t tx_cleanup_stall_delay;
 };
 
+int ena_mp_indirect_table_set(struct ena_adapter *adapter);
+int ena_mp_indirect_table_get(struct ena_adapter *adapter,
+			      uint32_t *indirect_table);
 int ena_rss_reta_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_reta_entry64 *reta_conf,
 			uint16_t reta_size);

drivers/net/ena/ena_rss.c +5 -5

index be4007e3f3..b6c4f76e38 100644
@@ -89,6 +89,8 @@ int ena_rss_reta_update(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+	/* Prevent RETA table structure update races */
+	rte_spinlock_lock(&adapter->admin_lock);
 	for (i = 0 ; i < reta_size ; i++) {
 		/* Each reta_conf is for 64 entries.
 		 * To support 128 we use 2 conf of 64.

@@ -109,8 +111,7 @@ int ena_rss_reta_update(struct rte_eth_dev *dev,
 		}
 	}
 
-	rte_spinlock_lock(&adapter->admin_lock);
-	rc = ena_com_indirect_table_set(ena_dev);
+	rc = ena_mp_indirect_table_set(adapter);
 	rte_spinlock_unlock(&adapter->admin_lock);
 	if (unlikely(rc != 0)) {
 		PMD_DRV_LOG(ERR, "Cannot set the indirection table\n");

@@ -128,9 +129,8 @@ int ena_rss_reta_query(struct rte_eth_dev *dev,
 		       struct rte_eth_rss_reta_entry64 *reta_conf,
 		       uint16_t reta_size)
 {
-	uint32_t indirect_table[ENA_RX_RSS_TABLE_SIZE] = { 0 };
+	uint32_t indirect_table[ENA_RX_RSS_TABLE_SIZE];
 	struct ena_adapter *adapter = dev->data->dev_private;
-	struct ena_com_dev *ena_dev = &adapter->ena_dev;
 	int rc;
 	int i;
 	int reta_conf_idx;

@@ -146,7 +146,7 @@ int ena_rss_reta_query(struct rte_eth_dev *dev,
 	}
 
 	rte_spinlock_lock(&adapter->admin_lock);
-	rc = ena_com_indirect_table_get(ena_dev, indirect_table);
+	rc = ena_mp_indirect_table_get(adapter, indirect_table);
 	rte_spinlock_unlock(&adapter->admin_lock);
 	if (unlikely(rc != 0)) {
 		PMD_DRV_LOG(ERR, "Cannot get indirection table\n");

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #65 lore raw

PATCH v3 11/21 net/ena: enable stats get function for MP mode

In reply to Michal Krawczyk

From: Stanislaw Kardach <[email protected]>

Since statistic gathering is now proxied safely to primary process, it can be enabled in secondary processes.

Signed-off-by Stanislaw Kardach <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/net/ena/ena_ethdev.c +0 -3

index e36d06ba91..38fbe1618e 100644
@@ -1068,9 +1068,6 @@ static int ena_stats_get(struct rte_eth_dev *dev,
 	int i;
 	int max_rings_stats;
 
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -ENOTSUP;
-
 	memset(&ena_stats, 0, sizeof(ena_stats));
 
 	rte_spinlock_lock(&adapter->admin_lock);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #66 lore raw

PATCH v3 12/21 net/ena/base: make IO memzone unique per port

In reply to Michal Krawczyk

Originally, the ena_com memzone counter was shared by ports, which caused the memzones to be harder to identify and could potentially lead to race and because of that the counter had to be atomic.

This atomic counter was global variable and it couldn't work in the multiprocess implementation.

The memzone is now being identified by the local to port memzone counter and the port ID - both of those information can be found in the shared data, so it can be probed easily.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/features/ena.ini     |  1 +
 drivers/net/ena/base/ena_plat_dpdk.h | 62 ++++++++--------------------
 drivers/net/ena/ena_ethdev.c         | 52 ++++++++++++++++++++++-
 drivers/net/ena/ena_ethdev.h         |  2 +
 4 files changed, 71 insertions(+), 46 deletions(-)

doc/guides/nics/features/ena.ini +1 -0

index 4db1db11f4..55690aaf5a 100644
@@ -17,6 +17,7 @@ L3 checksum offload  = Y
 L4 checksum offload  = Y
 Basic stats          = Y
 Extended stats       = Y
+Multiprocess aware   = Y
 Linux                = Y
 ARMv8                = Y
 x86-32               = Y

drivers/net/ena/base/ena_plat_dpdk.h +18 ~~-44~~

84 lines not on this page · raw message

drivers/net/ena/ena_ethdev.c +50 -2

64 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +2 -0

index 01cf0ef5db..ca3e5ed691 100644
@@ -308,6 +308,8 @@ struct ena_adapter {
 	uint64_t missing_tx_completion_to;
 	uint64_t missing_tx_completion_budget;
 	uint64_t tx_cleanup_stall_delay;
+
+	uint64_t memzone_cnt;
 };
 
 int ena_mp_indirect_table_set(struct ena_adapter *adapter);

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #67 lore raw

PATCH v3 13/21 net/ena: expose Tx cleanup function

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

ENA driver did not allow applications to call tx_cleanup. Freeing Tx mbufs was always done by the driver and it was not possible to manually request the driver to free mbufs.

Modify ena_tx_cleanup function to accept maximum number of packets to free and return number of packets that was freed.

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/features/ena.ini       |  1 +
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 27 ++++++++++++++++++--------
 3 files changed, 21 insertions(+), 8 deletions(-)

doc/guides/nics/features/ena.ini +1 -0

index 55690aaf5a..59c1ae85fa 100644
@@ -7,6 +7,7 @@
 Link status          = Y
 Link status event    = Y
 Rx interrupt         = Y
+Free Tx mbuf on demand = Y
 MTU update           = Y
 Scattered Rx         = Y
 TSO                  = Y

doc/guides/rel_notes/release_22_03.rst +1 -0

index 5c2521e870..95b31fc372 100644
@@ -114,6 +114,7 @@ New Features
   * Added support for the link status configuration.
   * Added optimized memcpy support for the ARM platforms.
   * Added ENA admin queue support for the MP applications.
+  * Added free Tx mbuf on demand feature support.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +19 -8

index 73c1d63174..5e7b964995 100644
@@ -170,7 +170,7 @@ static void ena_tx_map_mbuf(struct ena_ring *tx_ring,
 	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 int ena_tx_cleanup(void *txp, uint32_t free_pkt_cnt);
 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,

@@ -276,6 +276,7 @@ static const struct eth_dev_ops ena_dev_ops = {
 	.rx_queue_intr_disable = ena_rx_queue_intr_disable,
 	.rss_hash_update      = ena_rss_hash_update,
 	.rss_hash_conf_get    = ena_rss_hash_conf_get,
+	.tx_done_cleanup      = ena_tx_cleanup,
 };
 
 /*********************************************************************

@@ -2990,16 +2991,22 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	return 0;
 }
 
-static void ena_tx_cleanup(struct ena_ring *tx_ring)
+static int ena_tx_cleanup(void *txp, uint32_t free_pkt_cnt)
 {
+	struct ena_ring *tx_ring = (struct ena_ring *)txp;
 	unsigned int total_tx_descs = 0;
+	unsigned int total_tx_pkts = 0;
 	uint16_t cleanup_budget;
 	uint16_t next_to_clean = tx_ring->next_to_clean;
 
-	/* Attempt to release all Tx descriptors (ring_size - 1 -> size_mask) */
-	cleanup_budget = tx_ring->size_mask;
+	/*
+	 * If free_pkt_cnt is equal to 0, it means that the user requested
+	 * full cleanup, so attempt to release all Tx descriptors
+	 * (ring_size - 1 -> size_mask)
+	 */
+	cleanup_budget = (free_pkt_cnt == 0) ? tx_ring->size_mask : free_pkt_cnt;
 
-	while (likely(total_tx_descs < cleanup_budget)) {
+	while (likely(total_tx_pkts < cleanup_budget)) {
 		struct rte_mbuf *mbuf;
 		struct ena_tx_buffer *tx_info;
 		uint16_t req_id;

@@ -3021,6 +3028,7 @@ static void ena_tx_cleanup(struct ena_ring *tx_ring)
 		tx_ring->empty_tx_reqs[next_to_clean] = req_id;
 
 		total_tx_descs += tx_info->tx_descs;
+		total_tx_pkts++;
 
 		/* Put back descriptor to the ring for reuse */
 		next_to_clean = ENA_IDX_NEXT_MASKED(next_to_clean,

@@ -3034,8 +3042,11 @@ static void ena_tx_cleanup(struct ena_ring *tx_ring)
 		ena_com_update_dev_comp_head(tx_ring->ena_com_io_cq);
 	}
 
-	/* Notify completion handler that the cleanup was just called */
-	tx_ring->last_cleanup_ticks = rte_get_timer_cycles();
+	/* Notify completion handler that full cleanup was performed */
+	if (free_pkt_cnt == 0 || total_tx_pkts < cleanup_budget)
+		tx_ring->last_cleanup_ticks = rte_get_timer_cycles();
+
+	return total_tx_pkts;
 }
 
 static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,

@@ -3056,7 +3067,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 	available_desc = ena_com_free_q_entries(tx_ring->ena_com_io_sq);
 	if (available_desc < tx_ring->tx_free_thresh)
-		ena_tx_cleanup(tx_ring);
+		ena_tx_cleanup((void *)tx_ring, 0);
 
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx]))

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #68 lore raw

PATCH v3 14/21 net/ena: add API for probing xstat names by ID

In reply to Michal Krawczyk

ENA was only supporting retrieval of all the xstats name and wasn't implementing the eth_xstats_get_names_by_id API.

As this API may be more efficient than retrieving all the names, it tries to avoid excessive string copying.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/rel_notes/release_22_03.rst |   1 +
 drivers/net/ena/ena_ethdev.c           | 130 ++++++++++++++++++++-----
 2 files changed, 108 insertions(+), 23 deletions(-)

doc/guides/rel_notes/release_22_03.rst +1 -0

index 95b31fc372..1746abddbb 100644
@@ -115,6 +115,7 @@ New Features
   * Added optimized memcpy support for the ARM platforms.
   * Added ENA admin queue support for the MP applications.
   * Added free Tx mbuf on demand feature support.
+  * Added ``rte_eth_xstats_get_names_by_id`` API support.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +107 ~~-23~~

148 lines not on this page · raw message

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:21 UTC #69 lore raw

PATCH v3 15/21 net/ena: check if reset was already triggered

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

Fixes 2081d5e2e92d ("net/ena: add reset routine")

Cc [email protected]

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

drivers/net/ena/ena_ethdev.c +18 ~~-14~~

index 11c9bb05e6..3cdeba9fad 100644
@@ -539,6 +539,15 @@ ENA_PROXY_DESC(ena_com_indirect_table_get, ENA_MP_IND_TBL_GET,
 }),
 	struct ena_com_dev *ena_dev, u32 *ind_tbl);
 
+static inline void ena_trigger_reset(struct ena_adapter *adapter,
+				     enum ena_regs_reset_reason_types reason)
+{
+	if (likely(!adapter->trigger_reset)) {
+		adapter->reset_reason = reason;
+		adapter->trigger_reset = true;
+	}
+}
+
 static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
 				       struct rte_mbuf *mbuf,
 				       struct ena_com_rx_ctx *ena_rx_ctx,

@@ -666,8 +675,7 @@ static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id)
 
 	/* Trigger device reset */
 	++tx_ring->tx_stats.bad_req_id;
-	tx_ring->adapter->reset_reason = ENA_REGS_RESET_INV_TX_REQ_ID;
-	tx_ring->adapter->trigger_reset	= true;
+	ena_trigger_reset(tx_ring->adapter, ENA_REGS_RESET_INV_TX_REQ_ID);
 	return -EFAULT;
 }

@@ -1783,8 +1791,7 @@ static void check_for_missing_keep_alive(struct ena_adapter *adapter)
 	if (unlikely((rte_get_timer_cycles() - adapter->timestamp_wd) >=
 	    adapter->keep_alive_timeout)) {
 		PMD_DRV_LOG(ERR, "Keep alive timeout\n");
-		adapter->reset_reason = ENA_REGS_RESET_KEEP_ALIVE_TO;
-		adapter->trigger_reset = true;
+		ena_trigger_reset(adapter, ENA_REGS_RESET_KEEP_ALIVE_TO);
 		++adapter->dev_stats.wd_expired;
 	}
 }

@@ -1794,8 +1801,7 @@ static void check_for_admin_com_state(struct ena_adapter *adapter)
 {
 	if (unlikely(!ena_com_get_admin_running_state(&adapter->ena_dev))) {
 		PMD_DRV_LOG(ERR, "ENA admin queue is not in running state\n");
-		adapter->reset_reason = ENA_REGS_RESET_ADMIN_TO;
-		adapter->trigger_reset = true;
+		ena_trigger_reset(adapter, ENA_REGS_RESET_ADMIN_TO);
 	}
 }

@@ -2606,14 +2612,13 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				rc);
 			if (rc == ENA_COM_NO_SPACE) {
 				++rx_ring->rx_stats.bad_desc_num;
-				rx_ring->adapter->reset_reason =
-					ENA_REGS_RESET_TOO_MANY_RX_DESCS;
+				ena_trigger_reset(rx_ring->adapter,
+					ENA_REGS_RESET_TOO_MANY_RX_DESCS);
 			} else {
 				++rx_ring->rx_stats.bad_req_id;
-				rx_ring->adapter->reset_reason =
-					ENA_REGS_RESET_INV_RX_REQ_ID;
+				ena_trigger_reset(rx_ring->adapter,
+					ENA_REGS_RESET_INV_RX_REQ_ID);
 			}
-			rx_ring->adapter->trigger_reset = true;
 			return 0;
 		}

@@ -2978,9 +2983,8 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf)
 	if (unlikely(rc)) {
 		PMD_DRV_LOG(ERR, "Failed to prepare Tx buffers, rc: %d\n", rc);
 		++tx_ring->tx_stats.prepare_ctx_err;
-		tx_ring->adapter->reset_reason =
-		    ENA_REGS_RESET_DRIVER_INVALID_STATE;
-		tx_ring->adapter->trigger_reset = true;
+		ena_trigger_reset(tx_ring->adapter,
+			ENA_REGS_RESET_DRIVER_INVALID_STATE);
 		return rc;
 	}

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:22 UTC #70 lore raw

PATCH v3 16/21 net/ena: make Tx completion timeout configurable

In reply to Michal Krawczyk

The parameter is called 'miss_txc_to' and can be modified using the devargs interface:

./app -a <bdf>,miss_txc_to=UINT_NUMBER

This parameter accepts values from 0 to 60 and indicates number of seconds after which the Tx packet will be considered as missing.

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 doc/guides/nics/ena.rst                |  9 ++++
 doc/guides/rel_notes/release_22_03.rst |  1 +
 drivers/net/ena/ena_ethdev.c           | 66 ++++++++++++++++++++------
 drivers/net/ena/ena_ethdev.h           |  1 +
 4 files changed, 62 insertions(+), 15 deletions(-)

doc/guides/nics/ena.rst +9 -0

index 85c790e80f..3d780acac9 100644
@@ -87,6 +87,15 @@ Configuration information
      effect only if the device also supports large LLQ headers. Otherwise, the
      default value will be used.
 
+   * **miss_txc_to** (default 5)
+
+     Number of seconds after which the Tx packet will be considered missing.
+     If the missing packets number will exceed dynamically calculated threshold,
+     the driver will trigger the device reset which should be handled by the
+     application. Checking for missing Tx completions happens in the driver's
+     timer service. Setting this parameter to 0 disables this feature. Maximum
+     allowed value is 60 seconds.
+
 **ENA Configuration Parameters**
 
    * **Number of Queues**

doc/guides/rel_notes/release_22_03.rst +1 -0

index 1746abddbb..e04567ed5b 100644
@@ -116,6 +116,7 @@ New Features
   * Added ENA admin queue support for the MP applications.
   * Added free Tx mbuf on demand feature support.
   * Added ``rte_eth_xstats_get_names_by_id`` API support.
+  * Added ``miss_txc_to`` device argument for setting the Tx completion timeout.
 
 * **Updated Cisco enic driver.**

drivers/net/ena/ena_ethdev.c +51 ~~-15~~

109 lines not on this page · raw message

drivers/net/ena/ena_ethdev.h +1 -0

index ca3e5ed691..c0094b03ee 100644
@@ -40,6 +40,7 @@
 #define ENA_DEVICE_KALIVE_TIMEOUT (ENA_WD_TIMEOUT_SEC * rte_get_timer_hz())
 
 #define ENA_TX_TIMEOUT			(5 * rte_get_timer_hz())
+#define ENA_MAX_TX_TIMEOUT_SECONDS	60
 #define ENA_MONITORED_TX_QUEUES		3
 #define ENA_DEFAULT_MISSING_COMP	256U

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:22 UTC #71 lore raw

PATCH v3 17/21 net/ena: fix meta-desc DF flag setup

In reply to Michal Krawczyk

The requirements for the df field are as below:
* No offload used - value doesn't matter
* IPv4 - 0 or 1, depending on the DF flag in the IPv4 header
* IPv6 - 1

Setting DF to 0 causes the packet to enter the slow-path in the HW and as a result can noticeable impact the performance.

Moreover, as 'true' may not always be mapped to 1 depending on it's definition for the given platform/compiler, for safety DF field is being set explicitly to 1.

Fixes 1173fca25af9 ("ena: add polling-mode driver")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

Reviewed-by Dawid Gorecki <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +3 -1

index 0ca4269522..53a8071968 100644
@@ -624,6 +624,8 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 
 		if (mbuf->ol_flags & RTE_MBUF_F_TX_IPV6) {
 			ena_tx_ctx->l3_proto = ENA_ETH_IO_L3_PROTO_IPV6;
+			/* For the IPv6 packets, DF always needs to be true. */
+			ena_tx_ctx->df = 1;
 		} else {
 			ena_tx_ctx->l3_proto = ENA_ETH_IO_L3_PROTO_IPV4;

@@ -631,7 +633,7 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 			if (mbuf->packet_type &
 				(RTE_PTYPE_L4_NONFRAG
 				 | RTE_PTYPE_INNER_L4_NONFRAG))
-				ena_tx_ctx->df = true;
+				ena_tx_ctx->df = 1;
 		}
 
 		/* check if L4 checksum is needed */

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:22 UTC #72 lore raw

PATCH v3 18/21 net/ena: extend debug prints for invalid req ID resets

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

Add information about port id, queue id and req_id to error logs in validate_tx_req_id.

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/net/ena/ena_ethdev.c +4 -2

index 53a8071968..b471102ea9 100644
@@ -675,9 +675,11 @@ static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id)
 	}
 
 	if (tx_info)
-		PMD_TX_LOG(ERR, "tx_info doesn't have valid mbuf\n");
+		PMD_TX_LOG(ERR, "tx_info doesn't have valid mbuf. queue %d:%d req_id %u\n",
+			tx_ring->port_id, tx_ring->id, req_id);
 	else
-		PMD_TX_LOG(ERR, "Invalid req_id: %hu\n", req_id);
+		PMD_TX_LOG(ERR, "Invalid req_id: %hu in queue %d:%d\n",
+			req_id, tx_ring->port_id, tx_ring->id);
 
 	/* Trigger device reset */
 	++tx_ring->tx_stats.bad_req_id;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:22 UTC #73 lore raw

PATCH v3 19/21 net/ena: don't initialize LLQ when membar isn't exposed

In reply to Michal Krawczyk

From: Dawid Gorecki <[email protected]>

The ena_com_config_dev_mode() performs many calculations related to LLQ and then performs an admin queue call to configure LLQ in the device.

Fixes 2fca2a98c0d1 ("net/ena: support LLQv2")

Cc [email protected]

Signed-off-by Dawid Gorecki <[email protected]>

Reviewed-by Michal Krawczyk <[email protected]>

Reviewed-by Shai Brandes <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/net/ena/ena_ethdev.c +7 -7

index b471102ea9..151c688eec 100644
@@ -1953,6 +1953,13 @@ ena_set_queues_placement_policy(struct ena_adapter *adapter,
 		return 0;
 	}
 
+	if (adapter->dev_mem_base == NULL) {
+		PMD_DRV_LOG(ERR,
+			"LLQ is advertised as supported, but device doesn't expose mem bar\n");
+		ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
+		return 0;
+	}
+
 	rc = ena_com_config_dev_mode(ena_dev, llq, llq_default_configurations);
 	if (unlikely(rc)) {
 		PMD_INIT_LOG(WARNING,

@@ -1965,13 +1972,6 @@ ena_set_queues_placement_policy(struct ena_adapter *adapter,
 	if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_HOST)
 		return 0;
 
-	if (!adapter->dev_mem_base) {
-		PMD_DRV_LOG(ERR,
-			"Unable to access LLQ BAR resource. Fallback to host mode policy.\n");
-		ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
-		return 0;
-	}
-
 	ena_dev->mem_bar = adapter->dev_mem_base;
 
 	return 0;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:22 UTC #74 lore raw

PATCH v3 20/21 net/ena: don't indicate bad csum for L4 csum error

In reply to Michal Krawczyk

The statistics counters will still work as previously.

Fixes 05817057faba ("net/ena: fix indication of bad L4 Rx checksums")

Cc [email protected]

Signed-off-by Michal Krawczyk <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +7 -1

index 151c688eec..4b58dcda74 100644
@@ -583,7 +583,13 @@ static inline void ena_rx_mbuf_prepare(struct ena_ring *rx_ring,
 	} else {
 		if (unlikely(ena_rx_ctx->l4_csum_err)) {
 			++rx_stats->l4_csum_bad;
-			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
+			/*
+			 * For the L4 Rx checksum offload the HW may indicate
+			 * bad checksum although it's valid. Because of that,
+			 * we're setting the UNKNOWN flag to let the app
+			 * re-verify the checksum.
+			 */
+			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
 		} else {
 			++rx_stats->l4_csum_good;
 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;

Signature · 1 line

2.25.1

Michal Krawczyk

author

2022-02-23 12:22 UTC #75 lore raw

PATCH v3 21/21 net/ena: update version to 2.6.0

In reply to Michal Krawczyk

Signed-off-by Michal Krawczyk <[email protected]>

---
 drivers/net/ena/ena_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ena/ena_ethdev.c +1 -1

index 4b58dcda74..68768cab70 100644
@@ -21,7 +21,7 @@
 #include <ena_eth_io_defs.h>
 
 #define DRV_MODULE_VER_MAJOR	2
-#define DRV_MODULE_VER_MINOR	5
+#define DRV_MODULE_VER_MINOR	6
 #define DRV_MODULE_VER_SUBMINOR	0
 
 #define __MERGE_64B_H_L(h, l) (((uint64_t)h << 32) | l)

Signature · 1 line

2.25.1

Ferruh Yigit

2022-02-23 18:13 UTC #76 lore raw

In reply to Michal Krawczyk

Quoted 45 lines · Michal Krawczyk

Hi,
this set contains new ENA features:
* New xstats.
* Reconfigurable link status event.
* Usage of the optimized memcpy on arm/arm64.
* Better MP support.
* Reconfigurable Tx completion timeout value using devarg.
Beside that, this patchset contains multiple fixes, minor improvements, new DPDK API support and extra error checks.
v3:
* Fix build errors of the AQ proxy patch:
- for the FreeBSD, by changing ECOMM error to EIO,
- with assertions enabled, by fixing variable name.
v2:
* Fix reviewer's email (s/[email protected]/[email protected]/).
* Fix commit messages typos found by the checkpatch.
Dawid Gorecki (4):
net/ena: expose Tx cleanup function
net/ena: check if reset was already triggered net/ena: extend debug prints for invalid req ID resets net/ena: don't initialize LLQ when membar isn't exposed
Michal Krawczyk (15):
net/ena: remove linearization function net/ena: add assertion on Tx info mbuf net/ena: remove unused enumeration
net/ena: remove unused offloads variables net/ena: add extra Rx checksum related xstats net/ena: make LSC configurable
net/ena: skip timer if the reset is triggered net/ena: perform Tx cleanup before sending pkts net/ena/base: use optimized memcpy version also on Arm net/ena/base: make IO memzone unique per port net/ena: add API for probing xstat names by ID net/ena: make Tx completion timeout configurable net/ena: fix meta-desc DF flag setup
net/ena: don't indicate bad csum for L4 csum error net/ena: update version to 2.6.0

Quoted 4 lines

Stanislaw Kardach (2):
net/ena: proxy AQ calls to primary process
net/ena: enable stats get function for MP mode

Series applied to dpdk-next-net/main, thanks.