From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id DCAA7F60C for ; Thu, 2 Mar 2017 17:46:27 +0100 (CET) Received: by mail-wm0-f52.google.com with SMTP id n11so30451318wma.0 for ; Thu, 02 Mar 2017 08:46:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=oClhb5yGmyUMU8uuZIiYMUXGaClHBn7EtAJOIZpA7/M=; b=W3RTAyGS5PC+gxOKf9FDEKx3TjtHaiiYv+rMesoMNvRLVefe0AF2BWsteRo0WwVdne QHoceWhRtddMv869AMGZlBwxk/OcSHzK6/07jibtGu5hMui9DOZ8yset9eNWcPvf+ivA l2B6JBhogrBpz1wl6MipPYJSNelMbyFEOnMnh9Vody1CwnhRiwhwHyxJVOp0xryZQR5Y AHhCBfaOHMV6mS5bHpT1h0zuEnvHk/+eH+f+PCNL1t+33NuL2BxcvBG+ZpOHcuCMEOu9 8+1dTo39V4ox1RBcyo+AkVxsW1giTNoouQv9x5iHVnlG2GhC4SRdK7wCJAYIQsN7PKFp 6wsA== 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=oClhb5yGmyUMU8uuZIiYMUXGaClHBn7EtAJOIZpA7/M=; b=L7OivlNjV76kg9iUuLAQQD2WrQgjK1x/s23OcWlA9GFMhIG8S/mRB9UYGp6YrtAcYT 04z+8SMWZRdTTjJHdeL2ZFz36+VhFogo0gK5F6R7ha2x22+7m4kx6iXOYcOv2QlcRlw8 ezbv/CFxVDDF7krfMv7aRsBtJ+nw3YHRqVoAmAAbhYpyQStID+7W48jSnf0jpiWfFRqg 3SKkGF6+Bfvswva2Ukyt+rR/XUtaJ0k3lsf1Nqzu2dxnMlh8uqNNoi3l83K9DIgycJ7W nkPgIBnMwtv8/pyoCyp4kx6jaWs6PqCKP2r106hBSjVxW8adnnI4RnqgVoLdcF7S6n3r igzA== X-Gm-Message-State: AMke39m+KEfsTTeP5M24IG3nWZu4Yc36miW9jzJ9EWyrBGTaviue6PiUz3QDoKOT1jA3tA4o X-Received: by 10.28.153.139 with SMTP id b133mr8471053wme.93.1488473187468; Thu, 02 Mar 2017 08:46:27 -0800 (PST) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id e16sm11592258wra.36.2017.03.02.08.46.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 02 Mar 2017 08:46:27 -0800 (PST) Date: Thu, 2 Mar 2017 17:46:23 +0100 From: Olivier Matz To: "Ananyev, Konstantin" Cc: Jan Blunck , "Richardson, Bruce" , "dev@dpdk.org" Message-ID: <20170302174623.268592a7@platinum> In-Reply-To: <2601191342CEEE43887BDE71AB9772583F11EE7A@irsmsx105.ger.corp.intel.com> References: <1485271173-13408-1-git-send-email-olivier.matz@6wind.com> <20170221163808.GA213576@bricha3-MOBL3.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583F11B4CC@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583F11B633@irsmsx105.ger.corp.intel.com> <20170224150053.279e718d@platinum> <2601191342CEEE43887BDE71AB9772583F11E992@irsmsx105.ger.corp.intel.com> <20170228102359.5d601797@platinum> <2601191342CEEE43887BDE71AB9772583F11EA11@irsmsx105.ger.corp.intel.com> <20170228115043.3f78ce52@platinum> <2601191342CEEE43887BDE71AB9772583F11EA96@irsmsx105.ger.corp.intel.com> <20170228132825.37586902@platinum> <2601191342CEEE43887BDE71AB9772583F11EE7A@irsmsx105.ger.corp.intel.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC 0/8] mbuf: structure reorganization 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: , X-List-Received-Date: Thu, 02 Mar 2017 16:46:28 -0000 Hi Konstantin, On Tue, 28 Feb 2017 22:53:55 +0000, "Ananyev, Konstantin" wrote: > > > Another thing that doesn't look very convenient to me here - > > > We can have 2 different values of timestamp (both normalized and not) > > > and there is no clear way for the application to know which one is in > > > use right now. So each app writer would have to come-up with his own > > > solution. > > > > It depends: > > - the solution you describe is to have the application storing the > > normalized value in its private metadata. > > - another solution would be to store the normalized value in > > m->timestamp. In this case, we would need a flag to tell if the > > timestamp value is normalized. > > My first thought also was about second flag to specify was timestamp > already normalized or not. > Though I still in doubt - is it all really worth it: extra ol_flag, new function in eth_dev API. > My feeling that we trying to overcomplicate things. I don't see what is so complicated. The idea is just to let the application do the normalization if it is required. If the time is normalized in nanosecond in the PMD, we would still need to normalized the time reference (the 0). And for that we'd need a call to a synchronization code as well. > > The problem pointed out by Jan is that doing the timestamp > > normalization may take some CPU cycles, even if a small part of packets > > requires it. > > I understand that point, but from what I've seen with real example: > http://dpdk.org/ml/archives/dev/2016-October/048810.html > the amount of calculations at RX is pretty small. > I don't think it would affect performance in a noticeable way > (though I don't have any numbers here to prove it). I think we can consider by default that adding code in the data path impacts performance. > From other side, if user doesn't want a timestamp he can always disable > that feature anad save cycles, right? > > BTW, you and Jan both mention that not every packet would need a timestamp. > Instead we need sort of a timestamp for the group of packets? I think that for many applications the timestamp should be as precise as possible for each packet. > Is that really the only foreseen usage model? No, but it could be one. > If so, then why not to have a special function that would extract 'latest' timestamp > from the dev? > Or even have tx_burst_extra() that would return a latest timestamp (extra parameter or so). > Then there is no need to put timestamp into mbuf at all. Doing that will give a poor precision for the timestamp. > > > > Applications that > > > > are doing this are responsible of what they change. > > > > > > > > > > > > > 3. In theory with eth_dev_detach() - mbuf->port value might be > > > > > not valid at the point when application would decide to do > > > > > normalization. > > > > > > > > > > So to me all that approach with delayed normalization seems > > > > > unnecessary overcomplicated. Original one suggested by Olivier, > > > > > when normalization is done in PMD at RX look much cleaner and > > > > > more manageable. > > > > > > > > Detaching a device requires a synchronization between control and > > > > data plane, and not only for this use case. > > > > > > Of course it does. > > > But right now it is possible to do: > > > > > > eth_rx_burst(port=0, ..., &mbuf, 1); > > > eth_dev_detach(port=0, ...); > > > ... > > > /*process previously received mbuf */ > > > > > > With what you are proposing it would be not always possible any more. > > > > With your example, it does not work even without the timestamp feature, > > since the mbuf input port would reference an invalid port. > > This port is usually used in the application to do a lookup for an port structure, > > so it is expected that the entry is valid. It would be even worse if you > > do a detach + attach. > > I am not talking about the mbuf->port value usage. > Right now user can access/interpret all metadata fields set by PMD RX routines > (vlan, rss hash, ol_flags, ptype, etc.) without need to accessing the device data or > calling device functions. > With that change it wouldn't be the case anymore. That's the same for some other functions. If in my application I want to call eth_rx_queue_count(m->port), I will have the same problem. I think we also have something quite similar in examples/ptpclient: rte_eth_rx_burst(portid, 0, &m, 1); ... parse_ptp_frames(portid, m); ... ptp_data.portid = portid; ... rte_eth_timesync_read_tx_timestamp(ptp_data->portid, ...) So, really, I think it's an application issue: when the app deletes a port, it should ask itself if there are remaining references to it (m->port). > > So, I think it is already the responsibility of the application to do > > the sync (flush retrieved packets before detaching a port). > > The packets are not in RX or TX queue of detaching device any more. > I received a packet, after that I expect to have all its data and metadata inside mbuf. > So I can store mbufs somewhere and process them much later. > Or might be I would like to pass it to the secondary process for logging/analyzing, etc. Yes, but that's still an app problem for me. > > > >In the first solution, the normalization is > > > > partial: unit is nanosecond, but the time reference is different. > > > > > > Not sure I get you here... > > > > In the first solution I described, each PMD had to convert its unit > > into nanosecond. This is easy because we assume the PMD knows the > > value of its clock. But to get a fully normalized value, it also has to > > use the same time reference, so we would also need to manage an offset > > (we need a new API to give this value to the PMD). > > Yes, I suppose we do need an start timestamp and sort of factor() to convert > HW value, something like: > > mbuf->timestamp = rxq->start_timestamp + factor(hw_timestamp); > > Right? > Why passing start_timestamp at the configure() phase will be a problem? > > > > > I have another fear related to hardware clocks: if clocks are not > > synchronized between PMDs, the simple operation "t * ratio - offset" > > won't work. That's why I think we could delegate this job in a specific > > library that would manage this. > > But then that library would need to account all PMDs inside the system, > and be aware about each HW clock skew, etc. > Again, doesn't' sound like an simple task to me. Exactly, that's also why I want to let the specialists take care of it. Having non-normalized timestamps now allow to do the job later when required, while allowing basic usages as required by metrics libraries and mlx pmd. Olivier