From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id B23423978 for ; Tue, 18 Oct 2016 17:43:54 +0200 (CEST) Received: from lfbn-1-5996-232.w90-110.abo.wanadoo.fr ([90.110.195.232] helo=[192.168.1.13]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1bwWbe-0003yM-Tr for dev@dpdk.org; Tue, 18 Oct 2016 17:47:11 +0200 To: dev@dpdk.org References: <1476369308-17021-1-git-send-email-olegk@mellanox.com> <1476369308-17021-2-git-send-email-olegk@mellanox.com> From: Olivier Matz Message-ID: Date: Tue, 18 Oct 2016 17:43:49 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 MIME-Version: 1.0 In-Reply-To: <1476369308-17021-2-git-send-email-olegk@mellanox.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/3] mbuf: embedding timestamp into the packet X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Oct 2016 15:43:54 -0000 On 10/13/2016 04:35 PM, Oleg Kuporosov wrote: > The hard requirement of financial services industry is accurate > timestamping aligned with the packet itself. This patch is to satisfy > this requirement: > > - include uint64_t timestamp field into rte_mbuf with minimal impact to > throughput/latency. Keep it just simple uint64_t in ns (more than 580 > years) would be enough for immediate needs while using full > struct timespec with twice bigger size would have much stronger > performance impact as missed cacheline0. > > - it is possible as there is 6-bytes gap in 1st cacheline (fast path) > and moving uint16_t vlan_tci_outer field to 2nd cacheline. > > - such move will only impact for pretty rare usable VLAN RX stripping > mode for outer TCI (it used only for one NIC i40e from the whole set and > allows to keep minimal performance impact for RX/TX timestamps. This argument is difficult to accept. One can say you are adding a field for a pretty rare case used by only one NIC :) Honestly, I'm not able to judge whether timestamp is more important than vlan_tci_outer. As room is tight in the first cache line, your patch submission is the occasion to raise the question: how to decide what should be in the first part of the mbuf? There are also some other candidates for moving: m->seqn is only used in librte_reorder and it is not set in the RX part of a driver. About the timestamp, it would be valuable to have other opinions, not only about the placement of the field in the structure, but also to check that this API is also usable for other NICs. Have you measured the impact of having the timestamp in the second part of the mbuf? Changing the mbuf structure should happen as rarely as possible, and we have to make sure we take the correct decisions. I think we will discuss this at dpdk userland 2016. Apart from that, I wonder if an ol_flag should be added to tell that the timestamp field is valid in the mbuf. Regards, Olivier