net/ena: fix releasing Tx ring mbufs

Message ID 20210406002719.2556501-1-dharton@cisco.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series net/ena: fix releasing Tx ring mbufs |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success travis build: passed
ci/github-robot success github build: passed
ci/iol-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

David Harton April 6, 2021, 12:27 a.m. UTC
  When ena_tx_queue_release_bufs() frees the mbufs it does not clear
the mbuf pointers.  So, when the device starts and stops multiple
times it can cause the application to receive duplicate mbufs for
two different packets.  Fix the issue by clearing the mbuf pointer.

Also, while tracking down the "double free" issue the ena calls to
allocate and free mbufs in bulk were migrated to the mbuf based APIs
so the common mbuf alloc/free routines are exercised.

Fixes: 79405ee17585 ("net/ena: fix out of order completion")
Fixes: 1173fca25af9 ("ena: add polling-mode driver")

Signed-off-by: David Harton <dharton@cisco.com>
---
 drivers/net/ena/ena_ethdev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
  

Comments

Michal Krawczyk April 6, 2021, 5:25 a.m. UTC | #1
wt., 6 kwi 2021 o 02:27 David Harton <dharton@cisco.com> napisał(a):
>
> When ena_tx_queue_release_bufs() frees the mbufs it does not clear
> the mbuf pointers.  So, when the device starts and stops multiple
> times it can cause the application to receive duplicate mbufs for
> two different packets.  Fix the issue by clearing the mbuf pointer.
>
> Also, while tracking down the "double free" issue the ena calls to
> allocate and free mbufs in bulk were migrated to the mbuf based APIs
> so the common mbuf alloc/free routines are exercised.
>
> Fixes: 79405ee17585 ("net/ena: fix out of order completion")
> Fixes: 1173fca25af9 ("ena: add polling-mode driver")
>
> Signed-off-by: David Harton <dharton@cisco.com>
Acked-by: Michal Krawczyk <mk@semihalf.com>
> ---
>  drivers/net/ena/ena_ethdev.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 9aa51c9dc..f60e843b7 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -767,8 +767,10 @@ static void ena_tx_queue_release_bufs(struct ena_ring *ring)
>         for (i = 0; i < ring->ring_size; ++i) {
>                 struct ena_tx_buffer *tx_buf = &ring->tx_buffer_info[i];
>
> -               if (tx_buf->mbuf)
> +               if (tx_buf->mbuf) {
>                         rte_pktmbuf_free(tx_buf->mbuf);
> +                       tx_buf->mbuf = NULL;
> +               }
>         }
>  }
>
> @@ -1457,7 +1459,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
>                 "bad ring state\n");
>
>         /* get resources for incoming packets */
> -       rc = rte_mempool_get_bulk(rxq->mb_pool, (void **)mbufs, count);
> +       rc = rte_pktmbuf_alloc_bulk(rxq->mb_pool, mbufs, count);
>         if (unlikely(rc < 0)) {
>                 rte_atomic64_inc(&rxq->adapter->drv_stats->rx_nombuf);
>                 ++rxq->rx_stats.mbuf_alloc_fail;
> @@ -1486,8 +1488,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
>         if (unlikely(i < count)) {
>                 PMD_DRV_LOG(WARNING, "refilled rx qid %d with only %d "
>                         "buffers (from %d)\n", rxq->id, i, count);
> -               rte_mempool_put_bulk(rxq->mb_pool, (void **)(&mbufs[i]),
> -                                    count - i);
> +               rte_pktmbuf_free_bulk(&mbufs[i], count - i);
>                 ++rxq->rx_stats.refill_partial;
>         }
>
> --
> 2.26.2.Cisco
>
  
Ferruh Yigit April 7, 2021, 8:15 a.m. UTC | #2
On 4/6/2021 6:25 AM, Michał Krawczyk wrote:
> wt., 6 kwi 2021 o 02:27 David Harton <dharton@cisco.com> napisał(a):
>>
>> When ena_tx_queue_release_bufs() frees the mbufs it does not clear
>> the mbuf pointers.  So, when the device starts and stops multiple
>> times it can cause the application to receive duplicate mbufs for
>> two different packets.  Fix the issue by clearing the mbuf pointer.
>>
>> Also, while tracking down the "double free" issue the ena calls to
>> allocate and free mbufs in bulk were migrated to the mbuf based APIs
>> so the common mbuf alloc/free routines are exercised.
>>
>> Fixes: 79405ee17585 ("net/ena: fix out of order completion")
>> Fixes: 1173fca25af9 ("ena: add polling-mode driver")
>>
>> Signed-off-by: David Harton <dharton@cisco.com>
> Acked-by: Michal Krawczyk <mk@semihalf.com>

Applied to dpdk-next-net/main, thanks.
  

Patch

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 9aa51c9dc..f60e843b7 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -767,8 +767,10 @@  static void ena_tx_queue_release_bufs(struct ena_ring *ring)
 	for (i = 0; i < ring->ring_size; ++i) {
 		struct ena_tx_buffer *tx_buf = &ring->tx_buffer_info[i];
 
-		if (tx_buf->mbuf)
+		if (tx_buf->mbuf) {
 			rte_pktmbuf_free(tx_buf->mbuf);
+			tx_buf->mbuf = NULL;
+		}
 	}
 }
 
@@ -1457,7 +1459,7 @@  static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		"bad ring state\n");
 
 	/* get resources for incoming packets */
-	rc = rte_mempool_get_bulk(rxq->mb_pool, (void **)mbufs, count);
+	rc = rte_pktmbuf_alloc_bulk(rxq->mb_pool, mbufs, count);
 	if (unlikely(rc < 0)) {
 		rte_atomic64_inc(&rxq->adapter->drv_stats->rx_nombuf);
 		++rxq->rx_stats.mbuf_alloc_fail;
@@ -1486,8 +1488,7 @@  static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 	if (unlikely(i < count)) {
 		PMD_DRV_LOG(WARNING, "refilled rx qid %d with only %d "
 			"buffers (from %d)\n", rxq->id, i, count);
-		rte_mempool_put_bulk(rxq->mb_pool, (void **)(&mbufs[i]),
-				     count - i);
+		rte_pktmbuf_free_bulk(&mbufs[i], count - i);
 		++rxq->rx_stats.refill_partial;
 	}