[PATCH] IperfFlowMeasurement: fix parse_job_cpu duration get, again

From: Ondrej Lichtner <olichtne(a)redhat.com&gt;

One more fix for accessing the test duration when parsing test results
for cpu utilization of the iperf process.

The json dictionaries are inconsistent between tcp/udp stream on how
they report the end of test data - udp reports only a "sum" dictionary,
tcp only reports a "sum_sent" and "sum_received" dictionaries. So
instead we look at the test start and the *requested* test duration. Not
as precise as the actual duration of the test but probably good enough
considering how the duration is used in this case...

Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
---
 lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
index 87ef70f..722b49e 100644
--- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
+++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
@@ -187,5 +187,5 @@ def _parse_job_cpu(self, job):
         else:
             cpu_percent =
job.result["data"]["end"]["cpu_utilization_percent"]["host_total"]
             job_start =
job.result["data"]["start"]["timestamp"]["timesecs"]
-            duration =
job.result["data"]["end"]["sum"]["seconds"]
+            duration =
job.result["data"]["start"]["test_start"]["duration"]
             return PerfInterval(cpu_percent*duration, duration, "cpu_percent",
job_start)
-- 
2.30.0

[Ondrej Lichtner]

On Fri, Jan 29, 2021 at 09:57:16AM +0100, olichtne(a)redhat.com wrote:
>  From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>  
>  One more fix for accessing the test duration when parsing test results
>  for cpu utilization of the iperf process.
>  
>  The json dictionaries are inconsistent between tcp/udp stream on how
>  they report the end of test data - udp reports only a "sum" dictionary,
>  tcp only reports a "sum_sent" and "sum_received" dictionaries. So
>  instead we look at the test start and the *requested* test duration. Not
>  as precise as the actual duration of the test but probably good enough
>  considering how the duration is used in this case...
>  
>  Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>  ---
>   lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>  
>  diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
>  index 87ef70f..722b49e 100644
>  --- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
>  +++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
>  @@ -187,5 +187,5 @@ def _parse_job_cpu(self, job):
>           else:
>               cpu_percent =
> job.result["data"]["end"]["cpu_utilization_percent"]["host_total"]
>               job_start =
> job.result["data"]["start"]["timestamp"]["timesecs"]
>  -            duration =
> job.result["data"]["end"]["sum"]["seconds"]
>  +            duration =
> job.result["data"]["start"]["test_start"]["duration"]
>               return PerfInterval(cpu_percent*duration, duration, "cpu_percent",
> job_start)
>  -- 
>  2.30.0
>   
tests confirm that this fix finally works, pushing.

-Ondrej

[Jan Tluka]

Fri, Jan 29, 2021 at 09:57:16AM CET, olichtne(a)redhat.com wrote:
> From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>
> One more fix for accessing the test duration when parsing test results
> for cpu utilization of the iperf process.
>
> The json dictionaries are inconsistent between tcp/udp stream on how
> they report the end of test data - udp reports only a "sum" dictionary,
> tcp only reports a "sum_sent" and "sum_received" dictionaries. So
> instead we look at the test start and the *requested* test duration. Not
> as precise as the actual duration of the test but probably good enough
> considering how the duration is used in this case...
>
> Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> ---
>  lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> index 87ef70f..722b49e 100644
> --- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> +++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> @@ -187,5 +187,5 @@ def _parse_job_cpu(self, job):
>          else:
>              cpu_percent =
> job.result["data"]["end"]["cpu_utilization_percent"]["host_total"]
>              job_start =
> job.result["data"]["start"]["timestamp"]["timesecs"]
> -            duration =
> job.result["data"]["end"]["sum"]["seconds"]
> +            duration =
> job.result["data"]["start"]["test_start"]["duration"]
>              return PerfInterval(cpu_percent*duration, duration, "cpu_percent",
> job_start)
> -- 
> 2.30.0
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedorahosted.org/archives/list/[e-mail]...
Would it make sense to create an issue for iperf to unify this?

-Jan

[Ondrej Lichtner]

