From mboxrd@z Thu Jan 1 00:00:00 1970 From: santosh Subject: Re: [PATCH 2/2] ether/ethdev: Allow pmd to advertise preferred pool capability Date: Tue, 4 Jul 2017 18:09:33 +0530 Message-ID: References: <20170601080559.10684-1-santosh.shukla@caviumnetworks.com> <20170601080559.10684-3-santosh.shukla@caviumnetworks.com> <20170630161302.1c11ca46@platinum> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, hemant.agrawal@nxp.com, jerin.jacob@caviumnetworks.com To: Olivier Matz Return-path: Received: from NAM02-SN1-obe.outbound.protection.outlook.com (mail-sn1nam02on0054.outbound.protection.outlook.com [104.47.36.54]) by dpdk.org (Postfix) with ESMTP id 21462235 for ; Tue, 4 Jul 2017 14:39:48 +0200 (CEST) In-Reply-To: <20170630161302.1c11ca46@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" 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. > 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. 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;