All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] code clean up and fix in ipc
@ 2018-04-20  1:21 Jianfeng Tan
  2018-04-20  1:21 ` [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

* [PATCH 1/2] ipc: clearn up code
  2018-04-20  1:21 [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 ` [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
  2018-04-20 15:20 ` [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 related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] ipc: fix timeout not properly handled in async
  2018-04-20  1:21 [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20  1:21 ` [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 ` [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 related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] ipc: fix timeout not properly handled in async
  2018-04-20  1:21 ` [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: [PATCH 1/2] ipc: clearn up code
  2018-04-20  1:21 ` [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: [PATCH 2/2] ipc: fix timeout not properly handled in async
  2018-04-20  1:21 ` [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: [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

* [PATCH v2 0/2] code clean up and fix in ipc
  2018-04-20  1:21 [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20  1:21 ` [PATCH 1/2] ipc: clearn up code Jianfeng Tan
  2018-04-20  1:21 ` [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   ` [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

* [PATCH v2 1/2] ipc: clearn up code
  2018-04-20 15:20 ` [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   ` [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
  2018-04-23 20:48   ` [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 related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] ipc: fix timeout not properly handled in async
  2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20 15:20   ` [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan
@ 2018-04-20 15:20   ` Jianfeng Tan
  2018-04-23 20:48   ` [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 related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/2] code clean up and fix in ipc
  2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
  2018-04-20 15:20   ` [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan
  2018-04-20 15:20   ` [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 [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan
2018-04-20  1:21 ` [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 ` [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 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan
2018-04-20 15:20   ` [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan
2018-04-20 15:20   ` [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan
2018-04-23 20:48   ` [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.