bfdd: fix broken FSM in passive mode

Problem:
One is with active mode, the other is with passive mode. Sometimes
the one with active mode is in `Down` stauts, but the other one
with passive mode is unluckily stuck in `Init` status:
It doesn't answer its peer with any packets, even receiving continuous
`Down` packets.

Root Cause:
bfdd with passive mode answers its peer only *one* packet in `Down` status,
then it enters into `Init` status and ignores subsequent `Down` packets.
Unluckily that *one* answered packet is lost, at that moment its peer
with active mode can only have to send `Down` packets.

Fix:
1) With passive mode, bfdd should start xmittimer after received `Down` packet.
Refer to RFC5880:
"A system taking the Passive role MUST NOT begin sending BFD packets for
a particular session until it has received a BFD packet for that session, and
thus has learned the remote system's discriminator value."

2) Currently this added xmittimer for passive mode can be safely removed
except receiving `AdminDown` packet:
    - `bfd_session_enable/bfd_set_passive_mode` doesn't start xmittimer
    - `ptm_bfd_sess_dn/bfd_set_shutdown` can remove xmittimer
Per RFC5880, receiving `AdminDown` packet should be also regarded as `Down`,
so just call `ptm_bfd_sess_dn`, which will safely remove the added xmittimer
for passive mode. In summary, call `ptm_bfd_sess_dn` for two status changes
on receiving `AdminDown`: `Init`->`Down` and `Up`->`Down`.

Signed-off-by: anlan_cs <vic.lan@pica8.com>

diff --git a/bfdd/bfd.c b/bfdd/bfd.c
index c9a327490caa4aa24812d1e3f076353460072911..fbe0436b508133c03770f62983f6f43fae89982b 100644 (file)
--- a/bfdd/bfd.c
+++ b/bfdd/bfd.c
@@ -51,8 +51,6 @@ static void bs_admin_down_handler(struct bfd_session *bs, int nstate);
 static void bs_down_handler(struct bfd_session *bs, int nstate);
 static void bs_init_handler(struct bfd_session *bs, int nstate);
 static void bs_up_handler(struct bfd_session *bs, int nstate);
-static void bs_neighbour_admin_down_handler(struct bfd_session *bfd,
-                                           uint8_t diag);
 
 /**
  * Remove BFD profile from all BFD sessions so we don't leave dangling
@@ -997,9 +995,18 @@ static void bs_down_handler(struct bfd_session *bs, int nstate)
                 */
                bs->ses_state = PTM_BFD_INIT;
 
-               /* Answer peer with INIT immediately in passive mode. */
+               /*
+                * RFC 5880, Section 6.1.
+                * A system taking the Passive role MUST NOT begin
+                * sending BFD packets for a particular session until
+                * it has received a BFD packet for that session, and thus
+                * has learned the remote system's discriminator value.
+                *
+                * Now we can start transmission timer in passive mode.
+                */
                if (CHECK_FLAG(bs->flags, BFD_SESS_FLAG_PASSIVE))
-                       ptm_bfd_snd(bs, 0);
+                       ptm_bfd_xmt_TO(bs, 0);
+
                break;
 
        case PTM_BFD_INIT:
@@ -1026,7 +1033,7 @@ static void bs_init_handler(struct bfd_session *bs, int nstate)
                 * Remote peer doesn't want to talk, so lets make the
                 * connection down.
                 */
-               bs->ses_state = PTM_BFD_DOWN;
+               ptm_bfd_sess_dn(bs, BD_NEIGHBOR_DOWN);
                break;
 
        case PTM_BFD_DOWN:
@@ -1047,46 +1054,10 @@ static void bs_init_handler(struct bfd_session *bs, int nstate)
        }
 }
 
-static void bs_neighbour_admin_down_handler(struct bfd_session *bfd,
-                                           uint8_t diag)
-{
-       int old_state = bfd->ses_state;
-
-       bfd->local_diag = diag;
-       bfd->discrs.remote_discr = 0;
-       bfd->ses_state = PTM_BFD_DOWN;
-       bfd->polling = 0;
-       bfd->demand_mode = 0;
-       monotime(&bfd->downtime);
-
-       /* Slow down the control packets, the connection is down. */
-       bs_set_slow_timers(bfd);
-
-       /* only signal clients when going from up->down state */
-       if (old_state == PTM_BFD_UP)
-               control_notify(bfd, PTM_BFD_ADM_DOWN);
-
-       /* Stop echo packet transmission if they are active */
-       if (CHECK_FLAG(bfd->flags, BFD_SESS_FLAG_ECHO_ACTIVE))
-               ptm_bfd_echo_stop(bfd);
-
-       if (old_state != bfd->ses_state) {
-               bfd->stats.session_down++;
-               if (bglobal.debug_peer_event)
-                       zlog_debug("state-change: [%s] %s -> %s reason:%s",
-                                  bs_to_string(bfd), state_list[old_state].str,
-                                  state_list[bfd->ses_state].str,
-                                  get_diag_str(bfd->local_diag));
-       }
-}
-
 static void bs_up_handler(struct bfd_session *bs, int nstate)
 {
        switch (nstate) {
        case PTM_BFD_ADM_DOWN:
-               bs_neighbour_admin_down_handler(bs, BD_ADMIN_DOWN);
-               break;
-
        case PTM_BFD_DOWN:
                /* Peer lost or asked to shutdown connection. */
                ptm_bfd_sess_dn(bs, BD_NEIGHBOR_DOWN);