| Message ID | 1370272783.24311.148.camel@edumazet-glaptop |
|---|---|
| State | Superseded, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Mon, 2013-06-03 at 08:19 -0700, Eric Dumazet wrote: > On Mon, 2013-06-03 at 15:37 +0100, Ben Hutchings wrote: > > > This would be more understandable without a goto. I think this ordering > > would work: > > Yeah you're right, thanks ! > > [PATCH v3] get_rate: detect 32bit overflows > > Current rate limit is 34.359.738.360 bit per second, and > unfortunately 40Gbps links are above it. > > overflows in get_rate() are currently not detected, and some > users are confused. Let's detect this and complain. > > Note that some qdisc are ready to get extended range, but this will > need additional attributes and new iproute2 > > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > tc/tc_util.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/tc/tc_util.c b/tc/tc_util.c > index 8e62a01..912216c 100644 > --- a/tc/tc_util.c > +++ b/tc/tc_util.c > @@ -146,25 +146,29 @@ static const struct rate_suffix { > int get_rate(unsigned *rate, const char *str) > { > char *p; > - double bps = strtod(str, &p); > + long long bps = strtoll(str, &p, 0); Oops, I read this as being strtol() currently, not strtod(). Currently '1.5gbit' will work, but this change will break that. So I think you need to keep bps as a double. > const struct rate_suffix *s; > > if (p == str) > return -1; > > - if (*p == '\0') { > - *rate = bps / 8.; /* assume bits/sec */ > - return 0; > - } > - > for (s = suffixes; s->name; ++s) { > if (strcasecmp(s->name, p) == 0) { > - *rate = (bps * s->scale) / 8.; > - return 0; > + bps *= s->scale; > + p += strlen(p); > + break; > } > } > > - return -1; > + if (*p) > + return -1; /* unknown suffix */ > + bps /= 8; /* -> bytes per second */ > + *rate = bps; > + /* detect if an overflow happened */ > + if (*rate != bps) Then here I think the check should be *rate != floor(bps), i.e. accept rounding down of a non-integer number of bytes but any other change is assumed to be overflow. Ben. > + return -1; > + return 0; > } > > void print_rate(char *buf, int len, __u32 rate) > >
diff --git a/tc/tc_util.c b/tc/tc_util.c index 8e62a01..912216c 100644 --- a/tc/tc_util.c +++ b/tc/tc_util.c @@ -146,25 +146,29 @@ static const struct rate_suffix { int get_rate(unsigned *rate, const char *str) { char *p; - double bps = strtod(str, &p); + long long bps = strtoll(str, &p, 0); const struct rate_suffix *s; if (p == str) return -1; - if (*p == '\0') { - *rate = bps / 8.; /* assume bits/sec */ - return 0; - } - for (s = suffixes; s->name; ++s) { if (strcasecmp(s->name, p) == 0) { - *rate = (bps * s->scale) / 8.; - return 0; + bps *= s->scale; + p += strlen(p); + break; } } - return -1; + if (*p) + return -1; /* unknown suffix */ + + bps /= 8; /* -> bytes per second */ + *rate = bps; + /* detect if an overflow happened */ + if (*rate != bps) + return -1; + return 0; } void print_rate(char *buf, int len, __u32 rate)