net/softnic: add support for service cores

Message ID 20190103160432.147027-1-cristian.dumitrescu@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Cristian Dumitrescu
Headers
Series net/softnic: add support for service cores |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Cristian Dumitrescu Jan. 3, 2019, 4:04 p.m. UTC
  Add support to run the Soft NIC device on service cores, based on the
value of the sc parameter:

0: The current Soft NIC device is run explicitly by the application. The
   firmware creates one or several pipelines for this dev and maps them
   to CPU cores that should not be service cores. The app is required to
   call rte_pmd_softnic_run() to make this dev work.

1 (default): The current device is run on the service cores transparently
   to the app. The firmware creates one or several pipelines for this dev
   current device and maps them to CPU cores that should be service cores.
   Each of these service cores is calling rte_pmd_softnic_run()
   in order to make this dev work with no app intervention.

Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
---
 drivers/net/softnic/rte_eth_softnic.c         |  11 ++
 drivers/net/softnic/rte_eth_softnic.h         |  20 +++
 .../net/softnic/rte_eth_softnic_internals.h   |   6 +-
 .../net/softnic/rte_eth_softnic_pipeline.c    |  13 ++
 drivers/net/softnic/rte_eth_softnic_thread.c  | 139 ++++++++++++++----
 5 files changed, 161 insertions(+), 28 deletions(-)
  

Comments

Cristian Dumitrescu Jan. 7, 2019, 4:50 p.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Cristian Dumitrescu
> Sent: Thursday, January 3, 2019 4:05 PM
> To: dev@dpdk.org
> Cc: Singh, Jasvinder <jasvinder.singh@intel.com>
> Subject: [dpdk-dev] [PATCH] net/softnic: add support for service cores
> 
> Add support to run the Soft NIC device on service cores, based on the
> value of the sc parameter:
> 
> 0: The current Soft NIC device is run explicitly by the application. The
>    firmware creates one or several pipelines for this dev and maps them
>    to CPU cores that should not be service cores. The app is required to
>    call rte_pmd_softnic_run() to make this dev work.
> 
> 1 (default): The current device is run on the service cores transparently
>    to the app. The firmware creates one or several pipelines for this dev
>    current device and maps them to CPU cores that should be service cores.
>    Each of these service cores is calling rte_pmd_softnic_run()
>    in order to make this dev work with no app intervention.
> 
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> ---

Applied to next-qos tree, thanks!
  
Thomas Monjalon Jan. 9, 2019, 11:23 p.m. UTC | #2
07/01/2019 17:50, Dumitrescu, Cristian:
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Cristian Dumitrescu
> > Sent: Thursday, January 3, 2019 4:05 PM
> > To: dev@dpdk.org
> > Cc: Singh, Jasvinder <jasvinder.singh@intel.com>
> > Subject: [dpdk-dev] [PATCH] net/softnic: add support for service cores
> > 
> > Add support to run the Soft NIC device on service cores, based on the
> > value of the sc parameter:
> > 
> > 0: The current Soft NIC device is run explicitly by the application. The
> >    firmware creates one or several pipelines for this dev and maps them
> >    to CPU cores that should not be service cores. The app is required to
> >    call rte_pmd_softnic_run() to make this dev work.
> > 
> > 1 (default): The current device is run on the service cores transparently
> >    to the app. The firmware creates one or several pipelines for this dev
> >    current device and maps them to CPU cores that should be service cores.
> >    Each of these service cores is calling rte_pmd_softnic_run()
> >    in order to make this dev work with no app intervention.
> > 
> > Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > ---
> 
> Applied to next-qos tree, thanks!

It does not compile with 32-bit toolchain:

drivers/net/softnic/rte_eth_softnic_thread.c:140:28: error:
cast between incompatible function types
from ‘int (*)(uint16_t)’ {aka ‘int (*)(short unsigned int)’}
to ‘int32_t (*)(void *)’ {aka ‘int (*)(void *)’}
  service_params.callback = (rte_service_func)rte_pmd_softnic_run;
                            ^

Fixing in-tree.
  
