From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 108922BAA for ; Fri, 24 Feb 2017 10:58:53 +0100 (CET) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Feb 2017 01:58:51 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,200,1484035200"; d="scan'208";a="69101409" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.252.4.61]) ([10.252.4.61]) by fmsmga005.fm.intel.com with ESMTP; 24 Feb 2017 01:58:50 -0800 To: Shreyansh Jain References: <1487684578-28656-1-git-send-email-shreyansh.jain@nxp.com> <8958b9ca-0a7d-3df0-3b62-4b9c610d301c@intel.com> Cc: dev@dpdk.org, nhorman@tuxdriver.com, "hemant.agrawal@nxp.com" From: Ferruh Yigit Message-ID: <16fa9e1e-556e-a1b0-68ea-2feba58474d3@intel.com> Date: Fri, 24 Feb 2017 09:58:49 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCHv7 03/47] common/dpaa2: adding qbman driver 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: , X-List-Received-Date: Fri, 24 Feb 2017 09:58:54 -0000 On 2/22/2017 8:23 AM, Shreyansh Jain wrote: > (Modified the subject to: 'Re: [PATCHv7 03/47] common/dpaa2: adding > qbman driver' from 'Re: Hello Ferruh, Neil,') > > Hello Ferruh, > > On Tuesday 21 February 2017 08:09 PM, Ferruh Yigit wrote: >> On 2/21/2017 1:42 PM, Shreyansh Jain wrote: >>> Thanks for the suggestions about rte_* renaming in DPAA2 PMD. >>> I create a draft patch for a single symbol change. (applies over v7 >>> of DPAA2 PMD) >>> >>> Can you tell me if this is the direction you were suggesting? >>> >>> I see two issues in this approach which are somewhat problematic for >>> me to change all the symbols: >>> 1) We saw a drop of over 5% when I replaced only 3 symbols (one >>> of the most used ones, just for sampling). This also means that >>> when more of such symbols are replaced, it would bring further >>> drop. This was case when I used the Shared library approach. >>> (*) I am not well versed with gcc symbol aliasing to comment for >>> why such a drop would happen. But multiple test cycles confirm >>> this. >>> 2) I have to include a new header in almost all the source files for PMD/ >>> Pool/Bus etc. This is besides the STATIC_SYMBOL macros across the >>> code. Essentially, any internal repo patch cannot be directly >>> transposed to DPDK repo. Increased effort for each internal-> >>> external release >>> >>> Overall, I would like you to consider if this effort for changing names >>> for exposed symbols is really useful or not. >> >> As you showed below, this works for exporting proper APIs, but not sure >> if this change worth or not. > > Given such symbol aliasing is an impact on performance, probably there > is a need to discuss the strictness of rte_* appending for driver > symbols. > As for cost of maintaining such code base, it can be rationalized over > a period of time, but not performance. > >> >>> >>> There is another approach - that of not using a drivers/common library. >>> This again is problematic for us - NXP DPAA2 being a hardware, the lib >>> and state for Crypto and Net hardware is tied together - so, having >>> multiple instances of library breaks either of Crypto or Net PMD. >> >> Isn't is possible to keep folder structure same, but produce single library. >> Because these don't provide any API to the user application, perhaps not >> need to be library at all. >> >> Assuming that bus and pool won't be required without a driver existing, >> is it possible have a single driver library that contains others? >> >> For net driver, dependency is as following: >> bus_fslmc --> common_dpaa2_qbman >> pool_dpaa2 --> bus_fslmc, common_dpaa2_qbman >> pmd_dpaa2 --> pool_dpaa2, bus_fslmc, common_dpaa2_qbman >> >> So generating only "librte_pmd_dpaa2" which include above dependent ones. >> >> For cryptodev pmd, I assume it has dependency to same modules: >> pmd_crypto --> pool_dpaa2, bus_fslmc, common_dpaa2_qbman >> >> And this will generate only crypto pmd library, including all again. >> >> >> This will create duplication in binaries, but I think easier to manage >> library dependencies. >> >> >> And for above case, as far as I know, both net and crypto libraries can >> be linked against a binary even there are duplicate symbols. Are you >> getting error here? >> > > > Thanks for your comments. > > The key issue here is that driver/common is not actually a 'library' in > traditional sense. It is a driver support system. It provides > interfaces to interact with the hardware - and that includes the Net > and Crypto hardware. > > Being a 'driver', this also has its own state. For example, a mem area > to interact with hardware queues, whether net or crypto - there is a > single instance of it. > > This restricts its duplication as a library. > In fact, as of now the statefulness is quite limited, but once more > devices (like for eventdev) come into picture, this would become more > prominent. > > Now, we have these possibility: > 1. Have a shared library with non rte_* symbols > 2. We have shared library with rte_* symbols > 3. We have non-net devices (crypto, eventdev, ..) depend on net for > these hardware interfaces > > (2) is hitting performance significantly. > (3) it not a clean solution, having driver/crypto depend on driver/net. > When new devices are there, more dependencies will occur. > > In crux, probably we need to have a discussion on (1) and how strongly > we feel about that (specially in context of drivers). Insight of above information, I would be OK with (1). We can go with option (1) now, since these are not real APIs to user application, it can be possible to change them if better solution found. Do you think is it good idea to have different naming syntax for those libraries to clarify they are for PMD internal usage? > > - > Shreyansh >