[dpdk-dev] Incorrect latencystats implementation

[Pattan, Reshma]

Hi,

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, September 21, 2018 1:14 PM
> To: Pattan, Reshma <reshma.pattan at intel.com>; longtb5 at viettel.com.vn
> Cc: dev at dpdk.org
> Subject: RE: Incorrect latencystats implementation
> 
> Hi Reshma,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Pattan, Reshma
> > Sent: Friday, September 21, 2018 12:15 PM
> > To: longtb5 at viettel.com.vn
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] Incorrect latencystats implementation
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: longtb5 at viettel.com.vn [mailto:longtb5 at viettel.com.vn]
> > > Sent: Friday, September 21, 2018 2:58 AM
> > > To: Pattan, Reshma <reshma.pattan at intel.com>
> > > Cc: dev at dpdk.org
> > > Subject: RE: Incorrect latencystats implementation
> > >
> > > Hi Reshma,
> > >
> > > > -----Original Message-----
> > > > From: reshma.pattan at intel.com [mailto:reshma.pattan at intel.com]
> > > > Sent: Thursday, September 20, 2018 8:09 PM
> > > > To: longtb5 at viettel.com.vn
> > > > Cc: dev at dpdk.org
> > > > Subject: RE: Incorrect latencystats implementation
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: longtb5 at viettel.com.vn [mailto:longtb5 at viettel.com.vn]
> > > > > Sent: Wednesday, September 19, 2018 9:17 AM
> > > > > To: Pattan, Reshma <reshma.pattan at intel.com>
> > > > > Cc: dev at dpdk.org; Bao-Long Tran <longtb5 at viettel.com.vn>
> > > > > Subject: Incorrect latencystats implementation
> > > > >
> > > > >
> > > > > I have submit a patch to implement the trivial fix. For the drop
> > > > > case I can think of 2 options. We can either clear timestamp
> > > > > when putting mbufs back to their pool, or change lib
> > > > > latencystats implementation to perform packet selection at TX
> > > > > callback and let RX callback add
> > > timestamp
> > > > to every packet.
> > > > > Both option could affect performance but I think the second
> > > > > option is less aggressive.
> > > >
> > > > What happens when applications drop the packets? Do they free the
> mbuf?
> > > > In such case, can application set the timestamp to 0 before
> > > > freeing the
> > > mbuf,
> > > > instead of making these changes in latency library.?
> > > >
> > >
> > > Yes, applications can set the mbuf timestamp before freeing. But in
> > > my opinion that would not be a clean solution. Applications should
> > > not have to worry about the timestamp field at all, since that is an
> > > implementation detail of the library. For simple apps, wrapping
> > > rte_pktmbuf_free() to perform timestamp reset could be done without
> > > much hassle, but that kind of ad-hoc solution would become messy for
> > > more complex ones where packets are dropped at different places.
> > > From a usability point of view, as an user I want the lib to provide
> > > latency measurements without me having to touch existing codebase
> other than adding codes that use the APIs.
> > >
> >
> > I will send a patch to  add timestamp reset in rte_pktmbuf_free().  That will
> be a cleaner way  I think.
> > Let's see what other says on the patch.
> 
> That would probably affect performance.
> Actually, looking at rte_mbuf.h - timestamp field supposed to be valid only if
> PKT_RX_TIMESTAMP is set.
> From other side, as I remember, PMD RX routine should reset RX flags of the
> received PMD.
> So in theory you can rely on PKT_RX_TIMESTAMP to determine use this mbuf
> for latency calcs or not.
> Would that help?

Yes this will work. Thanks for the suggestion. Let me have a further look and send the patch.