Thomas Monjalon Jan. 9, 2019, 11:33 p.m. UTC | #3
10/01/2019 00:23, Thomas Monjalon:
> 07/01/2019 17:50, Dumitrescu, Cristian:
> > 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Cristian Dumitrescu
> > > Sent: Thursday, January 3, 2019 4:05 PM
> > > To: dev@dpdk.org
> > > Cc: Singh, Jasvinder <jasvinder.singh@intel.com>
> > > Subject: [dpdk-dev] [PATCH] net/softnic: add support for service cores
> > > 
> > > Add support to run the Soft NIC device on service cores, based on the
> > > value of the sc parameter:
> > > 
> > > 0: The current Soft NIC device is run explicitly by the application. The
> > >    firmware creates one or several pipelines for this dev and maps them
> > >    to CPU cores that should not be service cores. The app is required to
> > >    call rte_pmd_softnic_run() to make this dev work.
> > > 
> > > 1 (default): The current device is run on the service cores transparently
> > >    to the app. The firmware creates one or several pipelines for this dev
> > >    current device and maps them to CPU cores that should be service cores.
> > >    Each of these service cores is calling rte_pmd_softnic_run()
> > >    in order to make this dev work with no app intervention.
> > > 
> > > Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > > ---
> > 
> > Applied to next-qos tree, thanks!
> 
> It does not compile with 32-bit toolchain:
> 
> drivers/net/softnic/rte_eth_softnic_thread.c:140:28: error:
> cast between incompatible function types
> from ‘int (*)(uint16_t)’ {aka ‘int (*)(short unsigned int)’}
> to ‘int32_t (*)(void *)’ {aka ‘int (*)(void *)’}
>   service_params.callback = (rte_service_func)rte_pmd_softnic_run;
>                             ^
> 
> Fixing in-tree.

After a better look, I won't fix it myself because the simple fix
would be an API change.
It is removed from the pull in master for now, sorry.
  

Patch

diff --git a/drivers/net/softnic/rte_eth_softnic.c b/drivers/net/softnic/rte_eth_softnic.c
index 14d4a1030..32b001fd3 100644
--- a/drivers/net/softnic/rte_eth_softnic.c
+++ b/drivers/net/softnic/rte_eth_softnic.c
@@ -22,6 +22,7 @@ 
 #define PMD_PARAM_FIRMWARE                                 "firmware"
 #define PMD_PARAM_CONN_PORT                                "conn_port"
 #define PMD_PARAM_CPU_ID                                   "cpu_id"
+#define PMD_PARAM_SC                                       "sc"
 #define PMD_PARAM_TM_N_QUEUES                              "tm_n_queues"
 #define PMD_PARAM_TM_QSIZE0                                "tm_qsize0"
 #define PMD_PARAM_TM_QSIZE1                                "tm_qsize1"
@@ -32,6 +33,7 @@  static const char * const pmd_valid_args[] = {
 	PMD_PARAM_FIRMWARE,
 	PMD_PARAM_CONN_PORT,
 	PMD_PARAM_CPU_ID,
+	PMD_PARAM_SC,
 	PMD_PARAM_TM_N_QUEUES,
 	PMD_PARAM_TM_QSIZE0,
 	PMD_PARAM_TM_QSIZE1,
@@ -426,6 +428,7 @@  pmd_parse_args(struct pmd_params *p, const char *params)
 	memset(p, 0, sizeof(*p));
 	p->firmware = SOFTNIC_FIRMWARE;
 	p->cpu_id = SOFTNIC_CPU_ID;
+	p->sc = SOFTNIC_SC;
 	p->tm.n_queues = SOFTNIC_TM_N_QUEUES;
 	p->tm.qsize[0] = SOFTNIC_TM_QUEUE_SIZE;
 	p->tm.qsize[1] = SOFTNIC_TM_QUEUE_SIZE;
@@ -456,6 +459,14 @@  pmd_parse_args(struct pmd_params *p, const char *params)
 			goto out_free;
 	}
 
