All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler
@ 2018-05-09 12:47 Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 11/24] net/ena: add checking for admin queue state Michal Krawczyk
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin; +Cc: dev, matua

Keep alive is executing AENQ interrupt periodically. It allows to check
health of the device and trigger reset event if the device will stop
responding.

To check for the state of the device, the DPDK application must call
rte_timer_manage().

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 58 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/net/ena/ena_ethdev.h |  9 +++++++
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 160fcf04a..3956ec379 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -253,6 +253,7 @@ static bool ena_are_tx_queue_offloads_allowed(struct ena_adapter *adapter,
 static bool ena_are_rx_queue_offloads_allowed(struct ena_adapter *adapter,
 					      uint64_t offloads);
 static void ena_interrupt_handler_rte(void *cb_arg);
+static void ena_timer_wd_callback(struct rte_timer *timer, void *arg);

 static const struct eth_dev_ops ena_dev_ops = {
 	.dev_configure        = ena_dev_configure,
@@ -983,6 +984,7 @@ static int ena_start(struct rte_eth_dev *dev)
 {
 	struct ena_adapter *adapter =
 		(struct ena_adapter *)(dev->data->dev_private);
+	uint64_t ticks;
 	int rc = 0;

 	rc = ena_check_valid_conf(adapter);
@@ -1006,6 +1008,13 @@ static int ena_start(struct rte_eth_dev *dev)

 	ena_stats_restart(dev);

+	adapter->timestamp_wd = rte_get_timer_cycles();
+	adapter->keep_alive_timeout = ENA_DEVICE_KALIVE_TIMEOUT;
+
+	ticks = rte_get_timer_hz();
+	rte_timer_reset(&adapter->timer_wd, ticks, PERIODICAL, rte_lcore_id(),
+			ena_timer_wd_callback, adapter);
+
 	adapter->state = ENA_ADAPTER_STATE_RUNNING;

 	return 0;
@@ -1016,6 +1025,8 @@ static void ena_stop(struct rte_eth_dev *dev)
 	struct ena_adapter *adapter =
 		(struct ena_adapter *)(dev->data->dev_private);

+	rte_timer_stop_sync(&adapter->timer_wd);
+
 	adapter->state = ENA_ADAPTER_STATE_STOPPED;
 }

@@ -1377,7 +1388,8 @@ static int ena_device_init(struct ena_com_dev *ena_dev,
 	}

 	aenq_groups = BIT(ENA_ADMIN_LINK_CHANGE) |
-		      BIT(ENA_ADMIN_NOTIFICATION);
+		      BIT(ENA_ADMIN_NOTIFICATION) |
+		      BIT(ENA_ADMIN_KEEP_ALIVE);

 	aenq_groups &= get_feat_ctx->aenq.supported_groups;
 	rc = ena_com_set_aenq_config(ena_dev, aenq_groups);
@@ -1407,6 +1419,26 @@ static void ena_interrupt_handler_rte(void *cb_arg)
 		ena_com_aenq_intr_handler(ena_dev, adapter);
 }

+static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer,
+				  void *arg)
+{
+	struct ena_adapter *adapter = (struct ena_adapter *)arg;
+	struct rte_eth_dev *dev = adapter->rte_dev;
+
+	if (adapter->keep_alive_timeout == ENA_HW_HINTS_NO_TIMEOUT)
+		return;
+
+	/* Within reasonable timing range no memory barriers are needed */
+	if ((rte_get_timer_cycles() - adapter->timestamp_wd) >=
+	    adapter->keep_alive_timeout) {
+		RTE_LOG(ERR, PMD, "The ENA device is not responding - "
+			"performing device reset...");
+		adapter->reset_reason = ENA_REGS_RESET_KEEP_ALIVE_TO;
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
+			NULL);
+	}
+}
+
 static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev;
@@ -1509,6 +1541,10 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	ena_com_set_admin_polling_mode(ena_dev, false);
 	ena_com_admin_aenq_enable(ena_dev);

+	if (adapters_found == 0)
+		rte_timer_subsystem_init();
+	rte_timer_init(&adapter->timer_wd);
+
 	adapters_found++;
 	adapter->state = ENA_ADAPTER_STATE_INIT;

@@ -1864,6 +1900,16 @@ static void ena_update_hints(struct ena_adapter *adapter,
 		/* convert to usec */
 		adapter->ena_dev.mmio_read.reg_read_to =
 			hints->mmio_read_timeout * 1000;
+
+	if (hints->driver_watchdog_timeout) {
+		if (hints->driver_watchdog_timeout == ENA_HW_HINTS_NO_TIMEOUT)
+			adapter->keep_alive_timeout = ENA_HW_HINTS_NO_TIMEOUT;
+		else
+			// Convert msecs to ticks
+			adapter->keep_alive_timeout =
+				(hints->driver_watchdog_timeout *
+				rte_get_timer_hz()) / 1000;
+	}
 }

 static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
@@ -2084,6 +2130,14 @@ static void ena_notification(void *data,
 	}
 }

+static void ena_keep_alive(void *adapter_data,
+			   __rte_unused struct ena_admin_aenq_entry *aenq_e)
+{
+	struct ena_adapter *adapter = (struct ena_adapter *)adapter_data;
+
+	adapter->timestamp_wd = rte_get_timer_cycles();
+}
+
 /**
  * This handler will called for unknown event group or unimplemented handlers
  **/
@@ -2097,7 +2151,7 @@ static struct ena_aenq_handlers aenq_handlers = {
 	.handlers = {
 		[ENA_ADMIN_LINK_CHANGE] = ena_update_on_link_change,
 		[ENA_ADMIN_NOTIFICATION] = ena_notification,
-		[ENA_ADMIN_KEEP_ALIVE] = unimplemented_aenq_handler
+		[ENA_ADMIN_KEEP_ALIVE] = ena_keep_alive
 	},
 	.unimplemented_handler = unimplemented_aenq_handler
 };
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 79e9e655d..b44cca23e 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -34,8 +34,10 @@
 #ifndef _ENA_ETHDEV_H_
 #define _ENA_ETHDEV_H_

+#include <rte_cycles.h>
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
+#include <rte_timer.h>

 #include "ena_com.h"

@@ -50,6 +52,9 @@

 #define ENA_MMIO_DISABLE_REG_READ	BIT(0)

+#define ENA_WD_TIMEOUT_SEC	3
+#define ENA_DEVICE_KALIVE_TIMEOUT (ENA_WD_TIMEOUT_SEC * rte_get_timer_hz())
+
 struct ena_adapter;

 enum ena_ring_type {
@@ -185,6 +190,10 @@ struct ena_adapter {
 	bool link_status;

 	enum ena_regs_reset_reason_types reset_reason;
+
+	struct rte_timer timer_wd;
+	uint64_t timestamp_wd;
+	uint64_t keep_alive_timeout;
 };

 #endif /* _ENA_ETHDEV_H_ */
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 11/24] net/ena: add checking for admin queue state
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 12/24] net/ena: make watchdog configurable Michal Krawczyk
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin; +Cc: dev, matua

