| Message ID | 1516281992-6873-3-git-send-email-hemant.agrawal@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | Dynamic HW Mempool Detection Support | expand |
On Thu, Jan 18, 2018 at 06:56:27PM +0530, Hemant Agrawal wrote: > Add new API to set the user defined mbuf mempool ops name > i.e. set the provided ops name to `internal_config.mbuf_pool_ops_name`. > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com> > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > --- > lib/librte_eal/bsdapp/eal/eal.c | 6 ++++++ > lib/librte_eal/common/include/rte_eal.h | 9 +++++++++ > lib/librte_eal/linuxapp/eal/eal.c | 6 ++++++ > lib/librte_eal/rte_eal_version.map | 1 + > 4 files changed, 22 insertions(+) > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c > index c602d02..64f010a 100644 > --- a/lib/librte_eal/bsdapp/eal/eal.c > +++ b/lib/librte_eal/bsdapp/eal/eal.c > @@ -117,6 +117,12 @@ rte_eal_mbuf_default_mempool_ops(void) > return internal_config.user_mbuf_pool_ops_name; > } > > +void > +rte_eal_set_mbuf_user_mempool_ops(const char *ops_name) > +{ > + internal_config.user_mbuf_pool_ops_name = ops_name; > +} > + I think we should only have the "set" API in mbuf lib. What do you think about what I suggested in http://dpdk.org/ml/archives/dev/2018-January/087419.html ? """ 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. """ In that case, this patch could be dropped. Please refer to my comment in patch 3 to see what other modifications would be needed.
Hi Olivier, On 1/19/2018 3:31 PM, Olivier Matz wrote: > On Thu, Jan 18, 2018 at 06:56:27PM +0530, Hemant Agrawal wrote: >> Add new API to set the user defined mbuf mempool ops name >> i.e. set the provided ops name to `internal_config.mbuf_pool_ops_name`. >> >> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> >> --- >> lib/librte_eal/bsdapp/eal/eal.c | 6 ++++++ >> lib/librte_eal/common/include/rte_eal.h | 9 +++++++++ >> lib/librte_eal/linuxapp/eal/eal.c | 6 ++++++ >> lib/librte_eal/rte_eal_version.map | 1 + >> 4 files changed, 22 insertions(+) >> >> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c >> index c602d02..64f010a 100644 >> --- a/lib/librte_eal/bsdapp/eal/eal.c >> +++ b/lib/librte_eal/bsdapp/eal/eal.c >> @@ -117,6 +117,12 @@ rte_eal_mbuf_default_mempool_ops(void) >> return internal_config.user_mbuf_pool_ops_name; >> } >> >> +void >> +rte_eal_set_mbuf_user_mempool_ops(const char *ops_name) >> +{ >> + internal_config.user_mbuf_pool_ops_name = ops_name; >> +} >> + > > I think we should only have the "set" API in mbuf lib. > > What do you think about what I suggested in > http://dpdk.org/ml/archives/dev/2018-January/087419.html ? > > """ > 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. > """ > Will you please help me in understanding, how should I do it? 1. The is no standard librte_mbuf initialization routine. RTE_INIT will not work, as it will be executed before eal_init. dlopen is for shared lib only. Is there any other method? 2. If I call it on the very first call of rte_mbuf_user_mempool_ops_name or rte_mbuf_set_user_mempool_ops_name, I will be checking against NULL. It can be NULL always. So, no real meaning of maintaining it in librte_mbuf as well. Yes, I can maintain a flag to know, If I have synced with eal parse before. But that is not sounding to me clean. > In that case, this patch could be dropped. Please refer to my comment > in patch 3 to see what other modifications would be needed. >
On Fri, Jan 19, 2018 at 06:01:55PM +0530, Hemant Agrawal wrote: > Hi Olivier, > > On 1/19/2018 3:31 PM, Olivier Matz wrote: > > On Thu, Jan 18, 2018 at 06:56:27PM +0530, Hemant Agrawal wrote: > > > Add new API to set the user defined mbuf mempool ops name > > > i.e. set the provided ops name to `internal_config.mbuf_pool_ops_name`. > > > > > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com> > > > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > > > --- > > > lib/librte_eal/bsdapp/eal/eal.c | 6 ++++++ > > > lib/librte_eal/common/include/rte_eal.h | 9 +++++++++ > > > lib/librte_eal/linuxapp/eal/eal.c | 6 ++++++ > > > lib/librte_eal/rte_eal_version.map | 1 + > > > 4 files changed, 22 insertions(+) > > > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c > > > index c602d02..64f010a 100644 > > > --- a/lib/librte_eal/bsdapp/eal/eal.c > > > +++ b/lib/librte_eal/bsdapp/eal/eal.c > > > @@ -117,6 +117,12 @@ rte_eal_mbuf_default_mempool_ops(void) > > > return internal_config.user_mbuf_pool_ops_name; > > > } > > > > > > +void > > > +rte_eal_set_mbuf_user_mempool_ops(const char *ops_name) > > > +{ > > > + internal_config.user_mbuf_pool_ops_name = ops_name; > > > +} > > > + > > > > I think we should only have the "set" API in mbuf lib. > > > > What do you think about what I suggested in > > http://dpdk.org/ml/archives/dev/2018-January/087419.html ? > > > > """ > > 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. > > """ > > > > Will you please help me in understanding, how should I do it? > > 1. The is no standard librte_mbuf initialization routine. RTE_INIT will not > work, as it will be executed before eal_init. dlopen is for shared lib only. > Is there any other method? > > 2. If I call it on the very first call of rte_mbuf_user_mempool_ops_name or > rte_mbuf_set_user_mempool_ops_name, I will be checking against NULL. It can > be NULL always. So, no real meaning of maintaining it in librte_mbuf as > well. > Yes, I can maintain a flag to know, If I have synced with eal parse before. > But that is not sounding to me clean. My initial idea was to do it at init, but I came to the same conclusion than yours, it is not possible. Can you take a look to the comments of patch 3/7 and let me know if the proposition fits your needs?
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index c602d02..64f010a 100644 --- a/lib/librte_eal/bsdapp/eal/eal.c +++ b/lib/librte_eal/bsdapp/eal/eal.c @@ -117,6 +117,12 @@ rte_eal_mbuf_default_mempool_ops(void) return internal_config.user_mbuf_pool_ops_name; } +void +rte_eal_set_mbuf_user_mempool_ops(const char *ops_name) +{ + internal_config.user_mbuf_pool_ops_name = ops_name; +} + /* Return a pointer to the configuration structure */ struct rte_config * rte_eal_get_configuration(void) diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h index 2aba2c8..7645b34 100644 --- a/lib/librte_eal/common/include/rte_eal.h +++ b/lib/librte_eal/common/include/rte_eal.h @@ -307,6 +307,15 @@ enum rte_iova_mode rte_eal_iova_mode(void); const char * rte_eal_mbuf_default_mempool_ops(void); +/** + * Set user pool ops name for mbuf + * + * @param ops_name + * mempool ops name that is to be set as user defined. + */ +void +rte_eal_set_mbuf_user_mempool_ops(const char *ops_name); + #ifdef __cplusplus } #endif diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c index e8c7100..46b2bb3 100644 --- a/lib/librte_eal/linuxapp/eal/eal.c +++ b/lib/librte_eal/linuxapp/eal/eal.c @@ -127,6 +127,12 @@ rte_eal_mbuf_default_mempool_ops(void) return internal_config.user_mbuf_pool_ops_name; } +void +rte_eal_set_mbuf_user_mempool_ops(const char *ops_name) +{ + internal_config.user_mbuf_pool_ops_name = ops_name; +} + /* Return a pointer to the configuration structure */ struct rte_config * rte_eal_get_configuration(void) diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map index 7088b72..3529885 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: + rte_eal_set_mbuf_user_mempool_ops; rte_hypervisor_get; rte_hypervisor_get_name; rte_vfio_clear_group;