Index | Thread | Search

From:
Florian Obser <florian@openbsd.org>
Subject:
Re: [PATCH 3/4] evtimer_del(): remove redundant pending checks in routing daemons
To:
Bjorn Ketelaars <bket@openbsd.org>
Cc:
tech@openbsd.org
Date:
Wed, 21 Oct 2026 21:21:44 +0200

Download raw body.

Thread
reads OK florian

On 2026-10-16 14:34 +02, Bjorn Ketelaars <bket@openbsd.org> wrote:
> While reading event(3), I noticed that it says event_del() has no effect
> if an event “has already executed or has never been added”. The manual
> describes evtimer_del() as an abbreviation for the same event API, so
> the evtimer_pending() checks immediately before timer deletion are
> unnecessary.
>
> This patch removes those checks from timer stop and cleanup paths in
> dvmrpd, eigrpd, ldpd, ospf6d, ospfd, and ripd.
>
> These are mechanical cleanups; timer scheduling and protocol behavior
> are unchanged.
>
> Comments/OK?
>
> Index: usr.sbin/dvmrpd/rde_mfc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dvmrpd/rde_mfc.c,v
> diff -u -p -r1.12 rde_mfc.c
> --- usr.sbin/dvmrpd/rde_mfc.c	15 Dec 2025 23:38:34 -0000	1.12
> +++ usr.sbin/dvmrpd/rde_mfc.c	16 Oct 2026 12:25:23 -0000
> @@ -473,9 +473,8 @@ mfc_delete_prune(struct mfc_node *mn, st
>  {
>  	unsigned int	ifindex = pn->ifindex;
>  
> -	if (evtimer_pending(&pn->lifetime_timer, NULL))
> -		if (evtimer_del(&pn->lifetime_timer) == -1)
> -			fatal("mfc_delete_prune");
> +	if (evtimer_del(&pn->lifetime_timer) == -1)
> +		fatal("mfc_delete_prune");
>  
>  	LIST_REMOVE(pn, entry);
>  	free(pn);
> Index: usr.sbin/dvmrpd/rde_srt.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dvmrpd/rde_srt.c,v
> diff -u -p -r1.27 rde_srt.c
> --- usr.sbin/dvmrpd/rde_srt.c	21 Sep 2018 01:33:55 -0000	1.27
> +++ usr.sbin/dvmrpd/rde_srt.c	16 Oct 2026 12:25:23 -0000
> @@ -91,9 +91,8 @@ rt_start_expire_timer(struct rt_node *rn
>  
>  	rn->old_cost = 0;
>  
> -	if (evtimer_pending(&rn->holddown_timer, NULL))
> -		if (evtimer_del(&rn->holddown_timer) == -1)
> -			fatal("rt_start_expire_timer");
> +	if (evtimer_del(&rn->holddown_timer) == -1)
> +		fatal("rt_start_expire_timer");
>  
>  	timerclear(&tv);
>  	tv.tv_sec = ROUTE_EXPIRATION_TIME;
> Index: usr.sbin/eigrpd/interface.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/eigrpd/interface.c,v
> diff -u -p -r1.26 interface.c
> --- usr.sbin/eigrpd/interface.c	8 Mar 2023 04:43:13 -0000	1.26
> +++ usr.sbin/eigrpd/interface.c	16 Oct 2026 12:25:23 -0000
> @@ -464,8 +464,7 @@ eigrp_if_start_hello_timer(struct eigrp_
>  static void
>  eigrp_if_stop_hello_timer(struct eigrp_iface *ei)
>  {
> -	if (evtimer_pending(&ei->hello_timer, NULL) &&
> -	    evtimer_del(&ei->hello_timer) == -1)
> +	if (evtimer_del(&ei->hello_timer) == -1)
>  		fatal("eigrp_if_stop_hello_timer");
>  }
>  
> Index: usr.sbin/eigrpd/neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/eigrpd/neighbor.c,v
> diff -u -p -r1.11 neighbor.c
> --- usr.sbin/eigrpd/neighbor.c	8 Mar 2023 04:43:13 -0000	1.11
> +++ usr.sbin/eigrpd/neighbor.c	16 Oct 2026 12:25:23 -0000
> @@ -258,7 +258,6 @@ nbr_start_timeout(struct nbr *nbr)
>  static void
>  nbr_stop_timeout(struct nbr *nbr)
>  {
> -	if (evtimer_pending(&nbr->ev_hello_timeout, NULL) &&
> -	    evtimer_del(&nbr->ev_hello_timeout) == -1)
> +	if (evtimer_del(&nbr->ev_hello_timeout) == -1)
>  		fatal("nbr_stop_timeout");
>  }
> Index: usr.sbin/eigrpd/rde_dual.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/eigrpd/rde_dual.c,v
> diff -u -p -r1.31 rde_dual.c
> --- usr.sbin/eigrpd/rde_dual.c	8 Mar 2023 04:43:13 -0000	1.31
> +++ usr.sbin/eigrpd/rde_dual.c	16 Oct 2026 12:25:23 -0000
> @@ -510,8 +510,7 @@ reply_active_start_timer(struct reply_no
>  static void
>  reply_active_stop_timer(struct reply_node *reply)
>  {
> -	if (evtimer_pending(&reply->ev_active_timeout, NULL) &&
> -	    evtimer_del(&reply->ev_active_timeout) == -1)
> +	if (evtimer_del(&reply->ev_active_timeout) == -1)
>  		fatal("reply_active_stop_timer");
>  }
>  
> @@ -575,8 +574,7 @@ reply_sia_start_timer(struct reply_node 
>  static void
>  reply_sia_stop_timer(struct reply_node *reply)
>  {
> -	if (evtimer_pending(&reply->ev_sia_timeout, NULL) &&
> -	    evtimer_del(&reply->ev_sia_timeout) == -1)
> +	if (evtimer_del(&reply->ev_sia_timeout) == -1)
>  		fatal("reply_sia_stop_timer");
>  }
>  
> Index: usr.sbin/eigrpd/rtp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/eigrpd/rtp.c,v
> diff -u -p -r1.8 rtp.c
> --- usr.sbin/eigrpd/rtp.c	8 Mar 2023 04:43:13 -0000	1.8
> +++ usr.sbin/eigrpd/rtp.c	16 Oct 2026 12:25:23 -0000
> @@ -280,8 +280,7 @@ rtp_retrans_start_timer(struct packet *p
>  static void
>  rtp_retrans_stop_timer(struct packet *pkt)
>  {
> -	if (evtimer_pending(&pkt->ev_timeout, NULL) &&
> -	    evtimer_del(&pkt->ev_timeout) == -1)
> +	if (evtimer_del(&pkt->ev_timeout) == -1)
>  		fatal("rtp_retrans_stop_timer");
>  }
>  
> @@ -307,7 +306,6 @@ rtp_ack_start_timer(struct nbr *nbr)
>  void
>  rtp_ack_stop_timer(struct nbr *nbr)
>  {
> -	if (evtimer_pending(&nbr->ev_ack, NULL) &&
> -	    evtimer_del(&nbr->ev_ack) == -1)
> +	if (evtimer_del(&nbr->ev_ack) == -1)
>  		fatal("rtp_ack_stop_timer");
>  }
> Index: usr.sbin/ldpd/adjacency.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/adjacency.c,v
> diff -u -p -r1.28 adjacency.c
> --- usr.sbin/ldpd/adjacency.c	8 Mar 2023 04:43:13 -0000	1.28
> +++ usr.sbin/ldpd/adjacency.c	16 Oct 2026 12:25:23 -0000
> @@ -190,8 +190,7 @@ adj_start_itimer(struct adj *adj)
>  void
>  adj_stop_itimer(struct adj *adj)
>  {
> -	if (evtimer_pending(&adj->inactivity_timer, NULL) &&
> -	    evtimer_del(&adj->inactivity_timer) == -1)
> +	if (evtimer_del(&adj->inactivity_timer) == -1)
>  		fatal(__func__);
>  }
>  
> @@ -318,8 +317,7 @@ tnbr_start_hello_timer(struct tnbr *tnbr
>  static void
>  tnbr_stop_hello_timer(struct tnbr *tnbr)
>  {
> -	if (evtimer_pending(&tnbr->hello_timer, NULL) &&
> -	    evtimer_del(&tnbr->hello_timer) == -1)
> +	if (evtimer_del(&tnbr->hello_timer) == -1)
>  		fatal(__func__);
>  }
>  
> Index: usr.sbin/ldpd/interface.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/interface.c,v
> diff -u -p -r1.52 interface.c
> --- usr.sbin/ldpd/interface.c	8 Mar 2023 04:43:13 -0000	1.52
> +++ usr.sbin/ldpd/interface.c	16 Oct 2026 12:25:23 -0000
> @@ -397,8 +397,7 @@ if_start_hello_timer(struct iface_af *ia
>  static void
>  if_stop_hello_timer(struct iface_af *ia)
>  {
> -	if (evtimer_pending(&ia->hello_timer, NULL) &&
> -	    evtimer_del(&ia->hello_timer) == -1)
> +	if (evtimer_del(&ia->hello_timer) == -1)
>  		fatal(__func__);
>  }
>  
> Index: usr.sbin/ldpd/lde_lib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/lde_lib.c,v
> diff -u -p -r1.71 lde_lib.c
> --- usr.sbin/ldpd/lde_lib.c	8 Mar 2023 04:43:13 -0000	1.71
> +++ usr.sbin/ldpd/lde_lib.c	16 Oct 2026 12:25:23 -0000
> @@ -925,7 +925,6 @@ lde_gc_start_timer(void)
>  void
>  lde_gc_stop_timer(void)
>  {
> -	if (evtimer_pending(&gc_timer, NULL) &&
> -	    evtimer_del(&gc_timer) == -1)
> +	if (evtimer_del(&gc_timer) == -1)
>  		fatal(__func__);
>  }
> Index: usr.sbin/ldpd/neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/neighbor.c,v
> diff -u -p -r1.81 neighbor.c
> --- usr.sbin/ldpd/neighbor.c	28 Jun 2019 13:32:48 -0000	1.81
> +++ usr.sbin/ldpd/neighbor.c	16 Oct 2026 12:25:23 -0000
> @@ -404,8 +404,7 @@ nbr_start_ktimer(struct nbr *nbr)
>  void
>  nbr_stop_ktimer(struct nbr *nbr)
>  {
> -	if (evtimer_pending(&nbr->keepalive_timer, NULL) &&
> -	    evtimer_del(&nbr->keepalive_timer) == -1)
> +	if (evtimer_del(&nbr->keepalive_timer) == -1)
>  		fatal(__func__);
>  }
>  
> @@ -436,8 +435,7 @@ nbr_start_ktimeout(struct nbr *nbr)
>  void
>  nbr_stop_ktimeout(struct nbr *nbr)
>  {
> -	if (evtimer_pending(&nbr->keepalive_timeout, NULL) &&
> -	    evtimer_del(&nbr->keepalive_timeout) == -1)
> +	if (evtimer_del(&nbr->keepalive_timeout) == -1)
>  		fatal(__func__);
>  }
>  
> @@ -467,8 +465,7 @@ nbr_start_itimeout(struct nbr *nbr)
>  void
>  nbr_stop_itimeout(struct nbr *nbr)
>  {
> -	if (evtimer_pending(&nbr->init_timeout, NULL) &&
> -	    evtimer_del(&nbr->init_timeout) == -1)
> +	if (evtimer_del(&nbr->init_timeout) == -1)
>  		fatal(__func__);
>  }
>  
> @@ -515,8 +512,7 @@ nbr_start_idtimer(struct nbr *nbr)
>  void
>  nbr_stop_idtimer(struct nbr *nbr)
>  {
> -	if (evtimer_pending(&nbr->initdelay_timer, NULL) &&
> -	    evtimer_del(&nbr->initdelay_timer) == -1)
> +	if (evtimer_del(&nbr->initdelay_timer) == -1)
>  		fatal(__func__);
>  }
>  
> Index: usr.sbin/ldpd/packet.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/packet.c,v
> diff -u -p -r1.77 packet.c
> --- usr.sbin/ldpd/packet.c	21 Nov 2024 13:29:28 -0000	1.77
> +++ usr.sbin/ldpd/packet.c	16 Oct 2026 12:25:23 -0000
> @@ -747,8 +747,7 @@ pending_conn_new(int fd, int af, union l
>  void
>  pending_conn_del(struct pending_conn *pconn)
>  {
> -	if (evtimer_pending(&pconn->ev_timeout, NULL) &&
> -	    evtimer_del(&pconn->ev_timeout) == -1)
> +	if (evtimer_del(&pconn->ev_timeout) == -1)
>  		fatal(__func__);
>  
>  	TAILQ_REMOVE(&global.pending_conns, pconn, entry);
> Index: usr.sbin/ospf6d/interface.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/interface.c,v
> diff -u -p -r1.31 interface.c
> --- usr.sbin/ospf6d/interface.c	22 Jul 2026 16:37:46 -0000	1.31
> +++ usr.sbin/ospf6d/interface.c	16 Oct 2026 12:25:23 -0000
> @@ -272,12 +272,9 @@ if_del(struct iface *iface)
>  	while ((nbr = LIST_FIRST(&iface->nbr_list)) != NULL)
>  		nbr_del(nbr);
>  
> -	if (evtimer_pending(&iface->hello_timer, NULL))
> -		evtimer_del(&iface->hello_timer);
> -	if (evtimer_pending(&iface->wait_timer, NULL))
> -		evtimer_del(&iface->wait_timer);
> -	if (evtimer_pending(&iface->lsack_tx_timer, NULL))
> -		evtimer_del(&iface->lsack_tx_timer);
> +	evtimer_del(&iface->hello_timer);
> +	evtimer_del(&iface->wait_timer);
> +	evtimer_del(&iface->lsack_tx_timer);
>  
>  	ls_ack_list_clr(iface);
>  	TAILQ_REMOVE(&iflist, iface, list);
> Index: usr.sbin/ospf6d/neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/neighbor.c,v
> diff -u -p -r1.19 neighbor.c
> --- usr.sbin/ospf6d/neighbor.c	8 Mar 2023 04:43:14 -0000	1.19
> +++ usr.sbin/ospf6d/neighbor.c	16 Oct 2026 12:25:23 -0000
> @@ -335,16 +335,11 @@ nbr_del(struct nbr *nbr)
>  {
>  	ospfe_imsg_compose_rde(IMSG_NEIGHBOR_DOWN, nbr->peerid, 0, NULL, 0);
>  
> -	if (evtimer_pending(&nbr->inactivity_timer, NULL))
> -		evtimer_del(&nbr->inactivity_timer);
> -	if (evtimer_pending(&nbr->db_tx_timer, NULL))
> -		evtimer_del(&nbr->db_tx_timer);
> -	if (evtimer_pending(&nbr->lsreq_tx_timer, NULL))
> -		evtimer_del(&nbr->lsreq_tx_timer);
> -	if (evtimer_pending(&nbr->ls_retrans_timer, NULL))
> -		evtimer_del(&nbr->ls_retrans_timer);
> -	if (evtimer_pending(&nbr->adj_timer, NULL))
> -		evtimer_del(&nbr->adj_timer);
> +	evtimer_del(&nbr->inactivity_timer);
> +	evtimer_del(&nbr->db_tx_timer);
> +	evtimer_del(&nbr->lsreq_tx_timer);
> +	evtimer_del(&nbr->ls_retrans_timer);
> +	evtimer_del(&nbr->adj_timer);
>  
>  	/* clear lists */
>  	ls_retrans_list_clr(nbr);
> Index: usr.sbin/ospfd/interface.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v
> diff -u -p -r1.88 interface.c
> --- usr.sbin/ospfd/interface.c	21 Aug 2024 15:18:00 -0000	1.88
> +++ usr.sbin/ospfd/interface.c	16 Oct 2026 12:25:23 -0000
> @@ -227,12 +227,9 @@ if_del(struct iface *iface)
>  	while ((nbr = LIST_FIRST(&iface->nbr_list)) != NULL)
>  		nbr_del(nbr);
>  
> -	if (evtimer_pending(&iface->hello_timer, NULL))
> -		evtimer_del(&iface->hello_timer);
> -	if (evtimer_pending(&iface->wait_timer, NULL))
> -		evtimer_del(&iface->wait_timer);
> -	if (evtimer_pending(&iface->lsack_tx_timer, NULL))
> -		evtimer_del(&iface->lsack_tx_timer);
> +	evtimer_del(&iface->hello_timer);
> +	evtimer_del(&iface->wait_timer);
> +	evtimer_del(&iface->lsack_tx_timer);
>  
>  	ls_ack_list_clr(iface);
>  	md_list_clr(&iface->auth_md_list);
> Index: usr.sbin/ospfd/neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/neighbor.c,v
> diff -u -p -r1.53 neighbor.c
> --- usr.sbin/ospfd/neighbor.c	31 Aug 2026 07:57:09 -0000	1.53
> +++ usr.sbin/ospfd/neighbor.c	16 Oct 2026 12:25:23 -0000
> @@ -330,16 +330,11 @@ nbr_del(struct nbr *nbr)
>  {
>  	ospfe_imsg_compose_rde(IMSG_NEIGHBOR_DOWN, nbr->peerid, 0, NULL, 0);
>  
> -	if (evtimer_pending(&nbr->inactivity_timer, NULL))
> -		evtimer_del(&nbr->inactivity_timer);
> -	if (evtimer_pending(&nbr->db_tx_timer, NULL))
> -		evtimer_del(&nbr->db_tx_timer);
> -	if (evtimer_pending(&nbr->lsreq_tx_timer, NULL))
> -		evtimer_del(&nbr->lsreq_tx_timer);
> -	if (evtimer_pending(&nbr->ls_retrans_timer, NULL))
> -		evtimer_del(&nbr->ls_retrans_timer);
> -	if (evtimer_pending(&nbr->adj_timer, NULL))
> -		evtimer_del(&nbr->adj_timer);
> +	evtimer_del(&nbr->inactivity_timer);
> +	evtimer_del(&nbr->db_tx_timer);
> +	evtimer_del(&nbr->lsreq_tx_timer);
> +	evtimer_del(&nbr->ls_retrans_timer);
> +	evtimer_del(&nbr->adj_timer);
>  
>  	/* clear lists */
>  	ls_retrans_list_clr(nbr);
> Index: usr.sbin/ripd/neighbor.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ripd/neighbor.c,v
> diff -u -p -r1.12 neighbor.c
> --- usr.sbin/ripd/neighbor.c	8 Mar 2023 04:43:14 -0000	1.12
> +++ usr.sbin/ripd/neighbor.c	16 Oct 2026 12:25:23 -0000
> @@ -311,9 +311,8 @@ nbr_failed_find(struct iface *iface, u_i
>  void
>  nbr_failed_delete(struct nbr_failed *nbr_failed)
>  {
> -	if (evtimer_pending(&nbr_failed->timeout_timer, NULL))
> -		if (evtimer_del(&nbr_failed->timeout_timer) == -1)
> -			fatal("nbr_failed_delete");
> +	if (evtimer_del(&nbr_failed->timeout_timer) == -1)
> +		fatal("nbr_failed_delete");
>  
>  	LIST_REMOVE(nbr_failed, entry);
>  	free(nbr_failed);
>

-- 
In my defence, I have been left unsupervised.