The admin queue can stop responding or became inactive due to unexpected
behaviour of the device. In that case, the whole device should be
restarted.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 37 +++++++++++++++++++++++++++++--------
 drivers/net/ena/ena_ethdev.h |  2 ++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 3956ec379..2284a8c50 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -531,6 +531,8 @@ ena_dev_reset(struct rte_eth_dev *dev)
 	for (i = 0; i < nb_queues; ++i)
 		ena_tx_queue_setup(eth_dev, i, adapter->tx_ring_size, 0, NULL);

+	adapter->trigger_reset = false;
+
 	return 0;
 }

@@ -1419,21 +1421,40 @@ static void ena_interrupt_handler_rte(void *cb_arg)
 		ena_com_aenq_intr_handler(ena_dev, adapter);
 }

+static void check_for_missing_keep_alive(struct ena_adapter *adapter)
+{
+	if (adapter->keep_alive_timeout == ENA_HW_HINTS_NO_TIMEOUT)
+		return;
+
+	if (unlikely((rte_get_timer_cycles() - adapter->timestamp_wd) >=
+	    adapter->keep_alive_timeout)) {
+		RTE_LOG(ERR, PMD, "Keep alive timeout\n");
+		adapter->reset_reason = ENA_REGS_RESET_KEEP_ALIVE_TO;
+		adapter->trigger_reset = true;
+	}
+}
+
+/* Check if admin queue is enabled */
+static void check_for_admin_com_state(struct ena_adapter *adapter)
+{
+	if (unlikely(!ena_com_get_admin_running_state(&adapter->ena_dev))) {
+		RTE_LOG(ERR, PMD, "ENA admin queue is not in running state!\n");
+		adapter->reset_reason = ENA_REGS_RESET_ADMIN_TO;
+		adapter->trigger_reset = true;
+	}
+}
+
 static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer,
 				  void *arg)
 {
 	struct ena_adapter *adapter = (struct ena_adapter *)arg;
 	struct rte_eth_dev *dev = adapter->rte_dev;

-	if (adapter->keep_alive_timeout == ENA_HW_HINTS_NO_TIMEOUT)
-		return;
+	check_for_missing_keep_alive(adapter);
+	check_for_admin_com_state(adapter);

-	/* Within reasonable timing range no memory barriers are needed */
-	if ((rte_get_timer_cycles() - adapter->timestamp_wd) >=
-	    adapter->keep_alive_timeout) {
-		RTE_LOG(ERR, PMD, "The ENA device is not responding - "
-			"performing device reset...");
-		adapter->reset_reason = ENA_REGS_RESET_KEEP_ALIVE_TO;
+	if (unlikely(adapter->trigger_reset)) {
+		RTE_LOG(ERR, PMD, "Trigger reset is on\n");
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET,
 			NULL);
 	}
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index b44cca23e..1f6a7062f 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -194,6 +194,8 @@ struct ena_adapter {
 	struct rte_timer timer_wd;
 	uint64_t timestamp_wd;
 	uint64_t keep_alive_timeout;
+
+	bool trigger_reset;
 };

 #endif /* _ENA_ETHDEV_H_ */
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 12/24] net/ena: make watchdog configurable
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 11/24] net/ena: add checking for admin queue state Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 13/24] net/ena: add RX out of order completion Michal Krawczyk
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Add variable wd_state to make driver functional without keep alive
AENQ handler.
The watchdog will be executed only if the aenq group has keep alive
enabled.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 19 +++++++++++++++----
 drivers/net/ena/ena_ethdev.h |  2 ++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 2284a8c50..3383ba059 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -204,7 +204,8 @@ static const struct rte_pci_id pci_id_ena_map[] = {
 static struct ena_aenq_handlers aenq_handlers;

 static int ena_device_init(struct ena_com_dev *ena_dev,
-			   struct ena_com_dev_get_features_ctx *get_feat_ctx);
+			   struct ena_com_dev_get_features_ctx *get_feat_ctx,
+			   bool *wd_state);
 static int ena_dev_configure(struct rte_eth_dev *dev);
 static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 				  uint16_t nb_pkts);
@@ -489,6 +490,7 @@ ena_dev_reset(struct rte_eth_dev *dev)
 	struct ena_adapter *adapter;
 	int nb_queues;
 	int rc, i;
+	bool wd_state;

 	adapter = (struct ena_adapter *)(dev->data->dev_private);
 	ena_dev = &adapter->ena_dev;
@@ -514,11 +516,12 @@ ena_dev_reset(struct rte_eth_dev *dev)
 	ena_com_admin_destroy(ena_dev);
 	ena_com_mmio_reg_read_request_destroy(ena_dev);

-	rc = ena_device_init(ena_dev, &get_feat_ctx);
+	rc = ena_device_init(ena_dev, &get_feat_ctx, &wd_state);
 	if (rc) {
 		PMD_INIT_LOG(CRIT, "Cannot initialize device\n");
 		return rc;
 	}
+	adapter->wd_state = wd_state;

 	rte_intr_enable(intr_handle);
 	ena_com_set_admin_polling_mode(ena_dev, false);
@@ -1328,7 +1331,8 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 }

 static int ena_device_init(struct ena_com_dev *ena_dev,
-			   struct ena_com_dev_get_features_ctx *get_feat_ctx)
+			   struct ena_com_dev_get_features_ctx *get_feat_ctx,
+			   bool *wd_state)
 {
 	uint32_t aenq_groups;
 	int rc;
@@ -1400,6 +1404,8 @@ static int ena_device_init(struct ena_com_dev *ena_dev,
 		goto err_admin_init;
 	}

+	*wd_state = !!(aenq_groups & BIT(ENA_ADMIN_KEEP_ALIVE));
+
 	return 0;

 err_admin_init:
@@ -1423,6 +1429,9 @@ static void ena_interrupt_handler_rte(void *cb_arg)

 static void check_for_missing_keep_alive(struct ena_adapter *adapter)
 {
+	if (!adapter->wd_state)
+		return;
+
 	if (adapter->keep_alive_timeout == ENA_HW_HINTS_NO_TIMEOUT)
 		return;

@@ -1471,6 +1480,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	int queue_size, rc;

 	static int adapters_found;
+	bool wd_state;

 	memset(adapter, 0, sizeof(struct ena_adapter));
 	ena_dev = &adapter->ena_dev;
@@ -1514,11 +1524,12 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 		 adapter->id_number);

 	/* device specific initialization routine */
-	rc = ena_device_init(ena_dev, &get_feat_ctx);
+	rc = ena_device_init(ena_dev, &get_feat_ctx, &wd_state);
 	if (rc) {
 		PMD_INIT_LOG(CRIT, "Failed to init ENA device");
 		return -1;
 	}
+	adapter->wd_state = wd_state;

 	ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
 	adapter->num_queues = get_feat_ctx.max_queues.max_sq_num;
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 1f6a7062f..594e643e2 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -196,6 +196,8 @@ struct ena_adapter {
 	uint64_t keep_alive_timeout;

 	bool trigger_reset;
+
+	bool wd_state;
 };

 #endif /* _ENA_ETHDEV_H_ */
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 13/24] net/ena: add RX out of order completion
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 11/24] net/ena: add checking for admin queue state Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 12/24] net/ena: make watchdog configurable Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 14/24] net/ena: linearize Tx mbuf Michal Krawczyk
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin; +Cc: dev, matua

