From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 9418BA0A0C;
	Thu,  1 Jul 2021 23:36:50 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 18D8E40141;
	Thu,  1 Jul 2021 23:36:50 +0200 (CEST)
Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com
 [209.85.167.52]) by mails.dpdk.org (Postfix) with ESMTP id DBE964003E
 for <dev@dpdk.org>; Thu,  1 Jul 2021 23:36:48 +0200 (CEST)
Received: by mail-lf1-f52.google.com with SMTP id t17so14480777lfq.0
 for <dev@dpdk.org>; Thu, 01 Jul 2021 14:36:48 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=date:from:to:cc:subject:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding;
 bh=MshBDmKJdriKdQdr7RhEMF8Aj4fzpRY8pP2JatqvZ8c=;
 b=VOdT+8ZeVerAl/xyg/ktnevIbdmqrgDuB71P9WpSIlFx52Qtv8xMl4y7xog9txtwRU
 RaltndayMdPM/QZHSRXlIBzV/1zzaDYdNkoxvzcuvHoj+FyG6eQamOXkdHUQkNgbFb2s
 6HS750JQKz75ia9pNmcvmy88aq+NiJ4Xia57MPomFB3pArMuWeEM0A6FWZslAoij86xJ
 ImvmxP3zGfPQvTp5BOpCisHnCc9J9pFfYKeQQgGYDL2X9oDoqK3tQllbu3PrAbzVKJkN
 78DbQ6vXfR/lLVtIAYjJBFMUDsUmLDHbKneg3aXeTo7jKsdzDrAwJxJoM3xYrClczmHX
 s2zg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to
 :references:mime-version:content-transfer-encoding;
 bh=MshBDmKJdriKdQdr7RhEMF8Aj4fzpRY8pP2JatqvZ8c=;
 b=KW8XgvyLz+lQFIJvpfQySjqv852+iaazW4CuM7yasXfm9LQcRytsD87aRx9UKdix0r
 Riw5DObXLN301fCzOtyPqvBG7eNhKTWZbMFmxEi1mpPulWr1OAFIqceuOtws+VGr7qV7
 PD8GP84DEcidNM8FBbnp3noAjvUF8OtMZiKACIZiYYG24aUiqFaslTiPC01JJvpM7rPz
 /8F9kXSIiKr5EeFHgoYI1KlJfhX3PcuhCZtsjTNFCbNhK3/BqwfnUqxSF2B1uPj6kz5K
 pRRliGtOzCK/gfAZ/bvnEpo1ekVx6aMb8OKX4jD++oTZIe8s3m988+dNEaM01n6c5lml
 MnSQ==
X-Gm-Message-State: AOAM531jqlASfRg9Pynoq5BR9vKo5J2xSq+Mvi3VFvUnCmPPKnF98oa8
 +CIVBkrPZvObXt2x0chWxB4=
X-Google-Smtp-Source: ABdhPJyJYw60YtPQWccNQfN9c2KalMn5mP+OpSEErbWLTDXQMeiIC80sm+cwaxnXi/rgyi4USHWgwg==
X-Received: by 2002:ac2:546a:: with SMTP id e10mr1281109lfn.244.1625175408340; 
 Thu, 01 Jul 2021 14:36:48 -0700 (PDT)
Received: from sovereign (broadband-37-110-65-23.ip.moscow.rt.ru.
 [37.110.65.23])
 by smtp.gmail.com with ESMTPSA id u19sm142593ljl.99.2021.07.01.14.36.46
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Thu, 01 Jul 2021 14:36:47 -0700 (PDT)
Date: Fri, 2 Jul 2021 00:36:45 +0300
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
To: Tyler Retzlaff <roretzla@linux.microsoft.com>
Cc: Jie Zhou <jizh@linux.microsoft.com>, dev@dpdk.org,
 roretzla@microsoft.com, talshn@nvidia.com, pallavi.kadam@intel.com,
 navasile@linux.microsoft.com, dmitrym@microsoft.com
Message-ID: <20210702003645.13813371@sovereign>
In-Reply-To: <20210701162144.GA9873@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1624495001-16613-1-git-send-email-jizh@linux.microsoft.com>
 <20210701023129.2ee5e076@sovereign>
 <20210701162144.GA9873@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.32; x86_64-pc-linux-gnu)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v1] lib/eal: enforce alarm APIs parameters
 check
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2021-07-01 09:21 (UTC-0700), Tyler Retzlaff:
> On Thu, Jul 01, 2021 at 02:31:29AM +0300, Dmitry Kozlyuk wrote:
> > Hi Jie,
> > 
> > 2021-06-23 17:36 (UTC-0700), Jie Zhou:  
> > > From: Jie Zhou <jizh@microsoft.com>
> [...]
> > > +	/* Check if us is valid */
> > > +	if (us < 1 || us >(UINT64_MAX - US_PER_S)) {  
> > 
> > This condition is specific to Linux EAL. In fact, it's not very useful even
> > there, because actual upper bound for `us` depends on current time.
> > No bounds are specified in API description at all.
> > Windows check would be different, but these considerations remain valid.
> > 
> > Maybe it's alarm_autotest or API description that needs adjustments,
> > but not the implementation.  
> 
> i agree with your assessment. it's a bit silly to test a range
> constraint where the range is just some arbitrary range and not the real
> range. changing the implementation to calculate the "real" valid range
> isn't practical.  i'm sure linux implementors would argue that catching
> some extremely out of range values is better than none?

Why we care about overflow?
1. It likely indicates a bug in the app.
2. Alarm is can to be scheduled to some time in the past and fire
immediately, which means API did not do what it promises.
I see the only way to tell the interval is incorrect if it gives
(would give) time in the past when added to the current time.
But this is an implementation detail and does not need testing.
A separate patch can be submitted to change behavior for all OS.

> 
> i guess a correct test would would calculate the valid range and test
> against that but as per above the implementation won't pass the test.
> 
> so we are left with
> 
> * matching the range imposed in the linux implementation so we pass the
>   test as is.

It would be the worst thing one could do, defying the purpose of unit tests.

> * don't run the test with the input data that exercises this bogus range
>   constraint.
> 
> i guess based on your comments you prefer the latter? so is our action
> here to submit a patch for the test that doesn't test this range
> conditionally on execenv windows?

Yes, please remove this check from the test as it verifies no contract.