Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
ipsec: move bunch of `ipsecctl_vars' variables out of netlock
To:
Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date:
Sat, 10 May 2025 04:17:22 +0300

Download raw body.

Thread
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));
 	}
 }