This feature allows RX packets to be cleaned up out of order.

Signed-off-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 48 ++++++++++++++++++++++++++++++++++++++++----
 drivers/net/ena/ena_ethdev.h |  8 ++++++--
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 3383ba059..075621905 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -372,6 +372,19 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf,
 	}
 }

+static inline int validate_rx_req_id(struct ena_ring *rx_ring, uint16_t req_id)
+{
+	if (likely(req_id < rx_ring->ring_size))
+		return 0;
+
+	RTE_LOG(ERR, PMD, "Invalid rx req_id: %hu\n", req_id);
+
+	rx_ring->adapter->reset_reason = ENA_REGS_RESET_INV_RX_REQ_ID;
+	rx_ring->adapter->trigger_reset = true;
+
+	return -EFAULT;
+}
+
 static void ena_config_host_info(struct ena_com_dev *ena_dev)
 {
 	struct ena_admin_host_info *host_info;
@@ -728,6 +741,10 @@ static void ena_rx_queue_release(void *queue)
 		rte_free(ring->rx_buffer_info);
 	ring->rx_buffer_info = NULL;

+	if (ring->empty_rx_reqs)
+		rte_free(ring->empty_rx_reqs);
+	ring->empty_rx_reqs = NULL;
+
 	ring->configured = 0;

 	RTE_LOG(NOTICE, PMD, "RX Queue %d:%d released\n",
@@ -1187,7 +1204,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 		(struct ena_adapter *)(dev->data->dev_private);
 	struct ena_ring *rxq = NULL;
 	uint16_t ena_qid = 0;
-	int rc = 0;
+	int i, rc = 0;
 	struct ena_com_dev *ena_dev = &adapter->ena_dev;

 	rxq = &adapter->rx_ring[queue_idx];
@@ -1261,6 +1278,19 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 		return -ENOMEM;
 	}

+	rxq->empty_rx_reqs = rte_zmalloc("rxq->empty_rx_reqs",
+					 sizeof(uint16_t) * nb_desc,
+					 RTE_CACHE_LINE_SIZE);
+	if (!rxq->empty_rx_reqs) {
+		RTE_LOG(ERR, PMD, "failed to alloc mem for empty rx reqs\n");
+		rte_free(rxq->rx_buffer_info);
+		rxq->rx_buffer_info = NULL;
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < nb_desc; i++)
+		rxq->empty_tx_reqs[i] = i;
+
 	/* Store pointer to this queue in upper layer */
 	rxq->configured = 1;
 	dev->data->rx_queues[queue_idx] = rxq;
@@ -1275,7 +1305,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 	uint16_t ring_size = rxq->ring_size;
 	uint16_t ring_mask = ring_size - 1;
 	uint16_t next_to_use = rxq->next_to_use;
-	uint16_t in_use;
+	uint16_t in_use, req_id;
 	struct rte_mbuf **mbufs = &rxq->rx_buffer_info[0];

 	if (unlikely(!count))
@@ -1303,12 +1333,14 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		struct ena_com_buf ebuf;

 		rte_prefetch0(mbufs[((next_to_use + 4) & ring_mask)]);
+
+		req_id = rxq->empty_rx_reqs[next_to_use_masked];
 		/* prepare physical address for DMA transaction */
 		ebuf.paddr = mbuf->buf_iova + RTE_PKTMBUF_HEADROOM;
 		ebuf.len = mbuf->buf_len - RTE_PKTMBUF_HEADROOM;
 		/* pass resource to device */
 		rc = ena_com_add_single_rx_desc(rxq->ena_com_io_sq,
-						&ebuf, next_to_use_masked);
+						&ebuf, req_id);
 		if (unlikely(rc)) {
 			rte_mempool_put_bulk(rxq->mb_pool, (void **)(&mbuf),
 					     count - i);
@@ -1771,6 +1803,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	unsigned int ring_mask = ring_size - 1;
 	uint16_t next_to_clean = rx_ring->next_to_clean;
 	uint16_t desc_in_use = 0;
+	uint16_t req_id;
 	unsigned int recv_idx = 0;
 	struct rte_mbuf *mbuf = NULL;
 	struct rte_mbuf *mbuf_head = NULL;
@@ -1811,7 +1844,12 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			break;

 		while (segments < ena_rx_ctx.descs) {
-			mbuf = rx_buff_info[next_to_clean & ring_mask];
+			req_id = ena_rx_ctx.ena_bufs[segments].req_id;
+			rc = validate_rx_req_id(rx_ring, req_id);
+			if (unlikely(rc))
+				break;
+
+			mbuf = rx_buff_info[req_id];
 			mbuf->data_len = ena_rx_ctx.ena_bufs[segments].len;
 			mbuf->data_off = RTE_PKTMBUF_HEADROOM;
 			mbuf->refcnt = 1;
@@ -1828,6 +1866,8 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			mbuf_head->pkt_len += mbuf->data_len;

 			mbuf_prev = mbuf;
+			rx_ring->empty_rx_reqs[next_to_clean & ring_mask] =
+				req_id;
 			segments++;
 			next_to_clean++;
 		}
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 594e643e2..bba5ad53a 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -75,8 +75,12 @@ struct ena_ring {

 	enum ena_ring_type type;
 	enum ena_admin_placement_policy_type tx_mem_queue_type;
-	/* Holds the empty requests for TX OOO completions */
-	uint16_t *empty_tx_reqs;
+	/* Holds the empty requests for TX/RX OOO completions */
+	union {
+		uint16_t *empty_tx_reqs;
+		uint16_t *empty_rx_reqs;
+	};
+
 	union {
 		struct ena_tx_buffer *tx_buffer_info; /* contex of tx packet */
 		struct rte_mbuf **rx_buffer_info; /* contex of rx packet */
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 14/24] net/ena: linearize Tx mbuf
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (2 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 13/24] net/ena: add RX out of order completion Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 15/24] net/ena: add info about max number of Tx/Rx descriptors Michal Krawczyk
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Function ena_check_and_linearize_mbuf check Tx mbuf for number of
segments and linearize (defragment) it if necessary. It is called
before sending each packet.

Information about maximum number of segments is stored per each ring.

Maximum number of segments supported by NIC is taken from ENA COM in
ena_calc_queue_size function and stored in adapter structure.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 31 ++++++++++++++++++++++++++++++-
 drivers/net/ena/ena_ethdev.h |  2 ++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 075621905..759b80046 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -898,6 +898,7 @@ static int ena_check_valid_conf(struct ena_adapter *adapter)

 static int
 ena_calc_queue_size(struct ena_com_dev *ena_dev,
+		    u16 *max_tx_sgl_size,
 		    struct ena_com_dev_get_features_ctx *get_feat_ctx)
 {
 	uint32_t queue_size = ENA_DEFAULT_RING_SIZE;
@@ -920,6 +921,9 @@ ena_calc_queue_size(struct ena_com_dev *ena_dev,
 		return -EFAULT;
 	}

+	*max_tx_sgl_size = RTE_MIN(ENA_PKT_MAX_BUFS,
+		 get_feat_ctx->max_queues.max_packet_tx_descs);
+
 	return queue_size;
 }

@@ -1510,6 +1514,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	struct ena_com_dev *ena_dev = &adapter->ena_dev;
 	struct ena_com_dev_get_features_ctx get_feat_ctx;
 	int queue_size, rc;
+	u16 tx_sgl_size = 0;

 	static int adapters_found;
 	bool wd_state;
@@ -1566,13 +1571,15 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
 	adapter->num_queues = get_feat_ctx.max_queues.max_sq_num;

-	queue_size = ena_calc_queue_size(ena_dev, &get_feat_ctx);
+	queue_size = ena_calc_queue_size(ena_dev, &tx_sgl_size, &get_feat_ctx);
 	if ((queue_size <= 0) || (adapter->num_queues <= 0))
 		return -EFAULT;

 	adapter->tx_ring_size = queue_size;
 	adapter->rx_ring_size = queue_size;

+	adapter->max_tx_sgl_size = tx_sgl_size;
+
 	/* prepare ring structures */
 	ena_init_rings(adapter);

@@ -1687,6 +1694,7 @@ static void ena_init_rings(struct ena_adapter *adapter)
 		ring->id = i;
 		ring->tx_mem_queue_type = adapter->ena_dev.tx_mem_queue_type;
 		ring->tx_max_header_size = adapter->ena_dev.tx_max_header_size;
+		ring->sgl_size = adapter->max_tx_sgl_size;
 	}

 	for (i = 0; i < adapter->num_queues; i++) {
@@ -1984,6 +1992,23 @@ static void ena_update_hints(struct ena_adapter *adapter,
 	}
 }