+	/* Service cores (optional) */
+	if (rte_kvargs_count(kvlist, PMD_PARAM_SC) == 1) {
+		ret = rte_kvargs_process(kvlist, PMD_PARAM_SC,
+			&get_uint32, &p->sc);
+		if (ret < 0)
+			goto out_free;
+	}
+
 	/* TM number of queues (optional) */
 	if (rte_kvargs_count(kvlist, PMD_PARAM_TM_N_QUEUES) == 1) {
 		ret = rte_kvargs_process(kvlist, PMD_PARAM_TM_N_QUEUES,
diff --git a/drivers/net/softnic/rte_eth_softnic.h b/drivers/net/softnic/rte_eth_softnic.h
index 048dfe6bf..e8ba2bcaa 100644
--- a/drivers/net/softnic/rte_eth_softnic.h
+++ b/drivers/net/softnic/rte_eth_softnic.h
@@ -26,6 +26,26 @@  extern "C" {
 #define SOFTNIC_CPU_ID                                     0
 #endif
 
+/**
+ * Service cores:
+ *
+ * 0 = The current device is run explicitly by the application. The firmware
+ *     creates one or several pipelines for the current device and maps them to
+ *     CPU cores that should not be service cores. The application is required
+ *     to call rte_pmd_softnic_run() for the current device on each of these CPU
+ *     cores in order to make the current device work.
+ *
+ * 1 = The current device is run on the service cores transparently to the
+ *     application. The firmware creates one or several pipelines for the
+ *     current device and maps them to CPU cores that should be service cores.
+ *     Each of these service cores is calling rte_pmd_softnic_run() for the
+ *     current device in order to make the current device work. The application
+ *     is not allowed to call rte_pmd_softnic_run() for the current device.
+ */
+#ifndef SOFTNIC_SC
+#define SOFTNIC_SC                                         1
+#endif
+
 /** Traffic Manager: Number of scheduler queues. */
 #ifndef SOFTNIC_TM_N_QUEUES
 #define SOFTNIC_TM_N_QUEUES                                (64 * 1024)
diff --git a/drivers/net/softnic/rte_eth_softnic_internals.h b/drivers/net/softnic/rte_eth_softnic_internals.h
index 31698b9f0..3bc1de852 100644
--- a/drivers/net/softnic/rte_eth_softnic_internals.h
+++ b/drivers/net/softnic/rte_eth_softnic_internals.h
@@ -38,6 +38,7 @@  struct pmd_params {
 	const char *firmware;
 	uint16_t conn_port;
 	uint32_t cpu_id;
+	int sc; /**< Service cores. */
 
 	/** Traffic Management (TM) */
 	struct {
@@ -547,7 +548,7 @@  struct softnic_thread {
 	struct rte_ring *msgq_req;
 	struct rte_ring *msgq_rsp;
 
-	uint32_t enabled;
+	uint32_t service_id;
 };
 
 /**
@@ -843,6 +844,9 @@  softnic_pipeline_free(struct pmd_internals *p);
 void
 softnic_pipeline_disable_all(struct pmd_internals *p);
 
+uint32_t
+softnic_pipeline_thread_count(struct pmd_internals *p, uint32_t thread_id);
+
 struct pipeline *
 softnic_pipeline_find(struct pmd_internals *p, const char *name);
 
diff --git a/drivers/net/softnic/rte_eth_softnic_pipeline.c b/drivers/net/softnic/rte_eth_softnic_pipeline.c
index 5e180f8f7..308fd1534 100644
--- a/drivers/net/softnic/rte_eth_softnic_pipeline.c
+++ b/drivers/net/softnic/rte_eth_softnic_pipeline.c
@@ -108,6 +108,19 @@  softnic_pipeline_disable_all(struct pmd_internals *p)
 				pipeline->name);
 }
 
+uint32_t
+softnic_pipeline_thread_count(struct pmd_internals *p, uint32_t thread_id)
+{
+	struct pipeline *pipeline;
+	uint32_t count = 0;
+
+	TAILQ_FOREACH(pipeline, &p->pipeline_list, node)
+		if ((pipeline->enabled) && (pipeline->thread_id == thread_id))
+			count++;
+
+	return count;
+}
+
 struct pipeline *
 softnic_pipeline_find(struct pmd_internals *p,
 	const char *name)
diff --git a/drivers/net/softnic/rte_eth_softnic_thread.c b/drivers/net/softnic/rte_eth_softnic_thread.c
index 4572adfa6..68c1f50a6 100644
--- a/drivers/net/softnic/rte_eth_softnic_thread.c
+++ b/drivers/net/softnic/rte_eth_softnic_thread.c
@@ -7,6 +7,7 @@ 
 #include <rte_common.h>
 #include <rte_cycles.h>
 #include <rte_lcore.h>
+#include <rte_service_component.h>
 #include <rte_ring.h>
 
 #include <rte_table_acl.h>
@@ -41,7 +42,7 @@  softnic_thread_init(struct pmd_internals *softnic)
 {
 	uint32_t i;
 
-	RTE_LCORE_FOREACH_SLAVE(i) {
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
 		char ring_name[NAME_MAX];
 		struct rte_ring *msgq_req, *msgq_rsp;
 		struct softnic_thread *t = &softnic->thread[i];
@@ -80,7 +81,7 @@  softnic_thread_init(struct pmd_internals *softnic)
 		/* Master thread records */
 		t->msgq_req = msgq_req;
 		t->msgq_rsp = msgq_rsp;
-		t->enabled = 1;
+		t->service_id = UINT32_MAX;
 
 		/* Data plane thread records */
 		t_data->n_pipelines = 0;
@@ -95,6 +96,25 @@  softnic_thread_init(struct pmd_internals *softnic)
 	return 0;
 }
 
+static inline int
+thread_is_valid(struct pmd_internals *softnic, uint32_t thread_id)
+{
+	struct rte_config *cfg = rte_eal_get_configuration();
+	enum rte_lcore_role_t role;
+
+	if ((thread_id >= RTE_MAX_LCORE) ||
+		(thread_id == cfg->master_lcore))
+		return 0; /* FALSE */
+
+	role = cfg->lcore_role[thread_id];
+
+	if ((softnic->params.sc && (role == ROLE_SERVICE)) ||
+		(!softnic->params.sc && (role == ROLE_RTE)))
+		return 1; /* TRUE */
+
+	return 0; /* FALSE */
+}
+
 static inline int
 thread_is_running(uint32_t thread_id)
 {
@@ -104,6 +124,73 @@  thread_is_running(uint32_t thread_id)
 	return (thread_state == RUNNING)? 1 : 0;
 }
 
+static inline int
+thread_sc_service_up(struct pmd_internals *softnic, uint32_t thread_id)
+{
+	struct rte_service_spec service_params;
+	struct softnic_thread *t = &softnic->thread[thread_id];
+	int status;
+	uint16_t port_id;
+
+	/* service params */
+	rte_eth_dev_get_port_by_name(softnic->params.name, &port_id);
+	snprintf(service_params.name, sizeof(service_params.name), "%s_%d",
+		softnic->params.name,
+		thread_id);
+	service_params.callback = (rte_service_func)rte_pmd_softnic_run;
+	service_params.callback_userdata = (void *)((uint64_t)port_id);
+	service_params.capabilities = 0;
+	service_params.socket_id = (int)softnic->params.cpu_id;
+
+	/* service register */
+	status = rte_service_component_register(&service_params, &t->service_id);
+	if (status)
+		return status;
+
+	status = rte_service_component_runstate_set(t->service_id, 1);
+	if (status) {
+		rte_service_component_unregister(t->service_id);
+		t->service_id = UINT32_MAX;
+		return status;
+	}
+
+	status = rte_service_runstate_set(t->service_id, 1);
+	if (status) {
+		rte_service_component_runstate_set(t->service_id, 0);
+		rte_service_component_unregister(t->service_id);
+		t->service_id = UINT32_MAX;
+		return status;
+	}
+
+	/* service map to thread */
+	status = rte_service_map_lcore_set(t->service_id, thread_id, 1);
+	if (status) {
+		rte_service_runstate_set(t->service_id, 0);
+		rte_service_component_runstate_set(t->service_id, 0);
+		rte_service_component_unregister(t->service_id);
+		t->service_id = UINT32_MAX;
+		return status;
+	}
+
+	return 0;
+}
+
+static inline void
+thread_sc_service_down(struct pmd_internals *softnic, uint32_t thread_id)
+{
+	struct softnic_thread *t = &softnic->thread[thread_id];
+
+	/* service unmap from thread */
+	rte_service_map_lcore_set(t->service_id, thread_id, 0);
+
+	/* service unregister */
+	rte_service_runstate_set(t->service_id, 0);
+	rte_service_component_runstate_set(t->service_id, 0);
+	rte_service_component_unregister(t->service_id);
+
+	t->service_id = UINT32_MAX;
+}
+
 /**
  * Pipeline is running when:
  *    (A) Pipeline is mapped to a data plane thread AND
@@ -200,32 +287,34 @@  softnic_thread_pipeline_enable(struct pmd_internals *softnic,
 	const char *pipeline_name)
 {
 	struct pipeline *p = softnic_pipeline_find(softnic, pipeline_name);
-	struct softnic_thread *t;
 	struct thread_msg_req *req;
 	struct thread_msg_rsp *rsp;
-	uint32_t i;
+	uint32_t n_pipelines, i;
 	int status;
 
 	/* Check input params */
-	if ((thread_id >= RTE_MAX_LCORE) ||
+	if (!thread_is_valid(softnic, thread_id) ||
 		(p == NULL) ||
 		(p->n_ports_in == 0) ||
 		(p->n_ports_out == 0) ||
-		(p->n_tables == 0))
+		(p->n_tables == 0) ||
+		p->enabled)
 		return -1;
 
-	t = &softnic->thread[thread_id];
-	if ((t->enabled == 0) ||
-		p->enabled)
+	n_pipelines = softnic_pipeline_thread_count(softnic, thread_id);
+	if (n_pipelines >= THREAD_PIPELINES_MAX)
 		return -1;
 
+	if (softnic->params.sc && (n_pipelines == 0)) {
+		status = thread_sc_service_up(softnic, thread_id);
+		if (status)
+			return status;
+	}
+
 	if (!thread_is_running(thread_id)) {
 		struct softnic_thread_data *td = &softnic->thread_data[thread_id];
 		struct pipeline_data *tdp = &td->pipeline_data[td->n_pipelines];
 
-		if (td->n_pipelines >= THREAD_PIPELINES_MAX)
-			return -1;
-
 		/* Data plane thread */
 		td->p[td->n_pipelines] = p->p;
 
@@ -292,26 +381,20 @@  softnic_thread_pipeline_disable(struct pmd_internals *softnic,
 	const char *pipeline_name)
 {
 	struct pipeline *p = softnic_pipeline_find(softnic, pipeline_name);
-	struct softnic_thread *t;
 	struct thread_msg_req *req;
 	struct thread_msg_rsp *rsp;
+	uint32_t n_pipelines;
 	int status;
 
 	/* Check input params */
-	if ((thread_id >= RTE_MAX_LCORE) ||
-		(p == NULL))
-		return -1;
-
-	t = &softnic->thread[thread_id];
-	if (t->enabled == 0)
+	if (!thread_is_valid(softnic, thread_id) ||
+		(p == NULL) ||
+		(p->enabled && (p->thread_id != thread_id)))
 		return -1;
 
 	if (p->enabled == 0)
 		return 0;
 
-	if (p->thread_id != thread_id)
-		return -1;
-
 	if (!thread_is_running(thread_id)) {
 		struct softnic_thread_data *td = &softnic->thread_data[thread_id];
 		uint32_t i;
@@ -341,6 +424,9 @@  softnic_thread_pipeline_disable(struct pmd_internals *softnic,
 			break;
 		}
 
+		if (softnic->params.sc && (td->n_pipelines == 0))
+			thread_sc_service_down(softnic, thread_id);
+
 		return 0;
 	}
 
@@ -370,6 +456,10 @@  softnic_thread_pipeline_disable(struct pmd_internals *softnic,
 
 	p->enabled = 0;
 
+	n_pipelines = softnic_pipeline_thread_count(softnic, thread_id);
+	if (softnic->params.sc && (n_pipelines == 0))
+		thread_sc_service_down(softnic, thread_id);
+
 	return 0;
 }
 
@@ -409,11 +499,6 @@  thread_msg_handle_pipeline_enable(struct softnic_thread_data *t,
 	uint32_t i;
 
 	/* Request */
-	if (t->n_pipelines >= THREAD_PIPELINES_MAX) {
-		rsp->status = -1;
-		return rsp;
-	}
-
 	t->p[t->n_pipelines] = req->pipeline_enable.p;
 
 	p->p = req->pipeline_enable.p;