From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id E4274A05D3 for ; Fri, 26 Apr 2019 10:13:30 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BA7D01B111; Fri, 26 Apr 2019 10:13:29 +0200 (CEST) Received: from smtp-3.sys.kth.se (smtp-3.sys.kth.se [130.237.48.192]) by dpdk.org (Postfix) with ESMTP id 194861B101 for ; Fri, 26 Apr 2019 10:13:28 +0200 (CEST) Received: from smtp-3.sys.kth.se (localhost.localdomain [127.0.0.1]) by smtp-3.sys.kth.se (Postfix) with ESMTP id C5C339E7C; Fri, 26 Apr 2019 10:13:27 +0200 (CEST) X-Virus-Scanned: by amavisd-new at kth.se Received: from smtp-3.sys.kth.se ([127.0.0.1]) by smtp-3.sys.kth.se (smtp-3.sys.kth.se [127.0.0.1]) (amavisd-new, port 10024) with LMTP id rfbAc96J6mcj; Fri, 26 Apr 2019 10:13:27 +0200 (CEST) X-KTH-Auth: barbette [130.237.20.142] DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kth.se; s=default; t=1556266407; bh=cCucSK7g0OzbbhJ9Y0gUuRmbu50ZBJ55JWDy3iauUTY=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=JkDYqpWpZzWA8oKnhdWwKDDsoonvFZSiBp5EQ++38BxlJY3YM+g3jvlmk+UE9kgGi ftpd/y4ERPnHi9wrQN0PNOOyic6zSjRIx+xyisduin4B7Fg1523lqsbKKlU93+w75s ANSbYaHpgy03LbKMB3EiBTGeh6PkcPXapIrol2GU= X-KTH-mail-from: barbette@kth.se Received: from [130.237.20.142] (s2587.it.kth.se [130.237.20.142]) by smtp-3.sys.kth.se (Postfix) with ESMTPSA id 753799E7E; Fri, 26 Apr 2019 10:13:25 +0200 (CEST) To: "Wiles, Keith" Cc: dpdk-dev , "Richardson, Bruce" , "Mcnamara, John" , Thomas Monjalon , "Yigit, Ferruh" , Andrew Rybchenko , Shahaf Shuler , Yongseok Koh , "olivier.matz@6wind.com" References: <20190424173424.34628-1-barbette@kth.se> <20190424173424.34628-2-barbette@kth.se> From: Tom Barbette Message-ID: <91e23122-84a2-90dc-e279-175889627228@kth.se> Date: Fri, 26 Apr 2019 10:13:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 1/3] rte_ethdev: Add API function to read dev clock X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190426081325.1cHmn35V78vf6COMMgjx4FB5D8osc-wsglEdcwvoun4@z> @Andrew I applied your comments. Thanks. On 2019-04-25 20:28, Wiles, Keith wrote: > What is a raw clock value? It took me a bit to find that it is in nano-seconds need to document that point. It is not in nanosecond, it has no units. Finding the relation between the device clock and the real time is the whole point of this API. > Using timestamp variable is not very descriptive and some other name needs to be used. Also in the driver it is called *clock. The problem is that the "timestamp offload" feature filling the timestamp field of the pktmbuf is already badly named as it is given without time unit. Both of them are not timestamp but raw "clock" values, a number of ticks at an unknown frequency, with an unknow time base (ie is it the number of ticks since boot? Device is up? 1/1/1970?). One solution would be to have an union in the pktmbuf, changing the timestamp field into a "clock" or "timestamp" field. But it's a bit overkill. I propose to change the read_clock API to reference "clock" instead of timestamp everywhere. > Another question is why does this routine not perform the looping/delaying and calling read_clock and then return frequency instead. If you want a timestamp reading function then this one is not being described that way and I would think it should be done someplace else or should be. The frequency is one thing, and would allow to give a relative time in second between packets. In practice though, we change the frequency (as Linux does regarding the TSC ticks) to catch up corrections applied by NTP on the source clock. The second thing is the time reference, to convert the clock time (of the packets) to the current wall time. One may want to use the monotonic clock, the real clock (we do follow both). Or no system clock at all and directly follow an NTP source. The point is, having a function to give the frequency is not enough. One can derive the frequency and the time reference with this API. I could write a helper in a second step to get the frequency out of read_clock. But the time reference implies timers, choosing a source, and stuff we don't want here, I think... >> /** Valid if PKT_RX_TIMESTAMP is set. The unit and time reference >> * are not normalized but are always the same for a given port. >> + * Some devices allow to query rte_eth_read_clock that will return the >> + * current device timestamp. > > The message here is not a good place for this detail, I would put it in the function call doc above. I can remove these lines, but with read_clock referencing only clock I feel like it's even more needed. Someone reading the doc will see "oh, I can use timestamp offloading to get precise timing of when my packets were received, great ! But it has no unit and no reference... What do I do with that?". It miss the last step "how I see... I can look at rte_eth_read_clock documentation to read more about conversion." Thanks ! Tom