DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc
@ 2018-04-20  1:21 Jianfeng Tan
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 1/2] ipc: clearn up code Jianfeng Tan
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Jianfeng Tan @ 2018-04-20  1:21 UTC (permalink / raw)
  To: dev; +Cc: thomas, anatoly.burakov, Jianfeng Tan

Patch 1 is to rename some variable names for better readability.

Patch 2 is a bug fix.

Jianfeng Tan (2):
  ipc: clearn up code
  ipc: fix timeout not properly handled in async

 lib/librte_eal/common/eal_common_proc.c | 88 ++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 44 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH 1/2] ipc: clearn up code
  2018-04-20  1:21 [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan
@ 2018-04-20  1:21 ` Jianfeng Tan
  2018-04-20  7:55   ` Burakov, Anatoly
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
  2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
  2 siblings, 1 reply; 11+ messages in thread
From: Jianfeng Tan @ 2018-04-20  1:21 UTC (permalink / raw)
  To: dev; +Cc: thomas, anatoly.burakov, Jianfeng Tan

Following below commit, we change some internal function and variable
names:
  commit ce3a7312357b ("eal: rename IPC request as synchronous one")

Also use calloc to supersede malloc + memset for code clean up.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 82 +++++++++++++++------------------
 1 file changed, 38 insertions(+), 44 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 2179f3d..070a075 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -108,7 +108,7 @@ mp_send(struct rte_mp_msg *msg, const char *peer, int type);
 
 
 static struct pending_request *
-find_sync_request(const char *dst, const char *act_name)
+find_pending_request(const char *dst, const char *act_name)
 {
 	struct pending_request *r;
 
@@ -282,7 +282,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 static void
 process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 {
-	struct pending_request *sync_req;
+	struct pending_request *pending_req;
 	struct action_entry *entry;
 	struct rte_mp_msg *msg = &m->msg;
 	rte_mp_t action = NULL;
@@ -291,15 +291,15 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 
 	if (m->type == MP_REP || m->type == MP_IGN) {
 		pthread_mutex_lock(&pending_requests.lock);
-		sync_req = find_sync_request(s->sun_path, msg->name);
-		if (sync_req) {
-			memcpy(sync_req->reply, msg, sizeof(*msg));
+		pending_req = find_pending_request(s->sun_path, msg->name);
+		if (pending_req) {
+			memcpy(pending_req->reply, msg, sizeof(*msg));
 			/* -1 indicates that we've been asked to ignore */
-			sync_req->reply_received = m->type == MP_REP ? 1 : -1;
+			pending_req->reply_received = m->type == MP_REP ? 1 : -1;
 
-			if (sync_req->type == REQUEST_TYPE_SYNC)
-				pthread_cond_signal(&sync_req->sync.cond);
-			else if (sync_req->type == REQUEST_TYPE_ASYNC)
+			if (pending_req->type == REQUEST_TYPE_SYNC)
+				pthread_cond_signal(&pending_req->sync.cond);
+			else if (pending_req->type == REQUEST_TYPE_ASYNC)
 				pthread_cond_signal(
 					&pending_requests.async_cond);
 		} else
@@ -322,6 +322,7 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 			 * yet ready to process this request.
 			 */
 			struct rte_mp_msg dummy;
+
 			memset(&dummy, 0, sizeof(dummy));
 			snprintf(dummy.name, sizeof(dummy.name),
 					"%s", msg->name);
@@ -854,30 +855,27 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
 		struct async_request_param *param)
 {
 	struct rte_mp_msg *reply_msg;
-	struct pending_request *sync_req, *exist;
+	struct pending_request *pending_req, *exist;
 	int ret;
 
-	sync_req = malloc(sizeof(*sync_req));
-	reply_msg = malloc(sizeof(*reply_msg));
-	if (sync_req == NULL || reply_msg == NULL) {
+	pending_req = calloc(1, sizeof(*pending_req));
+	reply_msg = calloc(1, sizeof(*reply_msg));
+	if (pending_req == NULL || reply_msg == NULL) {
 		RTE_LOG(ERR, EAL, "Could not allocate space for sync request\n");
 		rte_errno = ENOMEM;
 		ret = -1;
 		goto fail;
 	}
 
-	memset(sync_req, 0, sizeof(*sync_req));
-	memset(reply_msg, 0, sizeof(*reply_msg));
-
-	sync_req->type = REQUEST_TYPE_ASYNC;
-	strcpy(sync_req->dst, dst);
-	sync_req->request = req;
-	sync_req->reply = reply_msg;
-	sync_req->async.param = param;
+	pending_req->type = REQUEST_TYPE_ASYNC;
+	strcpy(pending_req->dst, dst);
+	pending_req->request = req;
+	pending_req->reply = reply_msg;
+	pending_req->async.param = param;
 
 	/* queue already locked by caller */
 
-	exist = find_sync_request(dst, req->name);
+	exist = find_pending_request(dst, req->name);
 	if (exist) {
 		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
 		rte_errno = EEXIST;
@@ -895,13 +893,13 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
 		ret = 0;
 		goto fail;
 	}
-	TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next);
+	TAILQ_INSERT_TAIL(&pending_requests.requests, pending_req, next);
 
 	param->user_reply.nb_sent++;
 
 	return 0;
 fail:
-	free(sync_req);
+	free(pending_req);
 	free(reply_msg);
 	return ret;
 }
@@ -912,16 +910,16 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
 {
 	int ret;
 	struct rte_mp_msg msg, *tmp;
-	struct pending_request sync_req, *exist;
+	struct pending_request pending_req, *exist;
 
-	sync_req.type = REQUEST_TYPE_SYNC;
-	sync_req.reply_received = 0;
-	strcpy(sync_req.dst, dst);
-	sync_req.request = req;
-	sync_req.reply = &msg;
-	pthread_cond_init(&sync_req.sync.cond, NULL);
+	pending_req.type = REQUEST_TYPE_SYNC;
+	pending_req.reply_received = 0;
+	strcpy(pending_req.dst, dst);
+	pending_req.request = req;
+	pending_req.reply = &msg;
+	pthread_cond_init(&pending_req.sync.cond, NULL);
 
-	exist = find_sync_request(dst, req->name);
+	exist = find_pending_request(dst, req->name);
 	if (exist) {
 		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
 		rte_errno = EEXIST;
@@ -936,24 +934,24 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
 	} else if (ret == 0)
 		return 0;
 
-	TAILQ_INSERT_TAIL(&pending_requests.requests, &sync_req, next);
+	TAILQ_INSERT_TAIL(&pending_requests.requests, &pending_req, next);
 
 	reply->nb_sent++;
 
 	do {
-		ret = pthread_cond_timedwait(&sync_req.sync.cond,
+		ret = pthread_cond_timedwait(&pending_req.sync.cond,
 				&pending_requests.lock, ts);
 	} while (ret != 0 && ret != ETIMEDOUT);
 
-	TAILQ_REMOVE(&pending_requests.requests, &sync_req, next);
+	TAILQ_REMOVE(&pending_requests.requests, &pending_req, next);
 
-	if (sync_req.reply_received == 0) {
+	if (pending_req.reply_received == 0) {
 		RTE_LOG(ERR, EAL, "Fail to recv reply for request %s:%s\n",
 			dst, req->name);
 		rte_errno = ETIMEDOUT;
 		return -1;
 	}
-	if (sync_req.reply_received == -1) {
+	if (pending_req.reply_received == -1) {
 		RTE_LOG(DEBUG, EAL, "Asked to ignore response\n");
 		/* not receiving this message is not an error, so decrement
 		 * number of sent messages
@@ -1078,19 +1076,15 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
 		rte_errno = errno;
 		return -1;
 	}
-	copy = malloc(sizeof(*copy));
-	dummy = malloc(sizeof(*dummy));
-	param = malloc(sizeof(*param));
+	copy = calloc(1, sizeof(*copy));
+	dummy = calloc(1, sizeof(*dummy));
+	param = calloc(1, sizeof(*param));
 	if (copy == NULL || dummy == NULL || param == NULL) {
 		RTE_LOG(ERR, EAL, "Failed to allocate memory for async reply\n");
 		rte_errno = ENOMEM;
 		goto fail;
 	}
 
-	memset(copy, 0, sizeof(*copy));
-	memset(dummy, 0, sizeof(*dummy));
-	memset(param, 0, sizeof(*param));
-
 	/* copy message */
 	memcpy(copy, req, sizeof(*copy));
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async
  2018-04-20  1:21 [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 1/2] ipc: clearn up code Jianfeng Tan
@ 2018-04-20  1:21 ` Jianfeng Tan
  2018-04-20  1:22   ` Tan, Jianfeng
  2018-04-20  7:56   ` Burakov, Anatoly
  2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
  2 siblings, 2 replies; 11+ messages in thread
From: Jianfeng Tan @ 2018-04-20  1:21 UTC (permalink / raw)
  To: dev; +Cc: thomas, anatoly.burakov, Jianfeng Tan

In original implementation, timeout event for an async request
will be ignored. As a result, an async request will never
trigger the action if it cannot receive any reply any more.

We fix this by counting timeout as a processed reply.

Fixes: f05e26051c15 ("eal: add IPC asynchronous request")

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 070a075..27de16e 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -419,7 +419,13 @@ process_async_request(struct pending_request *sr, const struct timespec *now)
 	} else if (sr->reply_received == -1) {
 		/* we were asked to ignore this process */
 		reply->nb_sent--;
+	} else if (timeout) {
+		/* count it as processed reponse, but don't increment
+		 * nb_received.
+		 */
+		param->n_responses_processed++;
 	}
+
 	free(sr->reply);
 
 	last_msg = param->n_responses_processed == reply->nb_sent;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
@ 2018-04-20  1:22   ` Tan, Jianfeng
  2018-04-20  7:56   ` Burakov, Anatoly
  1 sibling, 0 replies; 11+ messages in thread
From: Tan, Jianfeng @ 2018-04-20  1:22 UTC (permalink / raw)
  To: dev; +Cc: thomas, Burakov, Anatoly



> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Friday, April 20, 2018 9:22 AM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Burakov, Anatoly; Tan, Jianfeng
> Subject: [PATCH 2/2] ipc: fix timeout not properly handled in async
> 
> In original implementation, timeout event for an async request
> will be ignored. As a result, an async request will never
> trigger the action if it cannot receive any reply any more.
> 
> We fix this by counting timeout as a processed reply.
> 
> Fixes: f05e26051c15 ("eal: add IPC asynchronous request")
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  lib/librte_eal/common/eal_common_proc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_eal/common/eal_common_proc.c
> b/lib/librte_eal/common/eal_common_proc.c
> index 070a075..27de16e 100644
> --- a/lib/librte_eal/common/eal_common_proc.c
> +++ b/lib/librte_eal/common/eal_common_proc.c
> @@ -419,7 +419,13 @@ process_async_request(struct pending_request *sr,
> const struct timespec *now)
>  	} else if (sr->reply_received == -1) {
>  		/* we were asked to ignore this process */
>  		reply->nb_sent--;
> +	} else if (timeout) {
> +		/* count it as processed reponse, but don't increment

Oops, a typo:
s/reponse/response

> +		 * nb_received.
> +		 */
> +		param->n_responses_processed++;
>  	}
> +
>  	free(sr->reply);
> 
>  	last_msg = param->n_responses_processed == reply->nb_sent;
> --
> 2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ipc: clearn up code
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 1/2] ipc: clearn up code Jianfeng Tan
@ 2018-04-20  7:55   ` Burakov, Anatoly
  2018-04-20 14:41     ` Tan, Jianfeng
  0 siblings, 1 reply; 11+ messages in thread
From: Burakov, Anatoly @ 2018-04-20  7:55 UTC (permalink / raw)
  To: Jianfeng Tan, dev; +Cc: thomas

On 20-Apr-18 2:21 AM, Jianfeng Tan wrote:
> Following below commit, we change some internal function and variable
> names:
>    commit ce3a7312357b ("eal: rename IPC request as synchronous one")
> 
> Also use calloc to supersede malloc + memset for code clean up.
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---

In general,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

However, i think it would be good to have it rebased/applied on top of 
IPC coverity fixes, in particular because this patch uses strcpy instead 
of instead of strlcpy.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
  2018-04-20  1:22   ` Tan, Jianfeng
@ 2018-04-20  7:56   ` Burakov, Anatoly
  1 sibling, 0 replies; 11+ messages in thread
From: Burakov, Anatoly @ 2018-04-20  7:56 UTC (permalink / raw)
  To: Jianfeng Tan, dev; +Cc: thomas

On 20-Apr-18 2:21 AM, Jianfeng Tan wrote:
> In original implementation, timeout event for an async request
> will be ignored. As a result, an async request will never
> trigger the action if it cannot receive any reply any more.
> 
> We fix this by counting timeout as a processed reply.
> 
> Fixes: f05e26051c15 ("eal: add IPC asynchronous request")
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Thanks for catching this!

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ipc: clearn up code
  2018-04-20  7:55   ` Burakov, Anatoly
@ 2018-04-20 14:41     ` Tan, Jianfeng
  0 siblings, 0 replies; 11+ messages in thread
From: Tan, Jianfeng @ 2018-04-20 14:41 UTC (permalink / raw)
  To: Burakov, Anatoly, dev; +Cc: thomas



On 4/20/2018 3:55 PM, Burakov, Anatoly wrote:
> On 20-Apr-18 2:21 AM, Jianfeng Tan wrote:
>> Following below commit, we change some internal function and variable
>> names:
>>    commit ce3a7312357b ("eal: rename IPC request as synchronous one")
>>
>> Also use calloc to supersede malloc + memset for code clean up.
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>
> In general,
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> However, i think it would be good to have it rebased/applied on top of 
> IPC coverity fixes, in particular because this patch uses strcpy 
> instead of instead of strlcpy.

Will rebase on those fixes patches. And send the v2.

Thanks,
Jianfeng

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc
  2018-04-20  1:21 [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 1/2] ipc: clearn up code Jianfeng Tan
  2018-04-20  1:21 ` [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
@ 2018-04-20 15:20 ` Jianfeng Tan
  2018-04-20 15:20   ` [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Jianfeng Tan @ 2018-04-20 15:20 UTC (permalink / raw)
  To: dev; +Cc: thomas, Jianfeng Tan

v2:
  - Fix typo.
  - Rebase on below patches:
    http://dpdk.org/dev/patchwork/patch/38358/
    http://dpdk.org/dev/patchwork/patch/38359/
    http://dpdk.org/dev/patchwork/patch/38360/

Patch 1 is to rename some variable names for better readability.

Patch 2 is a bug fix.

Jianfeng Tan (2):
  ipc: clearn up code
  ipc: fix timeout not properly handled in async

 lib/librte_eal/common/eal_common_proc.c | 90 +++++++++++++++++----------------
 1 file changed, 46 insertions(+), 44 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code
  2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
@ 2018-04-20 15:20   ` Jianfeng Tan
  2018-04-20 15:20   ` [dpdk-dev] [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
  2018-04-23 20:48   ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon
  2 siblings, 0 replies; 11+ messages in thread
From: Jianfeng Tan @ 2018-04-20 15:20 UTC (permalink / raw)
  To: dev; +Cc: thomas, Jianfeng Tan

Following below commit, we change some internal function and variable
names:
  commit ce3a7312357b ("eal: rename IPC request as synchronous one")

Also use calloc to supersede malloc + memset for code clean up.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 84 ++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 44 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index a329708..f8cb0e8 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -108,7 +108,7 @@ mp_send(struct rte_mp_msg *msg, const char *peer, int type);
 
 
 static struct pending_request *
-find_sync_request(const char *dst, const char *act_name)
+find_pending_request(const char *dst, const char *act_name)
 {
 	struct pending_request *r;
 
@@ -282,7 +282,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 static void
 process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 {
-	struct pending_request *sync_req;
+	struct pending_request *pending_req;
 	struct action_entry *entry;
 	struct rte_mp_msg *msg = &m->msg;
 	rte_mp_t action = NULL;
@@ -291,15 +291,16 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 
 	if (m->type == MP_REP || m->type == MP_IGN) {
 		pthread_mutex_lock(&pending_requests.lock);
-		sync_req = find_sync_request(s->sun_path, msg->name);
-		if (sync_req) {
-			memcpy(sync_req->reply, msg, sizeof(*msg));
+		pending_req = find_pending_request(s->sun_path, msg->name);
+		if (pending_req) {
+			memcpy(pending_req->reply, msg, sizeof(*msg));
 			/* -1 indicates that we've been asked to ignore */
-			sync_req->reply_received = m->type == MP_REP ? 1 : -1;
+			pending_req->reply_received =
+				m->type == MP_REP ? 1 : -1;
 
-			if (sync_req->type == REQUEST_TYPE_SYNC)
-				pthread_cond_signal(&sync_req->sync.cond);
-			else if (sync_req->type == REQUEST_TYPE_ASYNC)
+			if (pending_req->type == REQUEST_TYPE_SYNC)
+				pthread_cond_signal(&pending_req->sync.cond);
+			else if (pending_req->type == REQUEST_TYPE_ASYNC)
 				pthread_cond_signal(
 					&pending_requests.async_cond);
 		} else
@@ -322,6 +323,7 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
 			 * yet ready to process this request.
 			 */
 			struct rte_mp_msg dummy;
+
 			memset(&dummy, 0, sizeof(dummy));
 			strlcpy(dummy.name, msg->name, sizeof(dummy.name));
 			mp_send(&dummy, s->sun_path, MP_IGN);
@@ -855,30 +857,28 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
 		struct async_request_param *param)
 {
 	struct rte_mp_msg *reply_msg;
-	struct pending_request *sync_req, *exist;
+	struct pending_request *pending_req, *exist;
 	int ret;
 
-	sync_req = malloc(sizeof(*sync_req));
-	reply_msg = malloc(sizeof(*reply_msg));
-	if (sync_req == NULL || reply_msg == NULL) {
+	pending_req = calloc(1, sizeof(*pending_req));
+	reply_msg = calloc(1, sizeof(*reply_msg));
+	if (pending_req == NULL || reply_msg == NULL) {
 		RTE_LOG(ERR, EAL, "Could not allocate space for sync request\n");
 		rte_errno = ENOMEM;
 		ret = -1;
 		goto fail;
 	}
 
-	memset(sync_req, 0, sizeof(*sync_req));
-	memset(reply_msg, 0, sizeof(*reply_msg));
-
-	sync_req->type = REQUEST_TYPE_ASYNC;
-	strlcpy(sync_req->dst, dst, sizeof(sync_req->dst));
-	sync_req->request = req;
-	sync_req->reply = reply_msg;
-	sync_req->async.param = param;
+	pending_req->type = REQUEST_TYPE_ASYNC;
+	strlcpy(pending_req->dst, dst, sizeof(pending_req->dst));
+	strcpy(pending_req->dst, dst);
+	pending_req->request = req;
+	pending_req->reply = reply_msg;
+	pending_req->async.param = param;
 
 	/* queue already locked by caller */
 
-	exist = find_sync_request(dst, req->name);
+	exist = find_pending_request(dst, req->name);
 	if (exist) {
 		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
 		rte_errno = EEXIST;
@@ -896,13 +896,13 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
 		ret = 0;
 		goto fail;
 	}
-	TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next);
+	TAILQ_INSERT_TAIL(&pending_requests.requests, pending_req, next);
 
 	param->user_reply.nb_sent++;
 
 	return 0;
 fail:
-	free(sync_req);
+	free(pending_req);
 	free(reply_msg);
 	return ret;
 }
@@ -913,16 +913,16 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
 {
 	int ret;
 	struct rte_mp_msg msg, *tmp;
-	struct pending_request sync_req, *exist;
+	struct pending_request pending_req, *exist;
 
-	sync_req.type = REQUEST_TYPE_SYNC;
-	sync_req.reply_received = 0;
-	strlcpy(sync_req.dst, dst, sizeof(sync_req.dst));
-	sync_req.request = req;
-	sync_req.reply = &msg;
-	pthread_cond_init(&sync_req.sync.cond, NULL);
+	pending_req.type = REQUEST_TYPE_SYNC;
+	pending_req.reply_received = 0;
+	strlcpy(pending_req.dst, dst, sizeof(pending_req.dst));
+	pending_req.request = req;
+	pending_req.reply = &msg;
+	pthread_cond_init(&pending_req.sync.cond, NULL);
 
-	exist = find_sync_request(dst, req->name);
+	exist = find_pending_request(dst, req->name);
 	if (exist) {
 		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
 		rte_errno = EEXIST;
@@ -937,24 +937,24 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
 	} else if (ret == 0)
 		return 0;
 
-	TAILQ_INSERT_TAIL(&pending_requests.requests, &sync_req, next);
+	TAILQ_INSERT_TAIL(&pending_requests.requests, &pending_req, next);
 
 	reply->nb_sent++;
 
 	do {
-		ret = pthread_cond_timedwait(&sync_req.sync.cond,
+		ret = pthread_cond_timedwait(&pending_req.sync.cond,
 				&pending_requests.lock, ts);
 	} while (ret != 0 && ret != ETIMEDOUT);
 
-	TAILQ_REMOVE(&pending_requests.requests, &sync_req, next);
+	TAILQ_REMOVE(&pending_requests.requests, &pending_req, next);
 
-	if (sync_req.reply_received == 0) {
+	if (pending_req.reply_received == 0) {
 		RTE_LOG(ERR, EAL, "Fail to recv reply for request %s:%s\n",
 			dst, req->name);
 		rte_errno = ETIMEDOUT;
 		return -1;
 	}
-	if (sync_req.reply_received == -1) {
+	if (pending_req.reply_received == -1) {
 		RTE_LOG(DEBUG, EAL, "Asked to ignore response\n");
 		/* not receiving this message is not an error, so decrement
 		 * number of sent messages
@@ -1079,19 +1079,15 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
 		rte_errno = errno;
 		return -1;
 	}
-	copy = malloc(sizeof(*copy));
-	dummy = malloc(sizeof(*dummy));
-	param = malloc(sizeof(*param));
+	copy = calloc(1, sizeof(*copy));
+	dummy = calloc(1, sizeof(*dummy));
+	param = calloc(1, sizeof(*param));
 	if (copy == NULL || dummy == NULL || param == NULL) {
 		RTE_LOG(ERR, EAL, "Failed to allocate memory for async reply\n");
 		rte_errno = ENOMEM;
 		goto fail;
 	}
 
-	memset(copy, 0, sizeof(*copy));
-	memset(dummy, 0, sizeof(*dummy));
-	memset(param, 0, sizeof(*param));
-
 	/* copy message */
 	memcpy(copy, req, sizeof(*copy));
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] ipc: fix timeout not properly handled in async
  2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20 15:20   ` [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan
@ 2018-04-20 15:20   ` Jianfeng Tan
  2018-04-23 20:48   ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon
  2 siblings, 0 replies; 11+ messages in thread
From: Jianfeng Tan @ 2018-04-20 15:20 UTC (permalink / raw)
  To: dev; +Cc: thomas, Jianfeng Tan, Anatoly Burakov

In original implementation, timeout event for an async request
will be ignored. As a result, an async request will never
trigger the action if it cannot receive any reply any more.

We fix this by counting timeout as a processed reply.

Fixes: f05e26051c15 ("eal: add IPC asynchronous request")

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/common/eal_common_proc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index f8cb0e8..9136fb0 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -419,7 +419,13 @@ process_async_request(struct pending_request *sr, const struct timespec *now)
 	} else if (sr->reply_received == -1) {
 		/* we were asked to ignore this process */
 		reply->nb_sent--;
+	} else if (timeout) {
+		/* count it as processed response, but don't increment
+		 * nb_received.
+		 */
+		param->n_responses_processed++;
 	}
+
 	free(sr->reply);
 
 	last_msg = param->n_responses_processed == reply->nb_sent;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc
  2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20 15:20   ` [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan
  2018-04-20 15:20   ` [dpdk-dev] [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
@ 2018-04-23 20:48   ` Thomas Monjalon
  2 siblings, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2018-04-23 20:48 UTC (permalink / raw)
  To: Jianfeng Tan; +Cc: dev

20/04/2018 17:20, Jianfeng Tan:
> v2:
>   - Fix typo.
>   - Rebase on below patches:
>     http://dpdk.org/dev/patchwork/patch/38358/
>     http://dpdk.org/dev/patchwork/patch/38359/
>     http://dpdk.org/dev/patchwork/patch/38360/
> 
> Patch 1 is to rename some variable names for better readability.
> 
> Patch 2 is a bug fix.
> 
> Jianfeng Tan (2):
>   ipc: clearn up code
>   ipc: fix timeout not properly handled in async

Applied, thanks

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-04-23 20:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-20  1:21 [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan
2018-04-20  1:21 ` [dpdk-dev] [PATCH 1/2] ipc: clearn up code Jianfeng Tan
2018-04-20  7:55   ` Burakov, Anatoly
2018-04-20 14:41     ` Tan, Jianfeng
2018-04-20  1:21 ` [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
2018-04-20  1:22   ` Tan, Jianfeng
2018-04-20  7:56   ` Burakov, Anatoly
2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
2018-04-20 15:20   ` [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan
2018-04-20 15:20   ` [dpdk-dev] [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
2018-04-23 20:48   ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).