From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ferruh.yigit@intel.com>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
by dpdk.org (Postfix) with ESMTP id E6E1CDE5
for <dev@dpdk.org>; Wed, 25 Jan 2017 21:11:01 +0100 (CET)
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
by orsmga105.jf.intel.com with ESMTP; 25 Jan 2017 12:11:00 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.33,285,1477983600"; d="scan'208";a="1098900897"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38])
([10.237.220.38])
by fmsmga001.fm.intel.com with ESMTP; 25 Jan 2017 12:10:58 -0800
To: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>, olivier.matz@6wind.com
References: <1484801219-1312-1-git-send-email-s.vyazmitinov@brain4net.com>
<1484801219-1312-3-git-send-email-s.vyazmitinov@brain4net.com>
Cc: konstantin.ananyev@intel.com, stephen@networkplumber.org,
yuanhan.liu@linux.intel.com, dev@dpdk.org, mirqus@gmail.com
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <301e337b-1d1f-c3b2-3adf-af9beb0d6ad0@intel.com>
Date: Wed, 25 Jan 2017 20:10:58 +0000
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
Thunderbird/45.6.0
MIME-Version: 1.0
In-Reply-To: <1484801219-1312-3-git-send-email-s.vyazmitinov@brain4net.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCH v6 2/2] kni: Use bulk functions to allocate
and free mbufs
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 25 Jan 2017 20:11:02 -0000
On 1/19/2017 4:46 AM, Sergey Vyazmitinov wrote:
> Optimized kni_allocate_mbufs and kni_free_mbufs by using mbuf
> bulk functions. This can improve performance more than two times.
>
> Signed-off-by: Sergey Vyazmitinov <s.vyazmitinov@brain4net.com>
> ---
> v5:
> * use rte_pktmbuf_free_bulk for removing packets which was not
> put in alloc queue.
> v6:
> * fix c99 compilation error
> ---
> lib/librte_kni/rte_kni.c | 47 ++++++++++++++++++++-----------------------
> lib/librte_kni/rte_kni_fifo.h | 18 +++++++++++++++++
> 2 files changed, 40 insertions(+), 25 deletions(-)
>
> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c
> index a80cefd..6591f4f 100644
> --- a/lib/librte_kni/rte_kni.c
> +++ b/lib/librte_kni/rte_kni.c
> @@ -590,22 +590,21 @@ rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
> static void
> kni_free_mbufs(struct rte_kni *kni)
> {
> - int i, ret;
> + unsigned int freeing;
> struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM];
>
> - ret = kni_fifo_get(kni->free_q, (void **)pkts, MAX_MBUF_BURST_NUM);
> - if (likely(ret > 0)) {
> - for (i = 0; i < ret; i++)
> - rte_pktmbuf_free(pkts[i]);
> + freeing = kni_fifo_get(kni->free_q, (void **)pkts, MAX_MBUF_BURST_NUM);
> + if (likely(freeing > 0)) {
> + rte_pktmbuf_free_bulk(pkts, freeing);
> }
> }
>
> static void
> kni_allocate_mbufs(struct rte_kni *kni)
> {
> - int i, ret;
> - struct rte_mbuf *pkts[MAX_MBUF_BURST_NUM];
> - void *phys[MAX_MBUF_BURST_NUM];
> + unsigned int count, put, i;
> + struct rte_mbuf *pkts[KNI_FIFO_COUNT_MAX];
> + void *phys[KNI_FIFO_COUNT_MAX];
>
> RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, pool) !=
> offsetof(struct rte_kni_mbuf, pool));
> @@ -628,28 +627,26 @@ kni_allocate_mbufs(struct rte_kni *kni)
> return;
> }
>
> - for (i = 0; i < MAX_MBUF_BURST_NUM; i++) {
> - pkts[i] = rte_pktmbuf_alloc(kni->pktmbuf_pool);
> - if (unlikely(pkts[i] == NULL)) {
> - /* Out of memory */
> - RTE_LOG(ERR, KNI, "Out of memory\n");
> - break;
> - }
> - phys[i] = va2pa(pkts[i]);
> - }
> + /* Calculate alloc queue free space */
> + count = kni_fifo_free_count(kni->alloc_q);
This seems reason of the performance loss mentioned in cover letter.
Updating this line as following recovers the performance:
count = MAX_MBUF_BURST_NUM;
kni->alloc_q is fifo shared between kernel thread, which runs on its own
core, and the rx poll core.
Getting free count and allocating that much mbuf causing worse
performance, comparing to the original behavior.
Btw, I have tried something else, which is giving conflicting results
(it was better when I first tried, now getting worse result):
kni_allocate_mbufs() called every time received some packets from
kernel, because this means kernel thread consumed some mbufs from
alloc_q, and userspace needs to fill it back again.
So, instead of allocating fixed number of mbuf, or allocating free
number of fifo mbuf, what about allocating amount of mbufs that received
by kernel.
Since kernel consumed that much, fill back that much.
make this function:
kni_allocate_mbufs(struct rte_kni *kni, unsigned int num);
rte_kni_rx_burst() calls:
kni_allocate_mbufs(kni, ret);
What do you think?
If possible can you please try this?
Thanks,
ferruh
>
> - /* No pkt mbuf alocated */
> - if (i <= 0)
> + /* Get buffers from mempool */
> + if (rte_pktmbuf_alloc_bulk(kni->pktmbuf_pool, pkts, count) != 0) {
> + RTE_LOG(ERR, KNI, "Can`t allocate %d mbufs\n", count);
> return;
> + }
> +
> + for (i = 0; i < count; i++)
> + phys[i] = va2pa(pkts[i]);
>
> - ret = kni_fifo_put(kni->alloc_q, phys, i);
> + /* Put buffers into alloc queue */
> + put = kni_fifo_put(kni->alloc_q, (void **)phys, count);
>
> /* Check if any mbufs not put into alloc_q, and then free them */
> - if (ret >= 0 && ret < i && ret < MAX_MBUF_BURST_NUM) {
> - int j;
> -
> - for (j = ret; j < i; j++)
> - rte_pktmbuf_free(pkts[j]);
> + if (unlikely(put < count)) {
> + RTE_LOG(ERR, KNI, "Free %u of %u allocated buffers\n",
> + count - put, count);
> + rte_pktmbuf_free_bulk(pkts + put, count - put);
> }
> }
>
> diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
> index 8cb8587..361ddb0 100644
> --- a/lib/librte_kni/rte_kni_fifo.h
> +++ b/lib/librte_kni/rte_kni_fifo.h
> @@ -91,3 +91,21 @@ 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 unsigned
> +kni_fifo_count(struct rte_kni_fifo *fifo)
> +{
> + return (fifo->len + fifo->write - fifo->read) & (fifo->len - 1);
> +}
> +
> +/**
> + * Get the num of available elements in the fifo
> + */
> +static inline unsigned
> +kni_fifo_free_count(struct rte_kni_fifo *fifo)
> +{
> + return (fifo->read - fifo->write - 1) & (fifo->len - 1);
> +}
>