Currently, lib latencystats works as follow:
    * For every sample period, one packet is selected by RX callback to be later
      used for latency measurement. That packet is marked with a timestamp.  
    * In TX callback, mbufs with non-zero timestamp are used for calculation.

I see a problem with the current implementation. The timestamp field of a mbuf
will not be cleared when that mbuf is freed back to the pool. Thus TX callback
could potentially use an incorrect mbuf, one that had not been selected on RX
but still has non-zero timestamp anyway because it was inherited from a
previously selected packet. The simple fix for this is to reset the timestamp of
the mbuf after it was used for latency calculation.

However, that fix is not enough in the case where a selected mbuf get dropped by
the application before reaching TX. That timestamp will eventually be cleared
the next time that mbuf is used and not get dropped, but then the incorrect
residued timestamp will still be used and it would affect all measurement
results, most severely max_latency_ns.

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.

Bao-Long Tran (1):
  latency: clear mbuf timestamp after latency calculation

 lib/librte_latencystats/rte_latencystats.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
2.7.4

Reply via email to