pf 4.1 Update available for testing

[Nate Lawson]

Eygene Ryabinkin wrote:
> Nate, Max, good day.
> 
> Wed, Jun 20, 2007 at 07:26:09PM +0400, Eygene Ryabinkin wrote:
>> Fine, thanks!  So, you're happy with the way the problem was fixed?
>> I see that another function that uses tbr_callout is tbr_timeout,
>> but it will not be called before tbr_set.  So it seems to me that
>> callout initialisation only in tbr_set is enough.  But maybe I am
>> missing something?
> 
> After some thinking I came to the idea that one more patch must be
> applied.  The variables machclk_usepcc and machclk_per_tick can be
> left uninitialised following the same codepath as for tbr_callout:
> tsc_freq_changed() touches only machclk_freq, but init_machclk
> touches all three variables.
> 
> This error can potentially be responsible to the weird bandwidth
> values I am having with the altq on my notebook.  The issue is
> described on the thread
>   http://lists.freebsd.org/pipermail/freebsd-current/2007-April/070730.html
> Basically, I am setting one BW limit in pf.conf and seeing another
> one (much lower)  via the ifstat utility.
> 
> I was able only to test the compilation of the new patched kernel.
> No bandwidth tests were done: I have no access to the fast LAN link
> up to the Monday, 24th, sorry.  May be I will be able to setup
> ng_eiface and test with it, but I am not fluent with the netgraph.
> Will post an update if tests will be carried.
> 
> But I am pretty sure that the altq_subr.c should be patched to
> properly handle the initialization of these two variables.  The
> only question is how to do it: via my patch or using some different
> strategy.
> 
> No more words, the patch is attached.  Comments are welcome!
> 

I have tried to achieve the same goal with a simpler patch.  Here are
the changes:

Be sure to initialize the callout struct and other setup tasks before
proceeding.  Previously, machclk_freq could be set to a non-zero value
by tsc_freq_changed(), preventing the callout from being initialized.
To fix this, call init_machclk() from all paths.  init_machclk() is
split into two functions, one that only runs the first time it is
called.  The second half runs each time the frequency changes and
calibrates various items.  Also, static variables are zero so no need to
initialize them.

If you can test this, that would be great.

Thanks,
-- 
Nate
-------------- next part --------------
A non-text attachment was scrubbed...
Name: altq-fix-3.diff
Type: text/x-patch
Size: 1318 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-pf/attachments/20070703/6e4e328c/altq-fix-3.bin