DPDK patches and discussions
 help / color / mirror / Atom feed
From: longtb5@viettel.com.vn
To: reshma.pattan@intel.com
Cc: dev@dpdk.org, Bao-Long Tran <longtb5@viettel.com.vn>
Subject: [dpdk-dev] Incorrect latencystats implementation
Date: Wed, 19 Sep 2018 15:17:14 +0700 (ICT)	[thread overview]
Message-ID: <1537345496-70207-1-git-send-email-longtb5@viettel.com.vn> (raw)

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	other threads:[~2018-09-19  8:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19  8:17 longtb5 [this message]
2018-09-19  8:22 ` [dpdk-dev] [PATCH] latency: clear mbuf timestamp after latency calculation longtb5
2018-09-20 10:25   ` Pattan, Reshma
2018-09-20 12:16     ` longtb5
2018-09-20 11:16   ` [dpdk-dev] [PATCH v2] " longtb5
2018-09-20 13:08 ` [dpdk-dev] Incorrect latencystats implementation Pattan, Reshma
2018-09-21  1:58   ` longtb5
2018-09-21 11:15     ` Pattan, Reshma
2018-09-21 12:14       ` Ananyev, Konstantin
2018-09-21 14:58         ` Pattan, Reshma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1537345496-70207-1-git-send-email-longtb5@viettel.com.vn \
    --to=longtb5@viettel.com.vn \
    --cc=dev@dpdk.org \
    --cc=reshma.pattan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).