pfr_update_stats: assertion failed.

[Kristof Provost]

On 24 Jun 2018, at 21:07, Marek Zarychta wrote:
> On Sun, Jun 24, 2018 at 01:56:07PM +0200, Kristof Provost wrote:
>> On 23 Jun 2018, at 18:46, Marek Zarychta wrote:
>>> On Sat, Jun 23, 2018 at 05:27:29PM +0200, Marek Zarychta wrote:
>>>> On Sun, Oct 16, 2016 at 08:17:13PM +0200, Marek Zarychta wrote:
>>>>> The issue occurred first two years ago, after upgrade from 8 to 9
>>>>> branch. Now this i386 machine is running 11.0-STABLE and despite 
>>>>> it
>>>>> was
>>>>> compiled with "WITHOUT_ASSERT_DEBUG=yes", still from time to time
>>>>> message buffer is fed with:
>>>>>   pfr_update_stats: assertion failed.
>>>>>   pfr_update_stats: assertion failed.
>>>>>   pfr_update_stats: assertion failed.
>>>>>   pfr_update_stats: assertion failed.
>>>>>   pfr_update_stats: assertion failed.
>>>>>   pfr_update_stats: assertion failed.
>>>>>   pfr_update_stats: assertion failed.
>>>>
>>>> These messages are still filling system message buffer. According 
>>>> to
>>>> pfctl (8) there is nothing wrong with incrementing "XPass" counters
>>>> instead of the "Pass" counters. The message "pfr_update_stats:
>>>> assertion
>>>> failed" is printed for debugging purposes only. One could also
>>>> compare
>>>> the counters with the command "pfctl -sT -vv".
>>>>
>>>> OpenBSD converted printf()'s to DPFDEBUG() macro in their sources
>>>> almost
>>>> 8 years ago. Only this printf() in pf_table.c has been converted to
>>>> the
>>>> level of LOG_DEBUG [1].
>>>>
>>>> Perhaps this line of code could be removed from FreeBSD PF sources?
>>>>
>>>
>>> The previous patch was hastily prepared. It should rather look like
>>> this:
>>>
>>> --- sys/netpfil/pf/pf_table.orig.c	2018-06-23 16:40:14.876882000 
>>> +0200
>>> +++ sys/netpfil/pf/pf_table.c	2018-06-23 18:17:49.353490000 +0200
>>> @@ -1984,9 +1984,7 @@
>>>  		panic("%s: unknown address family %u", __func__, af);
>>>  	}
>>> -	if ((ke == NULL || ke->pfrke_not) != notrule) {
>>> -		if (op_pass != PFR_OP_PASS)
>>> -			printf("pfr_update_stats: assertion failed.\n");
>>> +	if ((ke == NULL || ke->pfrke_not) != notrule)
>>>  		op_pass = PFR_OP_XPASS;
>>> -	}
>>>  	kt->pfrkt_packets[dir_out][op_pass]++;
>>>  	kt->pfrkt_bytes[dir_out][op_pass] += len;
>>>
>> We could delete those lines and that’d get rid of the dmesg noise, 
>> but
>> I’m a bit worried that this demonstrates an actual problem.
>> It’s not at all clear to me what’s going on in this bit of the 
>> code,
>> and the OpenBSD repo doesn’t have any information about it either.
>>
>
> This machine acts as a NAT/firewall gateway for about a hundred users.
> A few hundred of PF rules + 20 tables are used. The error appeared
> suddenly after upgrade from 8-STABLE to 10-STABLE 3 years ago. It 
> never
> occurred when the firewall run PF on 8-STABLE.  I don't remember 
> whether
> firewall rules were changed at that time. If it is true then changes
> concerned only the compatibility with the newer version of PF.
>
> If it demonstrates an actual problem, then, please give me a clue how 
> to
> debug it. On the other hand, ~6 years ago PF was significantly 
> reworked.
> Is this piece of code still relevant there?
>
I honestly don’t know. I’d be good to actually investigate this 
before simply removing the warning.
Unfortunately I just don’t have the time to dig into this right now.
The only thing I can suggest is to look at the code and work out where 
the op_pass value comes from (and perhaps also what it’s used for. Why 
is PRF_OP_XPASS better than !PFR_OP_PASS?

It’s still present (though perhaps not logged) in OpenBSD too.

Regards,
Kristof