Re: [dpdk-dev] [PATCH v2 2/5] eal: add platform mempool ops name in internal config

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Mon, 15 Jan 2018 20:01:14 +0530
> From: Hemant Agrawal <hemant.agrawal@nxp.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: dev@dpdk.org, olivier.matz@6wind.com, santosh.shukla@caviumnetworks.com
> Subject: Re: [PATCH v2 2/5] eal: add platform mempool ops name in internal
>  config
> User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101
>  Thunderbird/45.8.0
> 
> On 1/15/2018 5:54 PM, Jerin Jacob wrote:
> > >  static int
> > > diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> > > index 1169fcc..12c5b8a 100644
> > > --- a/lib/librte_eal/common/eal_internal_cfg.h
> > > +++ b/lib/librte_eal/common/eal_internal_cfg.h
> > > @@ -54,6 +54,8 @@ struct internal_config {
> > >  	const char *hugepage_dir;         /**< specific hugetlbfs directory to use */
> > >  	const char *user_mbuf_pool_ops_name;
> > >  			/**< user defined mbuf pool ops name */
> > > +	const char *plat_mbuf_pool_ops_name;
> > > +			/**< platform configured mbuf pool ops name */
> > >  	unsigned num_hugepage_sizes;      /**< how many sizes on this system */
> > >  	struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
> > >  };
> > > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> > > index 3fa1e13..909691f 100644
> > > --- a/lib/librte_eal/rte_eal_version.map
> > > +++ b/lib/librte_eal/rte_eal_version.map
> > > @@ -203,6 +203,7 @@ DPDK_17.11 {
> > >  DPDK_18.02 {
> > >  	global:
> > > 
> > > +	internal_config;
> > 
> > I think, exposing the internal_config may not be a good idea. We may
> > need "plat_mbuf_pool_ops_name" value for multi process case too.
> > Considering the above points, How about adding it in
> > struct rte_config and then expose too rte_eal_get_configuration()
> > On the downside, it would be an ABI change.
> 
> Yes! I was also not sure about exposing internal_config.
> 
> rte_config is also a good option. If  we add these options in the end, it
> should not break ABI?

I think, it does break the ABI.

> 
> 
> > 
> > >  	rte_hypervisor_get;
> > >  	rte_hypervisor_get_name;
> > > 
> > > --
> > > 2.7.4
> > > 
> > 
>