patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>, dpdk stable <stable@dpdk.org>
Subject: [dpdk-stable] patch 'kni: fix possible memory leak' has been queued to LTS release 16.11.2
Date: Fri, 21 Apr 2017 14:19:44 +0800	[thread overview]
Message-ID: <1492755587-28967-19-git-send-email-yuanhan.liu@linux.intel.com> (raw)
In-Reply-To: <1492755587-28967-1-git-send-email-yuanhan.liu@linux.intel.com>

Hi,

FYI, your patch has been queued to LTS release 16.11.2

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable
yet. It will be pushed if I get no objections before 04/26/17.
So please shout if anyone has objections.

Thanks.

	--yliu

---
>From 9875d5f69835a951d17c1b9166880efaca7062d1 Mon Sep 17 00:00:00 2001
From: Ferruh Yigit <ferruh.yigit@intel.com>
Date: Tue, 18 Apr 2017 15:21:44 +0100
Subject: [PATCH] kni: fix possible memory leak

[ upstream commit 8eba5ebd181141f71115d01bf65fd2e9ad549210 ]

alloc_q and rx_q fifos holds physical address of the mbufs, and not able
to free those mbufs explicitly.

But kernel thread reads from rx_q and puts used mbufs into free_q (with
their virtual addresses.) And kernel thread stopped when application
close the /dev/kni file on exit. So rx_q has time to be consumed by
kernel thread but leak is technically possible.

Another fifo, alloc_q has physical addresses too, but all those coming
from same mempool provided by application, when application quit, all
mempool already returned back, so this leak can be ignored.

Added check and wait logic for rx_q to be sure kernel consumed the fifo,
an error message printed after some ammount of wait, and an explicit
mempool free added for alloc_q.

Fixes: 8451269e6d7b ("kni: remove continuous memory restriction")

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 lib/librte_kni/rte_kni.c      | 20 ++++++++++++++------
 lib/librte_kni/rte_kni_fifo.h |  9 +++++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
index a80cefd..52fcd4b 100644
--- a/lib/librte_kni/rte_kni.c
+++ b/lib/librte_kni/rte_kni.c
@@ -452,16 +452,16 @@ kni_free_fifo(struct rte_kni_fifo *fifo)
 }
 
 static void
-kni_free_fifo_phy(struct rte_kni_fifo *fifo)
+kni_free_fifo_phy(struct rte_mempool *pktmbuf_pool, struct rte_kni_fifo *fifo)
 {
 	void *mbuf_phys;
 	int ret;
 
+	rte_mempool_free(pktmbuf_pool);
+
+	/* All mbufs alredy freed with rte_mempoll_free, just free the fifo */
 	do {
 		ret = kni_fifo_get(fifo, &mbuf_phys, 1);
-		/*
-		 * TODO: free mbufs
-		 */
 	} while (ret);
 }
 
@@ -470,6 +470,7 @@ rte_kni_release(struct rte_kni *kni)
 {
 	struct rte_kni_device_info dev_info;
 	uint32_t slot_id;
+	uint32_t retry = 5;
 
 	if (!kni || !kni->in_use)
 		return -1;
@@ -481,9 +482,16 @@ rte_kni_release(struct rte_kni *kni)
 	}
 
 	/* mbufs in all fifo should be released, except request/response */
+
+	/* wait until all rxq packets processed by kernel */
+	while (kni_fifo_count(kni->rx_q) && retry--)
+		usleep(1000);
+
+	if (kni_fifo_count(kni->rx_q))
+		RTE_LOG(ERR, KNI, "Fail to free all Rx-q items\n");
+
+	kni_free_fifo_phy(kni->pktmbuf_pool, kni->alloc_q);
 	kni_free_fifo(kni->tx_q);
-	kni_free_fifo_phy(kni->rx_q);
-	kni_free_fifo_phy(kni->alloc_q);
 	kni_free_fifo(kni->free_q);
 
 	slot_id = kni->slot_id;
diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
index 8cb8587..c7cd5c2 100644
--- a/lib/librte_kni/rte_kni_fifo.h
+++ b/lib/librte_kni/rte_kni_fifo.h
@@ -91,3 +91,12 @@ kni_fifo_get(struct rte_kni_fifo *fifo, void **data, unsigned num)
 	fifo->read = new_read;
 	return i;
 }
+
+/**
+ * Get the num of elements in the fifo
+ */
+static inline uint32_t
+kni_fifo_count(struct rte_kni_fifo *fifo)
+{
+	return (fifo->len + fifo->write - fifo->read) & (fifo->len - 1);
+}
-- 
1.9.0

  parent reply	other threads:[~2017-04-21  6:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-21  6:19 [dpdk-stable] patch 'mk: fix quoting for ARM mtune argument' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/cxgbe: fix possible null pointer dereference' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/i40e: fix allocation check' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/i40e: fix VF link speed' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/i40e: add missing 25G " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/i40e: fix hash input set on X722' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/thunderx: fix stats access out of bounds' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/ena: cleanup if refilling of Rx descriptors fails' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/bonding: allow configuring jumbo frames without slaves' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/mlx4: fix Rx after mbuf alloc failure' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/i40e: ensure vector mode is not used with QinQ' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'vhost: fix use after free' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/virtio-user: fix address on 32-bit system' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'vhost: fix dequeue zero copy' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'crypto/qat: fix AES-GCM authentication length' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'crypto/qat: fix IV zero physical address' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'examples/l2fwd-crypto: fix AEAD tests when AAD is zero' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'examples/l2fwd-crypto: fix padding calculation' " Yuanhan Liu
2017-04-21  6:19 ` Yuanhan Liu [this message]
2017-04-21  6:19 ` [dpdk-stable] patch 'test/mempool: free mempool on exit' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/virtio: fix queue notify' " Yuanhan Liu
2017-04-21  6:19 ` [dpdk-stable] patch 'net/virtio: fix link status always being up' " Yuanhan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1492755587-28967-19-git-send-email-yuanhan.liu@linux.intel.com \
    --to=yuanhan.liu@linux.intel.com \
    --cc=ferruh.yigit@intel.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).