| Message ID | 1515996674-26338-4-git-send-email-hemant.agrawal@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | Dynamic HW Mempool Detection Support | expand |
-----Original Message----- > Date: Mon, 15 Jan 2018 11:41:12 +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 3/5] mbuf: support register mempool Hw ops name APIs > X-Mailer: git-send-email 2.7.4 > > With this patch the specific HW mempool are no longer required to be > specified in the config file at compile. A default platform hw mempool > can be detected dynamically and published to config at run time. > Only one type of HW mempool can be active default. > > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > --- > lib/librte_mbuf/Makefile | 1 + > lib/librte_mbuf/rte_mbuf.c | 42 +++++++++++++++++++++++++++++++++--- > lib/librte_mbuf/rte_mbuf.h | 20 +++++++++++++++++ > lib/librte_mbuf/rte_mbuf_version.map | 8 +++++++ > 4 files changed, 68 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_mbuf/Makefile b/lib/librte_mbuf/Makefile > index 398f724..85c4f9e 100644 > --- a/lib/librte_mbuf/Makefile > +++ b/lib/librte_mbuf/Makefile > @@ -7,6 +7,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > LIB = librte_mbuf.a > > CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 > +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common > LDLIBS += -lrte_eal -lrte_mempool > > EXPORT_MAP := rte_mbuf_version.map > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index c085c37..fd3b6f5 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -58,6 +58,7 @@ > #include <rte_hexdump.h> > #include <rte_errno.h> > #include <rte_memcpy.h> > +#include <eal_internal_cfg.h> > > /* > * ctrlmbuf constructor, given as a callback function to > @@ -148,6 +149,43 @@ rte_pktmbuf_init(struct rte_mempool *mp, > m->next = NULL; > } > > +int > +rte_mbuf_register_platform_mempool_ops(const char *ops_name) > +{ Should we also check the following? if (internal_config.plat_mbuf_pool_ops_name != NULL && strncmp(internal_config.plat_mbuf_pool_ops_name, ops_name, ..) == 0) return 0; i.e avoid returning error if the same the driver but another instance(different ethdev port) updates the same ops_name value.
-----Original Message----- > Date: Mon, 15 Jan 2018 11:41:12 +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 3/5] mbuf: support register mempool Hw ops name APIs > X-Mailer: git-send-email 2.7.4 > > With this patch the specific HW mempool are no longer required to be > specified in the config file at compile. A default platform hw mempool > can be detected dynamically and published to config at run time. > Only one type of HW mempool can be active default. > > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > --- > lib/librte_mbuf/Makefile | 1 + > lib/librte_mbuf/rte_mbuf.c | 42 +++++++++++++++++++++++++++++++++--- > lib/librte_mbuf/rte_mbuf.h | 20 +++++++++++++++++ > lib/librte_mbuf/rte_mbuf_version.map | 8 +++++++ > 4 files changed, 68 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_mbuf/Makefile b/lib/librte_mbuf/Makefile > index 398f724..85c4f9e 100644 > --- a/lib/librte_mbuf/Makefile > +++ b/lib/librte_mbuf/Makefile > @@ -7,6 +7,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > LIB = librte_mbuf.a > > CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 > +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common > LDLIBS += -lrte_eal -lrte_mempool > > EXPORT_MAP := rte_mbuf_version.map > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index c085c37..fd3b6f5 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -58,6 +58,7 @@ > #include <rte_hexdump.h> > #include <rte_errno.h> > #include <rte_memcpy.h> > +#include <eal_internal_cfg.h> > > /* > * ctrlmbuf constructor, given as a callback function to > @@ -148,6 +149,43 @@ rte_pktmbuf_init(struct rte_mempool *mp, > m->next = NULL; > } > > +int > +rte_mbuf_register_platform_mempool_ops(const char *ops_name) > +{ > + if (internal_config.plat_mbuf_pool_ops_name == NULL) { > + internal_config.plat_mbuf_pool_ops_name = ops_name; > + return 0; > + } > + RTE_LOG(ERR, MBUF, > + "%s is already registered as platform mbuf pool ops\n", > + internal_config.plat_mbuf_pool_ops_name); > + return -EACCES; > +} > + > +const char * > +rte_mbuf_platform_mempool_ops(void) > +{ > + return internal_config.plat_mbuf_pool_ops_name; > +} > + > +/* Return mbuf pool ops name */ > +static const char * > +rte_mbuf_best_mempool_ops(void) > +{ > + /* User defined mempool ops takes the priority */ > + const char *best_ops = rte_eal_mbuf_default_mempool_ops(); > + if (best_ops) > + return best_ops; > + > + /* Next choice is platform configured mempool ops */ > + best_ops = rte_mbuf_platform_mempool_ops(); > + if (best_ops) > + return best_ops; > + > + /* Last choice is to use the compile time config pool */ > + return RTE_MBUF_DEFAULT_MEMPOOL_OPS; > +} The mempool ops selection makes sense to me. We could add an "weight" based mempool ops selection in future here. http://dpdk.org/dev/patchwork/patch/32245/
On 1/15/2018 5:11 PM, Jerin Jacob wrote: >> >> +int >> +rte_mbuf_register_platform_mempool_ops(const char *ops_name) >> +{ > > Should we also check the following? > > if (internal_config.plat_mbuf_pool_ops_name != NULL && > strncmp(internal_config.plat_mbuf_pool_ops_name, ops_name, ..) == 0) > return 0; > > i.e avoid returning error if the same the driver but another > instance(different ethdev port) updates the same ops_name value. > No, there shall be only one default platform hw pool name. Any ethdev driven (may be weight based) replacement shall be dealt separately.
-----Original Message----- > Date: Mon, 15 Jan 2018 19:54:36 +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 3/5] mbuf: support register mempool Hw ops name APIs > User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 > Thunderbird/45.8.0 > > On 1/15/2018 5:11 PM, Jerin Jacob wrote: > > > > > > +int > > > +rte_mbuf_register_platform_mempool_ops(const char *ops_name) > > > +{ > > > > Should we also check the following? > > > > if (internal_config.plat_mbuf_pool_ops_name != NULL && > > strncmp(internal_config.plat_mbuf_pool_ops_name, ops_name, ..) == 0) > > return 0; > > > > i.e avoid returning error if the same the driver but another > > instance(different ethdev port) updates the same ops_name value. > > > > No, there shall be only one default platform hw pool name. > Any ethdev driven (may be weight based) replacement shall be dealt > separately. OK. Who calls the rte_mbuf_register_platform_mempool_ops() and when? mempool ops registration is using gcc construction scheme so it will be called all the time. I thought, ethdev drivers will call this API on pci or any other bus probe() and if its in probe() then rte_mbuf_register_platform_mempool_ops() will be called multiple time. What is your thought on this API usage? >
On Mon, Jan 15, 2018 at 11:41:12AM +0530, Hemant Agrawal wrote: > With this patch the specific HW mempool are no longer required to be > specified in the config file at compile. A default platform hw mempool > can be detected dynamically and published to config at run time. > Only one type of HW mempool can be active default. > > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> > --- > lib/librte_mbuf/Makefile | 1 + > lib/librte_mbuf/rte_mbuf.c | 42 +++++++++++++++++++++++++++++++++--- > lib/librte_mbuf/rte_mbuf.h | 20 +++++++++++++++++ > lib/librte_mbuf/rte_mbuf_version.map | 8 +++++++ > 4 files changed, 68 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_mbuf/Makefile b/lib/librte_mbuf/Makefile > index 398f724..85c4f9e 100644 > --- a/lib/librte_mbuf/Makefile > +++ b/lib/librte_mbuf/Makefile > @@ -7,6 +7,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > LIB = librte_mbuf.a > > CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 > +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common > LDLIBS += -lrte_eal -lrte_mempool > > EXPORT_MAP := rte_mbuf_version.map > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index c085c37..fd3b6f5 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -58,6 +58,7 @@ > #include <rte_hexdump.h> > #include <rte_errno.h> > #include <rte_memcpy.h> > +#include <eal_internal_cfg.h> > > /* > * ctrlmbuf constructor, given as a callback function to > @@ -148,6 +149,43 @@ rte_pktmbuf_init(struct rte_mempool *mp, > m->next = NULL; > } > > +int > +rte_mbuf_register_platform_mempool_ops(const char *ops_name) > +{ > + if (internal_config.plat_mbuf_pool_ops_name == NULL) { > + internal_config.plat_mbuf_pool_ops_name = ops_name; > + return 0; > + } > + RTE_LOG(ERR, MBUF, > + "%s is already registered as platform mbuf pool ops\n", > + internal_config.plat_mbuf_pool_ops_name); > + return -EACCES; > +} > + > +const char * > +rte_mbuf_platform_mempool_ops(void) > +{ > + return internal_config.plat_mbuf_pool_ops_name; > +} > + > +/* Return mbuf pool ops name */ > +static const char * > +rte_mbuf_best_mempool_ops(void) Shall we export this function? > +{ > + /* User defined mempool ops takes the priority */ > + const char *best_ops = rte_eal_mbuf_default_mempool_ops(); > + if (best_ops) > + return best_ops; > + > + /* Next choice is platform configured mempool ops */ > + best_ops = rte_mbuf_platform_mempool_ops(); > + if (best_ops) > + return best_ops; > + > + /* Last choice is to use the compile time config pool */ > + return RTE_MBUF_DEFAULT_MEMPOOL_OPS; > +} > + > /* helper to create a mbuf pool */ > struct rte_mempool * > rte_pktmbuf_pool_create(const char *name, unsigned n, > @@ -176,9 +214,7 @@ rte_pktmbuf_pool_create(const char *name, unsigned n, > if (mp == NULL) > return NULL; > > - mp_ops_name = rte_eal_mbuf_default_mempool_ops(); > - if (mp_ops_name == NULL) > - mp_ops_name = RTE_MBUF_DEFAULT_MEMPOOL_OPS; > + mp_ops_name = rte_mbuf_best_mempool_ops(); > ret = rte_mempool_set_ops_byname(mp, mp_ops_name, NULL); > if (ret != 0) { > RTE_LOG(ERR, MBUF, "error setting mempool handler\n"); > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index e4e3917..d26e8cd 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -1081,6 +1081,26 @@ rte_pktmbuf_pool_create(const char *name, unsigned n, > int socket_id); > > /** > + * Register the platform supported pktmbuf HW pool > + * > + * @param pool ops name description is missing here > + * @return > + * - 0: Success > + * - -EACCES: platform mempool is already registered. Maybe -EEXIST is more appropriate? > + */ > +int > +rte_mbuf_register_platform_mempool_ops(const char *ops_name); > + > +/** > + * Get registered platform supported pool ops name for mbuf > + * > + * @return > + * returns platform pool ops name. > + */ > +const char* I'm surprised that checkpatch does not complain about the missing space :) > +rte_mbuf_platform_mempool_ops(void); > + > +/** > * Get the data room size of mbufs stored in a pktmbuf_pool > * > * The data room size is the amount of data that can be stored in a > diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map > index 6e2ea84..b8e258f 100644 > --- a/lib/librte_mbuf/rte_mbuf_version.map > +++ b/lib/librte_mbuf/rte_mbuf_version.map > @@ -35,3 +35,11 @@ DPDK_16.11 { > rte_get_tx_ol_flag_list; > > } DPDK_2.1; > + > +DPDK_18.02 { > + global: > + > + rte_mbuf_platform_mempool_ops; > + rte_mbuf_register_platform_mempool_ops; > + > +} DPDK_16.11; > -- > 2.7.4 >
diff --git a/lib/librte_mbuf/Makefile b/lib/librte_mbuf/Makefile index 398f724..85c4f9e 100644 --- a/lib/librte_mbuf/Makefile +++ b/lib/librte_mbuf/Makefile @@ -7,6 +7,7 @@ include $(RTE_SDK)/mk/rte.vars.mk LIB = librte_mbuf.a CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common LDLIBS += -lrte_eal -lrte_mempool EXPORT_MAP := rte_mbuf_version.map diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index c085c37..fd3b6f5 100644 --- a/lib/librte_mbuf/rte_mbuf.c +++ b/lib/librte_mbuf/rte_mbuf.c @@ -58,6 +58,7 @@ #include <rte_hexdump.h> #include <rte_errno.h> #include <rte_memcpy.h> +#include <eal_internal_cfg.h> /* * ctrlmbuf constructor, given as a callback function to @@ -148,6 +149,43 @@ rte_pktmbuf_init(struct rte_mempool *mp, m->next = NULL; } +int +rte_mbuf_register_platform_mempool_ops(const char *ops_name) +{ + if (internal_config.plat_mbuf_pool_ops_name == NULL) { + internal_config.plat_mbuf_pool_ops_name = ops_name; + return 0; + } + RTE_LOG(ERR, MBUF, + "%s is already registered as platform mbuf pool ops\n", + internal_config.plat_mbuf_pool_ops_name); + return -EACCES; +} + +const char * +rte_mbuf_platform_mempool_ops(void) +{ + return internal_config.plat_mbuf_pool_ops_name; +} + +/* Return mbuf pool ops name */ +static const char * +rte_mbuf_best_mempool_ops(void) +{ + /* User defined mempool ops takes the priority */ + const char *best_ops = rte_eal_mbuf_default_mempool_ops(); + if (best_ops) + return best_ops; + + /* Next choice is platform configured mempool ops */ + best_ops = rte_mbuf_platform_mempool_ops(); + if (best_ops) + return best_ops; + + /* Last choice is to use the compile time config pool */ + return RTE_MBUF_DEFAULT_MEMPOOL_OPS; +} + /* helper to create a mbuf pool */ struct rte_mempool * rte_pktmbuf_pool_create(const char *name, unsigned n, @@ -176,9 +214,7 @@ rte_pktmbuf_pool_create(const char *name, unsigned n, if (mp == NULL) return NULL; - mp_ops_name = rte_eal_mbuf_default_mempool_ops(); - if (mp_ops_name == NULL) - mp_ops_name = RTE_MBUF_DEFAULT_MEMPOOL_OPS; + mp_ops_name = rte_mbuf_best_mempool_ops(); ret = rte_mempool_set_ops_byname(mp, mp_ops_name, NULL); if (ret != 0) { RTE_LOG(ERR, MBUF, "error setting mempool handler\n"); diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index e4e3917..d26e8cd 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -1081,6 +1081,26 @@ rte_pktmbuf_pool_create(const char *name, unsigned n, int socket_id); /** + * Register the platform supported pktmbuf HW pool + * + * @param pool ops name + * @return + * - 0: Success + * - -EACCES: platform mempool is already registered. + */ +int +rte_mbuf_register_platform_mempool_ops(const char *ops_name); + +/** + * Get registered platform supported pool ops name for mbuf + * + * @return + * returns platform pool ops name. + */ +const char* +rte_mbuf_platform_mempool_ops(void); + +/** * Get the data room size of mbufs stored in a pktmbuf_pool * * The data room size is the amount of data that can be stored in a diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map index 6e2ea84..b8e258f 100644 --- a/lib/librte_mbuf/rte_mbuf_version.map +++ b/lib/librte_mbuf/rte_mbuf_version.map @@ -35,3 +35,11 @@ DPDK_16.11 { rte_get_tx_ol_flag_list; } DPDK_2.1; + +DPDK_18.02 { + global: + + rte_mbuf_platform_mempool_ops; + rte_mbuf_register_platform_mempool_ops; + +} DPDK_16.11;
With this patch the specific HW mempool are no longer required to be specified in the config file at compile. A default platform hw mempool can be detected dynamically and published to config at run time. Only one type of HW mempool can be active default. Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com> --- lib/librte_mbuf/Makefile | 1 + lib/librte_mbuf/rte_mbuf.c | 42 +++++++++++++++++++++++++++++++++--- lib/librte_mbuf/rte_mbuf.h | 20 +++++++++++++++++ lib/librte_mbuf/rte_mbuf_version.map | 8 +++++++ 4 files changed, 68 insertions(+), 3 deletions(-) -- 2.7.4