mbox series

[v2,0/5] Dynamic HW Mempool Detection Support

Message ID 1515996674-26338-1-git-send-email-hemant.agrawal@nxp.com
Headers show
Series Dynamic HW Mempool Detection Support | expand

Message

Hemant Agrawal Jan. 15, 2018, 6:11 a.m. UTC
W.r.t the multiple discussions in the past about the ability to
dynamically detect the HW mempool support. [1],[2] & [3]

This patchset helps in removing the current static mempool selection
model and provides a flexible model to select the pktmbuf mempool
in more dynamic way. 

1) This patchset updates the hw mempool on the basis of device probe()),
   thus avoiding the need to specify the hw mempool in config file and 
   focing different binaries for diffirent config architectures.
2) Selection of mempool ops though --mbuf-pool-ops-name (cmd line arg)
   which can overridden the scheme(1)
3) A new best mempool ops selection logic.
4) A new wrapper for the pktmbuf_pool_create helper to take mempool ops
   name as an argument as well.

*Limitations and open points*

It was suggested to add all APIs in librte_mbuf, currently internal_config
is storing the mempool_ops names. So internal_config is exported in this
patchset. An alternate would be to keep these APIs in eal only and access
them indirectly from librte_mbuf.

Moreover, this logic can be further extended with addition for following
patch, which is still under discussion:

The ethdev PMD capability exposed through existing
rte_eth_dev_pool_ops_supported() to select the update the mempool ops with
some "weight" based algorithm like:
http://dpdk.org/dev/patchwork/patch/32245/

-----
[1]Multiple Pktmbuf mempool support
http://dpdk.org/ml/archives/dev/2017-September/076531.html
[2]Allow application set mempool handle
http://dpdk.org/ml/archives/dev/2017-June/067022.html
Other discussions
[3] http://dpdk.org/ml/archives/dev/2017-December/084775.html 
------
Changes in v2:
1. Changed the active mempool to platform mempool
2. Moved all the relavant APIs to librte_mbuf 
3. Added pktmbuf_create_pool_specific wrapper in this patch series.

Hemant Agrawal (5):
  eal: prefix mbuf pool ops name with user defined
  eal: add platform mempool ops name in internal config
  mbuf: support register mempool Hw ops name APIs
  mbuf: pktmbuf pool create helper for specific mempool ops
  mbuf: add user command line config mempools ops API

 doc/guides/rel_notes/deprecation.rst       |  7 +++
 lib/librte_eal/bsdapp/eal/eal.c            |  4 +-
 lib/librte_eal/common/eal_common_options.c |  3 +-
 lib/librte_eal/common/eal_internal_cfg.h   |  5 ++-
 lib/librte_eal/linuxapp/eal/eal.c          |  4 +-
 lib/librte_eal/rte_eal_version.map         |  1 +
 lib/librte_mbuf/Makefile                   |  1 +
 lib/librte_mbuf/rte_mbuf.c                 | 67 ++++++++++++++++++++++++---
 lib/librte_mbuf/rte_mbuf.h                 | 72 ++++++++++++++++++++++++++++++
 lib/librte_mbuf/rte_mbuf_version.map       | 10 +++++
 10 files changed, 162 insertions(+), 12 deletions(-)

-- 
2.7.4

Comments

Olivier Matz Jan. 16, 2018, 3:01 p.m. UTC | #1
On Mon, Jan 15, 2018 at 11:41:09AM +0530, Hemant Agrawal wrote:
> W.r.t the multiple discussions in the past about the ability to

> dynamically detect the HW mempool support. [1],[2] & [3]

> 

> This patchset helps in removing the current static mempool selection

> model and provides a flexible model to select the pktmbuf mempool

> in more dynamic way. 

> 

> 1) This patchset updates the hw mempool on the basis of device probe()),

>    thus avoiding the need to specify the hw mempool in config file and 

>    focing different binaries for diffirent config architectures.

> 2) Selection of mempool ops though --mbuf-pool-ops-name (cmd line arg)

>    which can overridden the scheme(1)

> 3) A new best mempool ops selection logic.

> 4) A new wrapper for the pktmbuf_pool_create helper to take mempool ops

>    name as an argument as well.

> 

> *Limitations and open points*

> 

