On Thu, Apr 21, 2022 at 03:25:03PM +0200, Alexander Bluhm wrote:
> Hi,
>
> As claudio@ wants to refactor router timer before making them MP
> safe, I would like to protect them with kernel lock. It should fix
> this panic.
>
> https://marc.info/?l=openbsd-tech&m=164038527425440&w=2
>
> I hope this is the final step before running IP forwarding in
> parallel.
>
> ok?
I prefer the other diff, I don't like that other diff but this one is worse.
> bluhm
>
> Index: netinet/ip_icmp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v
> retrieving revision 1.188
> diff -u -p -r1.188 ip_icmp.c
> --- netinet/ip_icmp.c 20 Apr 2022 09:38:26 -0000 1.188
> +++ netinet/ip_icmp.c 21 Apr 2022 12:45:40 -0000
> @@ -634,8 +634,10 @@ reflect:
> rtredirect(sintosa(&sdst), sintosa(&sgw),
> sintosa(&ssrc), &newrt, m->m_pkthdr.ph_rtableid);
> if (newrt != NULL && icmp_redirtimeout > 0) {
> + KERNEL_LOCK();
> rt_timer_add(newrt, icmp_redirect_timeout,
> icmp_redirect_timeout_q, m->m_pkthdr.ph_rtableid);
> + KERNEL_UNLOCK();
> }
> rtfree(newrt);
> pfctlinput(PRC_REDIRECT_HOST, sintosa(&sdst));
> @@ -884,8 +886,10 @@ icmp_sysctl(int *name, u_int namelen, vo
> NET_LOCK();
> error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> &icmp_redirtimeout, 0, INT_MAX);
> + KERNEL_LOCK();
> rt_timer_queue_change(icmp_redirect_timeout_q,
> icmp_redirtimeout);
> + KERNEL_UNLOCK();
> NET_UNLOCK();
> break;
>
> @@ -975,8 +979,10 @@ icmp_mtudisc_clone(struct in_addr dst, u
> rt = nrt;
> rtm_send(rt, RTM_ADD, 0, rtableid);
> }
> + KERNEL_LOCK();
> error = rt_timer_add(rt, icmp_mtudisc_timeout, ip_mtudisc_timeout_q,
> rtableid);
> + KERNEL_UNLOCK();
> if (error)
> goto bad;
>
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.367
> diff -u -p -r1.367 ip_input.c
> --- netinet/ip_input.c 20 Apr 2022 09:38:26 -0000 1.367
> +++ netinet/ip_input.c 21 Apr 2022 13:00:33 -0000
> @@ -1616,9 +1616,11 @@ ip_sysctl(int *name, u_int namelen, void
> NET_LOCK();
> error = sysctl_int(oldp, oldlenp, newp, newlen, &ip_mtudisc);
> if (ip_mtudisc == 0) {
> + KERNEL_LOCK();
> rt_timer_queue_destroy(ip_mtudisc_timeout_q);
> ip_mtudisc_timeout_q =
> rt_timer_queue_create(ip_mtudisc_timeout);
> + KERNEL_UNLOCK();
> }
> NET_UNLOCK();
> return error;
> @@ -1626,8 +1628,10 @@ ip_sysctl(int *name, u_int namelen, void
> NET_LOCK();
> error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> &ip_mtudisc_timeout, 0, INT_MAX);
> + KERNEL_LOCK();
> rt_timer_queue_change(ip_mtudisc_timeout_q,
> ip_mtudisc_timeout);
> + KERNEL_UNLOCK();
> NET_UNLOCK();
> return (error);
> #ifdef IPSEC
> Index: netinet/ip_mroute.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_mroute.c,v
> retrieving revision 1.131
> diff -u -p -r1.131 ip_mroute.c
> --- netinet/ip_mroute.c 15 Dec 2021 17:21:08 -0000 1.131
> +++ netinet/ip_mroute.c 21 Apr 2022 13:02:43 -0000
> @@ -520,7 +520,9 @@ ip_mrouter_init(struct socket *so, struc
> return (EADDRINUSE);
>
> ip_mrouter[rtableid] = so;
> + KERNEL_LOCK();
> mrouterq[rtableid] = rt_timer_queue_create(MCAST_EXPIRE_FREQUENCY);
> + KERNEL_UNLOCK();
>
> return (0);
> }
> @@ -572,7 +574,9 @@ ip_mrouter_done(struct socket *so)
>
> mrt_api_config = 0;
>
> + KERNEL_LOCK();
> rt_timer_queue_destroy(mrouterq[rtableid]);
> + KERNEL_UNLOCK();
> mrouterq[rtableid] = NULL;
> ip_mrouter[rtableid] = NULL;
> mrt_count[rtableid] = 0;
> @@ -799,8 +803,10 @@ mfc_expire_route(struct rtentry *rt, str
> /* Not expired, add it back to the queue. */
> if (mfc->mfc_expire == 0) {
> mfc->mfc_expire = 1;
> + KERNEL_LOCK();
> rt_timer_add(rt, mfc_expire_route, mrouterq[rtableid],
> rtableid);
> + KERNEL_UNLOCK();
> return;
> }
>
> @@ -834,8 +840,10 @@ mfc_add_route(struct ifnet *ifp, struct
>
> rt->rt_llinfo = (caddr_t)mfc;
>
> + KERNEL_LOCK();
> rt_timer_add(rt, mfc_expire_route, mrouterq[rtableid],
> rtableid);
> + KERNEL_UNLOCK();
>
> mfc->mfc_parent = mfccp->mfcc_parent;
> mfc->mfc_pkt_cnt = 0;
> @@ -1342,7 +1350,9 @@ mrt_mcast_del(struct rtentry *rt, unsign
> int error;
>
> /* Remove all timers related to this route. */
> + KERNEL_LOCK();
> rt_timer_remove_all(rt);
> + KERNEL_UNLOCK();
>
> free(rt->rt_llinfo, M_MRTABLE, sizeof(struct mfc));
> rt->rt_llinfo = NULL;
> Index: netinet6/icmp6.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
> retrieving revision 1.239
> diff -u -p -r1.239 icmp6.c
> --- netinet6/icmp6.c 20 Apr 2022 09:38:26 -0000 1.239
> +++ netinet6/icmp6.c 21 Apr 2022 12:48:18 -0000
> @@ -987,7 +987,9 @@ icmp6_mtudisc_update(struct ip6ctlparam
> * allow non-validated cases if memory is plenty, to make traffic
> * from non-connected pcb happy.
> */
> + KERNEL_LOCK();
> rtcount = rt_timer_queue_count(icmp6_mtudisc_timeout_q);
> + KERNEL_UNLOCK();
> if (validated) {
> if (0 <= icmp6_mtudisc_hiwat && rtcount > icmp6_mtudisc_hiwat)
> return;
> @@ -1383,7 +1385,9 @@ icmp6_redirect_input(struct mbuf *m, int
> * work just fine even if we do not install redirect route
> * (there will be additional hops, though).
> */
> + KERNEL_LOCK();
> rtcount = rt_timer_queue_count(icmp6_redirect_timeout_q);
> + KERNEL_UNLOCK();
> if (0 <= ip6_maxdynroutes && rtcount >= ip6_maxdynroutes)
> goto freeit;
> else if (0 <= icmp6_redirect_lowat &&
> @@ -1405,8 +1409,10 @@ icmp6_redirect_input(struct mbuf *m, int
> rtredirect(sin6tosa(&sdst), sin6tosa(&sgw), sin6tosa(&ssrc),
> &newrt, m->m_pkthdr.ph_rtableid);
> if (newrt != NULL && icmp6_redirtimeout > 0) {
> + KERNEL_LOCK();
> rt_timer_add(newrt, icmp6_redirect_timeout,
> icmp6_redirect_timeout_q, m->m_pkthdr.ph_rtableid);
> + KERNEL_UNLOCK();
> }
> rtfree(newrt);
> }
> @@ -1829,8 +1835,10 @@ icmp6_mtudisc_clone(struct sockaddr_in6
> rt = nrt;
> rtm_send(rt, RTM_ADD, 0, rtableid);
> }
> + KERNEL_LOCK();
> error = rt_timer_add(rt, icmp6_mtudisc_timeout, icmp6_mtudisc_timeout_q,
> rtableid);
> + KERNEL_UNLOCK();
> if (error)
> goto bad;
>
> @@ -1921,8 +1929,10 @@ icmp6_sysctl(int *name, u_int namelen, v
> NET_LOCK();
> error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> &icmp6_redirtimeout, 0, INT_MAX);
> + KERNEL_LOCK();
> rt_timer_queue_change(icmp6_redirect_timeout_q,
> icmp6_redirtimeout);
> + KERNEL_UNLOCK();
> NET_UNLOCK();
> break;
>
> Index: netinet6/ip6_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.241
> diff -u -p -r1.241 ip6_input.c
> --- netinet6/ip6_input.c 20 Apr 2022 09:38:26 -0000 1.241
> +++ netinet6/ip6_input.c 21 Apr 2022 12:48:51 -0000
> @@ -1456,8 +1456,10 @@ ip6_sysctl(int *name, u_int namelen, voi
> NET_LOCK();
> error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> &ip6_mtudisc_timeout, 0, INT_MAX);
> + KERNEL_LOCK();
> rt_timer_queue_change(icmp6_mtudisc_timeout_q,
> ip6_mtudisc_timeout);
> + KERNEL_UNLOCK();
> NET_UNLOCK();
> return (error);
> case IPV6CTL_IFQUEUE:
> Index: netinet6/ip6_mroute.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_mroute.c,v
> retrieving revision 1.127
> diff -u -p -r1.127 ip6_mroute.c
> --- netinet6/ip6_mroute.c 15 Dec 2021 17:21:08 -0000 1.127
> +++ netinet6/ip6_mroute.c 21 Apr 2022 12:50:04 -0000
> @@ -494,7 +494,9 @@ ip6_mrouter_init(struct socket *so, int
>
> ip6_mrouter[rtableid] = so;
> ip6_mrouter_ver = cmd;
> + KERNEL_LOCK();
> mrouter6q[rtableid] = rt_timer_queue_create(MCAST_EXPIRE_TIMEOUT);
> + KERNEL_UNLOCK();
>
> return (0);
> }
> @@ -544,7 +546,9 @@ ip6_mrouter_done(struct socket *so)
> ip6_mrouter_detach(ifp);
> }
>
> + KERNEL_LOCK();
> rt_timer_queue_destroy(mrouter6q[rtableid]);
> + KERNEL_UNLOCK();
> ip6_mrouter[inp->inp_rtableid] = NULL;
> ip6_mrouter_ver = 0;
> mrouter6q[rtableid] = NULL;
> @@ -682,7 +686,9 @@ mf6c_add_route(struct ifnet *ifp, struct
> }
>
> rt->rt_llinfo = (caddr_t)mf6c;
> + KERNEL_LOCK();
> rt_timer_add(rt, mf6c_expire_route, mrouter6q[rtableid], rtableid);
> + KERNEL_UNLOCK();
> mf6c->mf6c_parent = mf6cc->mf6cc_parent;
> rtfree(rt);
>
> @@ -1010,8 +1016,10 @@ mf6c_expire_route(struct rtentry *rt, st
>
> if (mf6c->mf6c_expire == 0) {
> mf6c->mf6c_expire = 1;
> + KERNEL_LOCK();
> rt_timer_add(rt, mf6c_expire_route, mrouter6q[rtableid],
> rtableid);
> + KERNEL_UNLOCK();
> return;
> }
>
> @@ -1272,7 +1280,9 @@ mrt6_mcast_del(struct rtentry *rt, unsig
> int error;
>
> /* Remove all timers related to this route. */
> + KERNEL_LOCK();
> rt_timer_remove_all(rt);
> + KERNEL_UNLOCK();
>
> free(rt->rt_llinfo, M_MRTABLE, sizeof(struct mf6c));
> rt->rt_llinfo = NULL;
>
--
:wq Claudio
