From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ananyev, Konstantin" Subject: Re: [RFC 0/8] mbuf: structure reorganization Date: Tue, 28 Feb 2017 11:48:20 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F11EA96@irsmsx105.ger.corp.intel.com> References: <1485271173-13408-1-git-send-email-olivier.matz@6wind.com> <20170217151708.20bf4a49@platinum> <20170221105400.2eba4747@glumotte.dev.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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Jan Blunck , "Richardson, Bruce" , "dev@dpdk.org" To: Olivier Matz Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id A17C02BB1 for ; Tue, 28 Feb 2017 12:48:24 +0100 (CET) In-Reply-To: <20170228115043.3f78ce52@platinum> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" >=20 > On Tue, 28 Feb 2017 10:29:41 +0000, "Ananyev, Konstantin" > wrote: > > > > > > Hi, > > > > > > On Tue, 28 Feb 2017 09:05:07 +0000, "Ananyev, Konstantin" > > > 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 t= oo. > > Probably not a big deal as most of PMDs do set port value anyway ri= ght now, > > but it means it would be hard to get rid/change mbuf->port in futur= e. >=20 > Currently, all PMDs must set m->port. > If in the future we remove m->port, the applications that use it will nee= d > to store the value in a mbuf metadata, or pass it as arguments through fu= nction > calls. >=20 >=20 > > 2. Applications would not allowed to change mbuf->port value before nor= malization 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 normaliz= ed, and which were not? >=20 > 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. What we are introducing here is basically a new dependency between 2 mbuf fields and new restriction.=20 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 r= ight now. So each app writer would have to come-up with his own solution. =20 > Applications that > are doing this are responsible of what they change. >=20 >=20 > > 3. In theory with eth_dev_detach() - mbuf->port value might be not vali= d 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. >=20 > Detaching a device requires a synchronization between control and data pl= ane, > and not only for this use case.=20 Of course it does. But right now it is possible to do: eth_rx_burst(port=3D0, ..., &mbuf, 1); eth_dev_detach(port=3D0, ...); ... /*process previously received mbuf */ With what you are proposing it would be not always possible any more. >In the first solution, the normalization is > partial: unit is nanosecond, but the time reference is different. Not sure I get you here... Konstantin >=20 > So, after the discussion, I'm more convinced by the second solution. >=20 >=20 > Regards, > Olivier