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 B6ABCA0524; Mon, 27 Jul 2020 00:17:28 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 567111C029; Mon, 27 Jul 2020 00:17:27 +0200 (CEST) Received: from mail-pg1-f194.google.com (mail-pg1-f194.google.com [209.85.215.194]) by dpdk.org (Postfix) with ESMTP id D1A091BFF5 for ; Mon, 27 Jul 2020 00:17:25 +0200 (CEST) Received: by mail-pg1-f194.google.com with SMTP id d4so8312291pgk.4 for ; Sun, 26 Jul 2020 15:17:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=BT2E0jGjKhwmOrHOP0UDEH1D6qvUWB3DsxruptPuqpQ=; b=Ax3BmmRUEzbMcl5KaekDNOtx+F/J8Uqqg3HLZhF/vV1aUPzLCCM0PXhufHO1BdEc/X fFpTEAt7KpDGerP/F9HuZ+m+H9sfFlHdsIqoi3361fLn0LG3fs7vtveubxQeaueRcRtT lugcb2hgPmlUfz3AL91OAsqsAcV7yUtudRiBQV8kYUkGW9p8+gLpqYEa4As3i1F61Rio 7akuhspSkLKEbYXodMkYgBc81axQ/7tyZHIUhai8O4DfUz55PxYSftDAYvatQohht+J+ 5BDip7nyLY7AqD9sUR5Uorf8RLbIZZTuFV7rZmbHcgp2KXLWVY/pdrBLcy1A8/G034nN yxPw== 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=BT2E0jGjKhwmOrHOP0UDEH1D6qvUWB3DsxruptPuqpQ=; b=NqZ3lYCFaH9DtYsphCykChtVTUxttZds5rmd2haxGfSFFLBrzRazHWFweRxo8AYRBu OLwvdWNAlnzkgCUQs5JuTv6zs9kqHFVsoPGZ+AIhacGH1gIMajXNGdmBDAFtH9BDf0oA XbA2tot12hBifmtCwYC34LlaTMgSrFHgDKK2u3wEhmmODu777MmjT9c+RuWiZPe+qF9I aWnf9f6L6hatZnbCINyuhx2r8TJn2YLgAZhbpqo9lEvxTaGzK84PZC1kq/Twmx2xR0pf WxagXN1wmUU3Ahr/OI2s+Ep+yAQVtbhpi+vkRVQZn92WYF1jahRnl44ijUfZOE50YfXF TTeA== X-Gm-Message-State: AOAM533QK8GLuTsC8Tvj1Fc5WsM7zL1HZAI3ifcvo9nXGCUA1YMxd3ZN e5LVaTpPWU5PKfA0+svJC5q1iw== X-Google-Smtp-Source: ABdhPJzqSYbXZ3dZ72XC23rglIpe3ek9frg3iJwZiLeaJ+2qI4jtaBXYPaUvIk0taKCHZjTGQm5u7A== X-Received: by 2002:a62:6484:: with SMTP id y126mr18922373pfb.166.1595801844671; Sun, 26 Jul 2020 15:17:24 -0700 (PDT) Received: from hermes.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id y19sm1676497pfn.77.2020.07.26.15.17.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 26 Jul 2020 15:17:24 -0700 (PDT) Date: Sun, 26 Jul 2020 15:17:16 -0700 From: Stephen Hemminger To: Patrick Keroulas Cc: dev@dpdk.org, Vivien Didelot Message-ID: <20200726151716.4873e7e4@hermes.lan> In-Reply-To: <20200724202315.19533-5-patrick.keroulas@radio-canada.ca> References: <20200724202315.19533-1-patrick.keroulas@radio-canada.ca> <20200724202315.19533-5-patrick.keroulas@radio-canada.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [[PATCH v3 4/4] 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" On Fri, 24 Jul 2020 16:23:15 -0400 Patrick Keroulas wrote: > From: Vivien Didelot > > 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 | 32 +++++++++++++----------- > lib/librte_pdump/rte_pdump.c | 34 ++++++++++++++------------ > 3 files changed, 37 insertions(+), 30 deletions(-) > > diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_notes/release_20_08.rst > index 89822bcb8d..bda2f7567a 100644 > --- a/doc/guides/rel_notes/release_20_08.rst > +++ b/doc/guides/rel_notes/release_20_08.rst > @@ -105,6 +105,7 @@ New Features > Updated PCAP driver with new features and improvements, including: > > * Support software Tx nanosecond timestamps precision. > + * Support hardware Tx timestamps. > > * **Updated Broadcom bnxt driver.** > > diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c > index 668cbd1fc7..45775951f7 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; > - > - cycles = rte_get_timer_cycles() - start_cycles; > - cur_time.tv_sec = cycles / hz; > - cur_time.tv_usec = (cycles % hz) * NSEC_PER_SEC / hz; > - > - ts->tv_sec = start_time.tv_sec + cur_time.tv_sec; > - ts->tv_usec = start_time.tv_usec + cur_time.tv_usec; > - if (ts->tv_usec >= NSEC_PER_SEC) { > - ts->tv_usec -= NSEC_PER_SEC; > - ts->tv_sec += 1; > +calculate_timestamp(const struct rte_mbuf *mbuf, struct timeval *ts) { DPDK style is to have { on seperate line > + if (mbuf->ol_flags & PKT_RX_TIMESTAMP) { > + ts->tv_sec = mbuf->timestamp / NSEC_PER_SEC; > + ts->tv_usec = mbuf->timestamp % NSEC_PER_SEC; > + } else { > + uint64_t cycles = rte_get_timer_cycles() - start_cycles; > + struct timeval cur_time = { > + .tv_sec = cycles / hz, > + .tv_usec = (cycles % hz) * NSEC_PER_SEC / hz, > + }; > + > + ts->tv_sec = start_time.tv_sec + cur_time.tv_sec; > + ts->tv_usec = start_time.tv_usec + cur_time.tv_usec; > + if (ts->tv_usec >= NSEC_PER_SEC) { > + ts->tv_usec -= NSEC_PER_SEC; > + ts->tv_sec += 1; > + } > } > } > > @@ -339,7 +343,7 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > caplen = sizeof(temp_data); > } > > - calculate_timestamp(&header.ts); > + calculate_timestamp(mbuf, &header.ts); On Tx pcap you don't want the hardware RX timestamp. You should update mbuf with software timestamp. > header.len = len; > header.caplen = caplen; > /* rte_pktmbuf_read() returns a pointer to the data directly > diff --git a/lib/librte_pdump/rte_pdump.c b/lib/librte_pdump/rte_pdump.c > index 8fd7e13af5..e8963e9dd8 100644 > --- a/lib/librte_pdump/rte_pdump.c > +++ b/lib/librte_pdump/rte_pdump.c > @@ -71,21 +71,6 @@ static struct pdump_rxtx_cbs { > } rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT], > tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT]; > > -static uint64_t hz; > -static uint64_t start_time; > -static uint64_t start_cycles; > - > -static inline void > -pdump_ts_to_ns(struct rte_mbuf **pkts, uint16_t nb_pkts) > -{ > - unsigned int i; > - > - for (i = 0; i < nb_pkts; i++) { > - if ((pkts[i]->ol_flags & PKT_RX_TIMESTAMP) && hz) > - pkts[i]->timestamp = start_time + > - (pkts[i]->timestamp - start_cycles) * NS_PER_S / hz; > - } > -} > > static inline void > pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params) > @@ -118,6 +103,23 @@ pdump_copy(struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params) > } > } > > +#define NSEC_PER_SEC 1000000000L > +static uint64_t hz; > +static uint64_t start_time; > +static uint64_t start_cycles; > + > +static inline void > +pdump_ts_to_ns(struct rte_mbuf **pkts, uint16_t nb_pkts) > +{ > + unsigned int i; > + > + for (i = 0; i < nb_pkts; i++) { > + if ((pkts[i]->ol_flags & PKT_RX_TIMESTAMP) && hz) > + pkts[i]->timestamp = start_time + > + (pkts[i]->timestamp - start_cycles) * NSEC_PER_SEC / hz; > + } > +} > + > static uint16_t > pdump_rx(uint16_t port __rte_unused, uint16_t qidx __rte_unused, > struct rte_mbuf **pkts, uint16_t nb_pkts, > @@ -154,7 +156,7 @@ pdump_register_rx_callbacks(uint16_t end_q, uint16_t port, uint16_t queue, > rte_eth_read_clock(port, &start_cycles); > rte_eth_get_clock_freq(port, &hz); > gettimeofday(&now, NULL); > - start_time = now.tv_sec * NS_PER_S + now.tv_usec * 1000; > + start_time = now.tv_sec * NSEC_PER_SEC + now.tv_usec * 1000; > > if (cbs->cb) { > PDUMP_LOG(ERR, Actually using the current way through rte_eth_pcap adds a lot of overhead. I dropped it when doing pcapng. Perhaps we should work together on the pcapng version.