[v2,2/5] eal: add platform mempool ops name in internal config

Message ID 1515996674-26338-3-git-send-email-hemant.agrawal@nxp.com
State New
Headers show
Series
  • Dynamic HW Mempool Detection Support
Related show

Commit Message

Hemant Agrawal Jan. 15, 2018, 6:11 a.m.
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

---
 lib/librte_eal/common/eal_common_options.c | 1 +
 lib/librte_eal/common/eal_internal_cfg.h   | 2 ++
 lib/librte_eal/rte_eal_version.map         | 1 +
 3 files changed, 4 insertions(+)

-- 
2.7.4

Comments

Jerin Jacob Jan. 15, 2018, 12:24 p.m. | #1
-----Original Message-----
> Date: Mon, 15 Jan 2018 11:41:11 +0530

> From: Hemant Agrawal <hemant.agrawal@nxp.com>

> To: dev@dpdk.org

> CC: jerin.jacob@caviumnetworks.com, olivier.matz@6wind.com,

>  santosh.shukla@caviumnetworks.com

> Subject: [PATCH v2 2/5] eal: add platform mempool ops name in internal

>  config

> X-Mailer: git-send-email 2.7.4

> 

> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

> ---

>  lib/librte_eal/common/eal_common_options.c | 1 +

>  lib/librte_eal/common/eal_internal_cfg.h   | 2 ++

>  lib/librte_eal/rte_eal_version.map         | 1 +

>  3 files changed, 4 insertions(+)

> 

> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c

> index b6d2762..1ed0ddb 100644

> --- a/lib/librte_eal/common/eal_common_options.c

> +++ b/lib/librte_eal/common/eal_common_options.c

> @@ -219,6 +219,7 @@ eal_reset_internal_config(struct internal_config *internal_cfg)

>  	internal_cfg->vmware_tsc_map = 0;

>  	internal_cfg->create_uio_dev = 0;

>  	internal_cfg->user_mbuf_pool_ops_name = NULL;

> +	internal_cfg->plat_mbuf_pool_ops_name = NULL;

>  }

>  

>  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.

>  	rte_hypervisor_get;

>  	rte_hypervisor_get_name;

>  

> -- 

> 2.7.4

>
Hemant Agrawal Jan. 15, 2018, 2:31 p.m. | #2
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?


>

>>  	rte_hypervisor_get;

>>  	rte_hypervisor_get_name;

>>

>> --

>> 2.7.4

>>

>
Jerin Jacob Jan. 15, 2018, 4:26 p.m. | #3
-----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

> > > 

> > 

>
Olivier Matz Jan. 16, 2018, 3:04 p.m. | #4
On Mon, Jan 15, 2018 at 09:56:36PM +0530, Jerin Jacob wrote:
> -----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.


What about a new API in librte_mbuf as suggested as a reply to the cover
letter?
Jerin Jacob Jan. 16, 2018, 3:08 p.m. | #5
-----Original Message-----
> Date: Tue, 16 Jan 2018 16:04:20 +0100

> From: Olivier Matz <olivier.matz@6wind.com>

> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> CC: Hemant Agrawal <hemant.agrawal@nxp.com>, dev@dpdk.org,

>  santosh.shukla@caviumnetworks.com

> Subject: Re: [PATCH v2 2/5] eal: add platform mempool ops name in internal

>  config

> User-Agent: NeoMutt/20170113 (1.7.2)

> 

> On Mon, Jan 15, 2018 at 09:56:36PM +0530, Jerin Jacob wrote:

> > -----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.

> 

> What about a new API in librte_mbuf as suggested as a reply to the cover

> letter?


Looks good to me.

Patch

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index b6d2762..1ed0ddb 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -219,6 +219,7 @@  eal_reset_internal_config(struct internal_config *internal_cfg)
 	internal_cfg->vmware_tsc_map = 0;
 	internal_cfg->create_uio_dev = 0;
 	internal_cfg->user_mbuf_pool_ops_name = NULL;
+	internal_cfg->plat_mbuf_pool_ops_name = NULL;
 }
 
 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;
 	rte_hypervisor_get;
 	rte_hypervisor_get_name;