DPDK patches and discussions
 help / color / mirror / Atom feed
From: santosh <santosh.shukla@caviumnetworks.com>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: dev@dpdk.org, thomas@monjalon.net,
	jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com
Subject: Re: [dpdk-dev] [PATCH v4 2/2] ethdev: get the supported pools for a port
Date: Mon, 25 Sep 2017 22:52:31 +0100	[thread overview]
Message-ID: <8b832367-e8fa-077a-1732-50627e977f9c@caviumnetworks.com> (raw)
In-Reply-To: <20170925073728.auchwn7mxx63b32j@platinum>

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 <santosh.shukla@caviumnetworks.com>
>>
>> [...]
>>
>> +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).

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? 

>> +	}
>> +
>> +	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

ok, v5.

>> + *   Maximum length of pool handle name is RTE_MEMPOOL_OPS_NAMESIZE.
> I don't think we should keep this
>
Ok

>
> Thanks,
> Olivier

  reply	other threads:[~2017-09-25 21:52 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-01  8:05 [dpdk-dev] [PATCH 0/2] Allow application set mempool handle Santosh Shukla
2017-06-01  8:05 ` [dpdk-dev] [PATCH 1/2] eal: Introducing option to " Santosh Shukla
2017-06-30 14:12   ` Olivier Matz
2017-07-04 12:33     ` santosh
2017-06-01  8:05 ` [dpdk-dev] [PATCH 2/2] ether/ethdev: Allow pmd to advertise preferred pool capability Santosh Shukla
2017-06-30 14:13   ` Olivier Matz
2017-07-04 12:39     ` santosh
2017-07-04 13:07       ` Olivier Matz
2017-07-04 14:12         ` Jerin Jacob
2017-06-19 11:52 ` [dpdk-dev] [PATCH 0/2] Allow application set mempool handle Hemant Agrawal
2017-06-19 13:01   ` Jerin Jacob
2017-06-20 10:37     ` Hemant Agrawal
2017-06-20 14:04       ` Jerin Jacob
2017-06-30 14:12         ` Olivier Matz
2017-07-04 12:25           ` santosh
2017-07-04 15:59             ` Olivier Matz
2017-07-05  7:48               ` santosh
2017-07-20  7:06 ` [dpdk-dev] [PATCH v2 0/2] Dynamically configure " Santosh Shukla
2017-07-20  7:06   ` [dpdk-dev] [PATCH v2 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-08-15  8:07     ` [dpdk-dev] [PATCH v3 0/2] Dynamically configure mempool handle Santosh Shukla
2017-08-15  8:07       ` [dpdk-dev] [PATCH v3 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-09-04 11:46         ` Olivier MATZ
2017-09-07  9:25         ` Hemant Agrawal
2017-08-15  8:07       ` [dpdk-dev] [PATCH v3 2/2] ethdev: allow pmd to advertise " Santosh Shukla
2017-09-04 12:11         ` Olivier MATZ
2017-09-04 13:14           ` santosh
2017-09-07  9:21             ` Hemant Agrawal
2017-09-07 10:06               ` santosh
2017-09-07 10:11                 ` santosh
2017-09-07 11:08                   ` Hemant Agrawal
2017-09-11  9:33                     ` Olivier MATZ
2017-09-11 12:40                       ` santosh
2017-09-11 13:00                         ` Olivier MATZ
2017-09-04  9:41       ` [dpdk-dev] [PATCH v3 0/2] Dynamically configure mempool handle Sergio Gonzalez Monroy
2017-09-04 13:20         ` santosh
2017-09-04 13:34         ` Olivier MATZ
2017-09-04 14:24           ` Sergio Gonzalez Monroy
2017-09-05  7:47             ` Olivier MATZ
2017-09-05  8:11               ` Jerin Jacob
2017-09-11 15:18       ` [dpdk-dev] [PATCH v4 " Santosh Shukla
2017-09-11 15:18         ` [dpdk-dev] [PATCH v4 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-09-25  7:28           ` Olivier MATZ
2017-09-25 21:23             ` santosh
2017-09-11 15:18         ` [dpdk-dev] [PATCH v4 2/2] ethdev: get the supported pools for a port Santosh Shukla
2017-09-25  7:37           ` Olivier MATZ
2017-09-25 21:52             ` santosh [this message]
2017-09-29  5:00               ` santosh
2017-09-29  8:32               ` Olivier MATZ
2017-09-29 10:16                 ` santosh
2017-09-29 11:21                   ` santosh
2017-09-29 11:23                   ` Olivier MATZ
2017-09-29 11:31                     ` santosh
2017-09-13 10:00         ` [dpdk-dev] [PATCH v4 0/2] Dynamically configure mempool handle santosh
2017-09-19  8:28           ` santosh
2017-09-25  7:24             ` Olivier MATZ
2017-09-25 21:58               ` santosh
2017-10-01  9:14         ` [dpdk-dev] [PATCH v5 " Santosh Shukla
2017-10-01  9:14           ` [dpdk-dev] [PATCH v5 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-10-02 14:29             ` Olivier MATZ
2017-10-06  0:29             ` Thomas Monjalon
2017-10-06  3:31               ` santosh
2017-10-06  8:39                 ` Thomas Monjalon
2017-10-06  7:45             ` [dpdk-dev] [PATCH v6 0/2] Dynamically configure mempool handle Santosh Shukla
2017-10-06  7:45               ` [dpdk-dev] [PATCH v6 1/2] eal: allow user to override default pool handle Santosh Shukla
2017-10-06  7:45               ` [dpdk-dev] [PATCH v6 2/2] ethdev: get the supported pool for a port Santosh Shukla
2017-10-06 18:58               ` [dpdk-dev] [PATCH v6 0/2] Dynamically configure mempool handle Thomas Monjalon
2017-10-01  9:14           ` [dpdk-dev] [PATCH v5 2/2] ethdev: get the supported pool for a port Santosh Shukla
2017-10-02 14:31             ` Olivier MATZ
2017-10-06  0:30             ` Thomas Monjalon
2017-10-06  3:32               ` santosh
2017-10-02  8:37           ` [dpdk-dev] [PATCH v5 0/2] Dynamically configure mempool handle santosh
2017-07-20  7:06   ` [dpdk-dev] [PATCH v2 2/2] ethdev: allow pmd to advertise pool handle Santosh Shukla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b832367-e8fa-077a-1732-50627e977f9c@caviumnetworks.com \
    --to=santosh.shukla@caviumnetworks.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=olivier.matz@6wind.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).