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 <santosh.shu...@caviumnetworks.com>
>
> [...]
>
> +int
> +rte_eth_dev_pools_ops_supported(uint8_t port_id, const char *pool)
pools -> pool?
> +{
> + 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.
I think that a PMD that does not implement ->pools_ops_supported
should always return 1 (mempool is supported).
> + }
> +
> + return (*dev->dev_ops->pools_ops_supported)(dev, pool);
> +}
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 0adf3274a..d90029b1e 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1425,6 +1425,10 @@ typedef int (*eth_get_dcb_info)(struct rte_eth_dev
> *dev,
> struct rte_eth_dcb_info *dcb_info);
> /**< @internal Get dcb information on an Ethernet device */
>
> +typedef int (*eth_pools_ops_supported_t)(struct rte_eth_dev *dev,
> + const char *pool);
> +/**< @internal Get the supported pools for a port */
> +
The comment should be something like:
Test if a port supports specific mempool ops.
> /**
> * @internal A structure containing the functions exported by an Ethernet
> driver.
> */
> @@ -1544,6 +1548,8 @@ struct eth_dev_ops {
>
> eth_tm_ops_get_t tm_ops_get;
> /**< Get Traffic Management (TM) operations. */
> + eth_pools_ops_supported_t pools_ops_supported;
> + /**< Get the supported pools for a port */
Same
> };
>
> /**
> @@ -4436,6 +4442,24 @@ int rte_eth_dev_adjust_nb_rx_tx_desc(uint8_t port_id,
> uint16_t *nb_rx_desc,
> uint16_t *nb_tx_desc);
>
> +
> +/**
> + * Get the supported pools for a port
Same
> + *
> + * @param port_id
> + * Port identifier of the Ethernet device.
> + * @param [in] pool
> + * The supported pool handle for this port.
The name of the pool operations to test
> + * Maximum length of pool handle name is RTE_MEMPOOL_OPS_NAMESIZE.
I don't think we should keep this
Thanks,
Olivier
