bfdd: simplify timer data structure

Remove some legacy left overs of the old timer data structure bits and
use a simpler version:

  We always keep the current configuration in the timer structure, but
  also keep the running timers (before poll transition) in
  `cur_timers`.

  With this we can remove `new_timers` and avoid timer copy
  configuration copy on final handler (this also simplifies peer
  show command).

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

bfdd/bfd.c

@@ -363,12 +363,29 @@ static struct bfd_session *bfd_session_new(int sd)
 
 	QOBJ_REG(bs, bfd_session);
 
-	bs->up_min_tx = BFD_DEFDESIREDMINTX;
+	bs->timers.desired_min_tx = BFD_DEFDESIREDMINTX;
 	bs->timers.required_min_rx = BFD_DEFREQUIREDMINRX;
 	bs->timers.required_min_echo = BFD_DEF_REQ_MIN_ECHO;
 	bs->detect_mult = BFD_DEFDETECTMULT;
 	bs->mh_ttl = BFD_DEF_MHOP_TTL;
 
+	/*
+	 * BFD connection startup must use slow timer.
+	 *
+	 * RFC 5880, Section 6.8.3.
+	 */
+	bs->cur_timers.desired_min_tx = BFD_DEF_SLOWTX;
+	bs->cur_timers.required_min_rx = BFD_DEF_SLOWTX;
+	bs->cur_timers.required_min_echo = 0;
+
+	/* Set the appropriated timeouts for slow connection. */
+	bs->detect_TO = (BFD_DEFDETECTMULT * BFD_DEF_SLOWTX);
+	bs->xmt_TO = BFD_DEF_SLOWTX;
+
+	/* Initiate remote settings as well. */
+	bs->remote_timers = bs->cur_timers;
+	bs->remote_detect_mult = BFD_DEFDETECTMULT;
+
 	bs->sock = sd;
 	monotime(&bs->uptime);
 	bs->downtime = bs->uptime;
@@ -432,7 +449,7 @@ static void _bfd_session_update(struct bfd_session *bs,
 
 skip_echo:
 	if (bpc->bpc_has_txinterval)
-		bs->up_min_tx = bpc->bpc_txinterval * 1000;
+		bs->timers.desired_min_tx = bpc->bpc_txinterval * 1000;
 
 	if (bpc->bpc_has_recvinterval)
 		bs->timers.required_min_rx = bpc->bpc_recvinterval * 1000;
@@ -599,12 +616,10 @@ struct bfd_session *ptm_bfd_sess_new(struct bfd_peer_cfg *bpc)
 	bfd->discrs.remote_discr = 0;
 	bfd->local_ip = bpc->bpc_local;
 	bfd->local_address = bpc->bpc_local;
-	bfd->timers.desired_min_tx = bfd->up_min_tx;
-	bfd->detect_TO = (bfd->detect_mult * BFD_DEF_SLOWTX);
-
-	/* Use detect_TO first for slow detection, then use recvtimer_update. */
 	bfd_recvtimer_update(bfd);
+	ptm_bfd_start_xmt_timer(bfd, false);
 
+	/* Registrate session into data structures. */
 	bfd_id_insert(bfd);
 
 	if (bpc->bpc_mhop) {
@@ -625,17 +640,8 @@ struct bfd_session *ptm_bfd_sess_new(struct bfd_peer_cfg *bpc)
 		bfd_shop_insert(bfd);
 	}
 
-	/*
-	 * XXX: session update triggers echo start, so we must have our
-	 * discriminator ID set first.
-	 */
 	_bfd_session_update(bfd, bpc);
 
-	/* Start transmitting with slow interval until peer responds */
-	bfd->xmt_TO = BFD_DEF_SLOWTX;
-
-	ptm_bfd_xmt_TO(bfd, 0);
-
 	log_info("session-new: %s", bs_to_string(bfd));
 
 	control_notify_config(BCM_NOTIFY_CONFIG_ADD, bfd);
@@ -680,8 +686,6 @@ void bfd_set_polling(struct bfd_session *bs)
 	 *
 	 * RFC 5880, Section 6.8.3.
 	 */
-	bs->new_timers.desired_min_tx = bs->up_min_tx;
-	bs->new_timers.required_min_rx = bs->timers.required_min_rx;
 	bs->polling = 1;
 }
 
