From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f179.google.com (mail-wr0-f179.google.com [209.85.128.179]) by dpdk.org (Postfix) with ESMTP id 2B9152C8 for ; Tue, 4 Jul 2017 15:07:18 +0200 (CEST) Received: by mail-wr0-f179.google.com with SMTP id c11so248957831wrc.3 for ; Tue, 04 Jul 2017 06:07:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=R/9dCNIW6sg4gu3JP66AJ8Yh13J/EcoaMwoY5V4zlVA=; b=hJ0csIg2FEz1GK9HaSObCWBII2ZM6aSklzLvhremyc+3UB4wy5BkCtjjIsxabz9p4M XdcGScPyIJUAmAWe0w9cywrVs3H32AVeBZNuj4fYk5JcICDdtq5E2enDfXZoMRuKWkWG LiQ+MnOLjArXfa4+Mva2OKdvzLU6kVVN5WESr+FrpN9Epr6aIA7U6vcQEEWpkqGgVF13 MCVojyeb16+MOO9Xokl+emTsp59t09lURxbyk7w8Ojf9AIjdfdiAc+9pqRdx5RRjItBq Cj8kthLEutxIfLPQydhdD0h7IGiDIbEc2DQbqVX1PxIz8Oc+fn7dU2dsfCCaksG3yxS4 DXeg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=R/9dCNIW6sg4gu3JP66AJ8Yh13J/EcoaMwoY5V4zlVA=; b=R4bbP1OJPsLBEwVcU49P1giGrB2nf9zOecC3a2x2ghTxed5wHFoe+d/ADa+iJuyW9G HTdNipgN0zZiHPMJwBlesM6Y2zWHWEd8Nm2eAIuXpkul/2QNW+JTdJ0qaxQ6CMGUl2ZN jqaYd4ttgjOYgk/QRFvWyb9KibikrH0ZtoVhQdOF10Hm6gj4yVTk51EPUOROxkLglkZH 4sYqyHrVu7aqFDlMlpytbmgz8NMD71tA2LQjOy6aTXDp7/X/l8WLYQOC0S+udinXZoxj XMAq+TMZ7KfyCdxfpQM0/yjs/L8A3P1kTBfPMhpFGKhJDMSJf/rayfDIdzb2oaIFL4Hh ufOQ== X-Gm-Message-State: AKS2vOzKqCnwBL5N+zNuU805WCVWV1D1MhKFqZIR6q8D8taCHSXkIWiu gCouMQzX6Q1GtSwOU/w= X-Received: by 10.223.153.238 with SMTP id y101mr33718311wrb.168.1499173637620; Tue, 04 Jul 2017 06:07:17 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id 46sm27108831wrz.8.2017.07.04.06.07.17 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 04 Jul 2017 06:07:17 -0700 (PDT) Date: Tue, 4 Jul 2017 15:07:14 +0200 From: Olivier Matz To: santosh Cc: dev@dpdk.org, hemant.agrawal@nxp.com, jerin.jacob@caviumnetworks.com Message-ID: <20170704150714.4feaa396@platinum> In-Reply-To: References: <20170601080559.10684-1-santosh.shukla@caviumnetworks.com> <20170601080559.10684-3-santosh.shukla@caviumnetworks.com> <20170630161302.1c11ca46@platinum> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 2/2] ether/ethdev: Allow pmd to advertise preferred pool capability 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: Tue, 04 Jul 2017 13:07:18 -0000 On Tue, 4 Jul 2017 18:09:33 +0530, santosh wrote: > On Friday 30 June 2017 07:43 PM, Olivier Matz wrote: > > > On Thu, 1 Jun 2017 13:35:59 +0530, Santosh Shukla wrote: > >> Platform with two different NICs like external PCI NIC and > >> Integrated NIC, May want to use their preferred pool handle. > >> Right now there is no way that two different NICs on same board, > >> Could use their choice of a pool. > >> Both NICs forced to use same pool, Which is statically configured > >> by setting CONFIG_RTE_MEMPOOL_DEFAULT_OPS=. > >> > >> So Introducing get_preferred_pool() API. Which allows PMD driver > >> to advertise their pool capability to Application. > >> Based on that hint, Application creates separate pool for > >> That driver. > >> > >> Signed-off-by: Santosh Shukla > >> --- > >> lib/librte_ether/rte_ethdev.c | 16 ++++++++++++++++ > >> lib/librte_ether/rte_ethdev.h | 21 +++++++++++++++++++++ > >> lib/librte_ether/rte_ether_version.map | 7 +++++++ > >> 3 files changed, 44 insertions(+) > >> > >> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > >> index 83898a8f7..4068a05b1 100644 > >> --- a/lib/librte_ether/rte_ethdev.c > >> +++ b/lib/librte_ether/rte_ethdev.c > >> @@ -3472,3 +3472,19 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id, > >> -ENOTSUP); > >> return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en); > >> } > >> + > >> +int > >> +rte_eth_dev_get_preferred_pool(uint8_t port_id, const char *pool) > >> +{ > >> + struct rte_eth_dev *dev; > >> + > >> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > >> + > >> + dev = &rte_eth_devices[port_id]; > >> + > >> + if (*dev->dev_ops->get_preferred_pool == NULL) { > >> + pool = RTE_MBUF_DEFAULT_MEMPOOL_OPS; > >> + return 0; > >> + } > >> + return (*dev->dev_ops->get_preferred_pool)(dev, pool); > >> +} > > Instead of this, what about: > > > > /* > > * Return values: > > * - -ENOTSUP: error, pool type is not supported > > * - on success, return the priority of the mempool (0 = highest) > > */ > > int > > rte_eth_dev_pool_ops_supported(uint8_t port_id, const char *pool) > > > > By default, always return 0 (i.e. all pools are supported). > > > > With this API, we can announce several supported pools (not only > > one preferred), and order them by preference. > > IMO: We should let application to decide on pool preference. Driver > only to advice his preferred or supported pool handle to application, > and its upto application to decide on pool selection scheme. The api I'm proposing does not prevent the application from taking the decision. On the contrary, it gives more clues to the application: an ordered list of supported pools, instead of just the preferred pool. > > > I also wonder if we should use a ops_index instead of a pool name > > for the second argument. > > > > > > > >> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h > >> index 0f38b45f8..8e5b06af7 100644 > >> --- a/lib/librte_ether/rte_ethdev.h > >> +++ b/lib/librte_ether/rte_ethdev.h > >> @@ -1381,6 +1381,10 @@ typedef int (*eth_l2_tunnel_offload_set_t) > >> uint8_t en); > >> /**< @internal enable/disable the l2 tunnel offload functions */ > >> > >> +typedef int (*eth_get_preferred_pool_t)(struct rte_eth_dev *dev, > >> + const char *pool); > >> +/**< @internal Get preferred pool handler for a device */ > >> + > >> #ifdef RTE_NIC_BYPASS > >> > >> enum { > >> @@ -1573,6 +1577,8 @@ struct eth_dev_ops { > >> /**< Get extended device statistic values by ID. */ > >> eth_xstats_get_names_by_id_t xstats_get_names_by_id; > >> /**< Get name of extended device statistics by ID. */ > >> + eth_get_preferred_pool_t get_preferred_pool; > >> + /**< Get preferred pool handler for a device */ > >> }; > >> > >> /** > >> @@ -4607,6 +4613,21 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id); > >> int > >> rte_eth_dev_get_name_by_port(uint8_t port_id, char *name); > >> > >> +/** > >> + * Get preferred pool handle for a device > >> + * > >> + * @param port_id > >> + * port identifier of the device > >> + * @param [out] pool > >> + * Preferred pool handle for this device. > >> + * Pool len shouldn't more than 256B. Allocated by pmd driver. > > [out] ?? > > I don't get why it is allocated by the driver > > > Driver to advice his preferred pool to application. That's why out. So how can it be const? Did you tried it? > > Thanks. > > > > >> + * @return > >> + * - (0) if successful. > >> + * - (-EINVAL) on failure. > >> + */ > >> +int > >> +rte_eth_dev_get_preferred_pool(uint8_t port_id, const char *pool); > >> + > >> #ifdef __cplusplus > >> } > >> #endif > >> diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map > >> index d6726bb1b..819fe800e 100644 > >> --- a/lib/librte_ether/rte_ether_version.map > >> +++ b/lib/librte_ether/rte_ether_version.map > >> @@ -156,3 +156,10 @@ DPDK_17.05 { > >> rte_eth_xstats_get_names_by_id; > >> > >> } DPDK_17.02; > >> + > >> +DPDK_17.08 { > >> + global: > >> + > >> + rte_eth_dev_get_preferred_pool; > >> + > >> +} DPDK_17.05; >