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

[Thomas Monjalon <thomas@monjalon.net>]

04/05/2021 21:12, Morten Brørup:
> > From: dev [mailto:dev-bounces@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@dpdk.org
> > >
> > > Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > > Signed-off-by: Min Hu (Connor) <humin29@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 see "return 0 for success, or -1 for failure".
Where is it said it cannot fail?

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

I think nobody hit such bug with DPDK IPC.