On Thu, Sep 07, 2017 at 04:38:39PM +0530, Hemant Agrawal wrote:
> On 9/7/2017 3:41 PM, santosh wrote:
> >
> >
> > On Thursday 07 September 2017 03:36 PM, santosh wrote:
> > > Hi Hemant,
> > >
> > >
> > > On Thursday 07 September 2017 02:51 PM, Hemant Agrawal wrote:
> > > > On 9/4/2017 6:44 PM, santosh wrote:
> > > > > Hi Olivier,
> > > > >
> > > > >
> > > > > On Monday 04 September 2017 05:41 PM, Olivier MATZ wrote:
> > > > > > Hi Santosh,
> > > > > >
> > > > > > On Tue, Aug 15, 2017 at 01:37:17PM +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 like to know `preferred mempool vs PMD driver`
> > > > > > > information in advance before port setup.
> > > > > > >
> > > > > > > Introducing rte_eth_dev_get_preferred_pool_ops() API,
> > > > > > > which allows PMD driver to advertise their pool capability to
> > > > > > > application.
> > > > > > >
> > > > > > > Application side programing sequence would be:
> > > > > > >
> > > > > > > char pref_mempool[RTE_MEMPOOL_OPS_NAMESIZE];
> > > > > > > rte_eth_dev_get_preferred_pool_ops(ethdev_port_id, pref_mempoolx
> > > > > > > /*out*/);
> > > > > > > rte_mempool_create_empty();
> > > > > > > rte_mempool_set_ops_byname( , pref_memppol, );
> > > > > > > rte_mempool_populate_default();
> > > > > > >
> > > > > > > Signed-off-by: Santosh Shukla <santosh.shu...@caviumnetworks.com>
> > > > > > > ---
> > > > > > > v2 --> v3:
> > > > > > > - Updated version.map entry to DPDK_v17.11.
> > > > > > >
> > > > > > > v1 --> v2:
> > > > > > > - Renamed _get_preferred_pool to _get_preferred_pool_ops().
> > > > > > > Per v1 review feedback, Olivier suggested to rename api
> > > > > > > to rte_eth_dev_pool_ops_supported(), considering that 2nd param
> > > > > > > for that api will return pool handle 'priority' for that port.
> > > > > > > However, per v1 [1], we're opting for approach 1) where
> > > > > > > ethdev API returns _preferred_ pool handle to application and Its
> > > > > > > upto
> > > > > > > application to decide on policy - whether application wants to
> > > > > > > create
> > > > > > > pool with received preferred pool handle or not. For more
> > > > > > > discussion details
> > > > > > > on this topic refer [1].
> > > > > > Well, I still think it would be more flexible to have an API like
> > > > > > rte_eth_dev_pool_ops_supported(uint8_t port_id, const char *pool)
> > > > > >
> > > > > > It supports the easy case (= one preferred mempool) without much
> > > > > > pain,
> > > > > > and provides a more precise manner to describe what is supported or
> > > > > > not
> > > > > > by the driver. Example: "pmd_foo" prefers "mempool_foo" (best
> > > > > > perf), but
> > > > > > also supporst "mempool_stack" and "mempool_ring", but "mempool_bar"
> > > > > > won't work at all.
> > > > > >
> > > > > > Having only one preferred pool_ops also prevents from smoothly
> > > > > > renaming
> > > > > > a pool (supporting both during some time) or to have 2 names for
> > > > > > different variants of the same pool_ops (ex: ring_mp_mc,
> > > > > > ring_sp_sc).
> > > > > >
> > > > > > But if the users (I guess at least Cavium and NXP) are happy with
> > > > > > what you propose, I'm fine with it.
> > > > > preferred handle based upon real world use-case and same thing raised
> > > > > at [1].
> > > > >
> > > > > Hi Hemant, Are you ok with proposed preferred API?
> > > > >
> > > > > [1] http://dpdk.org/dev/patchwork/patch/24944/
> > > > >
> > > > The current patch is ok, but it is better if you can extend it to
> > > > provide list of preferred pools (in preference order) instead of just
> > > > one pool. This will become helpful. I will avoid providing list of
> > > > not-supported pools etc.
> > > >
> > > > A driver can have more than one preferred pool, depend on the resource
> > > > availability one or other can be used. I am also proposing this in my
> > > > proposal[1].
> > > >
> > > > [1] http://dpdk.org/dev/patchwork/patch/26377/
> > > >
> > > Ok, then sticking to Olivier api but slight change in param,
> > > example:
> > > /** * Get list of supported pools for a port * * @param port_id [in] *
> > > Pointer to port identifier of the device. * @param pools [out] * Pointer
> > > to the list of supported pools for that port. * Returns with array of
> > > pool ops name handler of size * RTE_MEMPOOL_OPS_NAMESIZE. * @return *
> > > >=0: Success; PMD has updated supported pool list. * <0: Failure; */ int
> > > rte_eth_dev_pools_ops_supported(uint8_t port_id, char **pools) Hemant,
> > > Olivier: Does above api make sense? Pl. confirm. Thanks.
> >
> > Sorry for the font, resending proposed API:
> >
> > /**
> > * Get list of supported pools for a port
> > * @param port_id [in]
> > * Pointer to port identifier of the device.
> > * @param pools [out]
> > * Pointer to the list of supported pools for that port.
> > * Returns with array of pool ops name handler of size
> > * RTE_MEMPOOL_OPS_NAMESIZE.
> > * @return
> > * >=0: Success; PMD has updated supported pool list.
> > * <0: Failure;
> > */
> >
> > int rte_eth_dev_pools_ops_supported(uint8_t port_id, char **pools)
> >
> > Hemant, Olivier: Does above api make sense? Pl. confirm. Thanks.
> >
>
> looks ok to me.
I think that returning a list is harder to use in an application, instead of an
api that just returns an int (priority):
int rte_eth_dev_pool_ops_supported(uint8_t port_id, const char *pool)
The possible returned values are:
ENOTSUP: mempool ops not supported
< 0: any other error
0: best mempool ops choice for this pmd
1: this mempool ops are supported
Let's take an example. Our application wants to select ops that
will match all pmds. The pseudo code would be like this:
best_score = -1
best_ops = NULL
for ops in mempool_ops:
score = 0
for p in ports:
ret = rte_eth_dev_pools_ops_supported(p, ops.name)
if ret < 0:
score = -1
break
score += ret
if score == -1:
continue
if best_score == -1 || score < best_score:
best_score = score
best_ops = ops
if best_score == -1:
print "no matching mempool ops"
else:
print "selected ops: %s", best_ops.name
You can do the exercise with the API you are proposing, but I think
it would be harder.
Olivier
