diff mbox series

[v2,iproute2-next,1/3] tc: support conversions to or from 64 bit nanosecond-based time

Message ID 20180827024230.246445-2-ysseung@google.com
State Accepted, archived
Delegated to: David Ahern
Headers show
Series support delivering packets in | expand

Commit Message

Yousuk Seung Aug. 27, 2018, 2:42 a.m. UTC 
From: Dave Taht <dave.taht@gmail.com>

Using a 32 bit field to represent time in nanoseconds results in a
maximum value of about 4.3 seconds, which is well below many observed
delays in WiFi and LTE, and barely in the ballpark for a trip past the
Earth's moon, Luna.

Using 64 bit time fields in nanoseconds allows us to simulate
network diameters of several hundred light-years. However, only
conversions to and from ns, us, ms, and seconds are provided.

The iproute2 64 bit api uses signed values for time. Being able to
represent positive or negative time allows us to calculate +/- deltas
between, for example, the CLOCK_TAI and CLOCK_REALTIME clocks.

Time related utility functions in tc_util.c are moved to lib/utils.c.

Signed-off-by: Yousuk Seung <ysseung@google.com>
Signed-off-by: Dave Taht <dave.taht@gmail.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 include/utils.h   |  12 ++++++
 lib/utils.c       | 104 ++++++++++++++++++++++++++++++++++++++++++++++
 tc/tc_cbq.c       |   1 +
 tc/tc_core.c      |   1 +
 tc/tc_core.h      |   2 -
 tc/tc_estimator.c |   1 +
 tc/tc_util.c      |  46 --------------------
 tc/tc_util.h      |   3 --
 8 files changed, 119 insertions(+), 51 deletions(-)

Comments

Stephen Hemminger Aug. 27, 2018, 4:11 p.m. UTC | #1 
On Sun, 26 Aug 2018 19:42:28 -0700
Yousuk Seung <ysseung@google.com> wrote:

