From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: ipsec: move `psec_require_pfs' out of netlock
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Mon, 12 May 2025 21:33:23 +0200

On Sun, May 11, 2025 at 01:10:43PM +0300, Vitaliy Makkoveev wrote:
> Also atomically accessed integer local to pfkeyv2_acquire(). The rest
> ones of `ipsecctl_vars' belong to other source files.

OK bluhm@

> Index: sys/net/pfkeyv2.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.266
> diff -u -p -r1.266 pfkeyv2.c
> --- sys/net/pfkeyv2.c	12 May 2025 17:20:09 -0000	1.266
> +++ sys/net/pfkeyv2.c	12 May 2025 18:34:34 -0000
> @@ -2159,12 +2159,15 @@ pfkeyv2_acquire(struct ipsec_policy *ipo
>  	int i, j, registered;
>  
>  #ifdef IPSEC
> +	int require_pfs_local;
>  	int def_enc_local, def_comp_local, def_auth_local;
>  	int soft_allocations_local, exp_allocations_local;
>  	int soft_bytes_local, exp_bytes_local;
>  	int soft_timeout_local, exp_timeout_local;
>  	int soft_first_use_local, exp_first_use_local;
>  
> +	require_pfs_local = atomic_load_int(&ipsec_require_pfs);
> +
>  	def_enc_local = atomic_load_int(&ipsec_def_enc);
>  	def_comp_local = atomic_load_int(&ipsec_def_comp);
>  	def_auth_local = atomic_load_int(&ipsec_def_auth);
> @@ -2267,7 +2270,7 @@ pfkeyv2_acquire(struct ipsec_policy *ipo
>  	for (j = 0; j < sa_prop->sadb_prop_num; j++) {
>  		sadb_comb->sadb_comb_flags = 0;
>  #ifdef IPSEC
> -		if (ipsec_require_pfs)
> +		if (require_pfs_local)
>  			sadb_comb->sadb_comb_flags |= SADB_SAFLAGS_PFS;
>  
>  		if (ipo->ipo_sproto == IPPROTO_ESP) {
> Index: sys/netinet/ipsec_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> retrieving revision 1.211
> diff -u -p -r1.211 ipsec_input.c
> --- sys/netinet/ipsec_input.c	12 May 2025 17:20:09 -0000	1.211
> +++ sys/netinet/ipsec_input.c	12 May 2025 18:34:34 -0000
> @@ -107,7 +107,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_require_pfs = IPSEC_DEFAULT_PFS;
> +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] */
>  int ipsec_soft_bytes = IPSEC_DEFAULT_SOFT_BYTES;		/* [a] */
> @@ -176,10 +176,10 @@ const struct sysctl_bounded_args ipsecct
>  	{ IPSEC_ENCDEBUG, &encdebug, 0, 1 },
>  	{ IPSEC_EXPIRE_ACQUIRE, &ipsec_expire_acquire, 0, INT_MAX },
>  	{ IPSEC_EMBRYONIC_SA_TIMEOUT, &ipsec_keep_invalid, 0, INT_MAX },
> -	{ IPSEC_REQUIRE_PFS, &ipsec_require_pfs, 0, 1 },
>  };
>  
>  const struct sysctl_bounded_args ipsecctl_vars[] = {
> +	{ 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 },
>  	{ IPSEC_SOFT_BYTES, &ipsec_soft_bytes, 0, INT_MAX },
> @@ -651,7 +651,6 @@ ipsec_sysctl(int *name, u_int namelen, v
>  	case IPSEC_ENCDEBUG:
>  	case IPSEC_EXPIRE_ACQUIRE:
>  	case IPSEC_EMBRYONIC_SA_TIMEOUT:
> -	case IPSEC_REQUIRE_PFS:
>  		NET_LOCK();
>  		error = sysctl_bounded_arr(ipsecctl_vars_locked,
>  		    nitems(ipsecctl_vars_locked), name, namelen,