From fd2109e5756b34141053188df25e3a20aa92764e Mon Sep 17 00:00:00 2001 From: anlan_cs Date: Wed, 19 Jan 2022 02:10:18 -0500 Subject: 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 --- bfdd/bfd.c | 53 ++++++++++++----------------------------------------- 1 file changed, 12 insertions(+), 41 deletions(-) (limited to 'bfdd') diff --git a/bfdd/bfd.c b/bfdd/bfd.c index c9a327490..fbe0436b5 100644 --- 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); -- cgit v1.2.3