From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1275AA051C; Tue, 11 Feb 2020 13:18:19 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 52AE02A66; Tue, 11 Feb 2020 13:18:17 +0100 (CET) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 96B0CF72; Tue, 11 Feb 2020 13:18:15 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Feb 2020 04:18:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,428,1574150400"; d="scan'208";a="233433716" Received: from unknown (HELO [10.237.220.105]) ([10.237.220.105]) by orsmga003.jf.intel.com with ESMTP; 11 Feb 2020 04:18:12 -0800 To: Ferruh Yigit , Scott Wasson Cc: dev@dpdk.org, stable@dpdk.org, iryzhov@nfware.com References: <1581364020-4315-1-git-send-email-scott_wasson@affirmednetworks.com> <570fb7fd-6cac-ec87-66a1-d82b82fa62f6@intel.com> From: "Burakov, Anatoly" Message-ID: <7ef855f1-f032-fca0-c699-6d15ea481dbe@intel.com> Date: Tue, 11 Feb 2020 12:18:11 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.4.2 MIME-Version: 1.0 In-Reply-To: <570fb7fd-6cac-ec87-66a1-d82b82fa62f6@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] kni: fix bug 389 - Crash in librte_kni driver due to noncontiguous pages X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11-Feb-20 9:28 AM, Ferruh Yigit wrote: > On 2/10/2020 7:47 PM, Scott Wasson wrote: >> Fixes: edd2fafbc0b8 ("kni: allocate memory dynamically for each device") >> Cc: iryzhov@nfware.com >> Cc: stable@dpdk.org > > I suggest following patch subject: > "kni: fix not contiguous FIFO" > > And I little detail in the comment log can be good: > " > KNI requires FIFO to be physically contiguous, with existing > 'rte_memzone_reserve()' API this is not guaranteed by default and as a result > KNI rings and packet delivery is broken. > > Fixing it by providing 'RTE_MEMZONE_IOVA_CONTIG' flag to ask physically > contiguous memory. > " > > And the Fixes tag should show the commit that updates the > 'rte_memzone_reserve()' API to not provide physically contiguous memory. > @Anatoly, can you help finding that commit? The commit that added the ability to reserve contiguous memory was added before the memory subsystem itself started allocating non-contiguous memory, but all other drivers/libraries had added support for IOVA-contiguous memory as separate commits (see 74dbbcd6f8 for example). So, technically, the "offending commit" is the *omission* of KNI from these changes. But, the API itself was added in 23fa86e529e, so that's what we should use for Fixes: tag IMO. > > >> >> Signed-off-by: Scott Wasson >> --- >> lib/librte_kni/rte_kni.c | 14 +++++++------- >> 1 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c >> index e388751..df4a021 100644 >> --- a/lib/librte_kni/rte_kni.c >> +++ b/lib/librte_kni/rte_kni.c >> @@ -145,31 +145,31 @@ enum kni_ops_status { >> char mz_name[RTE_MEMZONE_NAMESIZE]; >> >> snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_TX_Q_MZ_NAME_FMT, kni->name); >> - kni->m_tx_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, 0); >> + kni->m_tx_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); > > Can you please break the line, so it doesn't go beyond 80 chars? > >> KNI_MEM_CHECK(kni->m_tx_q == NULL, tx_q_fail); >> >> snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_RX_Q_MZ_NAME_FMT, kni->name); >> - kni->m_rx_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, 0); >> + kni->m_rx_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); >> KNI_MEM_CHECK(kni->m_rx_q == NULL, rx_q_fail); >> >> snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_ALLOC_Q_MZ_NAME_FMT, kni->name); >> - kni->m_alloc_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, 0); >> + kni->m_alloc_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); >> KNI_MEM_CHECK(kni->m_alloc_q == NULL, alloc_q_fail); >> >> snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_FREE_Q_MZ_NAME_FMT, kni->name); >> - kni->m_free_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, 0); >> + kni->m_free_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); >> KNI_MEM_CHECK(kni->m_free_q == NULL, free_q_fail); >> >> snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_REQ_Q_MZ_NAME_FMT, kni->name); >> - kni->m_req_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, 0); >> + kni->m_req_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); >> KNI_MEM_CHECK(kni->m_req_q == NULL, req_q_fail); >> >> snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_RESP_Q_MZ_NAME_FMT, kni->name); >> - kni->m_resp_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, 0); >> + kni->m_resp_q = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); >> KNI_MEM_CHECK(kni->m_resp_q == NULL, resp_q_fail); >> >> snprintf(mz_name, RTE_MEMZONE_NAMESIZE, KNI_SYNC_ADDR_MZ_NAME_FMT, kni->name); >> - kni->m_sync_addr = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, 0); >> + kni->m_sync_addr = rte_memzone_reserve(mz_name, KNI_FIFO_SIZE, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG); >> KNI_MEM_CHECK(kni->m_sync_addr == NULL, sync_addr_fail); >> >> return 0; >> > -- Thanks, Anatoly