+static int ena_check_and_linearize_mbuf(struct ena_ring *tx_ring,
+					struct rte_mbuf *mbuf)
+{
+	int num_segments, rc;
+
+	num_segments = mbuf->nb_segs;
+
+	if (likely(num_segments < tx_ring->sgl_size))
+		return 0;
+
+	rc = rte_pktmbuf_linearize(mbuf);
+	if (unlikely(rc))
+		RTE_LOG(WARNING, PMD, "Mbuf linearize failed\n");
+
+	return rc;
+}
+
 static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 				  uint16_t nb_pkts)
 {
@@ -2014,6 +2039,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
 		mbuf = tx_pkts[sent_idx];

+		rc = ena_check_and_linearize_mbuf(tx_ring, mbuf);
+		if (unlikely(rc))
+			break;
+
 		req_id = tx_ring->empty_tx_reqs[next_to_use & ring_mask];
 		tx_info = &tx_ring->tx_buffer_info[req_id];
 		tx_info->mbuf = mbuf;
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index bba5ad53a..73c110ab9 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -101,6 +101,7 @@ struct ena_ring {
 	int configured;
 	struct ena_adapter *adapter;
 	uint64_t offloads;
+	u16 sgl_size;
 } __rte_cache_aligned;

 enum ena_adapter_state {
@@ -167,6 +168,7 @@ struct ena_adapter {
 	/* TX */
 	struct ena_ring tx_ring[ENA_MAX_NUM_QUEUES] __rte_cache_aligned;
 	int tx_ring_size;
+	u16 max_tx_sgl_size;

 	/* RX */
 	struct ena_ring rx_ring[ENA_MAX_NUM_QUEUES] __rte_cache_aligned;
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 15/24] net/ena: add info about max number of Tx/Rx descriptors
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (3 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 14/24] net/ena: linearize Tx mbuf Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 16/24] net/ena: unimplemented handler error Michal Krawczyk
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

In function ena_infos_get driver provides information about minimal
and maximal number of Rx and Tx descriptors.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 759b80046..c8ff62ccc 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -85,6 +85,9 @@

 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

+#define ENA_MAX_RING_DESC	ENA_DEFAULT_RING_SIZE
+#define ENA_MIN_RING_DESC	128
+
 enum ethtool_stringset {
 	ETH_SS_TEST             = 0,
 	ETH_SS_STATS,
@@ -1801,6 +1804,16 @@ static void ena_infos_get(struct rte_eth_dev *dev,

 	adapter->tx_supported_offloads = tx_feat;
 	adapter->rx_supported_offloads = rx_feat;
+
+	dev_info->rx_desc_lim.nb_max = ENA_MAX_RING_DESC;
+	dev_info->rx_desc_lim.nb_min = ENA_MIN_RING_DESC;
+
+	dev_info->tx_desc_lim.nb_max = ENA_MAX_RING_DESC;
+	dev_info->tx_desc_lim.nb_min = ENA_MIN_RING_DESC;
+	dev_info->tx_desc_lim.nb_seg_max = RTE_MIN(ENA_PKT_MAX_BUFS,
+					feat.max_queues.max_packet_tx_descs);
+	dev_info->tx_desc_lim.nb_mtu_seg_max = RTE_MIN(ENA_PKT_MAX_BUFS,
+					feat.max_queues.max_packet_tx_descs);
 }

 static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 16/24] net/ena: unimplemented handler error
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (4 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 15/24] net/ena: add info about max number of Tx/Rx descriptors Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 17/24] net/ena: rework configuration of IO queue numbers Michal Krawczyk
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Enable AENQ FATAL_ERROR and WARNING callbacks by setting flags
in aenq_groups. They are handled by "unimplemented handler".

If unimplemented handler is called, error is logged.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index c8ff62ccc..885d96b59 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1434,7 +1434,9 @@ static int ena_device_init(struct ena_com_dev *ena_dev,

 	aenq_groups = BIT(ENA_ADMIN_LINK_CHANGE) |
 		      BIT(ENA_ADMIN_NOTIFICATION) |
-		      BIT(ENA_ADMIN_KEEP_ALIVE);
+		      BIT(ENA_ADMIN_KEEP_ALIVE) |
+		      BIT(ENA_ADMIN_FATAL_ERROR) |
+		      BIT(ENA_ADMIN_WARNING);

 	aenq_groups &= get_feat_ctx->aenq.supported_groups;
 	rc = ena_com_set_aenq_config(ena_dev, aenq_groups);
@@ -2258,7 +2260,8 @@ static void ena_keep_alive(void *adapter_data,
 static void unimplemented_aenq_handler(__rte_unused void *data,
 				       __rte_unused struct ena_admin_aenq_entry *aenq_e)
 {
-	// Unimplemented handler
+	RTE_LOG(ERR, PMD, "Unknown event was received or event with "
+			  "unimplemented handler\n");
 }

 static struct ena_aenq_handlers aenq_handlers = {
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 17/24] net/ena: rework configuration of IO queue numbers
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (5 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 16/24] net/ena: unimplemented handler error Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 18/24] net/ena: validate Tx req id Michal Krawczyk
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Move configuration of IO queue numbers to separate function and take
into consideration max number of IO completion queues.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 885d96b59..3fabe196f 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1510,6 +1510,24 @@ static void ena_timer_wd_callback(__rte_unused struct rte_timer *timer,
 	}
 }

