From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hemant Agrawal Subject: Re: [PATCH 1/2] mbuf: update default Mempool ops with HW active pool Date: Thu, 28 Dec 2017 17:37:58 +0530 Message-ID: <64ba944d-a31e-a42f-1d9e-619dbeef59fe@nxp.com> References: <1499170968-23016-1-git-send-email-hemant.agrawal@nxp.com> <1513333483-4372-1-git-send-email-hemant.agrawal@nxp.com> <1513333483-4372-2-git-send-email-hemant.agrawal@nxp.com> <20171218085507.GA20578@jerin> <85485fb0-f602-78af-c40a-7bfb4bda561e@nxp.com> <20171222145957.tc56hzyzbxj65rg5@platinum> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: Jerin Jacob , , To: Olivier MATZ Return-path: Received: from NAM01-BY2-obe.outbound.protection.outlook.com (mail-by2nam01on0064.outbound.protection.outlook.com [104.47.34.64]) by dpdk.org (Postfix) with ESMTP id 0EA6E1B53 for ; Thu, 28 Dec 2017 13:08:03 +0100 (CET) In-Reply-To: <20171222145957.tc56hzyzbxj65rg5@platinum> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Olivier, On 12/22/2017 8:29 PM, Olivier MATZ wrote: > On Mon, Dec 18, 2017 at 03:06:21PM +0530, Hemant Agrawal wrote: >> On 12/18/2017 2:25 PM, Jerin Jacob wrote: >>> -----Original Message----- >>>> Date: Fri, 15 Dec 2017 15:54:42 +0530 >>>> From: Hemant Agrawal >>>> To: olivier.matz@6wind.com, santosh.shukla@caviumnetworks.com >>>> CC: dev@dpdk.org >>>> Subject: [dpdk-dev] [PATCH 1/2] mbuf: update default Mempool ops with HW >>>> active pool >>>> X-Mailer: git-send-email 2.7.4 >>>> >>>> With this patch the specific HW mempool are no longer required to be >>>> specified in the config file at compile. A default active hw mempool >>>> can be detected dynamically and published to default mempools ops >>>> config at run time. Only one type of HW mempool can be active default. >>> >>> For me, it looks very reasonable approach as it caters the basic use >>> case without any change in the application nor the additional(--mbuf-pool-ops-name) >>> EAL command line scheme to select different mempool ops. >>> Though, this option will not enough cater all the use case. I think, we can have >>> three options and the following order of precedence to select the mempool ops >>> >>> 1) This patch(update active mempool based on the device probe()) >>> 2) Selection of mempool ops though --mbuf-pool-ops-name= EAL commandline argument. >>> Which can overridden the scheme(1) >>> 3) More sophisticated mempool section based on >>> a) The ethdev PMD capability exposed through existing rte_eth_dev_pool_ops_supported() >>> b) Add mempool ops option in rte_pktmbuf_pool_create() >>> http://dpdk.org/ml/archives/dev/2017-December/083985.html >>> c) Use (a) and (b) to select the update the mempool ops with >>> some "weight" based algorithm like >>> http://dpdk.org/dev/patchwork/patch/32245/ >>> >> >> Yes! We need more options to fine tune control over the mempool uses, >> specially when dealing with HW mempools. >> >> Once the above mentioned mechanisms will be in place, it will be much easier >> and flexible. > > I'm inline with this description. It would be great if the same binary can work > on different platforms without configuration. > > I just feel it's a bit messy to have: > > - rte_eal_mbuf_default_mempool_ops() in eal API > return user-selected ops if any, or compile-time default > > - rte_pktmbuf_active_mempool_ops() in mbuf API > return platform ops except if a selected user ops != compile default > > Thomas suggested somewhere (but I don't remember in which thread) to have > rte_eal_mbuf_default_mempool_ops() in mbuf code, and I think he was right. > The idea is good. It will break ABI, but we can move around in systematic way. > I think the whole mbuf pool ops selection mechanism should be at the > same place. I could be in a specific file of librte_mbuf. > > The API could be: > - get compile time default ops We can get them from "RTE_MBUF_DEFAULT_MEMPOOL_OPS" or " const char *rte_get_mbuf_config_mempool_ops(void) > - get/set platform ops (NULL if none) const char *rte_get_mbuf_platform_mempool_ops(void); int rte_set_mbuf_platform_mempool_ops(const char *ops_name); > - get/set user ops (NULL if none) internal_config will only store the command line argument or NULL. rename : rte_eal_mbuf_default_mempool_ops(void) to const char *rte_get_mbuf_user_mempool_ops(void) > - get preferred ops from currently configured PMD > The following proposal is updating the mempool_ops name in internal_config, I thing that is not the best solution. We shall not update the internal config. eal: add API to set default mbuf mempool ops http://dpdk.org/ml/archives/dev/2017-December/083849.html rte_eth_dev_get_preferred_pool_name() > - get best ops: return user, or pmd-prefered, or platform, or default. > pktmbuf_pool_create is currently calling, *rte_eal_mbuf_default_mempool_ops* This should be changed to call *rte_get_mbuf_best_mempool_ops* in the following order 1. rte_get_mbuf_user_mempool_ops 2. rte_eth_dev_get_preferred_pool_name (whenever this API comes) 3. rte_get_mbuf_platform_mempool_ops 4. rte_get_mbuf_config_mempool_ops > rte_pktmbuf_pool_create() will use "get best ops" if no ops (NULL) is > passed as argument. > However, we shall still provide a additional API, *rte_pktmbuf_pool_create_specific* if user still wants fine control or don't want to use the default best logic.