bfdd: refactor timer handling

Move timer calculation code outside of the packet handling function
and explain how timers are calculated.

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

diff --git a/bfdd/bfd.c b/bfdd/bfd.c
index 235f9c06a8f242c49a6d49e84406644a49c87271..18c56196e8a8fea29306757cf72f708313ae1d4c 100644 (file)
--- a/bfdd/bfd.c
+++ b/bfdd/bfd.c
@@ -172,17 +172,13 @@ void ptm_bfd_ses_up(struct bfd_session *bfd)
 
        bfd->local_diag = 0;
        bfd->ses_state = PTM_BFD_UP;
-       bfd->polling = 1;
        monotime(&bfd->uptime);
 
-       /* If the peer is capable to receiving Echo pkts */
-       if (bfd->echo_xmt_TO && !BFD_CHECK_FLAG(bfd->flags, BFD_SESS_FLAG_MH)) {
-               ptm_bfd_echo_start(bfd);
-       } else {
-               bfd->new_timers.desired_min_tx = bfd->up_min_tx;
-               bfd->new_timers.required_min_rx = bfd->timers.required_min_rx;
-               ptm_bfd_snd(bfd, 0);
-       }
+       /* Connection is up, lets negotiate timers. */
+       bfd_set_polling(bfd);
+
+       /* Start sending control packets with poll bit immediately. */
+       ptm_bfd_snd(bfd, 0);
 
        control_notify(bfd);
 