@@ -864,10 +868,8 @@ void bs_echo_timer_handler(struct bfd_session *bs)
 void bs_final_handler(struct bfd_session *bs)
 {
 	/* Start using our new timers. */
-	bs->timers.desired_min_tx = bs->new_timers.desired_min_tx;
+	bs->cur_timers.desired_min_tx = bs->timers.desired_min_tx;
-	bs->timers.required_min_rx = bs->new_timers.required_min_rx;
+	bs->cur_timers.required_min_rx = bs->timers.required_min_rx;
-	bs->new_timers.desired_min_tx = 0;
-	bs->new_timers.required_min_rx = 0;
 
 	/*
 	 * TODO: demand mode. See RFC 5880 Section 6.1.

bfdd/bfd.h

@@ -214,8 +214,7 @@ struct bfd_session {
 
 	/* Timers */
 	struct bfd_timers timers;
-	struct bfd_timers new_timers;
+	struct bfd_timers cur_timers;
-	uint32_t up_min_tx;
 	uint64_t detect_TO;
 	struct thread *echo_recvtimer_ev;
 	struct thread *recvtimer_ev;

bfdd/bfd_packet.c

@@ -229,12 +229,20 @@ void ptm_bfd_snd(struct bfd_session *bfd, int fbit)
 	cp.discrs.remote_discr = htonl(bfd->discrs.remote_discr);
 	if (bfd->polling) {
 		cp.timers.desired_min_tx =
-			htonl(bfd->new_timers.desired_min_tx);
+			htonl(bfd->timers.desired_min_tx);
 		cp.timers.required_min_rx =
-			htonl(bfd->new_timers.required_min_rx);
+			htonl(bfd->timers.required_min_rx);
 	} else {
-		cp.timers.desired_min_tx = htonl(bfd->timers.desired_min_tx);
+		/*
-		cp.timers.required_min_rx = htonl(bfd->timers.required_min_rx);
+		 * We can only announce current setting on poll, this
+		 * avoids timing mismatch with our peer and give it
+		 * the oportunity to learn. See `bs_final_handler` for
+		 * more information.
+		 */
+		cp.timers.desired_min_tx =
+			htonl(bfd->cur_timers.desired_min_tx);
+		cp.timers.required_min_rx =
+			htonl(bfd->cur_timers.required_min_rx);
 	}
 	cp.timers.required_min_echo = htonl(bfd->timers.required_min_echo);
 

bfdd/bfdd_vty.c

@@ -200,10 +200,10 @@ DEFPY(bfd_peer_txinterval, bfd_peer_txinterval_cmd,
 	struct bfd_session *bs;
 
 	bs = VTY_GET_CONTEXT(bfd_session);
-	if (bs->up_min_tx == (uint32_t)(interval * 1000))
+	if (bs->timers.desired_min_tx == (uint32_t)(interval * 1000))
 		return CMD_SUCCESS;
 
-	bs->up_min_tx = interval * 1000;
+	bs->timers.desired_min_tx = interval * 1000;
 	bfd_set_polling(bs);
 
 	return CMD_SUCCESS;
@@ -430,20 +430,10 @@ static void _display_peer(struct vty *vty, struct bfd_session *bs)
 	vty_out(vty, "\t\tLocal timers:\n");
 	vty_out(vty, "\t\t\tReceive interval: %" PRIu32 "ms\n",
 		bs->timers.required_min_rx / 1000);
-	vty_out(vty, "\t\t\tTransmission interval: %" PRIu32 "ms",
+	vty_out(vty, "\t\t\tTransmission interval: %" PRIu32 "ms\n",
 		bs->timers.desired_min_tx / 1000);
-	if (bs->up_min_tx != bs->timers.desired_min_tx)
+	vty_out(vty, "\t\t\tEcho transmission interval: %" PRIu32 "ms\n",
-		vty_out(vty, " (configured %" PRIu32 "ms)\n",
+		bs->timers.required_min_echo / 1000);
-			bs->up_min_tx / 1000);
-	else
-		vty_out(vty, "\n");
-
-	vty_out(vty, "\t\t\tEcho transmission interval: ");
-	if (BFD_CHECK_FLAG(bs->flags, BFD_SESS_FLAG_ECHO))
-		vty_out(vty, "%" PRIu32 "ms\n",
-			bs->timers.required_min_echo / 1000);
-	else
-		vty_out(vty, "disabled\n");
 
 	vty_out(vty, "\t\tRemote timers:\n");
 	vty_out(vty, "\t\t\tReceive interval: %" PRIu32 "ms\n",
@@ -948,9 +938,9 @@ static void _bfdd_peer_write_config(struct hash_backet *hb, void *arg)
 	if (bs->timers.required_min_rx != (BPC_DEF_RECEIVEINTERVAL * 1000))
 		vty_out(vty, "  receive-interval %" PRIu32 "\n",
 			bs->timers.required_min_rx / 1000);
-	if (bs->up_min_tx != (BPC_DEF_TRANSMITINTERVAL * 1000))
+	if (bs->timers.desired_min_tx != (BPC_DEF_TRANSMITINTERVAL * 1000))
 		vty_out(vty, "  transmit-interval %" PRIu32 "\n",
-			bs->up_min_tx / 1000);
+			bs->timers.desired_min_tx / 1000);
 	if (bs->timers.required_min_echo != (BPC_DEF_ECHOINTERVAL * 1000))
 		vty_out(vty, "  echo-interval %" PRIu32 "\n",
 			bs->timers.required_min_echo / 1000);

bfdd/config.c

@@ -471,7 +471,8 @@ char *config_notify_config(const char *op, struct bfd_session *bs)
 	json_object_int_add(resp, "detect-multiplier", bs->detect_mult);
 	json_object_int_add(resp, "receive-interval",
 			    bs->timers.required_min_rx / 1000);
-	json_object_int_add(resp, "transmit-interval", bs->up_min_tx / 1000);
+	json_object_int_add(resp, "transmit-interval",
+			    bs->timers.desired_min_tx / 1000);
 	json_object_int_add(resp, "echo-interval",
 			    bs->timers.required_min_echo / 1000);