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>

diff --git a/bfdd/bfd.c b/bfdd/bfd.c
index a95f4b77b85d5333e60e492d6c642b16003f4173..40663b7f2ae1b5ac60687259dd69ee28ce75e7e7 100644 (file)
--- a/bfdd/bfd.c
+++ b/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->timers.required_min_rx = bs->new_timers.required_min_rx;
-       bs->new_timers.desired_min_tx = 0;
-       bs->new_timers.required_min_rx = 0;
+       bs->cur_timers.desired_min_tx = bs->timers.desired_min_tx;
+       bs->cur_timers.required_min_rx = bs->timers.required_min_rx;
 
        /*
         * TODO: demand mode. See RFC 5880 Section 6.1.

diff --git a/bfdd/bfd.h b/bfdd/bfd.h
index 66af6c79d81c1b7de10f1b2d73588f292f5e2b64..08204d673a640319e0f306ce5714b6de84cfb1b6 100644 (file)
--- a/bfdd/bfd.h
+++ b/bfdd/bfd.h
@@ -214,8 +214,7 @@ struct bfd_session {
 
        /* Timers */
        struct bfd_timers timers;
-       struct bfd_timers new_timers;
-       uint32_t up_min_tx;
+       struct bfd_timers cur_timers;
        uint64_t detect_TO;
        struct thread *echo_recvtimer_ev;
        struct thread *recvtimer_ev;

diff --git a/bfdd/bfd_packet.c b/bfdd/bfd_packet.c
index 1c55db39fc62184a1cc865622921b886ec117e5e..7a7a5a5d969e1db16f70c70d4295e19ad574306c 100644 (file)
--- a/bfdd/bfd_packet.c
+++ b/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);
 

diff --git a/bfdd/bfdd_vty.c b/bfdd/bfdd_vty.c
index eab860f18e617e8ed5cce4f0f2987f0276758afe..8ba8de9823d85f9f3f44fd6b7d5c0c195f50669e 100644 (file)
--- a/bfdd/bfdd_vty.c
+++ b/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, " (configured %" PRIu32 "ms)\n",
-                       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\t\tEcho transmission interval: %" PRIu32 "ms\n",
+               bs->timers.required_min_echo / 1000);
 
        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);

diff --git a/bfdd/config.c b/bfdd/config.c
index 06089780c6ba8c163d2e8db142f7076c5607cc02..921fa2b973a00f3f0ec0b6155c8ecf6d4f7c3f6c 100644 (file)
--- a/bfdd/config.c
+++ b/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);