Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Please test: shared solock for all intet sockets within knote(9) routines
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Tue, 30 Jan 2024 12:52:03 +0300

Download raw body.

Thread
On Mon, Jan 29, 2024 at 10:25:23PM +0300, Vitaliy Makkoveev wrote:
> On Mon, Jan 29, 2024 at 03:57:35PM +0100, Alexander Bluhm wrote:
> > On Mon, Jan 29, 2024 at 04:20:02PM +0300, Vitaliy Makkoveev wrote:
> > > So, I want to commit soassertlocked() right now. sofilt_lock() diff
> > > could be pushed to the snaps, so we could test them together.
> > 
> > Regress not finished, but here are the first findings.  I think it
> > is regress/sys/net/pf_divert which requires complicated setup with
> > two machines.
> > 
> > divert_packet runs with shared net lock, without rwlock socket lock,
> > but with inpcb mutex for receive socket buffer.  I think soassertlocked
> > has to take pru_lock() into account.
> > 
> > And for sorwakeup this is not sufficent, there I hope for your
> > sofilt_lock().
> > 
> > This is basically the same what my diff found a month ago.
> > 
> > bluhm
> 
> We should have no so{r,w}wakeup() calls outside pru_lock() or `so_lock'
> protection for shared netlock case. May be we have the only caller with
> shared netlock for socket instance, but this could be changed in the
> future. Any case this is very fragile.
> 
> Updated diff. We have no rw_status() analog for mutex, so I introduced
> mtx_owned() macro. The 'pr_usrreqs' structure received new optional
> `pru_locked' member to get the status of pru_lock(). soassertlocked()
> was adjusted to use it.
> 
> sorwakeup() call was moved under `inp_mtx' for both divert sockets and
> multicast routing.
> 
> This diff also includes the sofilt_lock() hunks to make the test
> coverage wider. I want to commit hem separately, so I remove them before
> commit.
> 
> The mutex man page is missing. I'll add it later if required.
> 

Please drop this, I have better idea.