[19/39] eventdev: add common initialize routine for eventmode devs

Message ID 1559583160-13944-20-git-send-email-anoobj@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series adding eventmode helper library |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Anoob Joseph June 3, 2019, 5:32 p.m. UTC
  Adding framework for common initialization routine for event mode.
Event mode would involve initialization of multiple devices, like
eventdev, ethdev etc and this routine would be the placeholder for all
initialization to come in.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
---
 lib/librte_eventdev/rte_eventdev_version.map |  2 ++
 lib/librte_eventdev/rte_eventmode_helper.c   | 51 +++++++++++++++++++++++++++-
 lib/librte_eventdev/rte_eventmode_helper.h   | 26 +++++++++++++-
 3 files changed, 77 insertions(+), 2 deletions(-)
  

Comments

Jerin Jacob Kollanukkaran June 10, 2019, 10:23 a.m. UTC | #1
> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Monday, June 3, 2019 11:02 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nikhil Rao
> <nikhil.rao@intel.com>; Erik Gabriel Carrillo <erik.g.carrillo@intel.com>;
> Abhinandan Gujjar <abhinandan.gujjar@intel.com>; Bruce Richardson
> <bruce.richardson@intel.com>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; dev@dpdk.org; Lukas Bartosik
> <lbartosik@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; Harry
> van Haaren <harry.van.haaren@intel.com>; Mattias Rönnblom
> <mattias.ronnblom@ericsson.com>; Liang Ma <liang.j.ma@intel.com>
> Subject: [PATCH 19/39] eventdev: add common initialize routine for
> eventmode devs
> 
> Adding framework for common initialization routine for event mode.
> Event mode would involve initialization of multiple devices, like eventdev,
> ethdev etc and this routine would be the placeholder for all initialization to
> come in.
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
> 
> +/* Helper functions for initialization, & launching workers */
> +
> +/**
> + * Initialize event mode devices
> + *
> + * Application could call this function to get the event device, eth
> +device
> + * and eth rx adapter initialized according to the conf populated using
> +the
> + * command line args.
> + *
> + * Application is expected to initialize the eth device and then the
> +eventmode
> + * helper subsystem will stop & start eth device according to it's
> requirement.
> + * So call to this function should be done after the eth device is
> +successfully
> + * initialized.
> + *
> + * @param mode_conf
> + *   Configuration of the mode in which app is doing packet handling
> + * @return
> + *  - 0 on success.
> + *  - (<0) on failure.
> + */
> +int32_t __rte_experimental
> +rte_eventmode_helper_initialize_devs(
> +		struct rte_eventmode_helper_conf *mode_conf);
> +

# Prefer to change to rte_event_helper_init() and 
introduce the counter part for the same(rte_event_helper_uninit() or rte_event_helper_fini())
# introduce params structure taking another paraments input instead of new APIs.
# let library return rte_event_helper_conf* object for further operations.


>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.7.4
  
Sunil Kumar Kori June 11, 2019, 8:58 a.m. UTC | #2
Regards
Sunil Kumar Kori

