The following reply was made to PR kern/189720; it has been noted by GNATS.

From: Luigi Rizzo <ri...@iet.unipi.it>
To: bycn82 <byc...@gmail.com>
Cc: bug-follo...@freebsd.org
Subject: Re: kern/189720: [ipfw] [patch] pps action for ipfw
Date: Fri, 30 May 2014 19:16:10 +0200

 On Sat, May 31, 2014 at 12:53:56AM +0800, bycn82 wrote:
 > 1.       Add static int to store the value of kern.hz
 
 this is unnecessary. There is already a global variable
 called "hz" which contains the correct information
 
 > 2.       Convert the duration into number of ticks based on  kern.hz
 
 this is done incorrectly.
 
 First, hz does not change at runtime so it is unnecessary to recompute
 the duration on every instace, even more since this is costing you
 one division.  You should adjust the value when the rule is injected
 in the kernel, perhaps adding a couple of fields in the rule
 so you can store the adjusted duration and threshold (see below).
 
 Second, you are still not doing the rounding correctly.
 When the requested interval is shorter than a tick, you adjust the
 interval but leave the limit unchanged, which means you are reducing
 the limit below what the user wants.
 Instead you should correct the limit so that it approximates
 the desired rate; one above or one below is not a problem,
 but your code might be off by a very large factor.
 
 BTW even in the other case you are always adding 1 tick unconditionally.
 A correct way to do the rounding is (pps->duration * hz + 999)/1000
 (and then again adjust the count according to the actual duration)
 
 cheers
 luigi
_______________________________________________
freebsd-ipfw@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw
To unsubscribe, send any mail to "freebsd-ipfw-unsubscr...@freebsd.org"

Reply via email to