@@ -688,9 +684,17 @@ int ptm_bfd_ses_del(struct bfd_peer_cfg *bpc)
 
 void bfd_set_polling(struct bfd_session *bs)
 {
+       /*
+        * Start polling procedure: the only timers that require polling
+        * to change value without losing connection are:
+        *
+        *   - Desired minimum transmission interval;
+        *   - Required minimum receive interval;
+        *
+        * 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->new_timers.required_min_echo = bs->timers.required_min_echo;
        bs->polling = 1;
 }
 
@@ -817,6 +821,123 @@ void bs_state_handler(struct bfd_session *bs, int nstate)
        }
 }
 
+/*
+ * Handles echo timer manipulation after updating timer.
+ */
+void bs_echo_timer_handler(struct bfd_session *bs)
+{
+       uint32_t old_timer;
+
+       /*
+        * Before doing any echo handling, check if it is possible to
+        * use it.
+        *
+        *   - Check for `echo-mode` configuration.
+        *   - Check that we are not using multi hop (RFC 5883,
+        *     Section 3).
+        *   - Check that we are already at the up state.
+        */
+       if (BFD_CHECK_FLAG(bs->flags, BFD_SESS_FLAG_ECHO) == 0
+           || BFD_CHECK_FLAG(bs->flags, BFD_SESS_FLAG_MH)
+           || bs->ses_state != PTM_BFD_UP)
+               return;
+
+       /* Remote peer asked to stop echo. */
+       if (bs->remote_timers.required_min_echo == 0) {
+               if (BFD_CHECK_FLAG(bs->flags, BFD_SESS_FLAG_ECHO_ACTIVE))
+                       ptm_bfd_echo_stop(bs, 0);
+
+               return;
+       }
+
+       /*
+        * Calculate the echo transmission timer: we must not send
+        * echo packets faster than the minimum required time
+        * announced by the remote system.
+        *
+        * RFC 5880, Section 6.8.9.
+        */
+       old_timer = bs->echo_xmt_TO;
+       if (bs->remote_timers.required_min_echo > bs->timers.required_min_echo)
+               bs->echo_xmt_TO = bs->remote_timers.required_min_echo;
+       else
+               bs->echo_xmt_TO = bs->timers.required_min_echo;
+
+       if (BFD_CHECK_FLAG(bs->flags, BFD_SESS_FLAG_ECHO_ACTIVE) == 0
+           || old_timer != bs->echo_xmt_TO)
+               ptm_bfd_echo_start(bs);
+}
+
+/*
+ * RFC 5880 Section 6.5.
+ *
+ * When a BFD control packet with the final bit is received, we must
+ * update the session parameters.
+ */
+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->timers.required_min_rx = bs->new_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.
+        *
+        * When using demand mode we must disable the detection timer
+        * for lost control packets.
+        */
+       if (bs->demand_mode) {
+               /* Notify watchers about changed timers. */
+               control_notify_config(BCM_NOTIFY_CONFIG_UPDATE, bs);
+               return;
+       }
+
+       /*
+        * Calculate detection time based on new timers.
+        *
+        * Transmission calculation:
+        * We must respect the RequiredMinRxInterval from the remote
+        * system: if our desired transmission timer is more than the
+        * minimum receive rate, then we must lower it to at least the
+        * minimum receive interval.
+        *
+        * RFC 5880, Section 6.8.3.
+        */
+       if (bs->timers.desired_min_tx > bs->remote_timers.required_min_rx)
+               bs->xmt_TO = bs->remote_timers.required_min_rx;
+       else
+               bs->xmt_TO = bs->timers.desired_min_tx;
+
+       /* Apply new transmission timer immediately. */
+       ptm_bfd_start_xmt_timer(bs, false);
+
+       /*
+        * Detection timeout calculation:
+        * The minimum detection timeout is the remote detection
+        * multipler (number of packets to be missed) times the agreed
+        * transmission interval.
+        *
+        * RFC 5880, Section 6.8.4.
+        *
+        * TODO: support sending/counting more packets inside detection
+        * timeout.
+        */
+       if (bs->remote_timers.required_min_rx > bs->timers.desired_min_tx)
+               bs->detect_TO = bs->remote_detect_mult
+                               * bs->remote_timers.required_min_rx;
+       else
+               bs->detect_TO = bs->remote_detect_mult
+                               * bs->timers.desired_min_tx;
+
+       /* Apply new receive timer immediately. */
+       bfd_recvtimer_update(bs);
+
+       /* Notify watchers about changed timers. */
+       control_notify_config(BCM_NOTIFY_CONFIG_UPDATE, bs);
+}
+
 
 /*
  * Helper functions.

diff --git a/bfdd/bfd.h b/bfdd/bfd.h
index dccc3dfcc68409a5b2df4cba4a3dba82c4f120dc..098db11f26ecc89f644066e6d828a5ff35b389f5 100644 (file)
--- a/bfdd/bfd.h
+++ b/bfdd/bfd.h
@@ -526,6 +526,8 @@ struct bfd_session *bs_peer_find(struct bfd_peer_cfg *bpc);
 int bfd_session_update_label(struct bfd_session *bs, const char *nlabel);
 void bfd_set_polling(struct bfd_session *bs);
 void bs_state_handler(struct bfd_session *, int);
+void bs_echo_timer_handler(struct bfd_session *);
+void bs_final_handler(struct bfd_session *);
 const char *satostr(struct sockaddr_any *sa);
 const char *diag2str(uint8_t diag);
 int strtosa(const char *addr, struct sockaddr_any *sa);

diff --git a/bfdd/bfd_packet.c b/bfdd/bfd_packet.c
index ef97e3462301e3eb4b661f17757739840800e29d..1a72800757dfcfeadf500bca7a59205b3c632292 100644 (file)
--- a/bfdd/bfd_packet.c
+++ b/bfdd/bfd_packet.c
@@ -523,7 +523,6 @@ int bfd_recv_cb(struct thread *t)
        struct bfd_pkt *cp;
        bool is_mhop;
        ssize_t mlen = 0;
-       uint32_t oldEchoXmt_TO, oldXmtTime;
        uint8_t ttl;
        struct sockaddr_any local, peer;
        char port[MAXNAMELEN + 1], vrfname[MAXNAMELEN + 1];
@@ -644,106 +643,42 @@ int bfd_recv_cb(struct thread *t)
 
        bfd->discrs.remote_discr = ntohl(cp->discrs.my_discr);
 
-       /* If received the Final bit, the new values should take effect */
-       if (bfd->polling && BFD_GETFBIT(cp->flags)) {
-               bfd->timers.desired_min_tx = bfd->new_timers.desired_min_tx;
-               bfd->timers.required_min_rx = bfd->new_timers.required_min_rx;
-               bfd->new_timers.desired_min_tx = 0;
-               bfd->new_timers.required_min_rx = 0;
-               bfd->polling = 0;
-       }
-
-       if (!bfd->demand_mode) {
-               /* Compute detect time */
-               bfd->detect_TO = cp->detect_mult
-                                * ((bfd->timers.required_min_rx
-                                    > ntohl(cp->timers.desired_min_tx))
-                                           ? bfd->timers.required_min_rx
-                                           : ntohl(cp->timers.desired_min_tx));
-               bfd->remote_detect_mult = cp->detect_mult;
-       } else
-               cp_debug(is_mhop, &peer, &local, port, vrfname,
-                        "unsupported demand mode");
-
        /* Save remote diagnostics before state switch. */
        bfd->remote_diag = cp->diag & BFD_DIAGMASK;
 
