I have a suggestion RE your pftop.c patch.
You are rounding multiple times, after each scale operation.
This is known as rounding the intermediate results of a calculation and
degrades accuracy.
If you're not familiar with the issue do a Google search on rounding
intermediate.
I suggest assigning the pf rate to an unsigned long and multiplying by 10.
This makes the LSD a guard digit.
After all scaling, round once (if guard digit >= 5 then add 10).
Yes, this may require one more scaling operation if it rounds up to 6 digits
(including guard digit).
At the very end divide by 10.
Note: This is essentially fixed point arithmetic with one decimal digit.
I modified pftop.c v1.37 to do this earlier today.
It was kind of tricky.
I need to see if it still looks OK on Tuesday (I'm busy Monday).
Let me know if you want a patch then.
I won't have actually tested it, though, so it's highly likely to have bugs.
You might be better off writing your own (and then perhaps comparing to mine).
RE my email client: It's Yahoo webmail -- nothing I can do about it.
--------------------------------------------
On Sat, 5/13/17, Mike Belopuhov <m...@belopuhov.com> wrote:
Subject: Re: pf queue definition: bandwidth resolution problem
To: "Carl Mascott" <cmasc...@yahoo.com>
Cc: tech@openbsd.org, t...@openbsd.org
Date: Saturday, May 13, 2017, 8:02 PM
Good call. This one is a bit more complicated
since we have 5 positions
to display and the
last one is sort of reserved for the unit specifier.
So ignoring the unit we can
display numbers from 1 to 9999.
However,
when truncating numbers like that we have to properly
round
them. I've been taught to round
this way: [12499 / 1000] = 12K and
[56500 /
1000] = 57K. I.e. I compare the remainder to 500.
YMMV.
The edge case where
we have to avoid adding this 1 is when rounding
in a previous cycle would affect rounding in
the next, i.e. 11499999
would be rounded to
12M if we would unconditionally add 1.
Does the diff below look good?
On Sat, May 13, 2017 at 21:28
+0000, Carl Mascott wrote:
> One more
thing:
> The BW column of "systat
queues" has the same truncation error.
> I'm guessing that "systat
queues" is running "pfctl -vsqueue"
periodically, but if that's not the case then the same
fix is needed in systat.
>
>
>
>
--------------------------------------------
> On Sat, 5/13/17, Carl Mascott <cmasc...@yahoo.com>
wrote:
>
> Subject:
Re: pf queue definition: bandwidth resolution problem
> To: "Mike Belopuhov" <m...@belopuhov.com>
> Cc: m...@openbsd.org
> Date: Saturday, May 13, 2017, 4:55 PM
>
> I forgot to ask:
How will I know when there's
> a
snapshot with a fixed pfctl binary?
>
Any problem with dropping the new pfctl
> binary into my 6.1-stable (i386)
system?
>
> P.S.
I'm new to OpenBSD.
>
You should not use snapshot
binaries on -stable. In fact, newer pfctl
won't work at all with an old kernel.
And please fix your email
client as it mangles mails that you quote.
Index:
sbin/pfctl/pfctl_parser.c
===================================================================
RCS file:
/home/cvs/src/sbin/pfctl/pfctl_parser.c,v
retrieving revision 1.309
diff
-u -p -r1.309 pfctl_parser.c
---
sbin/pfctl/pfctl_parser.c 26 Oct 2016 14:15:59
-0000 1.309
+++
sbin/pfctl/pfctl_parser.c 13 May 2017 19:18:19
-0000
@@ -1177,7 +1177,7 @@
print_bwspec(const char *prefix, struct
printf("%s%u%%", prefix,
bw->percent);
else if
(bw->absolute) {
rate =
bw->absolute;
- for (i = 0;
rate >= 1000 && i <= 3; i++)
+ for (i = 0; rate >= 1000
&& i <= 3 && (rate % 1000 == 0); i++)
rate /= 1000;
printf("%s%u%c",
prefix, rate, unit[i]);
}
Index: usr.bin/systat/pftop.c
===================================================================
RCS file:
/home/cvs/src/usr.bin/systat/pftop.c,v
retrieving revision 1.37
diff
-u -p -r1.37 pftop.c
---
usr.bin/systat/pftop.c 3 May 2017 14:01:29 -0000
1.37
+++ usr.bin/systat/pftop.c 13 May
2017 23:55:38 -0000
@@ -1608,7 +1608,7 @@
calc_pps(u_int64_t new_pkts, u_int64_t l
void
print_queue_node(struct
pfctl_queue_node *node)
{
- u_int rate;
+ u_int rate, rtmp;
int i;
double interval, pps, bps;
static const char unit[] = " KMG";
@@ -1624,8 +1624,12 @@ print_queue_node(struct
pfctl_queue_node
// XXX: missing
min, max, burst
tb_start();
rate =
node->qs.linkshare.m2.absolute;
- for (i = 0; rate
>= 1000 && i <= 3; i++)
- rate /= 1000;
+ for (i = 0; rate > 9999 && i
<= 3; i++) {
+ rtmp = rate /
1000;
+ if (rtmp < 9999)
+ rtmp += (rate % 1000) /
500;
+ rate = rtmp;
+ }
tbprintf("%u%c", rate, unit[i]);
print_fld_tb(FLD_BANDW);
