[dpdk-dev] [PATCH v1 07/28] eal/soc: add rte_eal_soc_register/unregister logic
On Wed, 15 Jun 2016 05:57:33 + Shreyansh Jain wrote: > Hi Jan, > > One more comment which I missed in previous reply: > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shreyansh Jain > > Sent: Monday, June 13, 2016 7:50 PM > > To: Jan Viktorin > > Cc: David Marchand ; Thomas Monjalon > > ; Bruce Richardson > intel.com>; > > Declan Doherty ; jianbo.liu at linaro.org; > > jerin.jacob at caviumnetworks.com; Keith Wiles ; > > Stephen > > Hemminger ; dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v1 07/28] eal/soc: add > > rte_eal_soc_register/unregister logic > > [...] > > > + > > > +/** > > > + * Empty PMD driver based on the SoC infra. > > > + * > > > + * The rte_soc_device is usually wrapped in some higher-level struct > > > + * (eth_driver). We simulate such a wrapper with an anonymous struct > > > here. > > > + */ > > > +struct test_wrapper { > > > + struct rte_soc_driver soc_drv; > > > +}; > > > + > > > +struct test_wrapper empty_pmd0 = { > > > + .soc_drv = { > > > + .name = "empty_pmd0", > > > + }, > > > +}; > > > + > > > +struct test_wrapper empty_pmd1 = { > > > + .soc_drv = { > > > + .name = "empty_pmd1", > > > + }, > > > +}; > > > + > > > +static int > > > +count_registered_socdrvs(void) > > > +{ > > > + int i; > > > + struct rte_soc_driver *drv; > > > + > > > + i = 0; > > > + TAILQ_FOREACH(drv, &soc_driver_list, next) > > > + i += 1; > > > + > > > + return i; > > > +} > > > + > > > +static int > > > +test_register_unregister(void) > > > +{ > > > + struct rte_soc_driver *drv; > > > + int count; > > > + > > > + rte_eal_soc_register(&empty_pmd0.soc_drv); [...] > > > + > > > +extern struct soc_driver_list soc_driver_list; /**< Global list of SoC > > > drivers. */ > > > > > > struct rte_soc_id { > > > const char *compatible; /**< OF compatible specification */ > > > @@ -131,4 +137,21 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr > > *a0, > > > return strcmp(a0->name, a1->name); > > > } > > > > > > +/** > > > + * Register a SoC driver. > > > + */ > > > +void rte_eal_soc_register(struct rte_soc_driver *driver); > > > + > > > +#define RTE_EAL_SOC_REGISTER(name) \ > > > +RTE_INIT(socinitfn_ ##name); \ > > > +static void socinitfn_ ##name(void) \ > > > +{ \ > > > + rte_eal_soc_register(&name.soc_drv); \ > > It should be 'rte_eal_soc_register(&name)'. > As a user of 'RTE_EAL_SOC_REGISTER', I would pass reference to > 'rte_soc_driver' object. It doesn't have any 'soc_drv' member. But eth_driver would have it. And other upper-level structs would have it as well. > > I am guessing that because you have created a wrapper structure > 'test_wrapper' in test_soc.c which contains a 'soc_drv', the macro reflects > that usage. I don't assume to use rte_soc_driver directly (similarly to rte_pci_driver). However, I agree that it is strange, we should have RTE_ETHDRV_SOC_REGISTER for such purpose (if needed). I wanted to avoid redundant arguments to the RTE_EAL_SOC_REGISTER because the name is used to create the constructor function. But, it seems that other parts of DPDK does not care of this so I will probably give up and make it: RTE_EAL_SOC_REGISTER(my_cool_drv, &my_cool_drv.soc_drv); Thanks for your opinion. I'll fix it in v2. Jan > [...] > > - > Shreyansh > -- Jan Viktorin E-mail: Viktorin at RehiveTech.com System Architect Web:www.RehiveTech.com RehiveTech Brno, Czech Republic
[dpdk-dev] [PATCH v1 07/28] eal/soc: add rte_eal_soc_register/unregister logic
Hi Jan, One more comment which I missed in previous reply: > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shreyansh Jain > Sent: Monday, June 13, 2016 7:50 PM > To: Jan Viktorin > Cc: David Marchand ; Thomas Monjalon > ; Bruce Richardson intel.com>; > Declan Doherty ; jianbo.liu at linaro.org; > jerin.jacob at caviumnetworks.com; Keith Wiles ; > Stephen > Hemminger ; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1 07/28] eal/soc: add > rte_eal_soc_register/unregister logic > > Another trivial comment inlined: > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Viktorin > > Sent: Friday, May 06, 2016 7:18 PM > > To: dev at dpdk.org > > Cc: Jan Viktorin ; David Marchand > > ; Thomas Monjalon > 6wind.com>; > > Bruce Richardson ; Declan Doherty > > ; jianbo.liu at linaro.org; > > jerin.jacob at caviumnetworks.com; Keith Wiles ; > Stephen > > Hemminger > > Subject: [dpdk-dev] [PATCH v1 07/28] eal/soc: add > > rte_eal_soc_register/unregister logic > > > > Signed-off-by: Jan Viktorin > > --- > > app/test/test_soc.c | 106 > > > > lib/librte_eal/bsdapp/eal/Makefile | 1 + > > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 3 + > > lib/librte_eal/common/eal_common_soc.c | 55 > > lib/librte_eal/common/include/rte_soc.h | 23 + > > lib/librte_eal/linuxapp/eal/Makefile| 1 + > > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 + > > 7 files changed, 193 insertions(+) > > create mode 100644 lib/librte_eal/common/eal_common_soc.c > > > > diff --git a/app/test/test_soc.c b/app/test/test_soc.c > > index a49fc9b..f6288dc 100644 > > --- a/app/test/test_soc.c > > +++ b/app/test/test_soc.c > > @@ -74,6 +74,103 @@ static int test_compare_addr(void) > > free(a2.name); > > free(a1.name); > > free(a0.name); > > + > > + return 0; > > +} > > + > > +/** > > + * Empty PMD driver based on the SoC infra. > > + * > > + * The rte_soc_device is usually wrapped in some higher-level struct > > + * (eth_driver). We simulate such a wrapper with an anonymous struct here. > > + */ > > +struct test_wrapper { > > + struct rte_soc_driver soc_drv; > > +}; > > + > > +struct test_wrapper empty_pmd0 = { > > + .soc_drv = { > > + .name = "empty_pmd0", > > + }, > > +}; > > + > > +struct test_wrapper empty_pmd1 = { > > + .soc_drv = { > > + .name = "empty_pmd1", > > + }, > > +}; > > + > > +static int > > +count_registered_socdrvs(void) > > +{ > > + int i; > > + struct rte_soc_driver *drv; > > + > > + i = 0; > > + TAILQ_FOREACH(drv, &soc_driver_list, next) > > + i += 1; > > + > > + return i; > > +} > > + > > +static int > > +test_register_unregister(void) > > +{ > > + struct rte_soc_driver *drv; > > + int count; > > + > > + rte_eal_soc_register(&empty_pmd0.soc_drv); > > + > > + TEST_ASSERT(!TAILQ_EMPTY(&soc_driver_list), > > + "No PMD is present but the empty_pmd0 should be there"); > > + drv = TAILQ_FIRST(&soc_driver_list); > > + TEST_ASSERT(!strcmp(drv->name, "empty_pmd0"), > > + "The registered PMD is not empty_pmd but '%s'", drv->name); > > Trivial: TEST_ASSERT Message should be: "... is not empty_pmd0 but..." > > > + > > + rte_eal_soc_register(&empty_pmd1.soc_drv); > > + > > + count = count_registered_socdrvs(); > > + TEST_ASSERT_EQUAL(count, 2, "Expected 2 PMDs but detected %d", count); > > + > > + rte_eal_soc_unregister(&empty_pmd0.soc_drv); > > + count = count_registered_socdrvs(); > > + TEST_ASSERT_EQUAL(count, 1, "Expected 1 PMDs but detected %d", count); > > + > > + rte_eal_soc_unregister(&empty_pmd1.soc_drv); > > + > > + printf("%s has been successful\n", __func__); > > + return 0; > > +} > > + > > +/* save real devices and drivers until the tests finishes */ > > +struct soc_driver_list real_soc_driver_list = > > + TAILQ_HEAD_INITIALIZER(real_soc_driver_list); > > + > > +static int test_soc_setup(void) > > +{ > > + struct rte_soc_driver *drv; > > + > &
[dpdk-dev] [PATCH v1 07/28] eal/soc: add rte_eal_soc_register/unregister logic
On Mon, 13 Jun 2016 14:19:39 + Shreyansh Jain wrote: > Another trivial comment inlined: > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Viktorin > > Sent: Friday, May 06, 2016 7:18 PM > > To: dev at dpdk.org > > Cc: Jan Viktorin ; David Marchand > > ; Thomas Monjalon > 6wind.com>; > > Bruce Richardson ; Declan Doherty > > ; jianbo.liu at linaro.org; > > jerin.jacob at caviumnetworks.com; Keith Wiles ; > > Stephen > > Hemminger > > Subject: [dpdk-dev] [PATCH v1 07/28] eal/soc: add > > rte_eal_soc_register/unregister logic > > > > Signed-off-by: Jan Viktorin > > --- > > app/test/test_soc.c | 106 > > > > lib/librte_eal/bsdapp/eal/Makefile | 1 + > > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 3 + > > lib/librte_eal/common/eal_common_soc.c | 55 > > lib/librte_eal/common/include/rte_soc.h | 23 + > > lib/librte_eal/linuxapp/eal/Makefile| 1 + > > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 + > > 7 files changed, 193 insertions(+) > > create mode 100644 lib/librte_eal/common/eal_common_soc.c > > > > diff --git a/app/test/test_soc.c b/app/test/test_soc.c > > index a49fc9b..f6288dc 100644 > > --- a/app/test/test_soc.c > > +++ b/app/test/test_soc.c > > @@ -74,6 +74,103 @@ static int test_compare_addr(void) > > free(a2.name); > > free(a1.name); > > free(a0.name); > > + > > + return 0; > > +} > > + > > +/** > > + * Empty PMD driver based on the SoC infra. > > + * > > + * The rte_soc_device is usually wrapped in some higher-level struct > > + * (eth_driver). We simulate such a wrapper with an anonymous struct here. > > + */ > > +struct test_wrapper { > > + struct rte_soc_driver soc_drv; > > +}; > > + > > +struct test_wrapper empty_pmd0 = { > > + .soc_drv = { > > + .name = "empty_pmd0", > > + }, > > +}; > > + > > +struct test_wrapper empty_pmd1 = { > > + .soc_drv = { > > + .name = "empty_pmd1", > > + }, > > +}; > > + > > +static int > > +count_registered_socdrvs(void) > > +{ > > + int i; > > + struct rte_soc_driver *drv; > > + > > + i = 0; > > + TAILQ_FOREACH(drv, &soc_driver_list, next) > > + i += 1; > > + > > + return i; > > +} > > + > > +static int > > +test_register_unregister(void) > > +{ > > + struct rte_soc_driver *drv; > > + int count; > > + > > + rte_eal_soc_register(&empty_pmd0.soc_drv); > > + > > + TEST_ASSERT(!TAILQ_EMPTY(&soc_driver_list), > > + "No PMD is present but the empty_pmd0 should be there"); > > + drv = TAILQ_FIRST(&soc_driver_list); > > + TEST_ASSERT(!strcmp(drv->name, "empty_pmd0"), > > + "The registered PMD is not empty_pmd but '%s'", drv->name); > > Trivial: TEST_ASSERT Message should be: "... is not empty_pmd0 but..." OK, thanks. > [...]
[dpdk-dev] [PATCH v1 07/28] eal/soc: add rte_eal_soc_register/unregister logic
Another trivial comment inlined: > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Viktorin > Sent: Friday, May 06, 2016 7:18 PM > To: dev at dpdk.org > Cc: Jan Viktorin ; David Marchand > ; Thomas Monjalon ; > Bruce Richardson ; Declan Doherty > ; jianbo.liu at linaro.org; > jerin.jacob at caviumnetworks.com; Keith Wiles ; > Stephen > Hemminger > Subject: [dpdk-dev] [PATCH v1 07/28] eal/soc: add > rte_eal_soc_register/unregister logic > > Signed-off-by: Jan Viktorin > --- > app/test/test_soc.c | 106 > > lib/librte_eal/bsdapp/eal/Makefile | 1 + > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 3 + > lib/librte_eal/common/eal_common_soc.c | 55 > lib/librte_eal/common/include/rte_soc.h | 23 + > lib/librte_eal/linuxapp/eal/Makefile| 1 + > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 + > 7 files changed, 193 insertions(+) > create mode 100644 lib/librte_eal/common/eal_common_soc.c > > diff --git a/app/test/test_soc.c b/app/test/test_soc.c > index a49fc9b..f6288dc 100644 > --- a/app/test/test_soc.c > +++ b/app/test/test_soc.c > @@ -74,6 +74,103 @@ static int test_compare_addr(void) > free(a2.name); > free(a1.name); > free(a0.name); > + > + return 0; > +} > + > +/** > + * Empty PMD driver based on the SoC infra. > + * > + * The rte_soc_device is usually wrapped in some higher-level struct > + * (eth_driver). We simulate such a wrapper with an anonymous struct here. > + */ > +struct test_wrapper { > + struct rte_soc_driver soc_drv; > +}; > + > +struct test_wrapper empty_pmd0 = { > + .soc_drv = { > + .name = "empty_pmd0", > + }, > +}; > + > +struct test_wrapper empty_pmd1 = { > + .soc_drv = { > + .name = "empty_pmd1", > + }, > +}; > + > +static int > +count_registered_socdrvs(void) > +{ > + int i; > + struct rte_soc_driver *drv; > + > + i = 0; > + TAILQ_FOREACH(drv, &soc_driver_list, next) > + i += 1; > + > + return i; > +} > + > +static int > +test_register_unregister(void) > +{ > + struct rte_soc_driver *drv; > + int count; > + > + rte_eal_soc_register(&empty_pmd0.soc_drv); > + > + TEST_ASSERT(!TAILQ_EMPTY(&soc_driver_list), > + "No PMD is present but the empty_pmd0 should be there"); > + drv = TAILQ_FIRST(&soc_driver_list); > + TEST_ASSERT(!strcmp(drv->name, "empty_pmd0"), > + "The registered PMD is not empty_pmd but '%s'", drv->name); Trivial: TEST_ASSERT Message should be: "... is not empty_pmd0 but..." > + > + rte_eal_soc_register(&empty_pmd1.soc_drv); > + > + count = count_registered_socdrvs(); > + TEST_ASSERT_EQUAL(count, 2, "Expected 2 PMDs but detected %d", count); > + > + rte_eal_soc_unregister(&empty_pmd0.soc_drv); > + count = count_registered_socdrvs(); > + TEST_ASSERT_EQUAL(count, 1, "Expected 1 PMDs but detected %d", count); > + > + rte_eal_soc_unregister(&empty_pmd1.soc_drv); > + > + printf("%s has been successful\n", __func__); > + return 0; > +} > + > +/* save real devices and drivers until the tests finishes */ > +struct soc_driver_list real_soc_driver_list = > + TAILQ_HEAD_INITIALIZER(real_soc_driver_list); > + > +static int test_soc_setup(void) > +{ > + struct rte_soc_driver *drv; > + > + /* no real drivers for the test */ > + while (!TAILQ_EMPTY(&soc_driver_list)) { > + drv = TAILQ_FIRST(&soc_driver_list); > + rte_eal_soc_unregister(drv); > + TAILQ_INSERT_TAIL(&real_soc_driver_list, drv, next); > + } > + > + return 0; > +} > + > +static int test_soc_cleanup(void) > +{ > + struct rte_soc_driver *drv; > + > + /* bring back real drivers after the test */ > + while (!TAILQ_EMPTY(&real_soc_driver_list)) { > + drv = TAILQ_FIRST(&real_soc_driver_list); > + TAILQ_REMOVE(&real_soc_driver_list, drv, next); > + rte_eal_soc_register(drv); > + } > + > return 0; > } > > @@ -83,6 +180,15 @@ test_soc(void) > if (test_compare_addr()) > return -1; > > + if (test_soc_setup()) > + return -1; > + > + if (test_register_unregister()) > + return -1; > + > + if (test_soc_cl
[dpdk-dev] [PATCH v1 07/28] eal/soc: add rte_eal_soc_register/unregister logic
Signed-off-by: Jan Viktorin --- app/test/test_soc.c | 106 lib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/bsdapp/eal/rte_eal_version.map | 3 + lib/librte_eal/common/eal_common_soc.c | 55 lib/librte_eal/common/include/rte_soc.h | 23 + lib/librte_eal/linuxapp/eal/Makefile| 1 + lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 + 7 files changed, 193 insertions(+) create mode 100644 lib/librte_eal/common/eal_common_soc.c diff --git a/app/test/test_soc.c b/app/test/test_soc.c index a49fc9b..f6288dc 100644 --- a/app/test/test_soc.c +++ b/app/test/test_soc.c @@ -74,6 +74,103 @@ static int test_compare_addr(void) free(a2.name); free(a1.name); free(a0.name); + + return 0; +} + +/** + * Empty PMD driver based on the SoC infra. + * + * The rte_soc_device is usually wrapped in some higher-level struct + * (eth_driver). We simulate such a wrapper with an anonymous struct here. + */ +struct test_wrapper { + struct rte_soc_driver soc_drv; +}; + +struct test_wrapper empty_pmd0 = { + .soc_drv = { + .name = "empty_pmd0", + }, +}; + +struct test_wrapper empty_pmd1 = { + .soc_drv = { + .name = "empty_pmd1", + }, +}; + +static int +count_registered_socdrvs(void) +{ + int i; + struct rte_soc_driver *drv; + + i = 0; + TAILQ_FOREACH(drv, &soc_driver_list, next) + i += 1; + + return i; +} + +static int +test_register_unregister(void) +{ + struct rte_soc_driver *drv; + int count; + + rte_eal_soc_register(&empty_pmd0.soc_drv); + + TEST_ASSERT(!TAILQ_EMPTY(&soc_driver_list), + "No PMD is present but the empty_pmd0 should be there"); + drv = TAILQ_FIRST(&soc_driver_list); + TEST_ASSERT(!strcmp(drv->name, "empty_pmd0"), + "The registered PMD is not empty_pmd but '%s'", drv->name); + + rte_eal_soc_register(&empty_pmd1.soc_drv); + + count = count_registered_socdrvs(); + TEST_ASSERT_EQUAL(count, 2, "Expected 2 PMDs but detected %d", count); + + rte_eal_soc_unregister(&empty_pmd0.soc_drv); + count = count_registered_socdrvs(); + TEST_ASSERT_EQUAL(count, 1, "Expected 1 PMDs but detected %d", count); + + rte_eal_soc_unregister(&empty_pmd1.soc_drv); + + printf("%s has been successful\n", __func__); + return 0; +} + +/* save real devices and drivers until the tests finishes */ +struct soc_driver_list real_soc_driver_list = + TAILQ_HEAD_INITIALIZER(real_soc_driver_list); + +static int test_soc_setup(void) +{ + struct rte_soc_driver *drv; + + /* no real drivers for the test */ + while (!TAILQ_EMPTY(&soc_driver_list)) { + drv = TAILQ_FIRST(&soc_driver_list); + rte_eal_soc_unregister(drv); + TAILQ_INSERT_TAIL(&real_soc_driver_list, drv, next); + } + + return 0; +} + +static int test_soc_cleanup(void) +{ + struct rte_soc_driver *drv; + + /* bring back real drivers after the test */ + while (!TAILQ_EMPTY(&real_soc_driver_list)) { + drv = TAILQ_FIRST(&real_soc_driver_list); + TAILQ_REMOVE(&real_soc_driver_list, drv, next); + rte_eal_soc_register(drv); + } + return 0; } @@ -83,6 +180,15 @@ test_soc(void) if (test_compare_addr()) return -1; + if (test_soc_setup()) + return -1; + + if (test_register_unregister()) + return -1; + + if (test_soc_cleanup()) + return -1; + return 0; } diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile index 9054ad6..d956808 100644 --- a/lib/librte_eal/bsdapp/eal/Makefile +++ b/lib/librte_eal/bsdapp/eal/Makefile @@ -71,6 +71,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_timer.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_memzone.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_log.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_launch.c +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_soc.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_pci.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_pci_uio.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_memory.c diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map index 4d075df..c430b4b 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -158,4 +158,7 @@ DPDK_16.07 { rte_eal_dev_attach; rte_eal_dev_detach; + rte_eal_soc_register; + rte_eal_soc_unregister; + } DPDK_16.04; diff --git a/lib/librte_eal/common/eal_common_soc.c b/lib/librte_eal/common/eal_common_soc.c new file mode 100644 index 000..afeed2f --- /dev/null +++ b/lib/librte_eal/com