[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, _driver_list, next)
> > > + i += 1;
> > > +
> > > + return i;
> > > +}
> > > +
> > > +static int
> > > +test_register_unregister(void)
> > > +{
> > > + struct rte_soc_driver *drv;
> > > + int count;
> > > +
> > > + rte_eal_soc_register(_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(_drv); \
>
> It should be 'rte_eal_soc_register()'.
> 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, _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, _driver_list, next)
> > + i += 1;
> > +
> > + return i;
> > +}
> > +
> > +static int
> > +test_register_unregister(void)
> > +{
> > + struct rte_soc_driver *drv;
> > + int count;
> > +
> > + rte_eal_soc_register(_pmd0.soc_drv);
> > +
> > + TEST_ASSERT(!TAILQ_EMPTY(_driver_list),
> > + "No PMD is present but the empty_pmd0 should be there");
> > + drv = TAILQ_FIRST(_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(_pmd1.soc_drv);
> > +
> > + count = count_registered_socdrvs();
> > + TEST_ASSERT_EQUAL(count, 2, "Expected 2 PMDs but detected %d", count);
> > +
> > + rte_eal_soc_unregister(_pmd0.soc_drv);
> > + count = count_registered_socdrvs();
> > + TEST_ASSERT_EQUAL(count, 1, "Expected 1 PMDs but detected %d", count);
> > +
> > + rte_eal_soc_unregister(_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 */
> > +
[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, _driver_list, next)
> > + i += 1;
> > +
> > + return i;
> > +}
> > +
> > +static int
> > +test_register_unregister(void)
> > +{
> > + struct rte_soc_driver *drv;
> > + int count;
> > +
> > + rte_eal_soc_register(_pmd0.soc_drv);
> > +
> > + TEST_ASSERT(!TAILQ_EMPTY(_driver_list),
> > + "No PMD is present but the empty_pmd0 should be there");
> > + drv = TAILQ_FIRST(_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, _driver_list, next)
> + i += 1;
> +
> + return i;
> +}
> +
> +static int
> +test_register_unregister(void)
> +{
> + struct rte_soc_driver *drv;
> + int count;
> +
> + rte_eal_soc_register(_pmd0.soc_drv);
> +
> + TEST_ASSERT(!TAILQ_EMPTY(_driver_list),
> + "No PMD is present but the empty_pmd0 should be there");
> + drv = TAILQ_FIRST(_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(_pmd1.soc_drv);
> +
> + count = count_registered_socdrvs();
> + TEST_ASSERT_EQUAL(count, 2, "Expected 2 PMDs but detected %d", count);
> +
> + rte_eal_soc_unregister(_pmd0.soc_drv);
> + count = count_registered_socdrvs();
> + TEST_ASSERT_EQUAL(count, 1, "Expected 1 PMDs but detected %d", count);
> +
> + rte_eal_soc_unregister(_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(_driver_list)) {
> + drv = TAILQ_FIRST(_driver_list);
> + rte_eal_soc_unregister(drv);
> + TAILQ_INSERT_TAIL(_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(_soc_driver_list)) {
> + drv = TAILQ_FIRST(_soc_driver_list);
> + TAILQ_REMOVE(_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
[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, _driver_list, next)
+ i += 1;
+
+ return i;
+}
+
+static int
+test_register_unregister(void)
+{
+ struct rte_soc_driver *drv;
+ int count;
+
+ rte_eal_soc_register(_pmd0.soc_drv);
+
+ TEST_ASSERT(!TAILQ_EMPTY(_driver_list),
+ "No PMD is present but the empty_pmd0 should be there");
+ drv = TAILQ_FIRST(_driver_list);
+ TEST_ASSERT(!strcmp(drv->name, "empty_pmd0"),
+ "The registered PMD is not empty_pmd but '%s'", drv->name);
+
+ rte_eal_soc_register(_pmd1.soc_drv);
+
+ count = count_registered_socdrvs();
+ TEST_ASSERT_EQUAL(count, 2, "Expected 2 PMDs but detected %d", count);
+
+ rte_eal_soc_unregister(_pmd0.soc_drv);
+ count = count_registered_socdrvs();
+ TEST_ASSERT_EQUAL(count, 1, "Expected 1 PMDs but detected %d", count);
+
+ rte_eal_soc_unregister(_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(_driver_list)) {
+ drv = TAILQ_FIRST(_driver_list);
+ rte_eal_soc_unregister(drv);
+ TAILQ_INSERT_TAIL(_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(_soc_driver_list)) {
+ drv = TAILQ_FIRST(_soc_driver_list);
+ TAILQ_REMOVE(_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/common/eal_common_soc.c
@@ -0,0 +1,55 @@
+/*-
+ * BSD LICENSE
+
