From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 3433C1B494 for ; Thu, 22 Nov 2018 17:53:19 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 968F21E321; Thu, 22 Nov 2018 16:53:18 +0000 (UTC) Received: from ktraynor.remote.csb (unknown [10.36.118.7]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A18560E3F; Thu, 22 Nov 2018 16:53:16 +0000 (UTC) From: Kevin Traynor To: Michal Krawczyk Cc: dpdk stable Date: Thu, 22 Nov 2018 16:49:49 +0000 Message-Id: <20181122164957.13003-57-ktraynor@redhat.com> In-Reply-To: <20181122164957.13003-1-ktraynor@redhat.com> References: <20181122164957.13003-1-ktraynor@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 22 Nov 2018 16:53:18 +0000 (UTC) Subject: [dpdk-stable] patch 'net/ena: recreate HW IO rings on start and stop' has been queued to stable release 18.08.1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Nov 2018 16:53:19 -0000 Hi, FYI, your patch has been queued to stable release 18.08.1 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 11/28/18. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. If the code is different (ie: not only metadata diffs), due for example to a change in context or macro names, please double check it. Thanks. Kevin Traynor --- >>From a867758113a2a3f60e58da58b650ae9c5636f0bf Mon Sep 17 00:00:00 2001 From: Michal Krawczyk Date: Thu, 25 Oct 2018 19:59:21 +0200 Subject: [PATCH] net/ena: recreate HW IO rings on start and stop [ upstream commit df238f84c0a21d642bb9517d0c75ba831eeceb46 ] On the start the driver was refilling all Rx buffs, but the old ones were not released. That way running start/stop for a few times was causing device to run out of descriptors. To fix the issue, IO rings are now being destroyed on stop, and recreated on start. That way the device is not losing any descriptors. Furthermore, there was also memory leak for the Rx mbufs, which were created on start and not destroyed on stop. Fixes: eb0ef49dd5d5 ("net/ena: add stop and uninit routines") Signed-off-by: Michal Krawczyk --- drivers/net/ena/ena_ethdev.c | 196 ++++++++++++++++------------------- 1 file changed, 91 insertions(+), 105 deletions(-) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index c255dc6db..de5d2edc2 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -240,4 +240,6 @@ static void ena_tx_queue_release_bufs(struct ena_ring *ring); static int ena_link_update(struct rte_eth_dev *dev, int wait_to_complete); +static int ena_create_io_queue(struct ena_ring *ring); +static void ena_free_io_queues_all(struct ena_adapter *adapter); static int ena_queue_restart(struct ena_ring *ring); static int ena_queue_restart_all(struct rte_eth_dev *dev, @@ -511,5 +513,6 @@ static void ena_close(struct rte_eth_dev *dev) (struct ena_adapter *)(dev->data->dev_private); - ena_stop(dev); + if (adapter->state == ENA_ADAPTER_STATE_RUNNING) + ena_stop(dev); adapter->state = ENA_ADAPTER_STATE_CLOSED; @@ -747,6 +750,4 @@ static void ena_rx_queue_release(void *queue) { struct ena_ring *ring = (struct ena_ring *)queue; - struct ena_adapter *adapter = ring->adapter; - int ena_qid; ena_assert_msg(ring->configured, @@ -755,11 +756,4 @@ static void ena_rx_queue_release(void *queue) "API violation"); - /* Destroy HW queue */ - ena_qid = ENA_IO_RXQ_IDX(ring->id); - ena_com_destroy_io_queue(&adapter->ena_dev, ena_qid); - - /* Free all bufs */ - ena_rx_queue_release_bufs(ring); - /* Free ring resources */ if (ring->rx_buffer_info) @@ -780,6 +774,4 @@ static void ena_tx_queue_release(void *queue) { struct ena_ring *ring = (struct ena_ring *)queue; - struct ena_adapter *adapter = ring->adapter; - int ena_qid; ena_assert_msg(ring->configured, @@ -788,8 +780,4 @@ static void ena_tx_queue_release(void *queue) "API violation"); - /* Destroy HW queue */ - ena_qid = ENA_IO_TXQ_IDX(ring->id); - ena_com_destroy_io_queue(&adapter->ena_dev, ena_qid); - /* Free all bufs */ ena_tx_queue_release_bufs(ring); @@ -1079,8 +1067,84 @@ static void ena_stop(struct rte_eth_dev *dev) rte_timer_stop_sync(&adapter->timer_wd); + ena_free_io_queues_all(adapter); adapter->state = ENA_ADAPTER_STATE_STOPPED; } +static int ena_create_io_queue(struct ena_ring *ring) +{ + struct ena_adapter *adapter; + struct ena_com_dev *ena_dev; + struct ena_com_create_io_ctx ctx = + /* policy set to _HOST just to satisfy icc compiler */ + { ENA_ADMIN_PLACEMENT_POLICY_HOST, + 0, 0, 0, 0, 0 }; + uint16_t ena_qid; + int rc; + + adapter = ring->adapter; + ena_dev = &adapter->ena_dev; + + if (ring->type == ENA_RING_TYPE_TX) { + ena_qid = ENA_IO_TXQ_IDX(ring->id); + ctx.direction = ENA_COM_IO_QUEUE_DIRECTION_TX; + ctx.mem_queue_type = ena_dev->tx_mem_queue_type; + ctx.queue_size = adapter->tx_ring_size; + } else { + ena_qid = ENA_IO_RXQ_IDX(ring->id); + ctx.direction = ENA_COM_IO_QUEUE_DIRECTION_RX; + ctx.queue_size = adapter->rx_ring_size; + } + ctx.qid = ena_qid; + ctx.msix_vector = -1; /* interrupts not used */ + ctx.numa_node = ena_cpu_to_node(ring->id); + + rc = ena_com_create_io_queue(ena_dev, &ctx); + if (rc) { + RTE_LOG(ERR, PMD, + "failed to create io queue #%d (qid:%d) rc: %d\n", + ring->id, ena_qid, rc); + return rc; + } + + rc = ena_com_get_io_handlers(ena_dev, ena_qid, + &ring->ena_com_io_sq, + &ring->ena_com_io_cq); + if (rc) { + RTE_LOG(ERR, PMD, + "Failed to get io queue handlers. queue num %d rc: %d\n", + ring->id, rc); + ena_com_destroy_io_queue(ena_dev, ena_qid); + return rc; + } + + if (ring->type == ENA_RING_TYPE_TX) + ena_com_update_numa_node(ring->ena_com_io_cq, ctx.numa_node); + + return 0; +} + +static void ena_free_io_queues_all(struct ena_adapter *adapter) +{ + struct rte_eth_dev *eth_dev = adapter->rte_dev; + struct ena_com_dev *ena_dev = &adapter->ena_dev; + int i; + uint16_t ena_qid; + uint16_t nb_rxq = eth_dev->data->nb_rx_queues; + uint16_t nb_txq = eth_dev->data->nb_tx_queues; + + for (i = 0; i < nb_txq; ++i) { + ena_qid = ENA_IO_TXQ_IDX(i); + ena_com_destroy_io_queue(ena_dev, ena_qid); + } + + for (i = 0; i < nb_rxq; ++i) { + ena_qid = ENA_IO_RXQ_IDX(i); + ena_com_destroy_io_queue(ena_dev, ena_qid); + + ena_rx_queue_release_bufs(&adapter->rx_ring[i]); + } +} + static int ena_queue_restart(struct ena_ring *ring) { @@ -1090,4 +1154,10 @@ static int ena_queue_restart(struct ena_ring *ring) "Trying to restart unconfigured queue\n"); + rc = ena_create_io_queue(ring); + if (rc) { + PMD_INIT_LOG(ERR, "Failed to create IO queue!\n"); + return rc; + } + ring->next_to_clean = 0; ring->next_to_use = 0; @@ -1112,15 +1182,8 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, const struct rte_eth_txconf *tx_conf) { - struct ena_com_create_io_ctx ctx = - /* policy set to _HOST just to satisfy icc compiler */ - { ENA_ADMIN_PLACEMENT_POLICY_HOST, - ENA_COM_IO_QUEUE_DIRECTION_TX, 0, 0, 0, 0 }; struct ena_ring *txq = NULL; struct ena_adapter *adapter = (struct ena_adapter *)(dev->data->dev_private); unsigned int i; - int ena_qid; - int rc; - struct ena_com_dev *ena_dev = &adapter->ena_dev; txq = &adapter->tx_ring[queue_idx]; @@ -1147,35 +1210,4 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, } - ena_qid = ENA_IO_TXQ_IDX(queue_idx); - - ctx.direction = ENA_COM_IO_QUEUE_DIRECTION_TX; - ctx.qid = ena_qid; - ctx.msix_vector = -1; /* admin interrupts not used */ - ctx.mem_queue_type = ena_dev->tx_mem_queue_type; - ctx.queue_size = adapter->tx_ring_size; - ctx.numa_node = ena_cpu_to_node(queue_idx); - - rc = ena_com_create_io_queue(ena_dev, &ctx); - if (rc) { - 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]; - - rc = ena_com_get_io_handlers(ena_dev, ena_qid, - &txq->ena_com_io_sq, - &txq->ena_com_io_cq); - if (rc) { - RTE_LOG(ERR, PMD, - "Failed to get TX queue handlers. TX queue num %d rc: %d\n", - queue_idx, rc); - 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; @@ -1189,6 +1221,5 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, if (!txq->tx_buffer_info) { RTE_LOG(ERR, PMD, "failed to alloc mem for tx buffer info\n"); - rc = -ENOMEM; - goto err_destroy_io_queue; + return -ENOMEM; } @@ -1198,6 +1229,6 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, if (!txq->empty_tx_reqs) { RTE_LOG(ERR, PMD, "failed to alloc mem for tx reqs\n"); - rc = -ENOMEM; - goto err_free; + rte_free(txq->tx_buffer_info); + return -ENOMEM; } @@ -1215,11 +1246,4 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, 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; } @@ -1231,14 +1255,8 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, struct rte_mempool *mp) { - struct ena_com_create_io_ctx ctx = - /* policy set to _HOST just to satisfy icc compiler */ - { ENA_ADMIN_PLACEMENT_POLICY_HOST, - ENA_COM_IO_QUEUE_DIRECTION_RX, 0, 0, 0, 0 }; struct ena_adapter *adapter = (struct ena_adapter *)(dev->data->dev_private); struct ena_ring *rxq = NULL; - uint16_t ena_qid = 0; - int i, rc = 0; - struct ena_com_dev *ena_dev = &adapter->ena_dev; + int i; rxq = &adapter->rx_ring[queue_idx]; @@ -1264,34 +1282,4 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, } - ena_qid = ENA_IO_RXQ_IDX(queue_idx); - - ctx.qid = ena_qid; - ctx.direction = ENA_COM_IO_QUEUE_DIRECTION_RX; - ctx.mem_queue_type = ENA_ADMIN_PLACEMENT_POLICY_HOST; - ctx.msix_vector = -1; /* admin interrupts not used */ - ctx.queue_size = adapter->rx_ring_size; - ctx.numa_node = ena_cpu_to_node(queue_idx); - - rc = ena_com_create_io_queue(ena_dev, &ctx); - 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]; - - rc = ena_com_get_io_handlers(ena_dev, ena_qid, - &rxq->ena_com_io_sq, - &rxq->ena_com_io_cq); - if (rc) { - RTE_LOG(ERR, PMD, - "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; rxq->next_to_clean = 0; @@ -1305,5 +1293,4 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, 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; } @@ -1316,5 +1303,4 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, rte_free(rxq->rx_buffer_info); rxq->rx_buffer_info = NULL; - ena_com_destroy_io_queue(ena_dev, ena_qid); return -ENOMEM; } @@ -1327,5 +1313,5 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev, dev->data->rx_queues[queue_idx] = rxq; - return rc; + return 0; } -- 2.19.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2018-11-22 16:47:33.767016466 +0000 +++ 0057-net-ena-recreate-HW-IO-rings-on-start-and-stop.patch 2018-11-22 16:47:32.000000000 +0000 @@ -1,8 +1,10 @@ -From df238f84c0a21d642bb9517d0c75ba831eeceb46 Mon Sep 17 00:00:00 2001 +From a867758113a2a3f60e58da58b650ae9c5636f0bf Mon Sep 17 00:00:00 2001 From: Michal Krawczyk Date: Thu, 25 Oct 2018 19:59:21 +0200 Subject: [PATCH] net/ena: recreate HW IO rings on start and stop +[ upstream commit df238f84c0a21d642bb9517d0c75ba831eeceb46 ] + On the start the driver was refilling all Rx buffs, but the old ones were not released. That way running start/stop for a few times was causing device to run out of descriptors. @@ -14,7 +16,6 @@ created on start and not destroyed on stop. Fixes: eb0ef49dd5d5 ("net/ena: add stop and uninit routines") -Cc: stable@dpdk.org Signed-off-by: Michal Krawczyk --- @@ -22,7 +23,7 @@ 1 file changed, 91 insertions(+), 105 deletions(-) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c -index c29a581e8..186ab0e6b 100644 +index c255dc6db..de5d2edc2 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -240,4 +240,6 @@ static void ena_tx_queue_release_bufs(struct ena_ring *ring);