From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: ipsec: move `ipsec_keep_invalid' out of netlock
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 13 May 2025 18:22:18 +0200

On Tue, May 13, 2025 at 01:29:39PM +0300, Vitaliy Makkoveev wrote:
> It is local to reserve_spi().

OK bluhm@

> Index: sys/netinet/ip_ipsp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_ipsp.c,v
> diff -u -p -r1.278 ip_ipsp.c
> --- sys/netinet/ip_ipsp.c	3 Dec 2023 10:50:25 -0000	1.278
> +++ sys/netinet/ip_ipsp.c	13 May 2025 10:26:22 -0000
> @@ -256,6 +256,9 @@ reserve_spi(u_int rdomain, u_int32_t ssp
>  	struct tdb *tdbp, *exists;
>  	u_int32_t spi;
>  	int nums;
> +#ifdef IPSEC
> +	int keep_invalid_local = atomic_load_int(&ipsec_keep_invalid);
> +#endif
>  
>  	/* Don't accept ranges only encompassing reserved SPIs. */
>  	if (sproto != IPPROTO_IPCOMP &&
> @@ -324,12 +327,12 @@ reserve_spi(u_int rdomain, u_int32_t ssp
>  
>  #ifdef IPSEC
>  		/* Setup a "silent" expiration (since TDBF_INVALID's set). */
> -		if (ipsec_keep_invalid > 0) {
> +		if (keep_invalid_local > 0) {
>  			mtx_enter(&tdbp->tdb_mtx);
>  			tdbp->tdb_flags |= TDBF_TIMER;
> -			tdbp->tdb_exp_timeout = ipsec_keep_invalid;
> +			tdbp->tdb_exp_timeout = keep_invalid_local;
>  			if (timeout_add_sec(&tdbp->tdb_timer_tmo,
> -			    ipsec_keep_invalid))
> +			    keep_invalid_local))
>  				tdb_ref(tdbp);
>  			mtx_leave(&tdbp->tdb_mtx);
>  		}
> Index: sys/netinet/ipsec_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> diff -u -p -r1.212 ipsec_input.c
> --- sys/netinet/ipsec_input.c	13 May 2025 09:16:33 -0000	1.212
> +++ sys/netinet/ipsec_input.c	13 May 2025 10:26:22 -0000
> @@ -106,7 +106,7 @@ void ipsec_common_ctlinput(u_int, int, s
>  
>  /* sysctl variables */
>  int encdebug = 0;
> -int ipsec_keep_invalid = IPSEC_DEFAULT_EMBRYONIC_SA_TIMEOUT;
> +int ipsec_keep_invalid = IPSEC_DEFAULT_EMBRYONIC_SA_TIMEOUT;	/* [a] */
>  int ipsec_require_pfs = IPSEC_DEFAULT_PFS;			/* [a] */
>  int ipsec_soft_allocations = IPSEC_DEFAULT_SOFT_ALLOCATIONS;	/* [a] */
>  int ipsec_exp_allocations = IPSEC_DEFAULT_EXP_ALLOCATIONS;	/* [a] */
> @@ -175,10 +175,10 @@ int ipsec_def_comp = IPSEC_COMP_DEFLATE;
>  const struct sysctl_bounded_args ipsecctl_vars_locked[] = {
>  	{ IPSEC_ENCDEBUG, &encdebug, 0, 1 },
>  	{ IPSEC_EXPIRE_ACQUIRE, &ipsec_expire_acquire, 0, INT_MAX },
> -	{ IPSEC_EMBRYONIC_SA_TIMEOUT, &ipsec_keep_invalid, 0, INT_MAX },
>  };
>  
>  const struct sysctl_bounded_args ipsecctl_vars[] = {
> +	{ IPSEC_EMBRYONIC_SA_TIMEOUT, &ipsec_keep_invalid, 0, INT_MAX },
>  	{ IPSEC_REQUIRE_PFS, &ipsec_require_pfs, 0, 1 },
>  	{ IPSEC_SOFT_ALLOCATIONS, &ipsec_soft_allocations, 0, INT_MAX },
>  	{ IPSEC_ALLOCATIONS, &ipsec_exp_allocations, 0, INT_MAX },
> @@ -650,7 +650,6 @@ ipsec_sysctl(int *name, u_int namelen, v
>  		return (ipsec_sysctl_ipsecstat(oldp, oldlenp, newp));
>  	case IPSEC_ENCDEBUG:
>  	case IPSEC_EXPIRE_ACQUIRE:
> -	case IPSEC_EMBRYONIC_SA_TIMEOUT:
>  		NET_LOCK();
>  		error = sysctl_bounded_arr(ipsecctl_vars_locked,
>  		    nitems(ipsecctl_vars_locked), name, namelen,