Hello,

On Sat, May 06, 2023 at 08:56:06PM +0000, Klemens Nanni wrote:
> On Sat, May 06, 2023 at 09:33:05PM +0200, Alexander Bluhm wrote:
> > On Sat, May 06, 2023 at 11:11:25AM +0000, Klemens Nanni wrote:
> > > pf_osfp.c contains all the locking for these three ioctls, this removes
> > > the net lock from it.
> > > 
> > > All data is protected by the pf lock, new asserts verify that.
> > > 
> > > Beside the pf ioctl handler, pf_match_rule()'s call to pf_osfp_match()
> > > is the only hook into it.
> > > 
> > > tcpbump still compiles pf_osfp.c without object change.
> > > 
> > > Feedback? OK?
> > 
> > Could you document that pf_osfp_list and fp_next is protected by
> > pf lock?
> 
> Picked [p] for pf_lock so as to not collide with pfvar_priv.h's comment
> about "Protection/owernership of pf_state members".
> 
> > There is nothing in pf_osfp_fingerprint() that needs pf lock directly.
> > The critical access is the SLIST_FOREACH in pf_osfp_find().  Place
> > the PF_ASSERT_LOCKED() there.
> 
> Misplaced, thanks.
> 
> > Why is pf_osfp_list passed to pf_osfp_find() and pf_osfp_find_exact()?
> > Can we remove the parameter and just use pf_osfp_list?
> 
> Wanted to clean this up in a separate diff, but we might as well do
> everything in one go.
> 
> 

looks OK to me

sashan

Reply via email to