json: add UNIX timestamp in milliseconds #1846

phip1611 · 2025-02-25T10:36:46Z

json: add UNIX timestamp in milliseconds

This extends the "timestamp" field with a UNIX timestamp in milliseconds
representing the current local wall clock time.

The "timestamp" field now has a new subkey:

"timestamp": {
  "time": "Tue, 25 Feb 2025 11:49:59 GMT",
  "timesecs": 1740484199,
  // new
  "timemillisecs": 1740484199926
},

This is helpful when one does fine-grained network tests with iperf,
where iperf events need to be aligned with events coming from other
resources. A time resolution based on seconds is too coarse-grained
for that.

It is enough to use milliseconds here, as networking always comes
with small timing offsets and delays. Using microseconds wouldn't
add a benefit.

It is sufficient to only change the code at this place, as the rel time
offset for all further events already includes milliseconds. Currently,
iperf3 prints this time offset in seconds encoded as a double with
a microsecond granularity.

Signed-off-by: Philipp Schuster philipp.schuster@cyberus-technology.de

PLEASE NOTE the following text from the iperf3 license. Submitting a
pull request to the iperf3 repository constitutes "[making]
Enhancements available...publicly":

You are under no obligation whatsoever to provide any bug fixes, patches, or
upgrades to the features, functionality or performance of the source code
("Enhancements") to anyone; however, if you choose to make your Enhancements
available either publicly, or directly to Lawrence Berkeley National
Laboratory, without imposing a separate written license agreement for such
Enhancements, then you hereby grant the following license: a non-exclusive,
royalty-free perpetual license to install, use, modify, prepare derivative
works, incorporate into other computer software, distribute, and sublicense
such enhancements or derivative works thereof, in binary and source code form.

The complete iperf3 license is available in the LICENSE file in the
top directory of the iperf3 source tree.

Version of iperf3 (or development branch, such as master or
3.1-STABLE) to which this pull request applies:
Issues fixed (if any):
Brief description of code changes (suitable for use as a commit message):

phip1611 · 2025-02-25T10:37:33Z

src/iperf_api.c

    if (test->json_output)
-	cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s  timesecs: %d", now_str, (int64_t) now_secs));
+	cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s  timesecs: %d  timestamp_ms: %d", now_str, (int64_t) time_spec.tv_sec, now_millisecs));


hint for reviewers: The %d is correct as iperf_json_printf() only knows %d for json numbers, no %lld or similar

snue

I see the need for a more accurate timestamp than second granularity occasionally. I like that this adds a separate new key instead of increasing the precision of existing timestamps. This should cause less churn for tools working with the existing schema.

I found an issue in the timestamp computation though.

src/iperf_api.c

snue · 2025-02-25T11:00:06Z

src/iperf_api.c

    if (test->json_output)
-	cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s  timesecs: %d", now_str, (int64_t) now_secs));
+	cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s  timesecs: %d  timemillisecs: %d", now_str, (int64_t) time_spec.tv_sec, now_millisecs));


nit: I won't claim I like that new key name. I see it fits with the existing one. Shorter alternatives would be time_ms or timemsecs ... I'm fine with using the proposed one here.

Any preference by the code owners? @bmah888

I have a weak preference for timemsecs

snue

LGTM. I'd be happy to get millisecond granularity for the start timestamp.

swlars · 2025-03-06T20:05:43Z

Thanks for the pull request! This seems like a nice, straightforward addition, so we'll try to get to it soon.

phip1611 · 2025-03-17T16:59:39Z

Any update here? @swlars

bmah888 · 2025-03-17T19:16:51Z

src/iperf_api.c

