From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 4CD861B1C6 for ; Fri, 29 Sep 2017 10:32:51 +0200 (CEST) Received: from lfbn-lil-1-182-75.w90-45.abo.wanadoo.fr ([90.45.31.75] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1dxqoc-0002jK-GP; Fri, 29 Sep 2017 10:38:36 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Fri, 29 Sep 2017 10:32:43 +0200 Date: Fri, 29 Sep 2017 10:32:43 +0200 From: Olivier MATZ To: santosh Cc: dev@dpdk.org, thomas@monjalon.net, jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com Message-ID: <20170929083242.a4arzpf2drfqyxpj@platinum> References: <20170815080717.9413-1-santosh.shukla@caviumnetworks.com> <20170911151837.25092-1-santosh.shukla@caviumnetworks.com> <20170911151837.25092-3-santosh.shukla@caviumnetworks.com> <20170925073728.auchwn7mxx63b32j@platinum> <8b832367-e8fa-077a-1732-50627e977f9c@caviumnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8b832367-e8fa-077a-1732-50627e977f9c@caviumnetworks.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v4 2/2] ethdev: get the supported pools for a port 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, 29 Sep 2017 08:32:51 -0000 On Mon, Sep 25, 2017 at 10:52:31PM +0100, santosh wrote: > Hi Olivier, > > > On Monday 25 September 2017 08:37 AM, Olivier MATZ wrote: > > Hi, > > > > On Mon, Sep 11, 2017 at 08:48:37PM +0530, Santosh Shukla wrote: > >> Now that dpdk supports more than one mempool drivers and > >> each mempool driver works best for specific PMD, example: > >> - sw ring based mempool for Intel PMD drivers. > >> - dpaa2 HW mempool manager for dpaa2 PMD driver. > >> - fpa HW mempool manager for Octeontx PMD driver. > >> > >> Application would like to know the best mempool handle > >> for any port. > >> > >> Introducing rte_eth_dev_pools_ops_supported() API, > >> which allows PMD driver to advertise > >> his supported pools capability to the application. > >> > >> Supported pools are categorized in below priority:- > >> - Best mempool handle for this port (Highest priority '0') > >> - Port supports this mempool handle (Priority '1') > >> > >> Signed-off-by: Santosh Shukla > >> > >> [...] > >> > >> +int > >> +rte_eth_dev_pools_ops_supported(uint8_t port_id, const char *pool) > > pools -> pool? > > ok. > > >> +{ > >> + struct rte_eth_dev *dev; > >> + const char *tmp; > >> + > >> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > >> + > >> + if (pool == NULL) > >> + return -EINVAL; > >> + > >> + dev = &rte_eth_devices[port_id]; > >> + > >> + if (*dev->dev_ops->pools_ops_supported == NULL) { > >> + tmp = rte_eal_mbuf_default_mempool_ops(); > >> + if (!strcmp(tmp, pool)) > >> + return 0; > >> + else > >> + return -ENOTSUP; > > I don't understand why we are comparing with > > rte_eal_mbuf_default_mempool_ops(). > > > > It means that the result of the function would be influenced > > by the parameter given by the user. > > But that will be only for ops not supported case and in that case, > function _must_ make sure that if inputted param is _default_ops_name > then function should return ops supported correct info (whether > returning '0' : Best ops or '1': ops does support > , this part is arguable.. meaning One can say that default_ops ='handle-name' is best possible handle Or > one of handle which platform supports). > > > I think that a PMD that does not implement ->pools_ops_supported > > should always return 1 (mempool is supported). > I don't agree. The result of this API (mempool ops supported or not by a PMD) should not depend on what user asks for. > Return 1 says: PMD support this ops.. > > So if ops is not supported and func returns with 1, then which ops application will use? > If that ops is default_ops.. then How application will distinguish when to use default ops or > param ops?.. as because in both cases func will return with value 1. > > The approach in the patch takes care of that condition and func will return -ENOTSUP > if (ops not support || inputted param not matching with default ops) otherwise will return > 0 or 1. > > At application side; > For error case: In case of -ENOTSUP, its upto application to use _default_ops or exit. > For good case: 0 or 1 case, func gaurantee that handle is either best handle for pool or pool supports > that handle.. However in your suggestion if ops not supported case returns 1 then application is not > sure which ops to use.. default_ops Or input ops given to func. > > make sense? My proposition is: - what a PMD returns does not depend on used parameter: - 0: best support - 1: support - -ENOTSUP: not supported - if a PMD does not implement the _ops_supported() API, assume all pools are supported (returns 1) - if the user does not pass a specific mempool ops, the application asks the PMDs, finds the best mempool ops, and use it. This could even be done in rte_pktmbuf_pool_create() as discussed at the summit. - if the user passes a specific mempool ops, we don't need to call the _ops_supported() api, we just try to use this pool. The _ops_supported() returns a property of a PMD, in my opinion it should not be impacted by a user argument.