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