>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Anoob Joseph
>Sent: Monday, June 3, 2019 11:02 PM
>To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nikhil Rao
><nikhil.rao@intel.com>; Erik Gabriel Carrillo <erik.g.carrillo@intel.com>;
>Abhinandan Gujjar <abhinandan.gujjar@intel.com>; Bruce Richardson
><bruce.richardson@intel.com>; Pablo de Lara
><pablo.de.lara.guarch@intel.com>
>Cc: Anoob Joseph <anoobj@marvell.com>; Narayana Prasad Raju Athreya
><pathreya@marvell.com>; dev@dpdk.org; Lukas Bartosik
><lbartosik@marvell.com>; Pavan Nikhilesh Bhagavatula
><pbhagavatula@marvell.com>; Hemant Agrawal
><hemant.agrawal@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; Harry
>van Haaren <harry.van.haaren@intel.com>; Mattias Rönnblom
><mattias.ronnblom@ericsson.com>; Liang Ma <liang.j.ma@intel.com>
>Subject: [EXT] [dpdk-dev] [PATCH 19/39] eventdev: add common initialize
>routine for eventmode devs
>
>External Email
>
>----------------------------------------------------------------------
>Adding framework for common initialization routine for event mode.
>Event mode would involve initialization of multiple devices, like eventdev,
>ethdev etc and this routine would be the placeholder for all initialization to
>come in.
>
>Signed-off-by: Anoob Joseph <anoobj@marvell.com>
>Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
>---
> lib/librte_eventdev/rte_eventdev_version.map |  2 ++
> lib/librte_eventdev/rte_eventmode_helper.c   | 51
>+++++++++++++++++++++++++++-
> lib/librte_eventdev/rte_eventmode_helper.h   | 26 +++++++++++++-
> 3 files changed, 77 insertions(+), 2 deletions(-)
>
>diff --git a/lib/librte_eventdev/rte_eventdev_version.map
>b/lib/librte_eventdev/rte_eventdev_version.map
>index 1199064..e156fa0 100644
>--- a/lib/librte_eventdev/rte_eventdev_version.map
>+++ b/lib/librte_eventdev/rte_eventdev_version.map
>@@ -130,4 +130,6 @@ EXPERIMENTAL {
> 	rte_event_eth_rx_adapter_stats_get;
> 	rte_eventmode_helper_print_options_list;
> 	rte_eventmode_helper_print_options_description;
>+	rte_eventmode_helper_parse_args;
>+	rte_eventmode_helper_initialize_devs;
> };
>diff --git a/lib/librte_eventdev/rte_eventmode_helper.c
>b/lib/librte_eventdev/rte_eventmode_helper.c
>index 38f1a2b..d1a569b 100644
>--- a/lib/librte_eventdev/rte_eventmode_helper.c
>+++ b/lib/librte_eventdev/rte_eventmode_helper.c
>@@ -3,6 +3,7 @@
>  */
> #include <getopt.h>
>
>+#include <rte_ethdev.h>
> #include <rte_eventmode_helper.h>
> #include <rte_malloc.h>
>
>@@ -92,7 +93,7 @@ em_initialize_helper_conf(struct
>rte_eventmode_helper_conf *conf)
> 	conf->eth_portmask = -1;
> }
>
>-struct rte_eventmode_helper_conf *
>+struct rte_eventmode_helper_conf * __rte_experimental
> rte_eventmode_helper_parse_args(int argc, char **argv)  {
> 	int32_t opt, ret;
>@@ -152,3 +153,51 @@ rte_eventmode_helper_parse_args(int argc, char
>**argv)
>
> 	return NULL;
> }
>+
>+int32_t __rte_experimental
>+rte_eventmode_helper_initialize_devs(
>+		struct rte_eventmode_helper_conf *mode_conf) {
>+	int ret;
>+	uint16_t portid;
>+
>+	if (mode_conf == NULL) {
>+		RTE_EM_HLPR_LOG_ERR("Invalid conf");
>+		return -1;
>+	}
>+
>+	if (mode_conf->mode !=
>RTE_EVENTMODE_HELPER_PKT_TRANSFER_MODE_EVENT)
>+		return 0;
>+
>+	if (mode_conf->mode_params == NULL) {
>+		RTE_EM_HLPR_LOG_ERR("Invalid mode params");
>+		return -1;
Better to return standard error number like EINVAL etc instead of having -1 or 0.
Consider it for all applicable places.

>+	}
>+
>+	/* Stop eth devices before setting up adapter */
>+	RTE_ETH_FOREACH_DEV(portid) {
>+
>+		/* Use only the ports enabled */
>+		if ((mode_conf->eth_portmask & (1 << portid)) == 0)
>+			continue;
>+
>+		rte_eth_dev_stop(portid);
>+	}
>+
>+	/* Start eth devices after setting up adapter */
>+	RTE_ETH_FOREACH_DEV(portid) {
>+
>+		/* Use only the ports enabled */
>+		if ((mode_conf->eth_portmask & (1 << portid)) == 0)
>+			continue;
>+
>+		ret = rte_eth_dev_start(portid);
>+		if (ret < 0) {
>+			RTE_EM_HLPR_LOG_ERR(
>+				"Error starting eth dev %d", portid);
>+			return -1;
>+		}
>+	}
>+
>+	return 0;
>+}
>diff --git a/lib/librte_eventdev/rte_eventmode_helper.h
>b/lib/librte_eventdev/rte_eventmode_helper.h
>index 77e69d0..0eafed3 100644
>--- a/lib/librte_eventdev/rte_eventmode_helper.h
>+++ b/lib/librte_eventdev/rte_eventmode_helper.h
>@@ -62,9 +62,33 @@
>rte_eventmode_helper_print_options_description(void);
>  * @return
>  *   Configuration generated by parsing the event mode args.
>  */
>-struct rte_eventmode_helper_conf *
>+struct rte_eventmode_helper_conf * __rte_experimental
> rte_eventmode_helper_parse_args(int argc, char **argv);
>
>+/* Helper functions for initialization, & launching workers */
>+
>+/**
>+ * Initialize event mode devices
>+ *
>+ * Application could call this function to get the event device, eth
>+device
>+ * and eth rx adapter initialized according to the conf populated using
>+the
>+ * command line args.
>+ *
>+ * Application is expected to initialize the eth device and then the
>+eventmode
>+ * helper subsystem will stop & start eth device according to it's requirement.
>+ * So call to this function should be done after the eth device is
>+successfully
>+ * initialized.
Comments should be updated for its default value of conf->eth_portmask.

>+ *
>+ * @param mode_conf
>+ *   Configuration of the mode in which app is doing packet handling
>+ * @return
>+ *  - 0 on success.
>+ *  - (<0) on failure.
>+ */
>+int32_t __rte_experimental
>+rte_eventmode_helper_initialize_devs(
>+		struct rte_eventmode_helper_conf *mode_conf);
>+
> #ifdef __cplusplus
> }
> #endif
>--
>2.7.4
  
