From: claudio Date: Mon, 27 Jun 2022 21:26:46 +0000 (+0000) Subject: Rework the rttimer code. Instead of a global queue and a global timeout X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=932c91099162e42518d474e732135401853f56a1;p=openbsd Rework the rttimer code. Instead of a global queue and a global timeout use a per rttimer struct timeout. On enqueue the struct rttimer belongs to the timeout, in case the route is removed before the timer fires cleanup based on the timeout_del() return value. If the timeout currently running then just clear the rtt_rt pointer and let the timeout handle the cleanup. This should hopefully fix the icmp_pmtu_timeout crashes reported by some people. OK bluhm@ --- diff --git a/sys/net/route.c b/sys/net/route.c index 56d927b9e7e..c9b83e892ba 100644 --- a/sys/net/route.c +++ b/sys/net/route.c @@ -1,4 +1,4 @@ -/* $OpenBSD: route.c,v 1.410 2022/05/05 13:57:40 claudio Exp $ */ +/* $OpenBSD: route.c,v 1.411 2022/06/27 21:26:46 claudio Exp $ */ /* $NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $ */ /* @@ -1361,7 +1361,16 @@ rt_ifa_purge_walker(struct rtentry *rt, void *vifa, unsigned int rtableid) */ struct mutex rttimer_mtx; -LIST_HEAD(, rttimer_queue) rttimer_queue_head; /* [T] */ + +struct rttimer { + TAILQ_ENTRY(rttimer) rtt_next; /* [T] entry on timer queue */ + LIST_ENTRY(rttimer) rtt_link; /* [T] timers per rtentry */ + struct timeout rtt_timeout; /* [I] timeout for this entry */ + struct rttimer_queue *rtt_queue; /* [I] back pointer to queue */ + struct rtentry *rtt_rt; /* [T] back pointer to route */ + time_t rtt_expire; /* [I] rt expire time */ + u_int rtt_tableid; /* [I] rtable id of rtt_rt */ +}; #define RTTIMER_CALLOUT(r) { \ if (r->rtt_queue->rtq_func != NULL) { \ @@ -1388,15 +1397,9 @@ LIST_HEAD(, rttimer_queue) rttimer_queue_head; /* [T] */ void rt_timer_init(void) { - static struct timeout rt_timer_timeout; - pool_init(&rttimer_pool, sizeof(struct rttimer), 0, IPL_MPFLOOR, 0, "rttmr", NULL); - mtx_init(&rttimer_mtx, IPL_MPFLOOR); - LIST_INIT(&rttimer_queue_head); - timeout_set_proc(&rt_timer_timeout, rt_timer_timer, &rt_timer_timeout); - timeout_add_sec(&rt_timer_timeout, 1); } void @@ -1407,10 +1410,6 @@ rt_timer_queue_init(struct rttimer_queue *rtq, int timeout, rtq->rtq_count = 0; rtq->rtq_func = func; TAILQ_INIT(&rtq->rtq_head); - - mtx_enter(&rttimer_mtx); - LIST_INSERT_HEAD(&rttimer_queue_head, rtq, rtq_link); - mtx_leave(&rttimer_mtx); } void @@ -1453,6 +1452,25 @@ rt_timer_queue_count(struct rttimer_queue *rtq) return (rtq->rtq_count); } +static inline struct rttimer * +rt_timer_unlink(struct rttimer *r) +{ + MUTEX_ASSERT_LOCKED(&rttimer_mtx); + + LIST_REMOVE(r, rtt_link); + r->rtt_rt = NULL; + + if (timeout_del(&r->rtt_timeout) == 0) { + /* timeout fired, so rt_timer_timer will do the cleanup */ + return NULL; + } + + TAILQ_REMOVE(&r->rtt_queue->rtq_head, r, rtt_next); + KASSERT(r->rtt_queue->rtq_count > 0); + r->rtt_queue->rtq_count--; + return r; +} + void rt_timer_remove_all(struct rtentry *rt) { @@ -1462,11 +1480,9 @@ rt_timer_remove_all(struct rtentry *rt) TAILQ_INIT(&rttlist); mtx_enter(&rttimer_mtx); while ((r = LIST_FIRST(&rt->rt_timer)) != NULL) { - LIST_REMOVE(r, rtt_link); - TAILQ_REMOVE(&r->rtt_queue->rtq_head, r, rtt_next); - TAILQ_INSERT_TAIL(&rttlist, r, rtt_next); - KASSERT(r->rtt_queue->rtq_count > 0); - r->rtt_queue->rtq_count--; + r = rt_timer_unlink(r); + if (r != NULL) + TAILQ_INSERT_TAIL(&rttlist, r, rtt_next); } mtx_leave(&rttimer_mtx); @@ -1476,41 +1492,52 @@ rt_timer_remove_all(struct rtentry *rt) } } +time_t +rt_timer_get_expire(const struct rtentry *rt) +{ + const struct rttimer *r; + time_t expire = 0; + + mtx_enter(&rttimer_mtx); + LIST_FOREACH(r, &rt->rt_timer, rtt_link) { + if (expire == 0 || expire > r->rtt_expire) + expire = r->rtt_expire; + } + mtx_leave(&rttimer_mtx); + + return expire; +} + int rt_timer_add(struct rtentry *rt, struct rttimer_queue *queue, u_int rtableid) { struct rttimer *r, *rnew; - time_t current_time; rnew = pool_get(&rttimer_pool, PR_NOWAIT | PR_ZERO); if (rnew == NULL) return (ENOBUFS); - current_time = getuptime(); - rnew->rtt_rt = rt; - rnew->rtt_time = current_time; rnew->rtt_queue = queue; rnew->rtt_tableid = rtableid; + rnew->rtt_expire = getuptime() + queue->rtq_timeout; + timeout_set_proc(&rnew->rtt_timeout, rt_timer_timer, rnew); mtx_enter(&rttimer_mtx); - rt->rt_expire = current_time + queue->rtq_timeout; /* * If there's already a timer with this action, destroy it before * we add a new one. */ LIST_FOREACH(r, &rt->rt_timer, rtt_link) { if (r->rtt_queue == queue) { - LIST_REMOVE(r, rtt_link); - TAILQ_REMOVE(&r->rtt_queue->rtq_head, r, rtt_next); - KASSERT(r->rtt_queue->rtq_count > 0); - r->rtt_queue->rtq_count--; + r = rt_timer_unlink(r); break; /* only one per list, so we can quit... */ } } LIST_INSERT_HEAD(&rt->rt_timer, rnew, rtt_link); TAILQ_INSERT_TAIL(&queue->rtq_head, rnew, rtt_next); + timeout_add_sec(&rnew->rtt_timeout, queue->rtq_timeout); rnew->rtt_queue->rtq_count++; mtx_leave(&rttimer_mtx); @@ -1523,37 +1550,25 @@ rt_timer_add(struct rtentry *rt, struct rttimer_queue *queue, u_int rtableid) void rt_timer_timer(void *arg) { - struct timeout *to = (struct timeout *)arg; - struct rttimer_queue *rtq; - struct rttimer *r; - TAILQ_HEAD(, rttimer) rttlist; - time_t current_time; - - current_time = getuptime(); - TAILQ_INIT(&rttlist); + struct rttimer *r = arg; + struct rttimer_queue *rtq = r->rtt_queue; NET_LOCK(); mtx_enter(&rttimer_mtx); - LIST_FOREACH(rtq, &rttimer_queue_head, rtq_link) { - while ((r = TAILQ_FIRST(&rtq->rtq_head)) != NULL && - (r->rtt_time + rtq->rtq_timeout) < current_time) { - LIST_REMOVE(r, rtt_link); - TAILQ_REMOVE(&rtq->rtq_head, r, rtt_next); - TAILQ_INSERT_TAIL(&rttlist, r, rtt_next); - KASSERT(rtq->rtq_count > 0); - rtq->rtq_count--; - } - } + + if (r->rtt_rt != NULL) + LIST_REMOVE(r, rtt_link); + TAILQ_REMOVE(&rtq->rtq_head, r, rtt_next); + KASSERT(rtq->rtq_count > 0); + rtq->rtq_count--; + mtx_leave(&rttimer_mtx); - while ((r = TAILQ_FIRST(&rttlist)) != NULL) { - TAILQ_REMOVE(&rttlist, r, rtt_next); + if (r->rtt_rt != NULL) RTTIMER_CALLOUT(r); - pool_put(&rttimer_pool, r); - } NET_UNLOCK(); - timeout_add_sec(to, 1); + pool_put(&rttimer_pool, r); } #ifdef MPLS diff --git a/sys/net/route.h b/sys/net/route.h index 0ad17bf7ec6..5692206e606 100644 --- a/sys/net/route.h +++ b/sys/net/route.h @@ -1,4 +1,4 @@ -/* $OpenBSD: route.h,v 1.194 2022/05/05 13:57:40 claudio Exp $ */ +/* $OpenBSD: route.h,v 1.195 2022/06/27 21:26:46 claudio Exp $ */ /* $NetBSD: route.h,v 1.9 1996/02/13 22:00:49 christos Exp $ */ /* @@ -92,6 +92,8 @@ struct rt_metrics { #include #include +struct rttimer; + /* * We distinguish between routes to hosts and routes to networks, * preferring the former if available. For each route we infer @@ -405,15 +407,6 @@ rtstat_inc(enum rtstat_counters c) * add,timer} functions all used with the kind permission of BSDI. * These allow functions to be called for routes at specific times. */ -struct rttimer { - TAILQ_ENTRY(rttimer) rtt_next; /* [T] entry on timer queue */ - LIST_ENTRY(rttimer) rtt_link; /* [T] timers per rtentry */ - struct rttimer_queue *rtt_queue; /* [T] back pointer to queue */ - struct rtentry *rtt_rt; /* [I] back pointer to route */ - time_t rtt_time; /* [I] when timer registered */ - u_int rtt_tableid; /* [I] rtable id of rtt_rt */ -}; - struct rttimer_queue { TAILQ_HEAD(, rttimer) rtq_head; /* [T] */ LIST_ENTRY(rttimer_queue) rtq_link; /* [T] */ @@ -461,6 +454,7 @@ void rt_timer_init(void); int rt_timer_add(struct rtentry *, struct rttimer_queue *, u_int); void rt_timer_remove_all(struct rtentry *); +time_t rt_timer_get_expire(const struct rtentry *); void rt_timer_queue_init(struct rttimer_queue *, int, void(*)(struct rtentry *, u_int)); void rt_timer_queue_change(struct rttimer_queue *, int); diff --git a/sys/net/rtsock.c b/sys/net/rtsock.c index 31fd8f8322b..7b4e544fced 100644 --- a/sys/net/rtsock.c +++ b/sys/net/rtsock.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rtsock.c,v 1.332 2022/06/27 17:15:35 bluhm Exp $ */ +/* $OpenBSD: rtsock.c,v 1.333 2022/06/27 21:26:46 claudio Exp $ */ /* $NetBSD: rtsock.c,v 1.18 1996/03/29 00:32:10 cgd Exp $ */ /* @@ -132,7 +132,7 @@ int rtm_xaddrs(caddr_t, caddr_t, struct rt_addrinfo *); int rtm_validate_proposal(struct rt_addrinfo *); void rtm_setmetrics(u_long, const struct rt_metrics *, struct rt_kmetrics *); -void rtm_getmetrics(const struct rt_kmetrics *, +void rtm_getmetrics(const struct rtentry *, struct rt_metrics *); int sysctl_iflist(int, struct walkarg *); @@ -674,7 +674,7 @@ rtm_report(struct rtentry *rt, u_char type, int seq, int tableid) rtm->rtm_flags = rt->rt_flags; rtm->rtm_pid = curproc->p_p->ps_pid; rtm->rtm_seq = seq; - rtm_getmetrics(&rt->rt_rmx, &rtm->rtm_rmx); + rtm_getmetrics(rt, &rtm->rtm_rmx); rtm->rtm_addrs = info.rti_addrs; #ifdef MPLS rtm->rtm_mpls = info.rti_mpls; @@ -1391,11 +1391,14 @@ rtm_setmetrics(u_long which, const struct rt_metrics *in, } void -rtm_getmetrics(const struct rt_kmetrics *in, struct rt_metrics *out) +rtm_getmetrics(const struct rtentry *rt, struct rt_metrics *out) { + const struct rt_kmetrics *in = &rt->rt_rmx; int64_t expire; expire = in->rmx_expire; + if (expire == 0) + expire = rt_timer_get_expire(rt); if (expire != 0) { expire -= getuptime(); expire += gettime(); @@ -1998,7 +2001,7 @@ sysctl_dumpentry(struct rtentry *rt, void *v, unsigned int id) rtm->rtm_pid = curproc->p_p->ps_pid; rtm->rtm_flags = RTF_DONE | rt->rt_flags; rtm->rtm_priority = rt->rt_priority & RTP_MASK; - rtm_getmetrics(&rt->rt_rmx, &rtm->rtm_rmx); + rtm_getmetrics(rt, &rtm->rtm_rmx); /* Do not account the routing table's reference. */ rtm->rtm_rmx.rmx_refcnt = rt->rt_refcnt - 1; rtm->rtm_index = rt->rt_ifidx;