From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <olivier.matz@6wind.com> Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id 4A1762B86 for <dev@dpdk.org>; Tue, 28 Feb 2017 13:28:28 +0100 (CET) Received: by mail-wm0-f52.google.com with SMTP id v77so83817459wmv.1 for <dev@dpdk.org>; Tue, 28 Feb 2017 04:28:28 -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=UyYMhWLdcoX5+cSnAnM8eY9lHTNulPAqF/Qip1g460Q=; b=f4OibqiPnBWmA5lOhgrKonINLTZ9r+BBHxnLSScywkcGUqzxMKvrXE1+Pb48jyHZ3s s3CqLBphW7JzUbSvxyWE2ziTpNkwraHhHnaFksD7PPAdBKEMkRK8zE8uczCD4H17XVUy dA1UXT67M6x++SaYSiRuYrIZ1oF0/AxGYjl/CTtr1wOjEowGPBHAuJkXzKT9TiZXQyTN DzJ8AwRoEB9FmUMdDfpky79EVvwAdVGxN5aloLdqdyW6s8SmeVYMOqdxrecSmOfYSosi fLn69nJwwfg5P95XGxjpjcKZjpfKXHgsnqthd0SzrnlAqYB7w6pG3iDr3mHo35W+Cw+z SYSw== 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=UyYMhWLdcoX5+cSnAnM8eY9lHTNulPAqF/Qip1g460Q=; b=RfYUN09BTlB2tNwulfIDi7+Hdss37/CGRGmiuDD34It7vuFubOwFXQexU/R8oyg3CG nNhSqqlUXaEU5gCWHpPU+w3m+yd007XLGS2gx9ljriTiUf/GjUNSPZfVsTtUdqr3APBj 1Hr9oRiJg1uaUNYmzBfioFqQ54EtDMlbPeRIP3Zx/XHjRdiAwsxGAmo79QO0rNpXqGf5 ZlXgkF1HXoM9zjW5aUPEgShSo5du/cDKzz2s9XtjYEoGYBSg+yG4fxjXrV2zvv3HZVaf iJLeqbSg+nf6qkNE1fuQ7iNceShbekSIlsZDJR0/4aQ1dlr4eSjDQxKC09mjnnUczxYS YpLQ== X-Gm-Message-State: AMke39lQbbeKF+2wiEBmCYSr2qpWTDGsDtyPHxz2d6+A3ArxQH0Ne9xTYfwVNjTfJ0Atet90 X-Received: by 10.28.72.193 with SMTP id v184mr17776612wma.105.1488284907746; Tue, 28 Feb 2017 04:28: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 o2sm2458714wmb.28.2017.02.28.04.28.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 28 Feb 2017 04:28:27 -0800 (PST) Date: Tue, 28 Feb 2017 13:28:24 +0100 From: Olivier Matz <olivier.matz@6wind.com> To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com> Cc: Jan Blunck <jblunck@infradead.org>, "Richardson, Bruce" <bruce.richardson@intel.com>, "dev@dpdk.org" <dev@dpdk.org> Message-ID: <20170228132825.37586902@platinum> In-Reply-To: <2601191342CEEE43887BDE71AB9772583F11EA96@irsmsx105.ger.corp.intel.com> References: <1485271173-13408-1-git-send-email-olivier.matz@6wind.com> <20170221105400.2eba4747@glumotte.dev.6wind.com> <CALe+Z03meh2od13-pfnFh0SpmCqxgKLD5MG2MF5Bj9Q8EtS=Hw@mail.gmail.com> <20170221163808.GA213576@bricha3-MOBL3.ger.corp.intel.com> <CALe+Z01pVFdEckOUabXTnh1q-xEOmJajTagEB1hvqYZazrG7iA@mail.gmail.com> <2601191342CEEE43887BDE71AB9772583F11B4CC@irsmsx105.ger.corp.intel.com> <CALe+Z01ozmTdWwxcc7mG+NhSV16K4+-Pe5uDWASzBBs5oMyh1g@mail.gmail.com> <2601191342CEEE43887BDE71AB9772583F11B633@irsmsx105.ger.corp.intel.com> <20170224150053.279e718d@platinum> <CALe+Z01+K8Odpz3oqk672qsKnjVAXif0TJCwsuPhbcwX+Z11Sg@mail.gmail.com> <2601191342CEEE43887BDE71AB9772583F11E992@irsmsx105.ger.corp.intel.com> <20170228102359.5d601797@platinum> <2601191342CEEE43887BDE71AB9772583F11EA11@irsmsx105.ger.corp.intel.com> <20170228115043.3f78ce52@platinum> <2601191342CEEE43887BDE71AB9772583F11EA96@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 <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Tue, 28 Feb 2017 12:28:28 -0000 On Tue, 28 Feb 2017 11:48:20 +0000, "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote: > > > > On Tue, 28 Feb 2017 10:29:41 +0000, "Ananyev, Konstantin" > > <konstantin.ananyev@intel.com> wrote: > > > > > > > > Hi, > > > > > > > > On Tue, 28 Feb 2017 09:05:07 +0000, "Ananyev, Konstantin" > > > > <konstantin.ananyev@intel.com> wrote: > > > > > Hi everyone, > > > > > > > > > > > > > > > > > > > In my opinion, if we have the room in the first cache > > > > > > > line, we should put it there. The only argument I see > > > > > > > against is "we may find something more important in the > > > > > > > future, and we won't have room for it in the first cache > > > > > > > line". I don't feel we should penalize today's use cases > > > > > > > for hypothetic future use cases. > > > > > > > > > > > > > > > > > > > > > > > > > > > >> 2. timestamp normalization point > > > > > > >> inside PMD RX vs somewhere later as user needs it > > > > > > >> (extra function in dev_ops?). > > > > > > > > > > > > > > This point could be changed. My initial proposition tries > > > > > > > to provide a generic API for timestamp. Let me remind it > > > > > > > here: > > > > > > > > > > > > > > a- the timestamp is in nanosecond > > > > > > > b- the reference is always the same for a given path: if > > > > > > > the timestamp is set in a PMD, all the packets for this > > > > > > > PMD will have the same reference, but for 2 different > > > > > > > PMDs (or a sw lib), the reference would not be the same. > > > > > > > > > > > > > > We may remove a-, and just have: > > > > > > > - the reference and the unit are always the same for a > > > > > > > given path: if the timestamp is set in a PMD, all the > > > > > > > packets for this PMD will have the same reference and > > > > > > > unit, but for 2 different PMDs (or a sw lib), they would > > > > > > > not be the same. > > > > > > > > > > > > > > In both cases, we would need a conversion code (maybe in a > > > > > > > library) if the application wants to work with timestamps > > > > > > > from several sources. The second solution removes the > > > > > > > normalization code in the PMD when not needed, it is > > > > > > > probably better. > > > > > > > > > > > > I agree. > > > > > > > > > > One question - does that mean that application would need to > > > > > keep a track from what PMD each particular packet came to do > > > > > the normalization? Konstantin > > > > > > > > I'd say yes. It does not look very difficult to do, since the > > > > mbuf contains the input port id. > > > > > > > > > > I understand that we can use mbuf->port here, but it means that > > > we'll introduce new implicit dependency between timestamp and > > > port values. From my point that introduces new implications: > > > 1. all PMDs that do set a timestamp would also have to set port > > > value too. Probably not a big deal as most of PMDs do set port > > > value anyway right now, but it means it would be hard to get > > > rid/change mbuf->port in future. > > > > Currently, all PMDs must set m->port. > > If in the future we remove m->port, the applications that use it > > will need to store the value in a mbuf metadata, or pass it as > > arguments through function calls. > > > > > > > 2. Applications would not allowed to change mbuf->port value > > > before normalization is done (from what I heard some apps do > > > update mbuf->port to store routing decisions). BTW, how the app > > > would keep track which mbufs were already normalized, and which > > > were not? > > > > I don't think it should be allowed to change m->port value. > > As far as I know it is allowed right now. > PMD RX routine sets mbuf->port, after that application is free to use > it in a way it likes. The descriptor or m->port is "Input port". If the applications stores something else than the input port, it is its responsibility if it breaks something else. Like changing any other field to put something that does not match the description. > What we are introducing here is basically a new dependency between 2 > mbuf fields and new restriction. On the other hand, there is no strong dependency: the API to do the normalization can take the port as a parameter. > > 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. 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. > > > 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 worst if you do a detach + attach. So, I think it is already the responsibility of the application to do the sync (flush retrieved packets before detaching a port). > > >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). 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. Having a non-normalized timestamp as of today would allow applications to take advantage of it for many use cases, even without the normalization library that could come later (and that may probably be more complex than expected). Olivier