rdr pass for proto tcp sometimes creates states with expire time zero and so breaking connections

[Gleb Smirnoff]

On Tue, Nov 13, 2018 at 10:01:14PM +0100, Andreas Longwitz wrote:
A> OK, in the meantime I did some more research and I am now quite sure the
A> problem with the bogus pf_default_rule->states_cur counter is not a
A> problem in pf. I am convinced it is a problem in counter(9) on i386
A> server. The critical code is the machine instruction cmpxchg8b used in
A> /sys/i386/include/counter.h.
A> 
A> From intel instruction set reference manual:
A> Zhis instruction can be used with a LOCK prefix allow the instruction to
A> be executed atomically.
A> 
A> We have two other sources in kernel using cmpxchg8b:
A>   /sys/i386/include/atomic.h   and
A>   /sys/cddl/contrib/opensolaris/common/atomic/i386/opensolaris_atomic.S

A single CPU instruction is atomic by definition, with regards to the CPU.
A preemption can not happen in a middle of instruction. What the "lock"
prefix does is memory locking to avoid unlocked parallel access to the
same address by different CPUs.

What is special about counter(9) is that %fs:%esi always points to a
per-CPU address, because %fs is unique for every CPU and is constant,
so no other CPU may write to this address, so lock prefix isn't needed.

Of course a true SMP i386 isn't a well tested arch, so I won't assert
that counter(9) doesn't have bugs on this arch. However, I don't see
lock prefix necessary here.

-- 
Gleb Smirnoff