On Tue, May 16, 2017 at 18:19 +0000, Carl Mascott wrote: > At the end is the patch I mentioned against pftop.c v1.37, using a guard > digit. > WARNING: Untested. I couldn't even try to compile it. > > At this point I don't see anything wrong with your patch, Mike. > > > -------------------------------------------- > On Mon, 5/15/17, Mike Belopuhov <m...@belopuhov.com> wrote: > > Subject: Re: pf queue definition: bandwidth resolution problem > To: "Carl Mascott" <cmasc...@yahoo.com> > Cc: "Theo Buehler" <t...@math.ethz.ch>, tech@openbsd.org > Date: Monday, May 15, 2017, 3:28 PM > > On Sun, May 14, 2017 at 21:08 +0000, Carl > Mascott wrote: > > OK, I was indeed missing > something. Thanks, Theo and Mike. > > > > I think I see another very minor problem, > though. > > Let the pf BW be 9,999,999. > > Shift right 3 digits (divide by 1000) : > yields 9,999K. > > (If we were doing > floating point arithmetic, would yield 9,999.999K.) > > You (Mike) don't round this, > presumably to avoid overflow to 10,000 (5 digits). > > However, since (i < 3) it could have > been rounded and scaled again, to 10M. > > > Slightly more accurate but not a big deal. > > > > You're right, initially I had a "<= > 9999" there but then I got > sidetracked > with a hypothetical problem that right now seems > like my imagination. I've double checked > numbers and indeed, > it should be "if > (rtmp <= 9999)". > > > > > > -------------------------------------------- > > On Sun, 5/14/17, Theo Buehler <t...@math.ethz.ch> > wrote: > > > > Subject: > Re: pf queue definition: bandwidth resolution problem > > To: tech@openbsd.org > > Date: Sunday, May 14, 2017, 4:35 PM > > > > On Sun, May 14, > 2017 at 08:29:18PM +0000, Carl > > > Mascott wrote: > > > It looks to me > like you > > are rounding on each > iteration of the for-loop: > > > > > > + for (i = 0; > > rate > 9999 && i <= 3; > i++) { > > > + rtmp = rate > / 1000; > > > + if (rtmp > < 9999) > > > > > This is only true in the last > > > iteration. > > > > > > + rtmp > > += (rate > % 1000) / 500; > > > + > > rate = rtmp; > > > > + } > > > > > > > Am I missing > > something? > > > > > > > > --- pftop.c.old 2017-05-14 09:51:58.620737200 -0400 > +++ pftop.c 2017-05-16 14:01:08.184811800 -0400 > @@ -1608,7 +1608,7 @@ > void > print_queue_node(struct pfctl_queue_node *node) > { > - u_int rate; > + u_int64_t rate; > int i; > double interval, pps, bps; > static const char unit[] = " KMG"; > @@ -1624,9 +1624,22 @@ > // XXX: missing min, max, burst > tb_start(); > rate = node->qs.linkshare.m2.absolute; > - for (i = 0; rate >= 1000 && i <= 3; i++) > - rate /= 1000; > - tbprintf("%u%c", rate, unit[i]); > + rate *= 10UL; > + for (i = 0; rate > 99999UL && i <= 3; i++) > + rate /= 1000UL; > + if (rate % 10UL >= 5UL) { > + rate += 10UL; /* LSD incorrect, don't care */ > + if (rate > 99999UL) { > + if (i < 3) { > + rate /= 1000UL; > + i++; > + } > + else > + rate -= 10UL; > + } > + } > + rate /= 10UL; > + tbprintf("%u%c", (unsigned) rate, unit[i]); > print_fld_tb(FLD_BANDW); > > if (node->qstats.valid && node->qstats_last.valid)
Thanks for the diff! But I must say this is a bit of an overkill (-: I've checked in my amended diff just now. Thanks for reporting and helping with the fix!