From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier Matz Subject: Re: [PATCH 0/2] Allow application set mempool handle Date: Tue, 4 Jul 2017 17:59:22 +0200 Message-ID: <20170704175922.02b4d916@platinum> References: <20170601080559.10684-1-santosh.shukla@caviumnetworks.com> <20170619130152.GA29671@jerin> <2e78b067-0a4d-a40d-799e-6137972bd7a9@nxp.com> <20170620140413.GA16157@jerin> <20170630161246.2af345e0@platinum> <16812056-cfa9-066a-6323-aa0ac7dbd9cb@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Jerin Jacob , Hemant Agrawal , dev@dpdk.org To: santosh Return-path: Received: from mail-wr0-f178.google.com (mail-wr0-f178.google.com [209.85.128.178]) by dpdk.org (Postfix) with ESMTP id 9A8F4374C for ; Tue, 4 Jul 2017 17:59:26 +0200 (CEST) Received: by mail-wr0-f178.google.com with SMTP id k67so250573891wrc.2 for ; Tue, 04 Jul 2017 08:59:26 -0700 (PDT) In-Reply-To: <16812056-cfa9-066a-6323-aa0ac7dbd9cb@caviumnetworks.com> 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 Santosh, On Tue, 4 Jul 2017 17:55:54 +0530, santosh wrote: > Hi Olivier, > > On Friday 30 June 2017 07:42 PM, Olivier Matz wrote: > > > Hi, > > > > On Tue, 20 Jun 2017 19:34:15 +0530, Jerin Jacob wrote: > >> -----Original Message----- > >>> Date: Tue, 20 Jun 2017 16:07:17 +0530 > >>> From: Hemant Agrawal > >>> To: Jerin Jacob > >>> CC: Santosh Shukla , > >>> olivier.matz@6wind.com, dev@dpdk.org > >>> Subject: Re: [PATCH 0/2] Allow application set mempool handle > >>> User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 > >>> Thunderbird/45.8.0 > >>> > >>> On 6/19/2017 6:31 PM, Jerin Jacob wrote: > >>>> -----Original Message----- > >>>>> Date: Mon, 19 Jun 2017 17:22:46 +0530 > >>>>> From: Hemant Agrawal > >>>>> To: Santosh Shukla , > >>>>> olivier.matz@6wind.com, dev@dpdk.org > >>>>> CC: jerin.jacob@caviumnetworks.com > >>>>> Subject: Re: [PATCH 0/2] Allow application set mempool handle > >>>>> User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 > >>>>> Thunderbird/45.8.0 > >>>>> > >>>>> On 6/1/2017 1:35 PM, Santosh Shukla wrote: > >>>>>> Some platform can have two different NICs for example external PCI Intel > >>>>>> 40G card and Integrated NIC like vNIC/octeontx/dpaa2. > >>>>>> > >>>>>> Both NICs like to use their preferred pool e.g. external PCI card/ vNIC's > >>>>>> preferred pool would be the ring based pool and octeontx/dpaa2 preferred would > >>>>>> be ext-mempools. > >>>>>> Right now, Framework doesn't support such case. Only one pool can be > >>>>>> used across two different NIC's. For that, user has to statically set > >>>>>> CONFIG_RTE_MEMPOOL_DEFAULT_OPS=. > >>>>>> > >>>>>> So proposing two approaches: > >>>>>> Patch 1) Introducing eal option --pkt-mempool= > >>>>>> Patch 2) Introducing ethdev API called _get_preferred_pool(), where PMD driver > >>>>>> gets a chance to advertise their pool capability to the application. And based > >>>>>> on that hint- application creates pools for that driver. > >>> If the system is having more than one heterogeneous ethernet device with > >>> different mempool, the application has to create different mempool for each > >>> of the ethernet device. > >>> > >>> However, let's take a case > >>> As system has a DPAA2 eth device, which only work with dpaa2 mempools. > >> dpaa2 ethdev will return dpaa2 mempool as preferred handler. > >> > >>> System also detect a standard PCI NIC, which can work with any software > >>> mempool (e.g ring_mp_mc) or with dpaa2 mempool. Given the preference, PCI > >>> NIC will have preferred as software mempool. > >>> how the application will choose between these, if it want to create only one > >>> mempool? > >> We need add some policy in common code to help application to choose > >> in case if application interested in creating in one pool for > >> heterogeneous cases. It is more of application problem, ethdev can > >> return the preferred handler, let application choose interested in one. > >> ethdev is depended on the specific mempool not any other object. > >> > >> We will provide option 1(eal argument based one) as one policy.More sophisticated > >> policies we need add in application. > >> > >> > >>> Or, how the scheme will work if the application want to create only one > >>> mempool? > >> option 1 (eal argument based) or we need to change the application to > >> choose from available ethdev count and its preferred mempool handler. > > I also think the approach in this patchset is not that bad: > > > > - The first step is to allow the user to specify the mempool > > dynamically (eal arg). > > > > One thing I don't really like is to have a mempool-related argument > > inside eal. It would be better if eal could provide a framework so > > that each libraries (ex: mbuf, mempool) can register their argument > > that could be changed through the command line or trough an API. > > > > Without this, it introduces a sort of dependency between eal and > > mempool, which I don't think is sane. > > Yes, eal has no such framework for the non-eal library. > > IIUC, then are you looking at something like below: > - All non-eal library to register their callback function with eal. > - EAL iterates through registered callbacks and calls them one by one. > - EAL don't do the parsing and those non-eal libs do the parsing. > - EAL passes char *string arg as input to those registered callback function. > - It is up to those callback function to parse and find out i/p arg is correct > or incorrect. > - Having said that, then in the mempool case; We need to add new API to list > the number of supported mempool handles(by name) and then compare/match > i/p string with mempool handle(byname). > > Are you referring to such framework? did I catch everything alright? Here is how I see this feature (very high level). The first step would be quite simple (no registration). The EAL manages a key value database, and provides a key/value API like this: /* return NULL if key is not in database */ const char *rte_eal_cfg_get(const char *key); /* value can be NULL to delete the key, return 0 on success */ int rte_eal_cfg_set(const char *key, const char *value); At startup, the EAL parses the arguments like this: --cfg=key:value Example: --cfg=mbuf.default_pool:ring Another way to set these options could be a config file (maybe the librte_cfgfile could be useful for that, I don't know). Probably something like: --cfgfile=file.conf The EAL parsing layer calls rte_eal_cfg_set() Then, a library like librte_mbuf can query a specific key through rte_eal_cfg_get("mbuf.default_pool"). No registration would be needed. We'd need to define a convention for the key names. It could be extented in a second step by adding a registration in the constructor of the library: /* check_cb is a function that is called to check if the parsing is * correct. Maybe an opaque arg could be added too. */ rte_eal_register_cfg(const char *key, rte_eal_cfg_check_cb_t check_cb); I'm sure many people will have an opinion on this topic, which could be different than mine. > > > - The second step is to be able to ask to the eth devices which > > mempool they prefer. If there is only one kind of port, it's > > quite easy. > > > > As suggested, more complexity could go in the application if > > required, or some helpers could be provided in the future. > > > > > > I'm sending some comments as replies to the patches. > > > If above eal framework approach is meeting your expectation then [1/4] need rework? > Or you want to keep [1/4] patch and I'll send v2 patch incorporating > your inline review comment, which one you prefer? Adding a specific EAL argument --pkt-mempool could do the job for now. But I'd be happy to see someone working on a generic cfg framework in EAL, which seems to be a longer term solution, and helpful for other libs. Some parts of EAL have currently no maintainer, which is a problem to get a good feedback. But I guess a proposition on this topic would trigger many comments. Regards, Olivier