+static int ena_calc_io_queue_num(__rte_unused struct ena_com_dev *ena_dev,
+				 struct ena_com_dev_get_features_ctx *get_feat_ctx)
+{
+	int io_sq_num, io_cq_num, io_queue_num;
+
+	io_sq_num = get_feat_ctx->max_queues.max_sq_num;
+	io_cq_num = get_feat_ctx->max_queues.max_cq_num;
+
+	io_queue_num = RTE_MIN(io_sq_num, io_cq_num);
+
+	if (unlikely(io_queue_num == 0)) {
+		RTE_LOG(ERR, PMD, "Number of IO queues should not be 0\n");
+		return -EFAULT;
+	}
+
+	return io_queue_num;
+}
+
 static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 {
 	struct rte_pci_device *pci_dev;
@@ -1574,7 +1592,8 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	adapter->wd_state = wd_state;

 	ena_dev->tx_mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST;
-	adapter->num_queues = get_feat_ctx.max_queues.max_sq_num;
+	adapter->num_queues = ena_calc_io_queue_num(ena_dev,
+						    &get_feat_ctx);

 	queue_size = ena_calc_queue_size(ena_dev, &tx_sgl_size, &get_feat_ctx);
 	if ((queue_size <= 0) || (adapter->num_queues <= 0))
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 18/24] net/ena: validate Tx req id
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (6 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 17/24] net/ena: rework configuration of IO queue numbers Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 19/24] net/ena: add (un)likely statements Michal Krawczyk
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Validate Tx req id during clearing completed packets.
If id is wrong, trigger NIC reset.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 3fabe196f..43e34a252 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -388,6 +388,27 @@ static inline int validate_rx_req_id(struct ena_ring *rx_ring, uint16_t req_id)
 	return -EFAULT;
 }