Anoob Joseph June 17, 2019, 10:22 a.m. UTC | #3
Hi Jerin,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran
> Sent: Monday, June 10, 2019 3:54 PM
> To: Anoob Joseph <anoobj@marvell.com>; Nikhil Rao
> <nikhil.rao@intel.com>; Erik Gabriel Carrillo <erik.g.carrillo@intel.com>;
> Abhinandan Gujjar <abhinandan.gujjar@intel.com>; Bruce Richardson
> <bruce.richardson@intel.com>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; dev@dpdk.org; Lukas Bartosik
> <lbartosik@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; Harry
> van Haaren <harry.van.haaren@intel.com>; Mattias Rönnblom
> <mattias.ronnblom@ericsson.com>; Liang Ma <liang.j.ma@intel.com>
> Subject: RE: [PATCH 19/39] eventdev: add common initialize routine for
> eventmode devs
> 
> > -----Original Message-----
> > From: Anoob Joseph <anoobj@marvell.com>
> > Sent: Monday, June 3, 2019 11:02 PM
> > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nikhil Rao
> > <nikhil.rao@intel.com>; Erik Gabriel Carrillo
> > <erik.g.carrillo@intel.com>; Abhinandan Gujjar
> > <abhinandan.gujjar@intel.com>; Bruce Richardson
> > <bruce.richardson@intel.com>; Pablo de Lara
> > <pablo.de.lara.guarch@intel.com>
> > Cc: Anoob Joseph <anoobj@marvell.com>; Narayana Prasad Raju Athreya
> > <pathreya@marvell.com>; dev@dpdk.org; Lukas Bartosik
> > <lbartosik@marvell.com>; Pavan Nikhilesh Bhagavatula
> > <pbhagavatula@marvell.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>;
> > Nipun Gupta <nipun.gupta@nxp.com>; Harry van Haaren
> > <harry.van.haaren@intel.com>; Mattias Rönnblom
> > <mattias.ronnblom@ericsson.com>; Liang Ma <liang.j.ma@intel.com>
> > Subject: [PATCH 19/39] eventdev: add common initialize routine for
> > eventmode devs
> >
> > Adding framework for common initialization routine for event mode.
> > Event mode would involve initialization of multiple devices, like
> > eventdev, ethdev etc and this routine would be the placeholder for all
> > initialization to come in.
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > ---
> >
> > +/* Helper functions for initialization, & launching workers */
> > +
> > +/**
> > + * Initialize event mode devices
> > + *
> > + * Application could call this function to get the event device, eth
> > +device
> > + * and eth rx adapter initialized according to the conf populated
> > +using the
> > + * command line args.
> > + *
> > + * Application is expected to initialize the eth device and then the
> > +eventmode
> > + * helper subsystem will stop & start eth device according to it's
> > requirement.
> > + * So call to this function should be done after the eth device is
> > +successfully
> > + * initialized.
> > + *
> > + * @param mode_conf
> > + *   Configuration of the mode in which app is doing packet handling
> > + * @return
> > + *  - 0 on success.
> > + *  - (<0) on failure.
> > + */
> > +int32_t __rte_experimental
> > +rte_eventmode_helper_initialize_devs(
> > +		struct rte_eventmode_helper_conf *mode_conf);
> > +
> 
> # Prefer to change to rte_event_helper_init() and introduce the counter part

[Anoob] This routine is doing init of devs (not the helper library). So are you okay with renaming it to rte_event_helper_devs_init() and rte_event_helper_devs_uninit(), and retain the functionality as before?

> for the same(rte_event_helper_uninit() or rte_event_helper_fini()) #
> introduce params structure taking another paraments input instead of new
> APIs.
> # let library return rte_event_helper_conf* object for further operations.

[Anoob] Parse args will parse the command line arguments and return the conf. Based on the conf we setup the required devices (eventdev, Rx adapter & Tx adapter, can be expanded to handle ethdev also). 

> 
> 
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > --
> > 2.7.4
  

Patch

