[dpdk-dev,v2,2/5] eal: add platform mempool ops name in internal config
Checks
Commit Message
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(+)
Comments
-----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
>
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
>>
>
-----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
> > >
> >
>
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?
-----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.
@@ -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
@@ -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];
};
@@ -203,6 +203,7 @@ DPDK_17.11 {
DPDK_18.02 {
global:
+ internal_config;
rte_hypervisor_get;
rte_hypervisor_get_name;