> It was suggested to add all APIs in librte_mbuf, currently internal_config

> is storing the mempool_ops names. So internal_config is exported in this

> patchset. An alternate would be to keep these APIs in eal only and access

> them indirectly from librte_mbuf.


Instead of storing the mempool_ops name in internal config, can't it be
stored inside librte_mbuf? The eal code can call
rte_mbuf_set_user_mempool_ops(name) while parsing the arguments.

It has to be done carefully so that it works with secondary processes.


> Moreover, this logic can be further extended with addition for following

> patch, which is still under discussion:

> 

> The ethdev PMD capability exposed through existing

> rte_eth_dev_pool_ops_supported() to select the update the mempool ops with

> some "weight" based algorithm like:

> http://dpdk.org/dev/patchwork/patch/32245/

> 

> -----

> [1]Multiple Pktmbuf mempool support

> http://dpdk.org/ml/archives/dev/2017-September/076531.html

> [2]Allow application set mempool handle

> http://dpdk.org/ml/archives/dev/2017-June/067022.html

> Other discussions

> [3] http://dpdk.org/ml/archives/dev/2017-December/084775.html 

> ------

> Changes in v2:

> 1. Changed the active mempool to platform mempool

> 2. Moved all the relavant APIs to librte_mbuf 

> 3. Added pktmbuf_create_pool_specific wrapper in this patch series.

> 

> Hemant Agrawal (5):

>   eal: prefix mbuf pool ops name with user defined

>   eal: add platform mempool ops name in internal config

>   mbuf: support register mempool Hw ops name APIs

>   mbuf: pktmbuf pool create helper for specific mempool ops

>   mbuf: add user command line config mempools ops API

> 

>  doc/guides/rel_notes/deprecation.rst       |  7 +++

>  lib/librte_eal/bsdapp/eal/eal.c            |  4 +-

>  lib/librte_eal/common/eal_common_options.c |  3 +-

>  lib/librte_eal/common/eal_internal_cfg.h   |  5 ++-

>  lib/librte_eal/linuxapp/eal/eal.c          |  4 +-

>  lib/librte_eal/rte_eal_version.map         |  1 +

>  lib/librte_mbuf/Makefile                   |  1 +

>  lib/librte_mbuf/rte_mbuf.c                 | 67 ++++++++++++++++++++++++---

>  lib/librte_mbuf/rte_mbuf.h                 | 72 ++++++++++++++++++++++++++++++


I think the code to manage the user/platform mempool ops could
go in a separate file. What about rte_mbuf_pool_ops.[ch] ?
Hemant Agrawal Jan. 18, 2018, 11:47 a.m. UTC | #2
On 1/16/2018 8:31 PM, Olivier Matz wrote:
> On Mon, Jan 15, 2018 at 11:41:09AM +0530, Hemant Agrawal wrote:

>> W.r.t the multiple discussions in the past about the ability to

>> dynamically detect the HW mempool support. [1],[2] & [3]

>>

>> This patchset helps in removing the current static mempool selection

>> model and provides a flexible model to select the pktmbuf mempool

>> in more dynamic way.

>>

>> 1) This patchset updates the hw mempool on the basis of device probe()),

>>    thus avoiding the need to specify the hw mempool in config file and

>>    focing different binaries for diffirent config architectures.

>> 2) Selection of mempool ops though --mbuf-pool-ops-name (cmd line arg)

>>    which can overridden the scheme(1)

>> 3) A new best mempool ops selection logic.

>> 4) A new wrapper for the pktmbuf_pool_create helper to take mempool ops

>>    name as an argument as well.

>>

>> *Limitations and open points*

>>

>> It was suggested to add all APIs in librte_mbuf, currently internal_config

>> is storing the mempool_ops names. So internal_config is exported in this

>> patchset. An alternate would be to keep these APIs in eal only and access

>> them indirectly from librte_mbuf.

>

> Instead of storing the mempool_ops name in internal config, can't it be

> stored inside librte_mbuf? The eal code can call

> rte_mbuf_set_user_mempool_ops(name) while parsing the arguments.


I doubt that eal can call mbuf APIs. It will be a issue in shared build?

>

> It has to be done carefully so that it works with secondary processes.

>

>

>> Moreover, this logic can be further extended with addition for following

>> patch, which is still under discussion:

