latency: clear mbuf timestamp after latency calculation
Checks
Commit Message
The timestamp of a mbuf should be cleared after that mbuf was used for
latency calculation, otherwise future packets which reuse the same mbuf
would inherit that previous timestamp. The latencystats library looks
for mbuf with non-zero timestamp, thus incorrectly inherited value would
result in incorrect latency measurement.
Cc: stable@dpdk.org
Signed-off-by: Bao-Long Tran <longtb5@viettel.com.vn>
---
lib/librte_latencystats/rte_latencystats.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Comments
Hi,
> -----Original Message-----
> From: longtb5@viettel.com.vn [mailto:longtb5@viettel.com.vn]
> Sent: Wednesday, September 19, 2018 9:23 AM
> To: Pattan, Reshma <reshma.pattan@intel.com>
> Cc: dev@dpdk.org; Bao-Long Tran <longtb5@viettel.com.vn>;
> stable@dpdk.org
> Subject: [PATCH] latency: clear mbuf timestamp after latency calculation
>
> The timestamp of a mbuf should be cleared after that mbuf was used for
> latency calculation, otherwise future packets which reuse the same mbuf
> would inherit that previous timestamp. The latencystats library looks for
> mbuf with non-zero timestamp, thus incorrectly inherited value would result
> in incorrect latency measurement.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: Bao-Long Tran <longtb5@viettel.com.vn>
You need to add the Fixes line just before CC: in the commit message.
Original commit that introduced the bug was 5cd3cac9ed. So fixes should be added like below
Fixes: 5cd3cac9ed ("latency: added new library for latency stats").
You can send v2 with fixes line and my ack. Other than that
Acked-by: Reshma Pattan <reshma.pattan@intel.com>
Hi,
Thanks, I have sent a v2.
Any comment on the problem of dropped mbuf that I described in the cover
letter? In our application the max_latency_ns metric is useless since
after running for a while it would always take on obviously incorrect value
(up to a few minutes). I suspect the impact on avg_latency_ns is much less
severe but significant nonetheless.
> -----Original Message-----
> From: reshma.pattan@intel.com [mailto:reshma.pattan@intel.com]
> Sent: Thursday, September 20, 2018 5:25 PM
> To: longtb5@viettel.com.vn
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] latency: clear mbuf timestamp after latency
calculation
>
> Hi,
>
> > -----Original Message-----
> > From: longtb5@viettel.com.vn [mailto:longtb5@viettel.com.vn]
> > Sent: Wednesday, September 19, 2018 9:23 AM
> > To: Pattan, Reshma <reshma.pattan@intel.com>
> > Cc: dev@dpdk.org; Bao-Long Tran <longtb5@viettel.com.vn>;
> > stable@dpdk.org
> > Subject: [PATCH] latency: clear mbuf timestamp after latency
> > calculation
> >
> > The timestamp of a mbuf should be cleared after that mbuf was used for
> > latency calculation, otherwise future packets which reuse the same
> > mbuf would inherit that previous timestamp. The latencystats library
> > looks for mbuf with non-zero timestamp, thus incorrectly inherited
> > value would result in incorrect latency measurement.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bao-Long Tran <longtb5@viettel.com.vn>
>
> You need to add the Fixes line just before CC: in the commit message.
>
> Original commit that introduced the bug was 5cd3cac9ed. So fixes should
be
> added like below
> Fixes: 5cd3cac9ed ("latency: added new library for latency stats").
>
> You can send v2 with fixes line and my ack. Other than that
>
> Acked-by: Reshma Pattan <reshma.pattan@intel.com>
@@ -156,8 +156,10 @@ calc_latency(uint16_t pid __rte_unused,
now = rte_rdtsc();
for (i = 0; i < nb_pkts; i++) {
- if (pkts[i]->timestamp)
+ if (pkts[i]->timestamp) {
latency[cnt++] = now - pkts[i]->timestamp;
+ pkts[i]->timestamp = 0;
+ }
}
for (i = 0; i < cnt; i++) {