From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: route gwroute mutex
To: Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date: Thu, 6 Mar 2025 17:47:59 +0300

On Thu, Mar 06, 2025 at 03:24:08PM +0100, Claudio Jeker wrote:
> On Thu, Mar 06, 2025 at 03:17:54PM +0100, Claudio Jeker wrote:
> > On Thu, Mar 06, 2025 at 02:47:10PM +0100, Alexander Bluhm wrote:
> > > Hi,
> > > 
> > > I am still trying to make the leaves of the routing tree MP safe.
> > > 
> > > A small step would be to protect rt_cachecnt and rt_gwroute writers
> > > with a per route mutex.  I am aware that this is not complete, e.g.
> > > rt_flags also looks fishy.  The READ_ONCE() in rt_getll() does not
> > > protect from use-after-free.  I want to avoid mutex in the hot path.
> > > A previous attempt with SMR failed when running BGP.  Let's make
> > > this first step and find a smarter solition later.  route_cleargateway()
> > > is not in the hot packet path, so I added the mutex there.
> > > 
> > > I have moved the rt_cachecnt from rt_setgwroute() to rt_putgwroute()
> > > to have the RTF_CACHED logic in one function.
> > > 
> > > ok?
> > 
> > I'm begrudgingly OK with this. Right now the situation is clearly
> > incorrect and the mutex solves this right now.
> > Long term we should move towards making struct rtentry more RCU friendly
> > so we can drop the mutex. But lets tackle that after 7.7 
> 
> But please use the mutex in all cases and don't try to trick around it.
> I know it sucks but at least then the code is correct.
> 

I agree, it's better to use mutex. I'm curious, is the performance impact
such significant?

> -- 
> :wq Claudio
>