>>

>> The ethdev PMD capability exposed through existing

>> rte_eth_dev_pool_ops_supported() to select the update the mempool ops with

>> some "weight" based algorithm like:

>> http://dpdk.org/dev/patchwork/patch/32245/

>>

>> -----

>> [1]Multiple Pktmbuf mempool support

>> http://dpdk.org/ml/archives/dev/2017-September/076531.html

>> [2]Allow application set mempool handle

>> http://dpdk.org/ml/archives/dev/2017-June/067022.html

>> Other discussions

>> [3] http://dpdk.org/ml/archives/dev/2017-December/084775.html

>> ------

>> Changes in v2:

>> 1. Changed the active mempool to platform mempool

>> 2. Moved all the relavant APIs to librte_mbuf

>> 3. Added pktmbuf_create_pool_specific wrapper in this patch series.

>>

>> Hemant Agrawal (5):

>>   eal: prefix mbuf pool ops name with user defined

>>   eal: add platform mempool ops name in internal config

>>   mbuf: support register mempool Hw ops name APIs

>>   mbuf: pktmbuf pool create helper for specific mempool ops

>>   mbuf: add user command line config mempools ops API

>>

>>  doc/guides/rel_notes/deprecation.rst       |  7 +++

>>  lib/librte_eal/bsdapp/eal/eal.c            |  4 +-

>>  lib/librte_eal/common/eal_common_options.c |  3 +-

>>  lib/librte_eal/common/eal_internal_cfg.h   |  5 ++-

>>  lib/librte_eal/linuxapp/eal/eal.c          |  4 +-

>>  lib/librte_eal/rte_eal_version.map         |  1 +

>>  lib/librte_mbuf/Makefile                   |  1 +

>>  lib/librte_mbuf/rte_mbuf.c                 | 67 ++++++++++++++++++++++++---

>>  lib/librte_mbuf/rte_mbuf.h                 | 72 ++++++++++++++++++++++++++++++

>

> I think the code to manage the user/platform mempool ops could

> go in a separate file. What about rte_mbuf_pool_ops.[ch] ?

>


Good suggestion.
Olivier Matz Jan. 18, 2018, 1:42 p.m. UTC | #3
Hi Hemant,

On Thu, Jan 18, 2018 at 05:17:17PM +0530, Hemant Agrawal wrote:
> On 1/16/2018 8:31 PM, Olivier Matz wrote:

> > On Mon, Jan 15, 2018 at 11:41:09AM +0530, Hemant Agrawal wrote:

> > > W.r.t the multiple discussions in the past about the ability to

> > > dynamically detect the HW mempool support. [1],[2] & [3]

> > > 

> > > This patchset helps in removing the current static mempool selection

> > > model and provides a flexible model to select the pktmbuf mempool

> > > in more dynamic way.

> > > 

> > > 1) This patchset updates the hw mempool on the basis of device probe()),

> > >    thus avoiding the need to specify the hw mempool in config file and

> > >    focing different binaries for diffirent config architectures.

> > > 2) Selection of mempool ops though --mbuf-pool-ops-name (cmd line arg)

> > >    which can overridden the scheme(1)

> > > 3) A new best mempool ops selection logic.

> > > 4) A new wrapper for the pktmbuf_pool_create helper to take mempool ops

> > >    name as an argument as well.

> > > 

> > > *Limitations and open points*

> > > 

> > > It was suggested to add all APIs in librte_mbuf, currently internal_config

> > > is storing the mempool_ops names. So internal_config is exported in this

> > > patchset. An alternate would be to keep these APIs in eal only and access

> > > them indirectly from librte_mbuf.

> > 

> > Instead of storing the mempool_ops name in internal config, can't it be

> > stored inside librte_mbuf? The eal code can call

> > rte_mbuf_set_user_mempool_ops(name) while parsing the arguments.

> 

> I doubt that eal can call mbuf APIs. It will be a issue in shared build?


You are right.

The proper way is maybe to keep the parsing in eal, and at librte_mbuf
initialization, query the eal library to get the user pool if any.
After that, all will be managed inside librte_mbuf. So the eal lib will
only do the argument parsing.

This is maybe what you already did in the v3 you just submitted, I'll
manage to have a look at it today.

Thanks,
Olivier