From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: ipsec: move `encdebug' out of netlock
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 14 May 2025 01:04:00 +0200

On Wed, May 14, 2025 at 12:38:22AM +0300, Vitaliy Makkoveev wrote:
> Tha last one from `ipsecctl_vars'. Widely used in the DPRINTF() macros,
> but disabled by default.
> 
> Do we really need to enforce load `encdebug' value each time?

At least it is consistent that way.

anyway OK bluhm@

> Index: sys/net/pfkeyv2_parsemessage.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkeyv2_parsemessage.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 pfkeyv2_parsemessage.c
> --- sys/net/pfkeyv2_parsemessage.c	23 Jul 2024 20:04:51 -0000	1.63
> +++ sys/net/pfkeyv2_parsemessage.c	13 May 2025 21:32:36 -0000
> @@ -86,7 +86,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet/ip_ah.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_ah.c,v
> retrieving revision 1.175
> diff -u -p -r1.175 ip_ah.c
> --- sys/netinet/ip_ah.c	2 Mar 2025 21:28:32 -0000	1.175
> +++ sys/netinet/ip_ah.c	13 May 2025 21:32:36 -0000
> @@ -73,7 +73,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet/ip_esp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_esp.c,v
> retrieving revision 1.197
> diff -u -p -r1.197 ip_esp.c
> --- sys/netinet/ip_esp.c	2 Mar 2025 21:28:32 -0000	1.197
> +++ sys/netinet/ip_esp.c	13 May 2025 21:32:36 -0000
> @@ -72,7 +72,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet/ip_ipcomp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_ipcomp.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 ip_ipcomp.c
> --- sys/netinet/ip_ipcomp.c	2 Mar 2025 21:28:32 -0000	1.93
> +++ sys/netinet/ip_ipcomp.c	13 May 2025 21:32:36 -0000
> @@ -59,7 +59,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet/ip_ipip.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_ipip.c,v
> retrieving revision 1.106
> diff -u -p -r1.106 ip_ipip.c
> --- sys/netinet/ip_ipip.c	2 Mar 2025 21:28:32 -0000	1.106
> +++ sys/netinet/ip_ipip.c	13 May 2025 21:32:36 -0000
> @@ -80,7 +80,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet/ip_ipsp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_ipsp.c,v
> retrieving revision 1.279
> diff -u -p -r1.279 ip_ipsp.c
> --- sys/netinet/ip_ipsp.c	13 May 2025 17:27:53 -0000	1.279
> +++ sys/netinet/ip_ipsp.c	13 May 2025 21:32:36 -0000
> @@ -83,7 +83,7 @@ void tdb_hashstats(void);
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet/ip_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.408
> diff -u -p -r1.408 ip_output.c
> --- sys/netinet/ip_output.c	21 Apr 2025 09:54:53 -0000	1.408
> +++ sys/netinet/ip_output.c	13 May 2025 21:32:36 -0000
> @@ -68,7 +68,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug)				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet/ipsec_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> retrieving revision 1.214
> diff -u -p -r1.214 ipsec_input.c
> --- sys/netinet/ipsec_input.c	13 May 2025 20:06:10 -0000	1.214
> +++ sys/netinet/ipsec_input.c	13 May 2025 21:32:36 -0000
> @@ -96,7 +96,7 @@ void ipsec_common_ctlinput(u_int, int, s
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> @@ -105,7 +105,7 @@ void ipsec_common_ctlinput(u_int, int, s
>  #endif
>  
>  /* sysctl variables */
> -int encdebug = 0;
> +int encdebug = 0;						/* [a] */
>  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] */
> @@ -172,11 +172,8 @@ 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_locked[] = {
> -	{ IPSEC_ENCDEBUG, &encdebug, 0, 1 },
> -};
> -
>  const struct sysctl_bounded_args ipsecctl_vars[] = {
> +	{ 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 },
> @@ -638,8 +635,6 @@ int
>  ipsec_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>      size_t newlen)
>  {
> -	int error;
> -
>  	switch (name[0]) {
>  	case IPCTL_IPSEC_ENC_ALGORITHM:
>  	case IPCTL_IPSEC_AUTH_ALGORITHM:
> @@ -648,13 +643,6 @@ ipsec_sysctl(int *name, u_int namelen, v
>  		    newp, newlen));
>  	case IPCTL_IPSEC_STATS:
>  		return (ipsec_sysctl_ipsecstat(oldp, oldlenp, newp));
> -	case IPSEC_ENCDEBUG:
> -		NET_LOCK();
> -		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));
> Index: sys/netinet/ipsec_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_output.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 ipsec_output.c
> --- sys/netinet/ipsec_output.c	14 Feb 2025 13:14:13 -0000	1.100
> +++ sys/netinet/ipsec_output.c	13 May 2025 21:32:36 -0000
> @@ -54,7 +54,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else
> Index: sys/netinet6/ip6_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
> retrieving revision 1.298
> diff -u -p -r1.298 ip6_output.c
> --- sys/netinet6/ip6_output.c	21 Apr 2025 09:54:53 -0000	1.298
> +++ sys/netinet6/ip6_output.c	13 May 2025 21:32:36 -0000
> @@ -109,7 +109,7 @@
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> -		if (encdebug)						\
> +		if (atomic_load_int(&encdebug))				\
>  			printf("%s: " fmt "\n", __func__, ## args);	\
>  	} while (0)
>  #else