Date: Fri, 30 May 2014 17:20:05 GMT From: Luigi Rizzo <rizzo@iet.unipi.it> To: freebsd-ipfw@FreeBSD.org Subject: Re: kern/189720: [ipfw] [patch] pps action for ipfw Message-ID: <201405301720.s4UHK5ES039852@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/189720; it has been noted by GNATS. From: Luigi Rizzo <rizzo@iet.unipi.it> To: bycn82 <bycn82@gmail.com> Cc: bug-followup@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201405301720.s4UHK5ES039852>