Re: [dpdk-dev] [RFC 0/8] mbuf: structure reorganization

[Olivier Matz <olivier.matz@6wind.com>]

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.


> > > About having the timestamp in the packet data, I don't think it is
> > > a good solution for a generic API in DPDK. The timestamp is a
> > > metadata, it has to go in the mbuf metadata. The packet data
> > > should not be modified when the timestamp is enabled.  
> > 
> > Good NICs already do that based on the packet type (e.g. NTP/PTP
> > packets). 
> > >
> > > But this would not prevent to have driver-specific features to do
> > > that. In that case, the application will be aware that it is
> > > using this specific driver and that it will receive a timestamp
> > > in the packet data.
> > >
> > > To summarize, the generic API could be:
> > > - an ethdev API to enable the timestamp in a PMD for received
> > > packets
> > > - a mbuf flag "timestamp present"
> > > - a mbuf 64b field to store the timestamp value
> > >
> > > Additionally, a driver-specific API can be added for a given PMD.
> > > Example:
> > > - the generic timestamp ethdev is disabled (or not supported)
> > > - a driver-specific feature "put timestamp in packet" is enabled
> > > It would have no additional cost compared to what we have today,
> > > since the timestamp in mbuf is not read/written.
> > >  
> > 
> > Thanks for the writeup. This sounds like a reasonable approach to
> > me.
> > 
> > Do you still want to call the 64bit field "timestamp" or rename it
> > to something neutral and document that it is used together with the
> > mbuf flags?  

I think timestamp is a good name. In the current RFC patchset, we have
this comment:

	/** Valid if PKT_RX_TIMESTAMP is set. The unit is nanoseconds */
	uint64_t timestamp;

We could change it to something like:

	/** Valid if PKT_RX_TIMESTAMP is set. The unit and time
	 *  reference are not normalized but are always the same
	 *  for a given port.
	 */
	uint64_t timestamp;


Regards,
Olivier