| Message ID | 952BE2E8-CE07-4D82-A47D-D181C229720A@8x8.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2,v3] tc: Fix rounding in tc_calc_xmittime and tc_calc_xmitsize. | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 2/24/25 11:42 AM, Jonathan Lennox wrote: > diff --git a/tc/tc_core.c b/tc/tc_core.c > index 37547e9b..32fd094f 100644 > --- a/tc/tc_core.c > +++ b/tc/tc_core.c > @@ -23,12 +23,12 @@ > static double tick_in_usec = 1; > static double clock_factor = 1; > > -static unsigned int tc_core_time2tick(unsigned int time) > +static double tc_core_time2tick(double time) > { > return time * tick_in_usec; > } > > -unsigned int tc_core_tick2time(unsigned int tick) > +double tc_core_tick2time(double tick) > { > return tick / tick_in_usec; > } > @@ -45,7 +45,7 @@ unsigned int tc_core_ktime2time(unsigned int ktime) > > unsigned int tc_calc_xmittime(__u64 rate, unsigned int size) > { > - return tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/(double)rate)); > + return ceil(tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/(double)rate))); > } > > unsigned int tc_calc_xmitsize(__u64 rate, unsigned int ticks) > diff --git a/tc/tc_core.h b/tc/tc_core.h > index 7a986ac2..c0fb7481 100644 > --- a/tc/tc_core.h > +++ b/tc/tc_core.h > @@ -12,7 +12,7 @@ enum link_layer { > }; > > > -unsigned tc_core_tick2time(unsigned tick); > +double tc_core_tick2time(double tick); > unsigned tc_core_time2ktime(unsigned time); > unsigned tc_core_ktime2time(unsigned ktime); > unsigned tc_calc_xmittime(__u64 rate, unsigned size); git am (and patch for that matter) is not liking your patch. Please make sure the patch is against iproute2-next and top of tree. You should also try sending the patch to yourself, saving to a file and applying using `git am`.
> On Feb 26, 2025, at 11:06 AM, David Ahern <dsahern@kernel.org> wrote: > > git am (and patch for that matter) is not liking your patch. Please make > sure the patch is against iproute2-next and top of tree. > > You should also try sending the patch to yourself, saving to a file and > applying using `git am`. Sorry, my mailer was mangling the patch (adding quoted-printable) and I couldn’t get git send-email to work with my corporate e-mail. I’ve resent it from my personal gmail account.
diff --git a/tc/tc_core.c b/tc/tc_core.c index 37547e9b..32fd094f 100644 --- a/tc/tc_core.c +++ b/tc/tc_core.c @@ -23,12 +23,12 @@ static double tick_in_usec = 1; static double clock_factor = 1; -static unsigned int tc_core_time2tick(unsigned int time) +static double tc_core_time2tick(double time) { return time * tick_in_usec; } -unsigned int tc_core_tick2time(unsigned int tick) +double tc_core_tick2time(double tick) { return tick / tick_in_usec; } @@ -45,7 +45,7 @@ unsigned int tc_core_ktime2time(unsigned int ktime) unsigned int tc_calc_xmittime(__u64 rate, unsigned int size) { - return tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/(double)rate)); + return ceil(tc_core_time2tick(TIME_UNITS_PER_SEC*((double)size/(double)rate))); } unsigned int tc_calc_xmitsize(__u64 rate, unsigned int ticks) diff --git a/tc/tc_core.h b/tc/tc_core.h index 7a986ac2..c0fb7481 100644 --- a/tc/tc_core.h +++ b/tc/tc_core.h @@ -12,7 +12,7 @@ enum link_layer { }; -unsigned tc_core_tick2time(unsigned tick); +double tc_core_tick2time(double tick); unsigned tc_core_time2ktime(unsigned time); unsigned tc_core_ktime2time(unsigned ktime); unsigned tc_calc_xmittime(__u64 rate, unsigned size);
Currently, tc_calc_xmittime and tc_calc_xmitsize round from double to int three times — once when they call tc_core_time2tick / tc_core_tick2time (whose argument is int), once when those functions return (their return value is int), and then finally when the tc_calc_* functions return. This leads to extremely granular and inaccurate conversions. As a result, for example, on my test system (where tick_in_usec=15.625, clock_factor=1, and hz=1000000000) for a bitrate of 1Gbps, all tc htb burst values between 0 and 999 bytes get encoded as 0 ticks; all values between 1000 and 1999 bytes get encoded as 15 ticks (equivalent to 960 bytes); all values between 2000 and 2999 bytes as 31 ticks (1984 bytes); etc. The patch changes the code so these calculations are done internally in floating-point, and only rounded to integer values when the value is returned. It also changes tc_calc_xmittime to round its calculated value up, rather than down, to ensure that the calculated time is actually sufficient for the requested size. Signed-off-by: Jonathan Lennox <jonathan.lennox@8x8.com> --- tc/tc_core.c | 6 +++--- tc/tc_core.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)