kern/189720: [ipfw] [patch] pps action for ipfw
Luigi Rizzo
rizzo at iet.unipi.it
Fri May 30 17:20:06 UTC 2014
The following reply was made to PR kern/189720; it has been noted by GNATS.
From: Luigi Rizzo <rizzo at iet.unipi.it>
To: bycn82 <bycn82 at gmail.com>
Cc: bug-followup at 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
More information about the freebsd-ipfw
mailing list