> +int get_time(unsigned int *time, const char *str)
> +{
> +	double t;
> +	char *p;
> +
> +	t = strtod(str, &p);
> +	if (p == str)
> +		return -1;
> +
> +	if (*p) {
> +		if (strcasecmp(p, "s") == 0 || strcasecmp(p, "sec") == 0 ||
> +		    strcasecmp(p, "secs") == 0)
> +			t *= TIME_UNITS_PER_SEC;
> +		else if (strcasecmp(p, "ms") == 0 || strcasecmp(p, "msec") == 0 ||
> +			 strcasecmp(p, "msecs") == 0)
> +			t *= TIME_UNITS_PER_SEC/1000;
> +		else if (strcasecmp(p, "us") == 0 || strcasecmp(p, "usec") == 0 ||
> +			 strcasecmp(p, "usecs") == 0)
> +			t *= TIME_UNITS_PER_SEC/1000000;
> +		else
> +			return -1;

Do we need to really support UPPER case.
Isn't existing matches semantics good enough?
Dave Taht Aug. 27, 2018, 4:39 p.m. UTC | #2 
On Mon, Aug 27, 2018 at 9:11 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Sun, 26 Aug 2018 19:42:28 -0700
> Yousuk Seung <ysseung@google.com> wrote:
>
> > +int get_time(unsigned int *time, const char *str)
> > +{
> > +     double t;
> > +     char *p;
> > +
> > +     t = strtod(str, &p);
> > +     if (p == str)
> > +             return -1;
> > +
> > +     if (*p) {
> > +             if (strcasecmp(p, "s") == 0 || strcasecmp(p, "sec") == 0 ||
> > +                 strcasecmp(p, "secs") == 0)
> > +                     t *= TIME_UNITS_PER_SEC;
> > +             else if (strcasecmp(p, "ms") == 0 || strcasecmp(p, "msec") == 0 ||
> > +                      strcasecmp(p, "msecs") == 0)
> > +                     t *= TIME_UNITS_PER_SEC/1000;
> > +             else if (strcasecmp(p, "us") == 0 || strcasecmp(p, "usec") == 0 ||
> > +                      strcasecmp(p, "usecs") == 0)
> > +                     t *= TIME_UNITS_PER_SEC/1000000;
> > +             else
> > +                     return -1;
>
> Do we need to really support UPPER case.

But that's  ALWAYS been the case in the 32 bit version of code above.
Imagine how many former VMS and MVS hackers you'd upset if they had to
turn caps-lock off!

> Isn't existing matches semantics good enough?

But that's the existing case for the 32 bit api, now replicated in the
64 bit api. ? I think the case-insensitive ship has sailed here. Can't
break userspace.

Well.. adding UTF-8 would be cool. We could start using the actual
greek  symbols for delta (δ) and beta (β) in particular. It would
replace a lot of typing, with a whole bunch more shift keys on a
single letter, fit better into
80 column lines, and so on, and tc inputs and outputs are already
pretty greek to many.
--

Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619
Dave Taht Oct. 3, 2018, 4:09 p.m. UTC | #3 
On Mon, Aug 27, 2018 at 9:39 AM Dave Taht <dave.taht@gmail.com> wrote:
>
> On Mon, Aug 27, 2018 at 9:11 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Sun, 26 Aug 2018 19:42:28 -0700
> > Yousuk Seung <ysseung@google.com> wrote:
> >
> > > +int get_time(unsigned int *time, const char *str)
> > > +{
> > > +     double t;
> > > +     char *p;
> > > +
> > > +     t = strtod(str, &p);
> > > +     if (p == str)
> > > +             return -1;
> > > +
> > > +     if (*p) {
> > > +             if (strcasecmp(p, "s") == 0 || strcasecmp(p, "sec") == 0 ||
> > > +                 strcasecmp(p, "secs") == 0)
> > > +                     t *= TIME_UNITS_PER_SEC;
> > > +             else if (strcasecmp(p, "ms") == 0 || strcasecmp(p, "msec") == 0 ||
> > > +                      strcasecmp(p, "msecs") == 0)
> > > +                     t *= TIME_UNITS_PER_SEC/1000;
> > > +             else if (strcasecmp(p, "us") == 0 || strcasecmp(p, "usec") == 0 ||
> > > +                      strcasecmp(p, "usecs") == 0)
> > > +                     t *= TIME_UNITS_PER_SEC/1000000;
> > > +             else
> > > +                     return -1;
> >
> > Do we need to really support UPPER case.
>
> But that's  ALWAYS been the case in the 32 bit version of code above.
> Imagine how many former VMS and MVS hackers you'd upset if they had to
> turn caps-lock off!

I was trying to be funny, of course. If you want us to rework the
patch to also downgrade to
being lowercase for both, ok... I'd rather like to finish getting this
upstream, there's
a change to netem enabling nsec time long stuck behind it.

>
> > Isn't existing matches semantics good enough?
>
> But that's the existing case for the 32 bit api, now replicated in the
> 64 bit api. ? I think the case-insensitive ship has sailed here. Can't
> break userspace.
>
> Well.. adding UTF-8 would be cool. We could start using the actual
> greek  symbols for delta (δ) and beta (β) in particular. It would
> replace a lot of typing, with a whole bunch more shift keys on a
> single letter, fit better into
> 80 column lines, and so on, and tc inputs and outputs are already
> pretty greek to many.
> --
>
> Dave Täht
> CEO, TekLibre, LLC
> http://www.teklibre.com
> Tel: 1-669-226-2619
diff mbox series

Patch

diff --git a/include/utils.h b/include/utils.h
index 8cb4349e8a89..eba67b6ecf44 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -46,6 +46,11 @@  void incomplete_command(void) __attribute__((noreturn));
 #define NEXT_ARG_FWD() do { argv++; argc--; } while(0)
 #define PREV_ARG() do { argv--; argc++; } while(0)
 
+#define TIME_UNITS_PER_SEC	1000000
+#define NSEC_PER_USEC 1000
+#define NSEC_PER_MSEC 1000000
+#define NSEC_PER_SEC 1000000000LL
+
 typedef struct
 {
 	__u16 flags;
@@ -310,4 +315,11 @@  size_t strlcat(char *dst, const char *src, size_t size);
 
 void drop_cap(void);
 
+int get_time(unsigned int *time, const char *str);
+int get_time64(__s64 *time, const char *str);
+void print_time(char *buf, int len, __u32 time);
+void print_time64(char *buf, int len, __s64 time);
+char *sprint_time(__u32 time, char *buf);
+char *sprint_time64(__s64 time, char *buf);
+
 #endif /* __UTILS_H__ */
diff --git a/lib/utils.c b/lib/utils.c
index 02ce67721915..34ec4ab12646 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1633,3 +1633,107 @@  void drop_cap(void)
 	}
 #endif
 }
+
+int get_time(unsigned int *time, const char *str)
+{
+	double t;
+	char *p;
+
+	t = strtod(str, &p);
+	if (p == str)
+		return -1;
+
+	if (*p) {
+		if (strcasecmp(p, "s") == 0 || strcasecmp(p, "sec") == 0 ||
+		    strcasecmp(p, "secs") == 0)
+			t *= TIME_UNITS_PER_SEC;
+		else if (strcasecmp(p, "ms") == 0 || strcasecmp(p, "msec") == 0 ||
+			 strcasecmp(p, "msecs") == 0)
+			t *= TIME_UNITS_PER_SEC/1000;
+		else if (strcasecmp(p, "us") == 0 || strcasecmp(p, "usec") == 0 ||
+			 strcasecmp(p, "usecs") == 0)
+			t *= TIME_UNITS_PER_SEC/1000000;
+		else
+			return -1;
+	}
+
+	*time = t;
+	return 0;
+}
+
+
+void print_time(char *buf, int len, __u32 time)
+{
+	double tmp = time;
+
+	if (tmp >= TIME_UNITS_PER_SEC)
+		snprintf(buf, len, "%.1fs", tmp/TIME_UNITS_PER_SEC);
+	else if (tmp >= TIME_UNITS_PER_SEC/1000)
+		snprintf(buf, len, "%.1fms", tmp/(TIME_UNITS_PER_SEC/1000));
+	else
+		snprintf(buf, len, "%uus", time);
+}
+
+char *sprint_time(__u32 time, char *buf)
+{
+	print_time(buf, SPRINT_BSIZE-1, time);
+	return buf;
+}
+
+/* 64 bit times are represented internally in nanoseconds */
+int get_time64(__s64 *time, const char *str)
+{
+	double nsec;
+	char *p;
+
+	nsec = strtod(str, &p);
+	if (p == str)
+		return -1;
+
+	if (*p) {
+		if (strcasecmp(p, "s") == 0 ||
+		    strcasecmp(p, "sec") == 0 ||
+		    strcasecmp(p, "secs") == 0)
+			nsec *= NSEC_PER_SEC;
+		else if (strcasecmp(p, "ms") == 0 ||
+			 strcasecmp(p, "msec") == 0 ||
+			 strcasecmp(p, "msecs") == 0)
+			nsec *= NSEC_PER_MSEC;
+		else if (strcasecmp(p, "us") == 0 ||
+			 strcasecmp(p, "usec") == 0 ||
+			 strcasecmp(p, "usecs") == 0)
+			nsec *= NSEC_PER_USEC;
+		else if (strcasecmp(p, "ns") == 0 ||
+			 strcasecmp(p, "nsec") == 0 ||
+			 strcasecmp(p, "nsecs") == 0)
+			nsec *= 1;
+		else
+			return -1;
+	}
+
+	*time = nsec;
+	return 0;
+}
+
+void print_time64(char *buf, int len, __s64 time)
+{
+	double nsec = time;
+
+	if (time >= NSEC_PER_SEC)
+		snprintf(buf, len, "%.3fs", nsec/NSEC_PER_SEC);
+	else if (time >= NSEC_PER_MSEC)
+		snprintf(buf, len, "%.3fms", nsec/NSEC_PER_MSEC);
+	else if (time >= NSEC_PER_USEC)
+		snprintf(buf, len, "%.3fus", nsec/NSEC_PER_USEC);
+	else
+		snprintf(buf, len, "%lldns", time);
+}
+
+char *sprint_time64(__s64 time, char *buf)
+{
+	print_time64(buf, SPRINT_BSIZE-1, time);
+	return buf;
+}
+
+
+
diff --git a/tc/tc_cbq.c b/tc/tc_cbq.c
index 4cd584a91a26..c811456b1627 100644
--- a/tc/tc_cbq.c
+++ b/tc/tc_cbq.c
@@ -20,6 +20,7 @@ 
 #include <arpa/inet.h>
 #include <string.h>
 
+#include "utils.h"
 #include "tc_core.h"
 #include "tc_cbq.h"
 
diff --git a/tc/tc_core.c b/tc/tc_core.c
index 1bde4d51e5dc..8eb11223eb9d 100644
--- a/tc/tc_core.c
+++ b/tc/tc_core.c
@@ -21,6 +21,7 @@ 
 #include <arpa/inet.h>
 #include <string.h>
 
+#include "utils.h"
 #include "tc_core.h"
 #include <linux/atm.h>
 
diff --git a/tc/tc_core.h b/tc/tc_core.h
index 1dfa9a4f773b..bd4a99f0d8dd 100644
--- a/tc/tc_core.h
+++ b/tc/tc_core.h
@@ -5,8 +5,6 @@ 
 #include <asm/types.h>
 #include <linux/pkt_sched.h>
 
-#define TIME_UNITS_PER_SEC	1000000
-
 enum link_layer {
 	LINKLAYER_UNSPEC,
 	LINKLAYER_ETHERNET,
diff --git a/tc/tc_estimator.c b/tc/tc_estimator.c
index e4edfc7e98d9..f494b7caa44e 100644
--- a/tc/tc_estimator.c
+++ b/tc/tc_estimator.c
@@ -20,6 +20,7 @@ 
 #include <arpa/inet.h>
 #include <string.h>
 
+#include "utils.h"
 #include "tc_core.h"
 
 int tc_setup_estimator(unsigned int A, unsigned int time_const, struct tc_estimator *est)
diff --git a/tc/tc_util.c b/tc/tc_util.c
index d7578528a31b..cafbe49f3ec8 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -334,52 +334,6 @@  char *sprint_rate(__u64 rate, char *buf)
 	return buf;
 }
 
-int get_time(unsigned int *time, const char *str)
-{
-	double t;
-	char *p;
-
-	t = strtod(str, &p);
-	if (p == str)
-		return -1;
-
-	if (*p) {
-		if (strcasecmp(p, "s") == 0 || strcasecmp(p, "sec") == 0 ||
-		    strcasecmp(p, "secs") == 0)
-			t *= TIME_UNITS_PER_SEC;
-		else if (strcasecmp(p, "ms") == 0 || strcasecmp(p, "msec") == 0 ||
-			 strcasecmp(p, "msecs") == 0)
-			t *= TIME_UNITS_PER_SEC/1000;
-		else if (strcasecmp(p, "us") == 0 || strcasecmp(p, "usec") == 0 ||
-			 strcasecmp(p, "usecs") == 0)
-			t *= TIME_UNITS_PER_SEC/1000000;
-		else
-			return -1;
-	}
-
-	*time = t;
-	return 0;
-}
-
-
-void print_time(char *buf, int len, __u32 time)
-{
-	double tmp = time;
-
-	if (tmp >= TIME_UNITS_PER_SEC)
-		snprintf(buf, len, "%.1fs", tmp/TIME_UNITS_PER_SEC);
-	else if (tmp >= TIME_UNITS_PER_SEC/1000)
-		snprintf(buf, len, "%.1fms", tmp/(TIME_UNITS_PER_SEC/1000));
-	else
-		snprintf(buf, len, "%uus", time);
-}
-
-char *sprint_time(__u32 time, char *buf)
-{
-	print_time(buf, SPRINT_BSIZE-1, time);
-	return buf;
-}
-
 char *sprint_ticks(__u32 ticks, char *buf)
 {
 	return sprint_time(tc_core_tick2time(ticks), buf);
diff --git a/tc/tc_util.h b/tc/tc_util.h
index 6632c4f9c528..76fd986d6e4c 100644
--- a/tc/tc_util.h
+++ b/tc/tc_util.h
@@ -81,13 +81,11 @@  int get_rate64(__u64 *rate, const char *str);
 int get_percent_rate64(__u64 *rate, const char *str, const char *dev);
 int get_size(unsigned int *size, const char *str);
 int get_size_and_cell(unsigned int *size, int *cell_log, char *str);
-int get_time(unsigned int *time, const char *str);
 int get_linklayer(unsigned int *val, const char *arg);
 
 void print_rate(char *buf, int len, __u64 rate);
 void print_size(char *buf, int len, __u32 size);
 void print_qdisc_handle(char *buf, int len, __u32 h);
-void print_time(char *buf, int len, __u32 time);
 void print_linklayer(char *buf, int len, unsigned int linklayer);
 void print_devname(enum output_type type, int ifindex);
 
@@ -95,7 +93,6 @@  char *sprint_rate(__u64 rate, char *buf);
 char *sprint_size(__u32 size, char *buf);
 char *sprint_qdisc_handle(__u32 h, char *buf);
 char *sprint_tc_classid(__u32 h, char *buf);
-char *sprint_time(__u32 time, char *buf);
 char *sprint_ticks(__u32 ticks, char *buf);
 char *sprint_linklayer(unsigned int linklayer, char *buf);