From: Vitaliy Makkoveev Subject: ipsec: move bunch of `ipsecctl_vars' variables out of netlock To: Alexander Bluhm , tech@openbsd.org Date: Sat, 10 May 2025 04:17:22 +0300 The `ipsec_soft_allocations', `ipsec_exp_allocations', `ipsec_soft_bytes', `ipsec_exp_bytes', `ipsec_soft_timeout', `ipsec_exp_timeout', `ipsec_soft_first_use' and `ipsec_exp_first_use' are local to pfkeyv2_acquire(). They are loaded together during `sadb_comb' initialization, so make sense to unlock them together too. It looks reasonable to drop 'ipsec_' prefix for local variables, otherwise the names are too long, and I think this should made code reading worse. For consistency I dropped 'ipsec_' prefix for `ipsec_def_*_local' too. The naming is discussable. Index: sys/net/pfkeyv2.c =================================================================== RCS file: /cvs/src/sys/net/pfkeyv2.c,v retrieving revision 1.265 diff -u -p -r1.265 pfkeyv2.c --- sys/net/pfkeyv2.c 9 May 2025 19:53:41 -0000 1.265 +++ sys/net/pfkeyv2.c 10 May 2025 00:40:50 -0000 @@ -2159,11 +2159,27 @@ pfkeyv2_acquire(struct ipsec_policy *ipo int i, j, registered; #ifdef IPSEC - int ipsec_def_enc_local, ipsec_def_comp_local, ipsec_def_auth_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; + + 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); + + soft_allocations_local = atomic_load_int(&ipsec_soft_allocations); + exp_allocations_local = atomic_load_int(&ipsec_exp_allocations); - ipsec_def_enc_local = atomic_load_int(&ipsec_def_enc); - ipsec_def_comp_local = atomic_load_int(&ipsec_def_comp); - ipsec_def_auth_local = atomic_load_int(&ipsec_def_auth); + soft_bytes_local = atomic_load_int(&ipsec_soft_bytes); + exp_bytes_local = atomic_load_int(&ipsec_exp_bytes); + + soft_timeout_local = atomic_load_int(&ipsec_soft_timeout); + exp_timeout_local = atomic_load_int(&ipsec_exp_timeout); + + soft_first_use_local = atomic_load_int(&ipsec_soft_first_use); + exp_first_use_local = atomic_load_int(&ipsec_exp_first_use); #endif mtx_enter(&pfkeyv2_mtx); @@ -2256,7 +2272,7 @@ pfkeyv2_acquire(struct ipsec_policy *ipo if (ipo->ipo_sproto == IPPROTO_ESP) { /* Set the encryption algorithm */ - switch(ipsec_def_enc_local) { + switch(def_enc_local) { case IPSEC_ENC_AES: sadb_comb->sadb_comb_encrypt = SADB_X_EALG_AES; sadb_comb->sadb_comb_encrypt_minbits = 128; @@ -2288,7 +2304,7 @@ pfkeyv2_acquire(struct ipsec_policy *ipo } } else if (ipo->ipo_sproto == IPPROTO_IPCOMP) { /* Set the compression algorithm */ - switch(ipsec_def_comp_local) { + switch(def_comp_local) { case IPSEC_COMP_DEFLATE: sadb_comb->sadb_comb_encrypt = SADB_X_CALG_DEFLATE; @@ -2299,7 +2315,7 @@ pfkeyv2_acquire(struct ipsec_policy *ipo } /* Set the authentication algorithm */ - switch(ipsec_def_auth_local) { + switch(def_auth_local) { case IPSEC_AUTH_HMAC_SHA1: sadb_comb->sadb_comb_auth = SADB_AALG_SHA1HMAC; sadb_comb->sadb_comb_auth_minbits = 160; @@ -2332,17 +2348,17 @@ pfkeyv2_acquire(struct ipsec_policy *ipo break; } - sadb_comb->sadb_comb_soft_allocations = ipsec_soft_allocations; - sadb_comb->sadb_comb_hard_allocations = ipsec_exp_allocations; + sadb_comb->sadb_comb_soft_allocations = soft_allocations_local; + sadb_comb->sadb_comb_hard_allocations = exp_allocations_local; - sadb_comb->sadb_comb_soft_bytes = ipsec_soft_bytes; - sadb_comb->sadb_comb_hard_bytes = ipsec_exp_bytes; + sadb_comb->sadb_comb_soft_bytes = soft_bytes_local; + sadb_comb->sadb_comb_hard_bytes = exp_bytes_local; - sadb_comb->sadb_comb_soft_addtime = ipsec_soft_timeout; - sadb_comb->sadb_comb_hard_addtime = ipsec_exp_timeout; + sadb_comb->sadb_comb_soft_addtime = soft_timeout_local; + sadb_comb->sadb_comb_hard_addtime = exp_timeout_local; - sadb_comb->sadb_comb_soft_usetime = ipsec_soft_first_use; - sadb_comb->sadb_comb_hard_usetime = ipsec_exp_first_use; + sadb_comb->sadb_comb_soft_usetime = soft_first_use_local; + sadb_comb->sadb_comb_hard_usetime = exp_first_use_local; #endif sadb_comb++; } Index: sys/netinet/ipsec_input.c =================================================================== RCS file: /cvs/src/sys/netinet/ipsec_input.c,v retrieving revision 1.210 diff -u -p -r1.210 ipsec_input.c --- sys/netinet/ipsec_input.c 9 May 2025 19:53:41 -0000 1.210 +++ sys/netinet/ipsec_input.c 10 May 2025 00:40:50 -0000 @@ -108,14 +108,14 @@ void ipsec_common_ctlinput(u_int, int, s int encdebug = 0; int ipsec_keep_invalid = IPSEC_DEFAULT_EMBRYONIC_SA_TIMEOUT; int ipsec_require_pfs = IPSEC_DEFAULT_PFS; -int ipsec_soft_allocations = IPSEC_DEFAULT_SOFT_ALLOCATIONS; -int ipsec_exp_allocations = IPSEC_DEFAULT_EXP_ALLOCATIONS; -int ipsec_soft_bytes = IPSEC_DEFAULT_SOFT_BYTES; -int ipsec_exp_bytes = IPSEC_DEFAULT_EXP_BYTES; -int ipsec_soft_timeout = IPSEC_DEFAULT_SOFT_TIMEOUT; -int ipsec_exp_timeout = IPSEC_DEFAULT_EXP_TIMEOUT; -int ipsec_soft_first_use = IPSEC_DEFAULT_SOFT_FIRST_USE; -int ipsec_exp_first_use = IPSEC_DEFAULT_EXP_FIRST_USE; +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] */ +int ipsec_exp_bytes = IPSEC_DEFAULT_EXP_BYTES; /* [a] */ +int ipsec_soft_timeout = IPSEC_DEFAULT_SOFT_TIMEOUT; /* [a] */ +int ipsec_exp_timeout = IPSEC_DEFAULT_EXP_TIMEOUT; /* [a] */ +int ipsec_soft_first_use = IPSEC_DEFAULT_SOFT_FIRST_USE; /* [a] */ +int ipsec_exp_first_use = IPSEC_DEFAULT_EXP_FIRST_USE; /* [a] */ int ipsec_expire_acquire = IPSEC_DEFAULT_EXPIRE_ACQUIRE; int esp_enable = 1; @@ -172,17 +172,20 @@ int ipsec_def_enc = IPSEC_ENC_AES; /* [ int ipsec_def_auth = IPSEC_AUTH_HMAC_SHA1; /* [a] */ int ipsec_def_comp = IPSEC_COMP_DEFLATE; /* [a] */ -const struct sysctl_bounded_args ipsecctl_vars[] = { +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 }, { IPSEC_REQUIRE_PFS, &ipsec_require_pfs, 0, 1 }, +}; + +const struct sysctl_bounded_args ipsecctl_vars[] = { { 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 }, { IPSEC_BYTES, &ipsec_exp_bytes, 0, INT_MAX }, { IPSEC_TIMEOUT, &ipsec_exp_timeout, 0, INT_MAX }, - { IPSEC_SOFT_TIMEOUT, &ipsec_soft_timeout,0, INT_MAX }, + { IPSEC_SOFT_TIMEOUT, &ipsec_soft_timeout, 0, INT_MAX }, { IPSEC_SOFT_FIRSTUSE, &ipsec_soft_first_use, 0, INT_MAX }, { IPSEC_FIRSTUSE, &ipsec_exp_first_use, 0, INT_MAX }, }; @@ -645,12 +648,19 @@ ipsec_sysctl(int *name, u_int namelen, v newp, newlen)); case IPCTL_IPSEC_STATS: return (ipsec_sysctl_ipsecstat(oldp, oldlenp, newp)); - default: + case IPSEC_ENCDEBUG: + case IPSEC_EXPIRE_ACQUIRE: + case IPSEC_EMBRYONIC_SA_TIMEOUT: + case IPSEC_REQUIRE_PFS: NET_LOCK(); - error = sysctl_bounded_arr(ipsecctl_vars, nitems(ipsecctl_vars), - name, namelen, oldp, oldlenp, newp, newlen); + error = sysctl_bounded_arr(ipsecctl_vars_locked, + nitems(ipsecctl_vars_locked), name, namelen, + oldp, oldlenp, newp, newlen); NET_UNLOCK(); return (error); + default: + return (sysctl_bounded_arr(ipsecctl_vars, nitems(ipsecctl_vars), + name, namelen, oldp, oldlenp, newp, newlen)); } }