diff --git a/lib/librte_eventdev/rte_eventdev_version.map b/lib/librte_eventdev/rte_eventdev_version.map
index 1199064..e156fa0 100644
--- a/lib/librte_eventdev/rte_eventdev_version.map
+++ b/lib/librte_eventdev/rte_eventdev_version.map
@@ -130,4 +130,6 @@  EXPERIMENTAL {
 	rte_event_eth_rx_adapter_stats_get;
 	rte_eventmode_helper_print_options_list;
 	rte_eventmode_helper_print_options_description;
+	rte_eventmode_helper_parse_args;
+	rte_eventmode_helper_initialize_devs;
 };
diff --git a/lib/librte_eventdev/rte_eventmode_helper.c b/lib/librte_eventdev/rte_eventmode_helper.c
index 38f1a2b..d1a569b 100644
--- a/lib/librte_eventdev/rte_eventmode_helper.c
+++ b/lib/librte_eventdev/rte_eventmode_helper.c
@@ -3,6 +3,7 @@ 
  */
 #include <getopt.h>
 
+#include <rte_ethdev.h>
 #include <rte_eventmode_helper.h>
 #include <rte_malloc.h>
 
@@ -92,7 +93,7 @@  em_initialize_helper_conf(struct rte_eventmode_helper_conf *conf)
 	conf->eth_portmask = -1;
 }
 
-struct rte_eventmode_helper_conf *
+struct rte_eventmode_helper_conf * __rte_experimental
 rte_eventmode_helper_parse_args(int argc, char **argv)
 {
 	int32_t opt, ret;
@@ -152,3 +153,51 @@  rte_eventmode_helper_parse_args(int argc, char **argv)
 
 	return NULL;
 }
+
+int32_t __rte_experimental
+rte_eventmode_helper_initialize_devs(
+		struct rte_eventmode_helper_conf *mode_conf)
+{
+	int ret;
+	uint16_t portid;
+
+	if (mode_conf == NULL) {
+		RTE_EM_HLPR_LOG_ERR("Invalid conf");
+		return -1;
+	}
+
+	if (mode_conf->mode != RTE_EVENTMODE_HELPER_PKT_TRANSFER_MODE_EVENT)
+		return 0;
+
+	if (mode_conf->mode_params == NULL) {
+		RTE_EM_HLPR_LOG_ERR("Invalid mode params");
+		return -1;
+	}
+
+	/* Stop eth devices before setting up adapter */
+	RTE_ETH_FOREACH_DEV(portid) {
+
+		/* Use only the ports enabled */
+		if ((mode_conf->eth_portmask & (1 << portid)) == 0)
+			continue;
+
+		rte_eth_dev_stop(portid);
+	}
+
+	/* Start eth devices after setting up adapter */
+	RTE_ETH_FOREACH_DEV(portid) {
+
+		/* Use only the ports enabled */
+		if ((mode_conf->eth_portmask & (1 << portid)) == 0)
+			continue;
+
+		ret = rte_eth_dev_start(portid);
+		if (ret < 0) {
+			RTE_EM_HLPR_LOG_ERR(
+				"Error starting eth dev %d", portid);
+			return -1;
+		}
+	}
+
+	return 0;
+}
diff --git a/lib/librte_eventdev/rte_eventmode_helper.h b/lib/librte_eventdev/rte_eventmode_helper.h
index 77e69d0..0eafed3 100644
--- a/lib/librte_eventdev/rte_eventmode_helper.h
+++ b/lib/librte_eventdev/rte_eventmode_helper.h
@@ -62,9 +62,33 @@  rte_eventmode_helper_print_options_description(void);
  * @return
  *   Configuration generated by parsing the event mode args.
  */
-struct rte_eventmode_helper_conf *
+struct rte_eventmode_helper_conf * __rte_experimental
 rte_eventmode_helper_parse_args(int argc, char **argv);
 
+/* Helper functions for initialization, & launching workers */
+
+/**
+ * Initialize event mode devices
+ *
+ * Application could call this function to get the event device, eth device
+ * and eth rx adapter initialized according to the conf populated using the
+ * command line args.
+ *
+ * Application is expected to initialize the eth device and then the eventmode
+ * helper subsystem will stop & start eth device according to it's requirement.
+ * So call to this function should be done after the eth device is successfully
+ * initialized.
+ *
+ * @param mode_conf
+ *   Configuration of the mode in which app is doing packet handling
+ * @return
+ *  - 0 on success.
+ *  - (<0) on failure.
+ */
+int32_t __rte_experimental
+rte_eventmode_helper_initialize_devs(
+		struct rte_eventmode_helper_conf *mode_conf);
+
 #ifdef __cplusplus
 }
 #endif