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