From: Olivier MATZ <olivier.matz@6wind.com>
To: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Cc: Santosh Shukla <santosh.shukla@caviumnetworks.com>,
dev@dpdk.org, thomas@monjalon.net,
jerin.jacob@caviumnetworks.com, hemant.agrawal@nxp.com
Subject: Re: [dpdk-dev] [PATCH v3 0/2] Dynamically configure mempool handle
Date: Tue, 5 Sep 2017 09:47:26 +0200 [thread overview]
Message-ID: <20170905074724.lmo3xitraq4cxwg2@neon> (raw)
In-Reply-To: <1b3482e9-7e49-5f15-afc9-56c0c098c1ab@intel.com>
On Mon, Sep 04, 2017 at 03:24:38PM +0100, Sergio Gonzalez Monroy wrote:
> Hi Olivier,
>
> On 04/09/2017 14:34, Olivier MATZ wrote:
> > Hi Sergio,
> >
> > On Mon, Sep 04, 2017 at 10:41:56AM +0100, Sergio Gonzalez Monroy wrote:
> > > On 15/08/2017 09:07, Santosh Shukla wrote:
> > > > * Application programming sequencing would be
> > > > char pref_mempool[RTE_MEMPOOL_OPS_NAMESIZE];
> > > > rte_eth_dev_get_preferred_pool_ops(ethdev_port_id, pref_mempool /* out */);
> > > > rte_mempool_create_empty();
> > > > rte_mempool_set_ops_byname( , pref_memppol, );
> > > > rte_mempool_populate_default();
> > > What about introducing an API like:
> > > rte_pktmbuf_poll_create_with_ops (..., ops_name, config_pool);
> > >
> > > I think that API would help for the case the application wants an mbuf pool
> > > with ie. stack handler.
> > > Sure we can do the empty/set_ops/populate sequence, but the only thing we
> > > want to change from default pktmbuf_pool_create API is the pool handler.
> > >
> > > Application just needs to decide the ops handler to use, either default or
> > > one suggested by PMD?
> > >
> > > I think ideally we would have similar APIs:
> > > - rte_mempool_create_with_ops (...)
> > > - rte_memppol_xmem_create_with_ops (...)
> > Today, we may only want to change the mempool handler, but if we
> > need to change something else tomorrow, we would need to add another
> > parameter again, breaking the ABI.
> >
> > If we pass a config structure, adding a new field in it would also break the
> > ABI, except if the structure is opaque, with accessors. These accessors would be
> > functions (ex: mempool_cfg_new, mempool_cfg_set_pool_ops, ...). This is not so
> > much different than what we have now.
> >
> > The advantage I can see of working on a config structure instead of directly on
> > a mempool is that the api can be reused to build a default config.
> >
> > That said, I think it's quite orthogonal to this patch since we still require
> > the ethdev api.
>
> Fair enough.
>
> Just to double check that we are on the same page:
> - rte_mempool_create is just there for backwards compatibility and a
> sequence of create_empty -> set_ops (optional) ->init -> populate_default ->
> obj_iter (optional) is the recommended way of creating mempools.
Yes, I think rte_mempool_create() has too many arguments.
> - if application wants to use rte_mempool_xmem_create with different pool
> handler needs to replicate function and add set_ops step.
Yes. And now that xen support is going to be removed, I'm wondering if
this function is still required, given the API is not that easy to
use. Calling rte_mempool_populate_phys() several times looks more
flexible. But this is also another topic.
> Now if rte_pktmbuf_pool_create is still the preferred mechanism for
> applications to create mbuf pools, wouldn't it make sense to offer the
> option of either pass the ops_name as suggested before or for the
> application to just set a different pool handler? I understand the it is
> either breaking API or introducing a new API, but is the solution to
> basically "copy" the whole function in the application and add an optional
> step (set_ops)?
I was quite reticent about introducing
rte_pktmbuf_pool_create_with_ops() because for the same reasons, we
would also want to introduce functions to create a mempool that use a
different pktmbuf_init() or pool_init() callback, or to create the pool
in external memory, ... and we would end up with a functions with too
many arguments.
I like the approach of having several simple functions, because it's
easier to read (even if the code is longer), and it's easily extensible.
Now if we feel that the mempool ops is more important than the other
parameters, we can consider to add it in rte_pktmbuf_pool_create().
> With these patches we can:
> - change the default pool handler with EAL option, which does *not* allow
> for pktmbuf_pool_create with different handlers.
> - We can check the PMDs preferred/supported handlers but then we need to
> implement function with whole sequence, cannot use pktmbuf_pool_create.
>
> It looks to me then that any application that wants to use different pool
> handlers than default/ring need to implement their own create_pool with
> set_ops.
next prev parent reply other threads:[~2017-09-05 7:47 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 " 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 [this message]
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
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=20170905074724.lmo3xitraq4cxwg2@neon \
--to=olivier.matz@6wind.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=santosh.shukla@caviumnetworks.com \
--cc=sergio.gonzalez.monroy@intel.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).