From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Yangchao Zhou <zhouyates@gmail.com>, dev@dpdk.org
Cc: stable@dpdk.org, thomas@monjalon.net
Subject: Re: [dpdk-dev] [PATCH v3] kni: fix possible rx_q mbuf leaks and speed up alloc_q release
Date: Tue, 17 Apr 2018 20:11:41 +0100 [thread overview]
Message-ID: <9a57896f-a2db-7f4b-f5c4-cf04970f1636@intel.com> (raw)
In-Reply-To: <1523937700-13716-1-git-send-email-zhouyates@gmail.com>
On 4/17/2018 5:01 AM, Yangchao Zhou wrote:
> rx_q fifo can only be released by kernel thread. There may be
> mbuf leaks in rx_q because kernel threads are randomly stopped.
>
> When the kni is released and netdev is unregisterd, convert the
> physical address mbufs in rx_q to the virtual address in free_q.
> By the way, alloc_q can be processed together to speed up the
> release rate in userspace.
Overall looks good to me. A few comments below.
Do you observe a speed up in userspace related alloc_q free?
>
> Signed-off-by: Yangchao Zhou <zhouyates@gmail.com>
> Suggested-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> kernel/linux/kni/kni_dev.h | 1 +
> kernel/linux/kni/kni_misc.c | 2 ++
> kernel/linux/kni/kni_net.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h
> index c9393d8..6275ef2 100644
> --- a/kernel/linux/kni/kni_dev.h
> +++ b/kernel/linux/kni/kni_dev.h
> @@ -92,6 +92,7 @@ struct kni_dev {
> void *alloc_va[MBUF_BURST_SZ];
> };
>
> +void kni_net_release_fifo_phy(struct kni_dev *kni);
> void kni_net_rx(struct kni_dev *kni);
> void kni_net_init(struct net_device *dev);
> void kni_net_config_lo_mode(char *lo_str);
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index 01574ec..fa69f8e 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -192,6 +192,8 @@ struct kni_net {
> free_netdev(dev->net_dev);
> }
>
> + kni_net_release_fifo_phy(dev);
> +
> return 0;
> }
>
> diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
> index 9f9b798..1d64d78 100644
> --- a/kernel/linux/kni/kni_net.c
> +++ b/kernel/linux/kni/kni_net.c
> @@ -163,6 +163,46 @@
> return (ret == 0) ? req.result : ret;
> }
>
> +static void
> +kni_fifo_trans_pa2va(struct kni_dev *kni,
> + struct rte_kni_fifo *src_pa, struct rte_kni_fifo *dst_va)
> +{
> + uint32_t ret, i, num_fq, num_rx;
> + void *kva;
> + do {
> + num_fq = kni_fifo_free_count(kni->free_q);
Since this function made more generic with src, dst arguments, it is better to
use dst_va here instead of kni->free_q
> + if (num_fq == 0)
And this variable name can be updated from free queue to dst.
> + return;
> +
> + num_rx = min_t(uint32_t, num_fq, MBUF_BURST_SZ);
> +
> + num_rx = kni_fifo_get(src_pa, kni->pa, num_rx);
> + if (num_rx == 0)
> + return;
> +
> + for (i = 0; i < num_rx; i++) {
> + kva = pa2kva(kni->pa[i]);
> + kni->va[i] = pa2va(kni->pa[i], kva);
> + }
> +
> + ret = kni_fifo_put(dst_va, kni->va, num_rx);
> + if (ret != num_rx) {
> + /* Failing should not happen */
> + pr_err("Fail to enqueue entries into dst_va\n");
> + return;
> + }
> + } while (1);
> +}
> +
> +/* Try to release mbufs when kni release */
> +void kni_net_release_fifo_phy(struct kni_dev *kni)
> +{
> + /* release rx_q first, because it can't release in userspace */
> + kni_fifo_trans_pa2va(kni, kni->rx_q, kni->free_q);
> + /* release alloc_q for speeding up kni release in userspace */
> + kni_fifo_trans_pa2va(kni, kni->alloc_q, kni->free_q);
> +}
> +
> /*
> * Configuration changes (passed on by ifconfig)
> */
>
next prev parent reply other threads:[~2018-04-17 19:11 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-11 12:34 [dpdk-dev] [PATCH v2] kni: fix possible rx_q mbuf leak " Yangchao Zhou
2018-04-17 4:01 ` [dpdk-dev] [PATCH v3] kni: fix possible rx_q mbuf leaks " Yangchao Zhou
2018-04-17 19:11 ` Ferruh Yigit [this message]
2018-04-19 3:12 ` [dpdk-dev] [PATCH v4] " Yangchao Zhou
2018-05-04 12:46 ` Ferruh Yigit
2018-05-13 22:02 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
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=9a57896f-a2db-7f4b-f5c4-cf04970f1636@intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
--cc=zhouyates@gmail.com \
/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).