Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: sysctl(2): unlock carp_sysctl()
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
Alexandr Nedvedicky <sashan@openbsd.org>, OpenBSD Tech <tech@openbsd.org>
Date:
Fri, 20 Dec 2024 00:21:13 +0300

Download raw body.

Thread
> On 20 Dec 2024, at 00:06, Alexander Bluhm <alexander.bluhm@gmx.net> wrote:
> 
> On Thu, Dec 12, 2024 at 09:39:03PM +0300, Vitaliy Makkoveev wrote:
>> This is the statistics implemented with per-CPU counters and atomically
>> accessed integers.
>> 
>> I replaced the `carp_opts' array with individual `carpctl_*' variables
>> and used sysctl_bounded_arr() instead former sysctl_int(). So, now we
>> have no used elements of `carp_opts'. Also we could define the bounds
>> for carp(4) controls. I intentionally left previous behaviour fro now,
>> but in the future I like to fix them too.
> 
> OK bluhm@
> 
> carpctl_allow and carpctl_preempt should be boolean 0 or 1.
> 
> carpctl_log is beween LOG_EMERG and LOG_DEBUG, so 0 ... 7
> Then it is actually unsigned and you don't need the (int) cast.

I intentionally left current behaviour. I doubt someone really have
net.inet.carp.preempt=-1, but carp.log could be negative to disable
logging.

> 
> I think CARPCTL_MAXID can be removed, but we should make a full
> build before.
> 

It is used by userland sysctl(8).

>> Index: sys/netinet/in_proto.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet/in_proto.c,v
>> diff -u -p -r1.116 in_proto.c
>> --- sys/netinet/in_proto.c	4 Dec 2024 22:48:41 -0000	1.116
>> +++ sys/netinet/in_proto.c	12 Dec 2024 18:28:46 -0000
>> @@ -331,7 +331,7 @@ const struct protosw inetsw[] = {
>>   .pr_type	= SOCK_RAW,
>>   .pr_domain	= &inetdomain,
>>   .pr_protocol	= IPPROTO_CARP,
>> -  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET,
>> +  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET|PR_MPSYSCTL,
>>   .pr_input	= carp_proto_input,
>>   .pr_ctloutput	= rip_ctloutput,
>>   .pr_usrreqs	= &rip_usrreqs,
>> Index: sys/netinet/ip_carp.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
>> diff -u -p -r1.364 ip_carp.c
>> --- sys/netinet/ip_carp.c	7 Dec 2024 01:14:45 -0000	1.364
>> +++ sys/netinet/ip_carp.c	12 Dec 2024 18:28:46 -0000
>> @@ -88,6 +88,11 @@
>> 
>> #include <netinet/ip_carp.h>
>> 
>> +/*
>> + * Locks used to protect data:
>> + *	a	atomic
>> + */
>> +
>> struct carp_mc_entry {
>> 	LIST_ENTRY(carp_mc_entry)	mc_entries;
>> 	union {
>> @@ -189,14 +194,23 @@ void	carp_sc_unref(void *, void *);
>> struct srpl_rc carp_sc_rc =
>>     SRPL_RC_INITIALIZER(carp_sc_ref, carp_sc_unref, NULL);
>> 
>> -int carp_opts[CARPCTL_MAXID] = { 0, 1, 0, LOG_CRIT };	/* XXX for now */
>> +int carpctl_allow = 1;		/* [a] */
>> +int carpctl_preempt = 0;	/* [a] */
>> +int carpctl_log = LOG_CRIT;	/* [a] */
>> +
>> +const struct sysctl_bounded_args carpctl_vars[] = {
>> +	{CARPCTL_ALLOW, &carpctl_allow, INT_MIN, INT_MAX},
>> +	{CARPCTL_PREEMPT, &carpctl_preempt, INT_MIN, INT_MAX},
>> +	{CARPCTL_LOG, &carpctl_log, INT_MIN, INT_MAX},
>> +};
>> +
>> struct cpumem *carpcounters;
>> 
>> int	carp_send_all_recur = 0;
>> 
>> #define	CARP_LOG(l, sc, s)						\
>> 	do {								\
>> -		if (carp_opts[CARPCTL_LOG] >= l) {			\
>> +		if ((int)atomic_load_int(&carpctl_log) >= l) {		\
>> 			if (sc)						\
>> 				log(l, "%s: ",				\
>> 				    (sc)->sc_if.if_xname);		\
>> @@ -451,7 +465,7 @@ carp_proto_input_if(struct ifnet *ifp, s
>> 
>> 	carpstat_inc(carps_ipackets);
>> 
>> -	if (!carp_opts[CARPCTL_ALLOW]) {
>> +	if (!atomic_load_int(&carpctl_allow)) {
>> 		m_freem(m);
>> 		return IPPROTO_DONE;
>> 	}
>> @@ -550,7 +564,7 @@ carp6_proto_input_if(struct ifnet *ifp, 
>> 
>> 	carpstat_inc(carps_ipackets6);
>> 
>> -	if (!carp_opts[CARPCTL_ALLOW]) {
>> +	if (!atomic_load_int(&carpctl_allow)) {
>> 		m_freem(m);
>> 		return IPPROTO_DONE;
>> 	}
>> @@ -707,7 +721,7 @@ carp_proto_input_c(struct ifnet *ifp, st
>> 		 * and do not have a better demote count, treat them as down.
>> 		 *
>> 		 */
>> -		if (carp_opts[CARPCTL_PREEMPT] &&
>> +		if (atomic_load_int(&carpctl_preempt) &&
>> 		    timercmp(&sc_tv, &ch_tv, <) &&
>> 		    ch->carp_demote >= carp_group_demote_count(sc)) {
>> 			carp_master_down(vhe);
>> @@ -765,8 +779,6 @@ int
>> carp_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>>     size_t newlen)
>> {
>> -	int error;
>> -
>> 	/* All sysctl names at this level are terminal. */
>> 	if (namelen != 1)
>> 		return (ENOTDIR);
>> @@ -775,13 +787,8 @@ carp_sysctl(int *name, u_int namelen, vo
>> 	case CARPCTL_STATS:
>> 		return (carp_sysctl_carpstat(oldp, oldlenp, newp));
>> 	default:
>> -		if (name[0] <= 0 || name[0] >= CARPCTL_MAXID)
>> -			return (ENOPROTOOPT);
>> -		NET_LOCK();
>> -		error = sysctl_int(oldp, oldlenp, newp, newlen,
>> -		    &carp_opts[name[0]]);
>> -		NET_UNLOCK();
>> -		return (error);
>> +		return (sysctl_bounded_arr(carpctl_vars, nitems(carpctl_vars),
>> +		    name, namelen, oldp, oldlenp, newp, newlen));
>> 	}
>> }
>> 
>> Index: sys/netinet6/in6_proto.c
>> ===================================================================
>> RCS file: /cvs/src/sys/netinet6/in6_proto.c,v
>> diff -u -p -r1.120 in6_proto.c
>> --- sys/netinet6/in6_proto.c	4 Dec 2024 22:48:41 -0000	1.120
>> +++ sys/netinet6/in6_proto.c	12 Dec 2024 18:28:46 -0000
>> @@ -278,7 +278,7 @@ const struct protosw inet6sw[] = {
>>   .pr_type	= SOCK_RAW,
>>   .pr_domain	= &inet6domain,
>>   .pr_protocol	= IPPROTO_CARP,
>> -  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET,
>> +  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSOCKET|PR_MPSYSCTL,
>>   .pr_input	= carp6_proto_input,
>>   .pr_ctloutput = rip6_ctloutput,
>>   .pr_usrreqs	= &rip6_usrreqs,