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 F3605A0527; Sat, 25 Jul 2020 10:35:53 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C88301C02A; Sat, 25 Jul 2020 10:35:53 +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 6B9911C025 for ; Sat, 25 Jul 2020 10:35:52 +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 4A2F132B7; Sat, 25 Jul 2020 10:35:52 +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 00imbDbHUtwB; Sat, 25 Jul 2020 10:35:51 +0200 (CEST) X-KTH-Auth: barbette [109.217.63.52] DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kth.se; s=default; t=1595666151; bh=sFIi/ghIRBMnwZXXGv0PL6Xh3BprV5ao653teqDvoUs=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=dv8BPPlHvIwtewBYDV6HDzB3yDEgm+8c9ofIZK0Hz5cQ3BAV65MBXwMF8HcMq+dLp 7czx4oaqgO4FTO29vOO2SNQpjNGqb/CjbX7PmnLXAYYo0jryzhIsZqa7tas7jj6GMO 18dlVbq/UF8eMLt5PNlXIb72YO1BY0iRL6aSHyLI= X-KTH-mail-from: barbette@kth.se Received: from [192.168.1.86] (anancy-652-1-470-52.w109-217.abo.wanadoo.fr [109.217.63.52]) by smtp-3.sys.kth.se (Postfix) with ESMTPSA id D4B363013; Sat, 25 Jul 2020 10:35:50 +0200 (CEST) To: Patrick Keroulas , dev@dpdk.org Cc: Vivien Didelot References: <20200724202315.19533-1-patrick.keroulas@radio-canada.ca> <20200724202315.19533-5-patrick.keroulas@radio-canada.ca> From: Tom Barbette Message-ID: Date: Sat, 25 Jul 2020 10:35:48 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20200724202315.19533-5-patrick.keroulas@radio-canada.ca> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit 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" Nice patch! However I would use a multiply and shift approach instead of a division in the fast path. When we did that it was really hurting the performance. Cheers, Tom Le 24/07/2020 à 22:23, Patrick Keroulas a écrit : > 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) { > + 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); > 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, >