[dpdk-dev] [PATCH v2] eal: fix use wrong time API

[Morten Brørup]

> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, May 4, 2021 6:50 PM
> 
> 29/04/2021 04:10, Min Hu (Connor):
> > 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: stable at dpdk.org
> >
> > Signed-off-by: Chengwen Feng <fengchengwen at huawei.com>
> > Signed-off-by: Min Hu (Connor) <humin29 at huawei.com>
> > ---
> [...]
> > --- a/lib/eal/common/eal_common_proc.c
> > +++ b/lib/eal/common/eal_common_proc.c
> > -	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_MONOTONIC, &ts_now);
> 
> Why not testing the return value?

Because it is guaranteed not to fail. Ref:
https://linux.die.net/man/3/clock_gettime
https://www.freebsd.org/cgi/man.cgi?query=clock_gettime

> 
> I think this change would not be appropriate after -rc1.
> If you agree, I will postpone to DPDK 21.08.

It does fix a serious bug, where IPC timeouts can incorrectly happen. And this is not a theoretical bug; I have seen errors happen due to using the wrong clock source in other projects.

However, I have no clue if these IPC library functions are important or not. So I have no qualified opinion about postponing the change.