From: Vitaliy Makkoveev Subject: Re: sysctl(2): unlock carp_sysctl() To: Alexander Bluhm Cc: Alexandr Nedvedicky , OpenBSD Tech Date: Fri, 20 Dec 2024 00:21:13 +0300 > On 20 Dec 2024, at 00:06, Alexander Bluhm 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 >> >> +/* >> + * 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,