+       /* Update remote timers settings. */
+       bfd->remote_timers.desired_min_tx = ntohl(cp->timers.desired_min_tx);
+       bfd->remote_timers.required_min_rx = ntohl(cp->timers.required_min_rx);
+       bfd->remote_timers.required_min_echo =
+               ntohl(cp->timers.required_min_echo);
+       bfd->remote_detect_mult = cp->detect_mult;
+
        /* State switch from section 6.2. */
        bs_state_handler(bfd, BFD_GETSTATE(cp->flags));
 
-       /*
-        * Handle echo packet status:
-        * - Start echo packets if configured and permitted
-        *   (required_min_echo > 0);
-        * - Stop echo packets if not allowed (required_min_echo == 0);
-        * - Recalculate echo packet interval;
-        */
-       if (BFD_CHECK_FLAG(bfd->flags, BFD_SESS_FLAG_ECHO)) {
-               if (BFD_CHECK_FLAG(bfd->flags, BFD_SESS_FLAG_ECHO_ACTIVE)) {
-                       if (!ntohl(cp->timers.required_min_echo)) {
-                               ptm_bfd_echo_stop(bfd, 1);
-                       } else {
-                               oldEchoXmt_TO = bfd->echo_xmt_TO;
-                               bfd->echo_xmt_TO =
-                                       bfd->timers.required_min_echo;
-                               if (ntohl(cp->timers.required_min_echo)
-                                   > bfd->echo_xmt_TO)
-                                       bfd->echo_xmt_TO = ntohl(
-                                               cp->timers.required_min_echo);
-                               if (oldEchoXmt_TO != bfd->echo_xmt_TO)
-                                       ptm_bfd_echo_start(bfd);
-                       }
-               } else if (ntohl(cp->timers.required_min_echo)) {
-                       bfd->echo_xmt_TO = bfd->timers.required_min_echo;
-                       if (ntohl(cp->timers.required_min_echo)
-                           > bfd->echo_xmt_TO)
-                               bfd->echo_xmt_TO =
-                                       ntohl(cp->timers.required_min_echo);
-                       ptm_bfd_echo_start(bfd);
-               }
-       }
-
-       if (BFD_CHECK_FLAG(bfd->flags, BFD_SESS_FLAG_ECHO_ACTIVE)) {
-               bfd->echo_xmt_TO = bfd->timers.required_min_echo;
-               if (ntohl(cp->timers.required_min_echo) > bfd->echo_xmt_TO)
-                       bfd->echo_xmt_TO = ntohl(cp->timers.required_min_echo);
-       }
-
-       /* Calculate new transmit time */
-       oldXmtTime = bfd->xmt_TO;
-       bfd->xmt_TO =
-               (bfd->timers.desired_min_tx > ntohl(cp->timers.required_min_rx))
-                       ? bfd->timers.desired_min_tx
-                       : ntohl(cp->timers.required_min_rx);
+       /* RFC 5880, Section 6.5: handle POLL/FINAL negotiation sequence. */
+       if (bfd->polling && BFD_GETFBIT(cp->flags)) {
+               /* Disable pooling. */
+               bfd->polling = 0;
 
-       /* If transmit time has changed, and too much time until next xmt,
-        * restart
-        */
-       if (BFD_GETPBIT(cp->flags)) {
-               ptm_bfd_xmt_TO(bfd, 1);
-       } else if (oldXmtTime != bfd->xmt_TO) {
-               /* XXX add some skid to this as well */
-               ptm_bfd_start_xmt_timer(bfd, false);
+               /* Handle poll finalization. */
+               bs_final_handler(bfd);
+       } else {
+               /* Received a packet, lets update the receive timer. */
+               bfd_recvtimer_update(bfd);
        }
 
-       /* Restart detection timer (packet received) */
-       if (!bfd->demand_mode)
-               bfd_recvtimer_update(bfd);
+       /* Handle echo timers changes. */
+       bs_echo_timer_handler(bfd);
 
        /*
-        * Save the timers and state sent by the remote end
-        * for debugging and statistics.
+        * We've received a packet with the POLL bit set, we must send
+        * a control packet back with the FINAL bit set.
+        *
+        * RFC 5880, Section 6.5.
         */
-       if (BFD_GETFBIT(cp->flags)) {
-               bfd->remote_timers.desired_min_tx =
-                       ntohl(cp->timers.desired_min_tx);
-               bfd->remote_timers.required_min_rx =
-                       ntohl(cp->timers.required_min_rx);
-               bfd->remote_timers.required_min_echo =
-                       ntohl(cp->timers.required_min_echo);
-
-               control_notify_config(BCM_NOTIFY_CONFIG_UPDATE, bfd);
-       }
+       if (BFD_GETPBIT(cp->flags))
+               ptm_bfd_snd(bfd, 1);
 
        return 0;
 }