> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Min Hu (Connor)
> Sent: Wednesday, April 28, 2021 10:13 AM
>
> From: Chengwen Feng <fengcheng...@huawei.com>
>
> Currently, the mp uses gettimeofday() API to get the time, and used as
> timeout parameter.
>
> But the time which gets from gettimeofday() API isn't monotonically
> increasing. The process may fail if the system time is changed.
>
> This fixes it by using clock_gettime() API with monotonic attribution.
>
> Fixes: 783b6e54971d ("eal: add synchronous multi-process
> communication")
> Fixes: f05e26051c15 ("eal: add IPC asynchronous request")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com>
> Signed-off-by: Min Hu (Connor) <humi...@huawei.com>
> ---
> lib/eal/common/eal_common_proc.c | 45 +++++++++++++++++---------------
> --------
> 1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_proc.c
> b/lib/eal/common/eal_common_proc.c
> index 6d1af3c..7f08826 100644
> --- a/lib/eal/common/eal_common_proc.c
> +++ b/lib/eal/common/eal_common_proc.c
> @@ -40,6 +40,12 @@ static char mp_dir_path[PATH_MAX]; /* The directory
> path for all mp sockets */
> static pthread_mutex_t mp_mutex_action = PTHREAD_MUTEX_INITIALIZER;
> static char peer_name[PATH_MAX];
>
> +#ifdef CLOCK_MONOTONIC_RAW /* Defined in glibc bits/time.h */
> +#define CLOCK_TYPE_ID CLOCK_MONOTONIC_RAW
> +#else
> +#define CLOCK_TYPE_ID CLOCK_MONOTONIC
> +#endif
Just out of curiosity: Why do you prefer CLOCK_MONOTONIC_RAW over
CLOCK_MONOTONIC?
> +
> struct action_entry {
> TAILQ_ENTRY(action_entry) next;
> char action_name[RTE_MP_MAX_NAME_LEN];
> @@ -490,14 +496,8 @@ async_reply_handle_thread_unsafe(void *arg)
> struct pending_request *req = (struct pending_request *)arg;
> enum async_action action;
> struct timespec ts_now;
> - struct timeval now;
>
> - if (gettimeofday(&now, NULL) < 0) {
> - RTE_LOG(ERR, EAL, "Cannot get current time\n");
> - goto no_trigger;
> - }
> - ts_now.tv_nsec = now.tv_usec * 1000;
> - ts_now.tv_sec = now.tv_sec;
> + clock_gettime(CLOCK_TYPE_ID, &ts_now);
>
> action = process_async_request(req, &ts_now);
>
> @@ -896,6 +896,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg
> *req,
> struct rte_mp_reply *reply, const struct timespec *ts)
> {
> int ret;
> + pthread_condattr_t attr;
> struct rte_mp_msg msg, *tmp;
> struct pending_request pending_req, *exist;
>
> @@ -904,7 +905,9 @@ mp_request_sync(const char *dst, struct rte_mp_msg
> *req,
> 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);
> + pthread_condattr_init(&attr);
> + pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
Shouldn't CLOCK_MONOTONIC be CLOCK_TYPE_ID here too?
> + pthread_cond_init(&pending_req.sync.cond, &attr);
>
> exist = find_pending_request(dst, req->name);
> if (exist) {
> @@ -967,8 +970,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
> rte_mp_reply *reply,
> int dir_fd, ret = -1;
> DIR *mp_dir;
> struct dirent *ent;
> - struct timeval now;
> - struct timespec end;
> + struct timespec now, end;
> const struct internal_config *internal_conf =
> eal_get_internal_configuration();
>
> @@ -987,15 +989,10 @@ rte_mp_request_sync(struct rte_mp_msg *req,
> struct rte_mp_reply *reply,
> return -1;
> }
>
> - if (gettimeofday(&now, NULL) < 0) {
> - RTE_LOG(ERR, EAL, "Failed to get current time\n");
> - rte_errno = errno;
> - goto end;
> - }
> -
> - end.tv_nsec = (now.tv_usec * 1000 + ts->tv_nsec) % 1000000000;
> + clock_gettime(CLOCK_TYPE_ID, &now);
> + end.tv_nsec = (now.tv_nsec + ts->tv_nsec) % 1000000000;
> end.tv_sec = now.tv_sec + ts->tv_sec +
> - (now.tv_usec * 1000 + ts->tv_nsec) / 1000000000;
> + (now.tv_nsec + ts->tv_nsec) / 1000000000;
>
> /* for secondary process, send request to the primary process
> only */
> if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> @@ -1069,7 +1066,7 @@ rte_mp_request_async(struct rte_mp_msg *req,
> const struct timespec *ts,
> int dir_fd, ret = 0;
> DIR *mp_dir;
> struct dirent *ent;
> - struct timeval now;
> + struct timespec now;
> struct timespec *end;
> bool dummy_used = false;
> const struct internal_config *internal_conf =
> @@ -1086,11 +1083,6 @@ rte_mp_request_async(struct rte_mp_msg *req,
> const struct timespec *ts,
> return -1;
> }
>
> - if (gettimeofday(&now, NULL) < 0) {
> - RTE_LOG(ERR, EAL, "Failed to get current time\n");
> - rte_errno = errno;
> - return -1;
> - }
> copy = calloc(1, sizeof(*copy));
> dummy = calloc(1, sizeof(*dummy));
> param = calloc(1, sizeof(*param));
> @@ -1108,9 +1100,10 @@ rte_mp_request_async(struct rte_mp_msg *req,
> const struct timespec *ts,
> end = ¶m->end;
> reply = ¶m->user_reply;
>
> - end->tv_nsec = (now.tv_usec * 1000 + ts->tv_nsec) % 1000000000;
> + clock_gettime(CLOCK_TYPE_ID, &now);
> + end->tv_nsec = (now.tv_nsec + ts->tv_nsec) % 1000000000;
> end->tv_sec = now.tv_sec + ts->tv_sec +
> - (now.tv_usec * 1000 + ts->tv_nsec) / 1000000000;
> + (now.tv_nsec + ts->tv_nsec) / 1000000000;
> reply->nb_sent = 0;
> reply->nb_received = 0;
> reply->msgs = NULL;
> --
> 2.7.4
>
