From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9AB89A0350; Thu, 25 Jun 2020 18:35:36 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6B3E3E07; Thu, 25 Jun 2020 18:35:35 +0200 (CEST) Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by dpdk.org (Postfix) with ESMTP id DBDF4A3 for ; Thu, 25 Jun 2020 18:35:33 +0200 (CEST) Received: by mail-wm1-f67.google.com with SMTP id u26so7098103wmn.1 for ; Thu, 25 Jun 2020 09:35:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=HnjVVACIZUtVdpNPeCvNPr3IvnVp14UgJd5CWT58YT0=; b=SSfvPTDdGbvD9OBSdM6EeQ63om7WAC4Z5ofHQ6C4TR3t7bsz0hmvm3raAV3c6vP9lG wcP8i9YYXYV75Ti8y4Wek785cJ31gqOXUkH9g09mLFLPigdYwlla2yZE5Xyl/gfL5ln6 KePItrSj+Q0rG4ycVeT9isUUdlfDmeYa08XVf2GO0xcvEa+vLXwduJ5CnKUfzm674jOg 1WdHfaIF7JBLmoFb93MQITtUSULxIFPt52mh/3OTr+OKk+W2oTJn2vCZ+ysdurf9objI XiWdyRyOmCFb+sBQh26t04Np7bk+boN9QyM1tNOpTRi+Qz9h9i/Kg5XWjCkUem3Vcu39 JJ/A== 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:references :mime-version:content-disposition:in-reply-to:user-agent; bh=HnjVVACIZUtVdpNPeCvNPr3IvnVp14UgJd5CWT58YT0=; b=qb+vELmGwfqIby3aTfzOtf9pAfclw+DVRddyxgjWI+akewJExYCmuy+AGNYT+LPD3l EawoDIU+A1IQb+6PHMUsXAjvrxPJ9Z7i0qJ6R10M8Olk5FJXZbYHYYfKISlgYai5mBt+ 3cZVGerKHjfDdoDx9VJs5tkUSTBP1DSUcn9iy2H75B6sapMHssmfbZGc/9bTcQ0KklVH 7umDJfsg3TSln3HTFrz6d/W044t3dK/ORBP/rSwOyuh0iuzX1K6V+sgm3gvzLhwt41ma 2xmrgt3QxYEvcR0IZ3LpI7qdnI3gu1Upo30rir6dVJgCsJ7lVQ290X2YbsKXvmltVF3c NhoQ== X-Gm-Message-State: AOAM531XgwF2XAYHn5LoRnayPjoMIROnXnHJ+B2qUo5lpRGvewvD46xS md5EfciwF/IEfqf4Fd68BLsKaA== X-Google-Smtp-Source: ABdhPJzDt6CnfA2vDFsG4u7Em8AT60wykSKa07RXqN6Cw53YFoJ7QtReIArjwScqD5W0VWBbm8H7Vw== X-Received: by 2002:a1c:4804:: with SMTP id v4mr4263356wma.139.1593102933411; Thu, 25 Jun 2020 09:35:33 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id u23sm19054236wru.94.2020.06.25.09.35.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jun 2020 09:35:32 -0700 (PDT) Date: Thu, 25 Jun 2020 18:35:32 +0200 From: Olivier Matz To: Vivien Didelot Cc: Ferruh Yigit , dev@dpdk.org, PATRICK KEROULAS Message-ID: <20200625163532.GU12564@platinum> References: <20200610193938.218768-1-vivien.didelot@gmail.com> <5991f5d8-5d7f-4caf-733c-a5d29110a046@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH] net/pcap: support hardware Tx timestamps 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" Hi Vivien, On Tue, Jun 23, 2020 at 06:10:09PM -0400, Vivien Didelot wrote: > > On Wed, Jun 17, 2020 at 4:16 AM Ferruh Yigit wrote: > > > On 6/10/2020 8:39 PM, Vivien Didelot wrote: > > > When hardware timestamping is enabled on Rx path, system time should > > > no longer be used to calculate the timestamps when dumping packets. > > > > > > Instead, use the value stored by the driver in mbuf->timestamp > > > and assume it is already converted to nanoseconds (otherwise the > > > application may edit the packet headers itself afterwards). > > > > > > Signed-off-by: Vivien Didelot > > > Signed-off-by: Patrick Keroulas > > > --- > > > doc/guides/rel_notes/release_20_08.rst | 1 + > > > drivers/net/pcap/rte_eth_pcap.c | 30 +++++++++++++++----------- > > > 2 files changed, 18 insertions(+), 13 deletions(-) > > > > > > diff --git a/doc/guides/rel_notes/release_20_08.rst > > b/doc/guides/rel_notes/release_20_08.rst > > > index 7a67c960c..cedceaf9d 100644 > > > --- a/doc/guides/rel_notes/release_20_08.rst > > > +++ b/doc/guides/rel_notes/release_20_08.rst > > > @@ -61,6 +61,7 @@ New Features > > > Updated PCAP driver with new features and improvements, including: > > > > > > * Support software Tx nanosecond timestamps precision. > > > + * Support hardware Tx timestamps. > > > > > > * **Updated Mellanox mlx5 driver.** > > > > > > diff --git a/drivers/net/pcap/rte_eth_pcap.c > > b/drivers/net/pcap/rte_eth_pcap.c > > > index 13a3d0ac7..3d80b699b 100644 > > > --- a/drivers/net/pcap/rte_eth_pcap.c > > > +++ b/drivers/net/pcap/rte_eth_pcap.c > > > @@ -290,19 +290,23 @@ eth_null_rx(void *queue __rte_unused, > > > #define NSEC_PER_SEC 1000000000L > > > > > > static inline void > > > -calculate_timestamp(struct timeval *ts) { > > > - uint64_t cycles; > > > - struct timeval cur_time; > > > +calculate_timestamp(const struct rte_mbuf *mbuf, struct timeval *ts) { > > > + if (mbuf->ol_flags & PKT_RX_TIMESTAMP) { > > > + ts->tv_sec = mbuf->timestamp / NSEC_PER_SEC; > > > + ts->tv_usec = mbuf->timestamp % NSEC_PER_SEC; > > > > Hi Vivien, > > > > No objection from pcap PMD point of view. > > > > But should we have a Tx mbuf flag, 'PKT_TX_TIMESTAMP', for applications to > > request drivers to use the timestamp field on Tx path? Not sure if there > > can be > > any problem on using Rx flag on both direction? > > > > Also the metric is not defined for the 'mbuf->timestamp', it doesn't need > > to be > > nanoseconds, not sure if it is correct to assume it is. Or should we > > define a > > metric for timestamp on the Tx path? > > > > cc'ed Oliver, I think he can comment better on above two questions. > > > > Thanks, > > ferruh > > > > > Hi Oliver, > > Surprisingly, dumping PCAP with hardware timestamps seems to be a niche, > but we do need this feature for our network analyzing tool. > > Do you guys have objections for this patch? > > Regards, > Vivien > As said by Ferruh, the unit of timestamp in mbuf is not normalized to nanosecs, as seen in rte_mbuf_core.h: /** 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. */ uint64_t timestamp; Using the timestamp generated from a port which is not a pmd-pcap would require a conversion, using rte_eth_read_clock() on mbuf->port (assuming it was not modified, which should be true except if event eth Tx adapter is used). Also, note that the timestamp corresponds to the Rx timestamp. I think it could be an issue in case the mbuf is reassembled by the application: the timestamp in reassembled mbuf would be the one from the first fragment. So, I share Ferruh's concerns. If the problem is about calculate_timestamp() speed, I think there is some room for optimization: 1e6 is a float, so it will probably slow down the function. I think the following should also work, and would be faster: static inline void calculate_timestamp(struct timeval *ts) { uint64_t cycles; struct timeval cur_time; cycles = rte_get_timer_cycles() - start_cycles; cur_time.tv_sec = cycles / hz; cycles -= cur_time.tv_sec * hz; cur_time.tv_usec = (cycles * 1000000) / hz; timeradd(&start_time, &cur_time, ts); } I also think the call to timeradd() could be removed if an offset is added to start_cycles. Regards, Olivier