On Sat, Jun 21, 2025 at 11:30:32PM +0200, Alexander Bluhm wrote:
> On Sat, Jun 21, 2025 at 10:21:02PM +0300, Vitaliy Makkoveev wrote:
> > They both are the same and both do `rtgeneration' update. As I
> > understand the code, we don't need to serialize `ipmultipath' and
> > `rtgeneration' update with the netstack, however, want to increment
> > `rtgeneration' before `ipmultipath' update. IIRC, atomic operations will
> > not be reordered and we don't need any extra barriers here.
> 
> This is architecture dependent.  membar_enter_after_atomic() is not
> defined everywhere.  I think membar_consumer() is needed here.  See
> comments inline.
> 
> bluhm
> 
>     atomic_store_int(&ipmultipath, newval);
>     membar_producer();
>     atomic_inc_long(&rtgeneration);
> 
> This ensures that route cache gets invalid after ipmultipath changes.
> You need that as rt_hash() result is based on ipmultipath.  That
> way the ipmultipath loads in rt_hash() and route_cache() can be
> kept independent.  No lock needed.

Could we have the side effects while two threads set `ipmultipath' to
opposing values? I mean

	/* ipmultipath is 1 */
	atomic_store_int(&ipmultipath, 0); /* thread A */
	atomic_store_int(&ipmultipath, 1); /* thread B */
	atomic_inc_long(&rtgeneration); /* thread B */
	atomic_inc_long(&rtgeneration); /* thread A */

If so, I like to keep patch serialized.