Upstream bugs in FRR’s BFD daemon · 5 of 6

The RFC says negotiate, but nobody was listening

Echo mode is the part of BFD where you send a packet addressed to yourself, the peer’s forwarding plane loops it back, and you time the round trip. It tests the peer’s data path rather than its control plane, which is the whole point.

RFC 5880 section 6.8.9 puts a limit on it: you must not transmit echo packets faster than the interval the remote system advertises in its Required Min Echo RX field. The effective interval is the larger of what you want and what the peer says it can take.

While implementing echo in xdp-bfd, I found that for offloaded sessions bfdd never performed that negotiation.

Where the negotiation lives, and why it was unreachable

For a normal session, bfdd does the calculation in bs_echo_timer_handler(). That function has exactly two callers:

  • the control packet receive path in bfd_packet.c, which bfdd does not execute for offloaded sessions, because the dataplane is processing control packets
  • bfd_set_echo(), where the call sits behind if (bs->bdc == NULL)

bs->bdc is the dataplane context. It is non-NULL precisely when the session is offloaded. So the guard excludes exactly the sessions that reach this code through the first path being unavailable.

The result is that bs->echo_xmt_TO stays at 0, and _bfd_dplane_session_fill() puts the locally configured bs->timers.desired_min_echo_tx into the min_echo_tx field of DP_ADD_SESSION. The dataplane faithfully transmits at that rate.

Configure the peer to advertise 200ms and the local side to want 50ms, and you get echoes every 50ms: four times faster than the peer said it could receive them.

The information was already there

The frustrating part, and the reason the fix is small, is that bfdd already has the peer’s advertisement. bfd_dplane_session_state_change() receives state->required_echo_rx from the dataplane and stores it into bs->remote_timers.required_min_echo.

It was being recorded and never used. show bfd peer even displays it correctly, which makes this genuinely hard to spot from the CLI: the operator sees the peer’s 200ms reported back accurately while the wire carries 50ms. Only the value sent to the dataplane, and the actual packet cadence, show the problem.

The fix

A bfd_dplane_echo_negotiate() that takes the larger of the two values, called from the state-change handler and from bfd_set_echo(), so that enabling echo on an already-established session negotiates too rather than waiting for the next state change:

void bfd_dplane_echo_negotiate(struct bfd_session *bs)
{
        uint32_t negotiated;

        if (bs->bdc == NULL || !CHECK_FLAG(bs->flags, BFD_SESS_FLAG_ECHO))
                return;

        negotiated = bs->remote_timers.required_min_echo >
                             bs->timers.desired_min_echo_tx
                     ? bs->remote_timers.required_min_echo
                     : bs->timers.desired_min_echo_tx;

        if (bs->echo_xmt_TO == negotiated)
                return;

        bs->echo_xmt_TO = negotiated;
        bfd_dplane_update_session(bs);
}

The whole fix is that comparison, and the reason it took a bug report rather than a glance is that both of its inputs were already sitting in the session struct. Nothing had to be plumbed anywhere. The two guards are what keep it inert: it does nothing for a session that is not offloaded or has echo disabled, and it does not push an update to the dataplane unless the value actually changed.

min_echo_tx then carries the negotiated interval, and the field’s comment says so explicitly, since the next person to read it deserves to know it is not the configured value.

Two things I flagged for reviewers, because both look like behaviour changes and neither is:

The wire format does not change. min_echo_tx now carries a negotiated value instead of a configured one, but no dataplane could have implemented echo correctly before this, because it was never told what the peer required. There was no working behaviour to break.

The fix is inert against a dataplane that does not populate required_echo_rx in BFD_STATE_CHANGE. In that case remote_timers.required_min_echo stays 0, the negotiation is a no-op, and the configured value is used exactly as before. Which also means testing bfdd on its own shows no change at all, and I said so explicitly rather than letting a reviewer discover it.

Verified against a dataplane that does report the field: observed cadence moved from 50ms to 200ms against a peer advertising 200ms, with no other configuration change. Merged as #22805.

What I took from it

This bug needed two implementations to be visible. The control plane had the peer’s requirement and did not use it. The dataplane used what it was told and had no way to know better. Each side was locally reasonable.

It also only shows up on the wire. Every piece of CLI output was correct throughout. If I had been checking show bfd peer rather than counting packet intervals, I would have concluded it worked.