On Mon, Feb 01, 2021 at 09:24:08AM +0100, Jan Tluka wrote:
>  Fri, Jan 29, 2021 at 09:57:16AM CET, olichtne(a)redhat.com wrote:
>  >From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>  >
>  >One more fix for accessing the test duration when parsing test results
>  >for cpu utilization of the iperf process.
>  >
>  >The json dictionaries are inconsistent between tcp/udp stream on how
>  >they report the end of test data - udp reports only a "sum" dictionary,
>  >tcp only reports a "sum_sent" and "sum_received" dictionaries.
> So
>  >instead we look at the test start and the *requested* test duration. Not
>  >as precise as the actual duration of the test but probably good enough
>  >considering how the duration is used in this case...
>  >
>  >Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
>  >---
>  > lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +-
>  > 1 file changed, 1 insertion(+), 1 deletion(-)
>  >
>  >diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
>  >index 87ef70f..722b49e 100644
>  >--- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
>  >+++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
>  >@@ -187,5 +187,5 @@ def _parse_job_cpu(self, job):
>  >         else:
>  >             cpu_percent =
> job.result["data"]["end"]["cpu_utilization_percent"]["host_total"]
>  >             job_start =
> job.result["data"]["start"]["timestamp"]["timesecs"]
>  >-            duration =
> job.result["data"]["end"]["sum"]["seconds"]
>  >+            duration =
> job.result["data"]["start"]["test_start"]["duration"]
>  >             return PerfInterval(cpu_percent*duration, duration,
> "cpu_percent", job_start)
>  >-- 
>  >2.30.0
>  >_______________________________________________
>  >LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
>  >To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
>  >Fedora Code of Conduct:
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  >List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  >List Archives:
> https://lists.fedorahosted.org/archives/list/[e-mail]...
>  
>  Would it make sense to create an issue for iperf to unify this?
>  
>  -Jan 
I think this is already reported, or at least discussed in this:

    https://github.com/esnet/iperf/issues/584

which is quite an old bug with an explanation that:

    It's not on the roadmap right now. Fixing this actually requires a
    change to the state machine and protocol between the client and
    server.

We can try to ping it but I wouldn't keep my hopes up...

-Ondrej

[Jan Tluka]

Mon, Feb 01, 2021 at 09:30:18AM CET, olichtne(a)redhat.com wrote:
> On Mon, Feb 01, 2021 at 09:24:08AM +0100, Jan Tluka wrote:
> > Fri, Jan 29, 2021 at 09:57:16AM CET, olichtne(a)redhat.com wrote:
> > >From: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> > >
> > >One more fix for accessing the test duration when parsing test results
> > >for cpu utilization of the iperf process.
> > >
> > >The json dictionaries are inconsistent between tcp/udp stream on how
> > >they report the end of test data - udp reports only a "sum"
> dictionary,
> > >tcp only reports a "sum_sent" and "sum_received"
> dictionaries. So
> > >instead we look at the test start and the *requested* test duration. Not
> > >as precise as the actual duration of the test but probably good enough
> > >considering how the duration is used in this case...
> > >
> > >Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> > >---
> > > lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> > >index 87ef70f..722b49e 100644
> > >--- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> > >+++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py
> > >@@ -187,5 +187,5 @@ def _parse_job_cpu(self, job):
> > >         else:
> > >             cpu_percent =
> job.result["data"]["end"]["cpu_utilization_percent"]["host_total"]
> > >             job_start =
> job.result["data"]["start"]["timestamp"]["timesecs"]
> > >-            duration =
> job.result["data"]["end"]["sum"]["seconds"]
> > >+            duration =
> job.result["data"]["start"]["test_start"]["duration"]
> > >             return PerfInterval(cpu_percent*duration, duration,
> "cpu_percent", job_start)
> > >-- 
> > >2.30.0
> > >_______________________________________________
> > >LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> > >To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> > >Fedora Code of Conduct:
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > >List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > >List Archives:
> https://lists.fedorahosted.org/archives/list/[e-mail]...
> > 
> > Would it make sense to create an issue for iperf to unify this?
> > 
> > -Jan
>
> I think this is already reported, or at least discussed in this:
>
>     https://github.com/esnet/iperf/issues/584
>
> which is quite an old bug with an explanation that:
>
>     It's not on the roadmap right now. Fixing this actually requires a
>     change to the state machine and protocol between the client and
>     server.
>
> We can try to ping it but I wouldn't keep my hopes up...
>
> -Ondrej 
Ok, makes sense.

Thanks for fixing this.

J.