+static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id)
+{
+	struct ena_tx_buffer *tx_info = NULL;
+
+	if (likely(req_id < tx_ring->ring_size)) {
+		tx_info = &tx_ring->tx_buffer_info[req_id];
+		if (likely(tx_info->mbuf))
+			return 0;
+	}
+
+	if (tx_info)
+		RTE_LOG(ERR, PMD, "tx_info doesn't have valid mbuf\n");
+	else
+		RTE_LOG(ERR, PMD, "Invalid req_id: %hu\n", req_id);
+
+	/* Trigger device reset */
+	tx_ring->adapter->reset_reason = ENA_REGS_RESET_INV_TX_REQ_ID;
+	tx_ring->adapter->trigger_reset	= true;
+	return -EFAULT;
+}
+
 static void ena_config_host_info(struct ena_com_dev *ena_dev)
 {
 	struct ena_admin_host_info *host_info;
@@ -2154,6 +2175,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,

 	/* Clear complete packets  */
 	while (ena_com_tx_comp_req_id_get(tx_ring->ena_com_io_cq, &req_id) >= 0) {
+		rc = validate_tx_req_id(tx_ring, req_id);
+		if (rc)
+			break;
+
 		/* Get Tx info & store how many descs were processed  */
 		tx_info = &tx_ring->tx_buffer_info[req_id];
 		total_tx_descs += tx_info->tx_descs;
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 19/24] net/ena: add (un)likely statements
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (7 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 18/24] net/ena: validate Tx req id Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 20/24] net/ena: adjust error checking and cleaning Michal Krawczyk
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Add likely and unlikely statements to increase performance.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 43e34a252..b3aa751e9 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -940,7 +940,7 @@ ena_calc_queue_size(struct ena_com_dev *ena_dev,
 	if (!rte_is_power_of_2(queue_size))
 		queue_size = rte_align32pow2(queue_size >> 1);

-	if (queue_size == 0) {
+	if (unlikely(queue_size == 0)) {
 		PMD_INIT_LOG(ERR, "Invalid queue size");
 		return -EFAULT;
 	}
@@ -1379,7 +1379,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 	}

 	/* When we submitted free recources to device... */
-	if (i > 0) {
+	if (likely(i > 0)) {
 		/* ...let HW know that it can fill buffers with data */
 		rte_wmb();
 		ena_com_write_sq_doorbell(rxq->ena_com_io_sq);
@@ -1485,7 +1485,7 @@ static void ena_interrupt_handler_rte(void *cb_arg)
 	struct ena_com_dev *ena_dev = &adapter->ena_dev;

 	ena_com_admin_q_comp_intr_handler(ena_dev);
-	if (adapter->state != ENA_ADAPTER_STATE_CLOSED)
+	if (likely(adapter->state != ENA_ADAPTER_STATE_CLOSED))
 		ena_com_aenq_intr_handler(ena_dev, adapter);
 }

@@ -1917,7 +1917,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			mbuf->data_off = RTE_PKTMBUF_HEADROOM;
 			mbuf->refcnt = 1;
 			mbuf->next = NULL;
-			if (segments == 0) {
+			if (unlikely(segments == 0)) {
 				mbuf->nb_segs = ena_rx_ctx.descs;
 				mbuf->port = rx_ring->port_id;
 				mbuf->pkt_len = 0;
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 20/24] net/ena: adjust error checking and cleaning
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (8 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 19/24] net/ena: add (un)likely statements Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 21/24] net/ena: update numa node Michal Krawczyk
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Adjust error checking and cleaning to Linux driver:
 * add checking if MTU is to small,
 * fix error messages (mismatched Rx and Tx),
 * return error received from base driver or proper error
   code instead of -1,
 * in case of error release occupied resources,
 * in case of Rx error trigger NIC reset.

Fixes: 1173fca25af9 ("ena: add polling-mode driver")
Fixes: 1daff5260ff8 ("net/ena: use unmasked head and tail")

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 158 ++++++++++++++++++++++++++++---------------
 drivers/net/ena/ena_ethdev.h |   2 +
 2 files changed, 104 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index b3aa751e9..6a2c4699a 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -440,9 +440,12 @@ static void ena_config_host_info(struct ena_com_dev *ena_dev)

 	rc = ena_com_set_host_attributes(ena_dev);
 	if (rc) {
-		RTE_LOG(ERR, PMD, "Cannot set host attributes\n");
-		if (rc != -ENA_COM_UNSUPPORTED)
-			goto err;
+		if (rc == -ENA_COM_UNSUPPORTED)
+			RTE_LOG(WARNING, PMD, "Cannot set host attributes\n");
+		else
+			RTE_LOG(ERR, PMD, "Cannot set host attributes\n");
+
+		goto err;
 	}

 	return;
@@ -493,9 +496,12 @@ static void ena_config_debug_area(struct ena_adapter *adapter)

 	rc = ena_com_set_host_attributes(&adapter->ena_dev);
 	if (rc) {
-		RTE_LOG(WARNING, PMD, "Cannot set host attributes\n");
-		if (rc != -ENA_COM_UNSUPPORTED)
-			goto err;
+		if (rc == -ENA_COM_UNSUPPORTED)
+			RTE_LOG(WARNING, PMD, "Cannot set host attributes\n");
+		else
+			RTE_LOG(ERR, PMD, "Cannot set host attributes\n");
+
+		goto err;
 	}

 	return;
@@ -538,7 +544,9 @@ ena_dev_reset(struct rte_eth_dev *dev)

 	ena_com_set_admin_running_state(ena_dev, false);

-	ena_com_dev_reset(ena_dev, adapter->reset_reason);
+	rc = ena_com_dev_reset(ena_dev, adapter->reset_reason);
+	if (rc)
+		RTE_LOG(ERR, PMD, "Device reset failed\n");

 	for (i = 0; i < nb_queues; i++)
 		mb_pool_rx[i] = adapter->rx_ring[i].mb_pool;
@@ -583,7 +591,7 @@ static int ena_rss_reta_update(struct rte_eth_dev *dev,
 	struct ena_adapter *adapter =
 		(struct ena_adapter *)(dev->data->dev_private);
 	struct ena_com_dev *ena_dev = &adapter->ena_dev;
-	int ret, i;
+	int rc, i;
 	u16 entry_value;
 	int conf_idx;
 	int idx;
@@ -595,8 +603,7 @@ static int ena_rss_reta_update(struct rte_eth_dev *dev,
 		RTE_LOG(WARNING, PMD,
 			"indirection table %d is bigger than supported (%d)\n",
 			reta_size, ENA_RX_RSS_TABLE_SIZE);
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}

 	for (i = 0 ; i < reta_size ; i++) {
@@ -608,29 +615,28 @@ static int ena_rss_reta_update(struct rte_eth_dev *dev,
 		if (TEST_BIT(reta_conf[conf_idx].mask, idx)) {
 			entry_value =
 				ENA_IO_RXQ_IDX(reta_conf[conf_idx].reta[idx]);
-			ret = ena_com_indirect_table_fill_entry(ena_dev,
-								i,
-								entry_value);
-			if (unlikely(ret && (ret != ENA_COM_UNSUPPORTED))) {
+
+			rc = ena_com_indirect_table_fill_entry(ena_dev,
+							       i,
+							       entry_value);
+			if (unlikely(rc && rc != ENA_COM_UNSUPPORTED)) {
 				RTE_LOG(ERR, PMD,
 					"Cannot fill indirect table\n");
-				ret = -ENOTSUP;
-				goto err;
+				return rc;
 			}
 		}
 	}

-	ret = ena_com_indirect_table_set(ena_dev);
-	if (unlikely(ret && (ret != ENA_COM_UNSUPPORTED))) {
+	rc = ena_com_indirect_table_set(ena_dev);
+	if (unlikely(rc && rc != ENA_COM_UNSUPPORTED)) {
 		RTE_LOG(ERR, PMD, "Cannot flush the indirect table\n");
-		ret = -ENOTSUP;
-		goto err;
+		return rc;
 	}

 	RTE_LOG(DEBUG, PMD, "%s(): RSS configured %d entries  for port %d\n",
 		__func__, reta_size, adapter->rte_dev->data->port_id);
-err:
-	return ret;
+
+	return 0;
 }

 /* Query redirection table. */
@@ -641,7 +647,7 @@ static int ena_rss_reta_query(struct rte_eth_dev *dev,
 	struct ena_adapter *adapter =
 		(struct ena_adapter *)(dev->data->dev_private);
 	struct ena_com_dev *ena_dev = &adapter->ena_dev;
-	int ret;
+	int rc;
 	int i;
 	u32 indirect_table[ENA_RX_RSS_TABLE_SIZE] = {0};
 	int reta_conf_idx;
@@ -651,11 +657,10 @@ static int ena_rss_reta_query(struct rte_eth_dev *dev,
 	    (reta_size > RTE_RETA_GROUP_SIZE && ((reta_conf + 1) == NULL)))
 		return -EINVAL;

-	ret = ena_com_indirect_table_get(ena_dev, indirect_table);
-	if (unlikely(ret && (ret != ENA_COM_UNSUPPORTED))) {
+	rc = ena_com_indirect_table_get(ena_dev, indirect_table);
+	if (unlikely(rc && rc != ENA_COM_UNSUPPORTED)) {
 		RTE_LOG(ERR, PMD, "cannot get indirect table\n");
-		ret = -ENOTSUP;
-		goto err;
+		return -ENOTSUP;
 	}

 	for (i = 0 ; i < reta_size ; i++) {
@@ -665,8 +670,8 @@ static int ena_rss_reta_query(struct rte_eth_dev *dev,
 			reta_conf[reta_conf_idx].reta[reta_idx] =
 				ENA_IO_RXQ_IDX_REV(indirect_table[i]);
 	}
-err:
-	return ret;
+
+	return 0;
 }

 static int ena_rss_init_default(struct ena_adapter *adapter)
@@ -888,7 +893,7 @@ static int ena_queue_restart_all(struct rte_eth_dev *dev,
 				PMD_INIT_LOG(ERR,
 					     "failed to restart queue %d type(%d)",
 					     i, ring_type);
-				return -1;
+				return rc;
 			}
 		}
 	}
@@ -912,9 +917,11 @@ static int ena_check_valid_conf(struct ena_adapter *adapter)
 {
 	uint32_t max_frame_len = ena_get_mtu_conf(adapter);

-	if (max_frame_len > adapter->max_mtu) {
-		PMD_INIT_LOG(ERR, "Unsupported MTU of %d", max_frame_len);
-		return -1;
+	if (max_frame_len > adapter->max_mtu || max_frame_len < ENA_MIN_MTU) {
+		PMD_INIT_LOG(ERR, "Unsupported MTU of %d. "
+				  "max mtu: %d, min mtu: %d\n",
+			     max_frame_len, adapter->max_mtu, ENA_MIN_MTU);
+		return ENA_COM_UNSUPPORTED;
 	}

 	return 0;
@@ -1012,12 +1019,12 @@ static int ena_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	ena_dev = &adapter->ena_dev;
 	ena_assert_msg(ena_dev != NULL, "Uninitialized device");

-	if (mtu > ena_get_mtu_conf(adapter)) {
+	if (mtu > ena_get_mtu_conf(adapter) || mtu < ENA_MIN_MTU) {
 		RTE_LOG(ERR, PMD,
-			"Given MTU (%d) exceeds maximum MTU supported (%d)\n",
-			mtu, ena_get_mtu_conf(adapter));
-		rc = -EINVAL;
-		goto err;
+			"Invalid MTU setting. new_mtu: %d "
+			"max mtu: %d min mtu: %d\n",
+			mtu, ena_get_mtu_conf(adapter), ENA_MIN_MTU);
+		return -EINVAL;
 	}

 	rc = ena_com_set_dev_mtu(ena_dev, mtu);
@@ -1026,7 +1033,6 @@ static int ena_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	else
 		RTE_LOG(NOTICE, PMD, "Set MTU: %d\n", mtu);

-err:
 	return rc;
 }

@@ -1097,7 +1103,7 @@ static int ena_queue_restart(struct ena_ring *ring)
 	rc = ena_populate_rx_queue(ring, bufs_num);
 	if (rc != bufs_num) {
 		PMD_INIT_LOG(ERR, "Failed to populate rx ring !");
-		return (-1);
+		return ENA_COM_FAULT;
 	}

 	return 0;
@@ -1127,12 +1133,12 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 		RTE_LOG(CRIT, PMD,
 			"API violation. Queue %d is already configured\n",
 			queue_idx);
-		return -1;
+		return ENA_COM_FAULT;
 	}

 	if (!rte_is_power_of_2(nb_desc)) {
 		RTE_LOG(ERR, PMD,
-			"Unsupported size of RX queue: %d is not a power of 2.",
+			"Unsupported size of TX queue: %d is not a power of 2.",
 			nb_desc);
 		return -EINVAL;
 	}
@@ -1167,6 +1173,7 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 		RTE_LOG(ERR, PMD,
 			"failed to create io TX queue #%d (qid:%d) rc: %d\n",
 			queue_idx, ena_qid, rc);
+		return rc;
 	}
 	txq->ena_com_io_cq = &ena_dev->io_cq_queues[ena_qid];
 	txq->ena_com_io_sq = &ena_dev->io_sq_queues[ena_qid];
@@ -1178,8 +1185,7 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 		RTE_LOG(ERR, PMD,
 			"Failed to get TX queue handlers. TX queue num %d rc: %d\n",
 			queue_idx, rc);
-		ena_com_destroy_io_queue(ena_dev, ena_qid);
-		goto err;
+		goto err_destroy_io_queue;
 	}

 	txq->port_id = dev->data->port_id;
@@ -1193,7 +1199,8 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 					  RTE_CACHE_LINE_SIZE);
 	if (!txq->tx_buffer_info) {
 		RTE_LOG(ERR, PMD, "failed to alloc mem for tx buffer info\n");
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto err_destroy_io_queue;
 	}

 	txq->empty_tx_reqs = rte_zmalloc("txq->empty_tx_reqs",
@@ -1201,9 +1208,10 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 					 RTE_CACHE_LINE_SIZE);
 	if (!txq->empty_tx_reqs) {
 		RTE_LOG(ERR, PMD, "failed to alloc mem for tx reqs\n");
-		rte_free(txq->tx_buffer_info);
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto err_free;
 	}
+
 	for (i = 0; i < txq->ring_size; i++)
 		txq->empty_tx_reqs[i] = i;

@@ -1213,7 +1221,14 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 	/* Store pointer to this queue in upper layer */
 	txq->configured = 1;
 	dev->data->tx_queues[queue_idx] = txq;
-err:
+
+	return 0;
+
+err_free:
+	rte_free(txq->tx_buffer_info);
+
+err_destroy_io_queue:
+	ena_com_destroy_io_queue(ena_dev, ena_qid);
 	return rc;
 }

@@ -1240,12 +1255,12 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 		RTE_LOG(CRIT, PMD,
 			"API violation. Queue %d is already configured\n",
 			queue_idx);
-		return -1;
+		return ENA_COM_FAULT;
 	}

 	if (!rte_is_power_of_2(nb_desc)) {
 		RTE_LOG(ERR, PMD,
-			"Unsupported size of TX queue: %d is not a power of 2.",
+			"Unsupported size of RX queue: %d is not a power of 2.",
 			nb_desc);
 		return -EINVAL;
 	}
