* [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.