@@ -944,7 +944,7 @@ mapped_v4_to_regular_v4(char *str)
 void
 iperf_on_connect(struct iperf_test *test)
 {
-    time_t now_secs;
+    struct timespec time_spec;


Thanks for the PR and apologies it's taken so long to actually look at it!

First, I agree with @swlars that this is good functionality to have. As I'm reviewing the change it's not quite as easy as we thought originally (for reasons that are entirely due to existing iperf3 code). We might want to implement this slightly differently to use some abstractions for some of the time library calls. Basically iperf3 started off not using clock_gettime() in any way (using gettimeofday() instead), but it was added somewhere around iperf-3.7 (see PR #738). At the time it wasn't clear if just switching over to clock_gettime() would break anything, so the aforementioned PR added some abstraction and conditional compilation to make sure that iperf3 would use clock_gettime() on hosts that have it, and gettimeofday() on hosts that didn't. That's all in iperf_time.[ch].

Trying to use those abstractions would mean doing something like this (completely untested):

struct iperf_time now; uint64_t now_millisecs; time_t now_secs; iperf_time_now(&now); now_millisecs = iperf_time_in_usecs(&now) / 1000L; now_secs = (time_t) iperf_time_in_secs(&now); // <-- yes this is awful (void) strftime(now_str, sizeof(now_str), rfc1123_fmt, gmtime(&now_secs)); if (test->json_output) cJSON_AddItemToObject(test->json_start, "timestamp", iperf_json_printf("time: %s timesecs: %d timemillisecs: %d", now_str, (int64_t) now_secs, now_millisecs))

I'm not super-happy with what I just wrote, maybe you can come up with a better way to do this.

I note that so far I haven't seen an example of a host that runs iperf3 that doesn't also support clock_gettime() so it might be possible to just assume we always have it and get rid of a bunch of the backward compatibility stuff. That's kind of a bigger task though.

Hey! Reusing iperf_time.h makes total sense, no need to reinvent the weel. I adapted the code with one change. No need for

now_secs = (time_t) iperf_time_in_secs(&now); // <-- yes this is awful

when we also can use

now_secs = (time_t) (now_millisecs / 1000);

I'd like to point out that I had to change the parameter for the clock_gettime system call from CLOCK_MONOTONIC to CLOCK_REALTIME, as otherwise, we don't get the timestamp we expect.

Firstly, the first returns the seconds since the system was booted (Linux) and secondly, this way we align the behaviour with the implementation based on gettimeofday().

So either we change the behaviour (current state of PR) with all side-effects this has, or we add a new function.. Thoughts, @bmah888 ?

I'm leaning towards a new function that is like iperf_time_now() but uses CLOCK_REALTIME instead. The existing use of CLOCK_MONOTONIC was there for a specific reason (according to the person who did PR #738), although I totally understand the reasoning for CLOCK_REALTIME for this use case.

Note we need some way to do the same thing for the case where the host doesn't support clock_gettime(), which I'm not sure is even applicable to modern UNIX-like systems.

I have a couple build fixes that I'll comment on separately. Thanks for working with me on this!

This extends the "timestamp" field with a UNIX timestamp in milliseconds representing the current local wall clock time. The "timestamp" field now has a new subkey: "timestamp": { "time": "Tue, 25 Feb 2025 11:49:59 GMT", "timesecs": 1740484199, // new "timemillisecs": 1740484199926 }, This is helpful when one does fine-grained network tests with iperf, where iperf events need to be aligned with events coming from other resources. A time resolution based on seconds is too coarse-grained for that. It is enough to use milliseconds here, as networking always comes with small timing offsets and delays. Using microseconds wouldn't add a benefit. It is sufficient to only change the code at this place, as the relative time offset for all further events already includes milliseconds. Currently, iperf3 prints this time offset in seconds encoded as a double with a microsecond granularity. The parameter for the clock_gettime system call was changed from CLOCK_MONOTONIC to CLOCK_REALTIME. Firstly, the first returns the seconds since the system was booted (Linux) and secondly, this way we align the behaviour with the implementation based on gettimeofday(). Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com

bmah888 · 2025-03-28T18:28:57Z

src/iperf_api.c

+#include <stdio.h>
+    printf("millisecs= %lld ms\n", now_millisecs);
+    printf("secs= %ld s\n", now_secs);


Is this debugging code?

Yikes, sorry! This wasn't supposed to end up in my commit

bmah888 · 2025-03-28T18:29:44Z

src/iperf_api.c

@@ -953,11 +954,19 @@ iperf_on_connect(struct iperf_test *test)
    struct sockaddr_in *sa_inP;
    struct sockaddr_in6 *sa_in6P;
    socklen_t len;
+    time_t now_secs;


This duplicates the declaration of now_secs in line 948. However we do need a declaration of struct iperf_time now; somewhere in this function.

phip1611 commented Feb 25, 2025

View reviewed changes

phip1611 force-pushed the timestamp branch 2 times, most recently from f8f2e44 to 1f6e6f3 Compare February 25, 2025 10:50

snue suggested changes Feb 25, 2025

View reviewed changes

phip1611 force-pushed the timestamp branch from 1f6e6f3 to 1d2eb6f Compare February 25, 2025 11:22

phip1611 requested a review from snue February 25, 2025 11:22

snue approved these changes Feb 25, 2025

View reviewed changes

phip1611 force-pushed the timestamp branch from 1d2eb6f to 877ed2e Compare February 25, 2025 11:52

bmah888 reviewed Mar 17, 2025

View reviewed changes

phip1611 force-pushed the timestamp branch 4 times, most recently from ac88bbc to 0fc0f1e Compare March 21, 2025 08:31

phip1611 force-pushed the timestamp branch from 0fc0f1e to 1be2dd7 Compare March 21, 2025 08:32

bmah888 reviewed Mar 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

json: add UNIX timestamp in milliseconds #1846

json: add UNIX timestamp in milliseconds #1846

phip1611 commented Feb 25, 2025 •

edited

Loading

phip1611 Feb 25, 2025

snue left a comment

snue Feb 25, 2025

phip1611 Feb 25, 2025

bmah888 Mar 17, 2025

snue left a comment

swlars commented Mar 6, 2025

phip1611 commented Mar 17, 2025

bmah888 Mar 17, 2025

phip1611 Mar 21, 2025

phip1611 Mar 21, 2025 •

edited

Loading

bmah888 Mar 28, 2025

bmah888 Mar 28, 2025

phip1611 Mar 28, 2025

bmah888 Mar 28, 2025

json: add UNIX timestamp in milliseconds #1846

Are you sure you want to change the base?

json: add UNIX timestamp in milliseconds #1846

Conversation

phip1611 commented Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

snue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snue left a comment

Choose a reason for hiding this comment

swlars commented Mar 6, 2025

phip1611 commented Mar 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phip1611 Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phip1611 commented Feb 25, 2025 •

edited

Loading

phip1611 Mar 21, 2025 •

edited

Loading