@@ -1275,9 +1290,11 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 	ctx.numa_node = ena_cpu_to_node(queue_idx);

 	rc = ena_com_create_io_queue(ena_dev, &ctx);
-	if (rc)
+	if (rc) {
 		RTE_LOG(ERR, PMD, "failed to create io RX queue #%d rc: %d\n",
 			queue_idx, rc);
+		return rc;
+	}

 	rxq->ena_com_io_cq = &ena_dev->io_cq_queues[ena_qid];
 	rxq->ena_com_io_sq = &ena_dev->io_sq_queues[ena_qid];
@@ -1290,6 +1307,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 			"Failed to get RX queue handlers. RX queue num %d rc: %d\n",
 			queue_idx, rc);
 		ena_com_destroy_io_queue(ena_dev, ena_qid);
+		return rc;
 	}

 	rxq->port_id = dev->data->port_id;
@@ -1303,6 +1321,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 					  RTE_CACHE_LINE_SIZE);
 	if (!rxq->rx_buffer_info) {
 		RTE_LOG(ERR, PMD, "failed to alloc mem for rx buffer info\n");
+		ena_com_destroy_io_queue(ena_dev, ena_qid);
 		return -ENOMEM;
 	}

@@ -1313,6 +1332,7 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
 		RTE_LOG(ERR, PMD, "failed to alloc mem for empty rx reqs\n");
 		rte_free(rxq->rx_buffer_info);
 		rxq->rx_buffer_info = NULL;
+		ena_com_destroy_io_queue(ena_dev, ena_qid);
 		return -ENOMEM;
 	}

@@ -1363,6 +1383,10 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		rte_prefetch0(mbufs[((next_to_use + 4) & ring_mask)]);

 		req_id = rxq->empty_rx_reqs[next_to_use_masked];
+		rc = validate_rx_req_id(rxq, req_id);
+		if (unlikely(rc < 0))
+			break;
+
 		/* prepare physical address for DMA transaction */
 		ebuf.paddr = mbuf->buf_iova + RTE_PKTMBUF_HEADROOM;
 		ebuf.len = mbuf->buf_len - RTE_PKTMBUF_HEADROOM;
@@ -1378,9 +1402,17 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		next_to_use++;
 	}

+	if (unlikely(i < count))
+		RTE_LOG(WARNING, PMD, "refilled rx qid %d with only %d "
+			"buffers (from %d)\n", rxq->id, i, count);
+
 	/* When we submitted free recources to device... */
 	if (likely(i > 0)) {
-		/* ...let HW know that it can fill buffers with data */
+		/* ...let HW know that it can fill buffers with data
+		 *
+		 * Add memory barrier to make sure the desc were written before
+		 * issue a doorbell
+		 */
 		rte_wmb();
 		ena_com_write_sq_doorbell(rxq->ena_com_io_sq);

@@ -1608,7 +1640,7 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	rc = ena_device_init(ena_dev, &get_feat_ctx, &wd_state);
 	if (rc) {
 		PMD_INIT_LOG(CRIT, "Failed to init ENA device");
-		return -1;
+		goto err;
 	}
 	adapter->wd_state = wd_state;

@@ -1617,8 +1649,10 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 						    &get_feat_ctx);

 	queue_size = ena_calc_queue_size(ena_dev, &tx_sgl_size, &get_feat_ctx);
-	if ((queue_size <= 0) || (adapter->num_queues <= 0))
-		return -EFAULT;
+	if (queue_size <= 0 || adapter->num_queues <= 0) {
+		rc = -EFAULT;
+		goto err_device_destroy;
+	}

 	adapter->tx_ring_size = queue_size;
 	adapter->rx_ring_size = queue_size;
@@ -1647,7 +1681,8 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 					 RTE_CACHE_LINE_SIZE);
 	if (!adapter->drv_stats) {
 		RTE_LOG(ERR, PMD, "failed to alloc mem for adapter stats\n");
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto err_delete_debug_area;
 	}

 	rte_intr_callback_register(intr_handle,
@@ -1665,6 +1700,16 @@ static int eth_ena_dev_init(struct rte_eth_dev *eth_dev)
 	adapter->state = ENA_ADAPTER_STATE_INIT;

 	return 0;
+
+err_delete_debug_area:
+	ena_com_delete_debug_area(ena_dev);
+
+err_device_destroy:
+	ena_com_delete_host_info(ena_dev);
+	ena_com_admin_destroy(ena_dev);
+
+err:
+	return rc;
 }

 static int eth_ena_dev_uninit(struct rte_eth_dev *eth_dev)
@@ -1900,6 +1945,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				    &ena_rx_ctx);
 		if (unlikely(rc)) {
 			RTE_LOG(ERR, PMD, "ena_com_rx_pkt error %d\n", rc);
+			rx_ring->adapter->trigger_reset = true;
 			return 0;
 		}

diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 73c110ab9..2dc8129e0 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -50,6 +50,8 @@
 #define ENA_NAME_MAX_LEN	20
 #define ENA_PKT_MAX_BUFS	17

+#define ENA_MIN_MTU		128
+
 #define ENA_MMIO_DISABLE_REG_READ	BIT(0)

 #define ENA_WD_TIMEOUT_SEC	3
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 21/24] net/ena: update numa node
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (9 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 20/24] net/ena: adjust error checking and cleaning Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 22/24] net/ena: check pointer before memset Michal Krawczyk
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

During initializing tx queues update Non-Uniform Memory Access
configuration in NIC firmware.

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/ena_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 6a2c4699a..5042696ab 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1188,6 +1188,8 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
 		goto err_destroy_io_queue;
 	}

+	ena_com_update_numa_node(txq->ena_com_io_cq, ctx.numa_node);
+
 	txq->port_id = dev->data->port_id;
 	txq->next_to_clean = 0;
 	txq->next_to_use = 0;
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 22/24] net/ena: check pointer before memset
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (10 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 21/24] net/ena: update numa node Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 23/24] net/ena: change memory type Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 24/24] net/ena: fix GENMASK_ULL macro Michal Krawczyk
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

Need to check if memory allocation succeed before using it.
Using memset on NULL pointer cause segfault.

Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index dd54515b2..9d4cc7246 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -210,10 +210,15 @@ extern uint32_t ena_alloc_cnt;
 				"ena_alloc_%d", ena_alloc_cnt++);	\
 		mz = rte_memzone_reserve(z_name, size, SOCKET_ID_ANY,	\
 				RTE_MEMZONE_IOVA_CONTIG);		\
-		memset(mz->addr, 0, size);				\
-		virt = mz->addr;					\
-		phys = mz->iova;					\
 		handle = mz;						\
+		if (mz == NULL) {					\
+			virt = NULL;					\
+			phys = 0;					\
+		} else {						\
+			memset(mz->addr, 0, size);			\
+			virt = mz->addr;				\
+			phys = mz->iova;				\
+		}							\
 	} while (0)
 #define ENA_MEM_FREE_COHERENT(dmadev, size, virt, phys, handle) 	\
 		({ ENA_TOUCH(size); ENA_TOUCH(phys);			\
@@ -231,9 +236,14 @@ extern uint32_t ena_alloc_cnt;
 		mz = rte_memzone_reserve(z_name, size, node,		\
 				RTE_MEMZONE_IOVA_CONTIG);		\
 		mem_handle = mz;					\
-		memset(mz->addr, 0, size);				\
-		virt = mz->addr;					\
-		phys = mz->iova;					\
+		if (mz == NULL) {					\
+			virt = NULL;					\
+			phys = 0;					\
+		} else {						\
+			memset(mz->addr, 0, size);			\
+			virt = mz->addr;				\
+			phys = mz->iova;				\
+		}							\
 	} while (0)

 #define ENA_MEM_ALLOC_NODE(dmadev, size, virt, node, dev_node) \
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 23/24] net/ena: change memory type
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (11 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 22/24] net/ena: check pointer before memset Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  2018-05-09 12:47 ` [PATCH v1 24/24] net/ena: fix GENMASK_ULL macro Michal Krawczyk
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

ENA_MEM_ALLOC_NODE not need to use contiguous physical memory.
Also using memset without checking if allocation succeed can cause
segmentation fault.

To avoid both issue use rte_zmalloc_socket.

Fixes: 3d3edc265fc8 ("net/ena: make coherent memory allocation NUMA-aware")

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index 9d4cc7246..b89cdb55d 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -248,15 +248,8 @@ extern uint32_t ena_alloc_cnt;

 #define ENA_MEM_ALLOC_NODE(dmadev, size, virt, node, dev_node) \
 	do {								\
-		const struct rte_memzone *mz;				\
-		char z_name[RTE_MEMZONE_NAMESIZE];			\
 		ENA_TOUCH(dmadev); ENA_TOUCH(dev_node);			\
-		snprintf(z_name, sizeof(z_name),			\
-				"ena_alloc_%d", ena_alloc_cnt++);	\
-		mz = rte_memzone_reserve(z_name, size, node,		\
-				RTE_MEMZONE_IOVA_CONTIG);		\
-		memset(mz->addr, 0, size);				\
-		virt = mz->addr;					\
+		virt = rte_zmalloc_socket(NULL, size, 0, node);		\
 	} while (0)

 #define ENA_MEM_ALLOC(dmadev, size) rte_zmalloc(NULL, size, 1)
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v1 24/24] net/ena: fix GENMASK_ULL macro
  2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
                   ` (12 preceding siblings ...)
  2018-05-09 12:47 ` [PATCH v1 23/24] net/ena: change memory type Michal Krawczyk
@ 2018-05-09 12:47 ` Michal Krawczyk
  13 siblings, 0 replies; 15+ messages in thread
From: Michal Krawczyk @ 2018-05-09 12:47 UTC (permalink / raw)
  To: Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin
  Cc: dev, matua, Rafal Kozik

From: Rafal Kozik <rk@semihalf.com>

When use GENMASK_ULL(63,0) left shift by 64 bits is performed.
Shifting by number greater or equal then word length
is undefined operation and failed on some platforms.

Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")

Signed-off-by: Rafal Kozik <rk@semihalf.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index b89cdb55d..58ef57d2b 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -116,11 +116,13 @@ typedef uint64_t dma_addr_t;
 #define ENA_MIN16(x, y) RTE_MIN((x), (y))
 #define ENA_MIN8(x, y) RTE_MIN((x), (y))

+#define BITS_PER_LONG_LONG (__SIZEOF_LONG_LONG__ * 8)
 #define U64_C(x) x ## ULL
 #define BIT(nr)         (1UL << (nr))
 #define BITS_PER_LONG	(__SIZEOF_LONG__ * 8)
 #define GENMASK(h, l)	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
-#define GENMASK_ULL(h, l) (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l))
+#define GENMASK_ULL(h, l) (((~0ULL) - (1ULL << (l)) + 1) & \
+			  (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))

 #ifdef RTE_LIBRTE_ENA_COM_DEBUG
 #define ena_trc_dbg(format, arg...)					\
--
2.14.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-05-09 12:47 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-09 12:47 [PATCH v1 10/24] net/ena: add watchdog and keep alive AENQ handler Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 11/24] net/ena: add checking for admin queue state Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 12/24] net/ena: make watchdog configurable Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 13/24] net/ena: add RX out of order completion Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 14/24] net/ena: linearize Tx mbuf Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 15/24] net/ena: add info about max number of Tx/Rx descriptors Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 16/24] net/ena: unimplemented handler error Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 17/24] net/ena: rework configuration of IO queue numbers Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 18/24] net/ena: validate Tx req id Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 19/24] net/ena: add (un)likely statements Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 20/24] net/ena: adjust error checking and cleaning Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 21/24] net/ena: update numa node Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 22/24] net/ena: check pointer before memset Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 23/24] net/ena: change memory type Michal Krawczyk
2018-05-09 12:47 ` [PATCH v1 24/24] net/ena: fix GENMASK_ULL macro Michal Krawczyk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.