Re: [RFC 0/8] mbuf: structure reorganization

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

Hi,

On Tue, 21 Feb 2017 20:30:57 +0000, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com> wrote:
> > -----Original Message-----
> > From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of Jan
> > Blunck Sent: Tuesday, February 21, 2017 7:18 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Olivier MATZ
> > <olivier.matz@6wind.com>; dev@dpdk.org Subject: Re: [dpdk-dev] [RFC
> > 0/8] mbuf: structure reorganization
> > 
> > On Tue, Feb 21, 2017 at 6:26 PM, Ananyev, Konstantin
> > <konstantin.ananyev@intel.com> wrote:  
> > > Hi Jan,
> > >  
> > >> -----Original Message-----
> > >> From: jblunck@gmail.com [mailto:jblunck@gmail.com] On Behalf Of
> > >> Jan Blunck Sent: Tuesday, February 21, 2017 5:05 PM
> > >> To: Richardson, Bruce <bruce.richardson@intel.com>
> > >> Cc: Olivier MATZ <olivier.matz@6wind.com>; Ananyev, Konstantin
> > >> <konstantin.ananyev@intel.com>; dev@dpdk.org Subject: Re:
> > >> [dpdk-dev] [RFC 0/8] mbuf: structure reorganization
> > >>
> > >> On Tue, Feb 21, 2017 at 5:38 PM, Bruce Richardson
> > >> <bruce.richardson@intel.com> wrote:  
> > >> > On Tue, Feb 21, 2017 at 05:12:12PM +0100, Jan Blunck wrote:  
> > >> >>
> > >> >> Access through PMD specific function pointers should be
> > >> >> relatively fast on access. Modern architecture optimize that
> > >> >> use case well enough.
> > >> >>  
> > >> > The cost of doing a function call per packet to access data
> > >> > starts to add up very, very fast. For the app, once the data
> > >> > is written to the mbuf, it should be in the L1 cache, giving
> > >> > very fast access to it in a few cycles. However, if a function
> > >> > call has to be made in order to do the read, that makes the
> > >> > read of that field many times more expensive. 
> > >>
> > >> Exactly. Right now the timestamp normalization is done before
> > >> writing to each mbuf. Timestamps are usually read at most
> > >> once ... if at all.  
> > >
> > > Well we don't know for sure right?
> > > Someone can argue that there are plenty of scenarios  when
> > > other fields might also never be used/updated (rss, vlan, etc).
> > >
> > > So, are you suggesting to do normalization later?
> > > If so, then what would be the benefit (data still need to be in
> > > mbuf)?  
> > 
> > Yes, postponing normalization prevents you from doing unnecessary
> > work upfront. AFAIK not all NICs store timestamp data OOB, e.g. in
> > CQ.  
> 
> Yes, postponing normalization might help a bit (though I don't think
> much) in terms of calculations performed inside PMD.
> But we still need 8B inside mbuf to store the timestamp value,
> either normalized or raw one.
> So to clarify where is the disagreement:
> 1. timestamp position:
>     mbufs 1-st cacheline vs 2-nd cacheline


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.


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.

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.



Olivier