Hardening, and a review that found real edges

Part 5 ended with a working benchmark and an embarrassing list: no session continuity, no real validation, one session tested out of sixty-four, and three RFC 5880 features missing outright.

This part takes the first two off that list. It is the unglamorous middle of the project, where nothing gets faster and the interesting question is no longer “does it work” but “does it work when someone is trying to break it, or when the control plane restarts underneath it”.

Nothing here changes the steady-state path, so the numbers from part 5 stand unchanged throughout.

Surviving a control-plane restart

The most visible gap was that restarting bfdd dropped every session. The dataplane tore them down when the control plane disconnected and bfdd re-added them on reconnect, which is correct in the sense that nothing lies about state, and useless in the sense that a routine daemon restart takes your routes with it.

The fix is a hold mode, --dp-hold, that keeps wire sessions alive across the gap. On disconnect the sessions are orphaned rather than deleted. When bfdd comes back and re-adds them, they are adopted by address pair with discriminator continuity, and a mark-and-sweep pass reconciles anything that did not come back. I took the shape of this from Rafael Zalamena’s guidance on the FRR dev list, since the protocol’s intent matters more than my reading of it. Two back-to-back FRR restarts now produce zero peer-visible events.

It is not the default, and should not be. Drop-and-recreate is what the protocol expects unless an operator has decided otherwise, so continuity is something you ask for.

The same milestone added self-initiated Poll sequences on parameter change, per RFC 5880 s6.8.3, along with transitional transmission during the change. Before that, altering a timer mid-session could flap it against either side, which is a poor reward for reconfiguring something.

Refusing traffic properly

The validation story until this point was thin. It now enforces GTSM at TTL 255, demux validation per s6.8.6, and session creation gated on control-plane config so nothing can conjure a session by sending packets at the box. Everything that fails lands on XDP_DROP, so a rejected packet never reaches userspace at all rather than being filtered somewhere later.

The demux check is the one that does the real work against spoofing:

__u8 rstate = BFD_STATE(bfd);
if (cfg && cfg->my_disc) {
        __u32 ydisc = bpf_ntohl(bfd->your_disc);
        if (ydisc != cfg->my_disc &&
            !(ydisc == 0 && rstate <= 1)) {
                count(3);
                return XDP_DROP;
        }
}

A packet has to name our discriminator back at us. The exception is the reason it is not simply an equality test: a peer that has lost its state and is restarting legitimately sends your_disc of 0, so that is allowed, but only while it also reports Down or AdminDown. Without the state condition, zero becomes a wildcard anyone can send.

What makes this a validation fix rather than a parsing detail is what happens on success. A packet that gets past here refreshes the session’s liveness and gets echoed back by the transmit path. Anything that can forge its way through is therefore able to keep a dead session up indefinitely, which is precisely the failure BFD exists to prevent. All of it was tested by injecting from a third host.

One measurement note cost me an hour and is worth passing on. XDP_DROP consumes the frame before any capture hook sees it, so tcpdump shows you nothing at all. The only trustworthy signal that a drop happened is the map counters read through bpftool. A test that “sees no packets” is otherwise indistinguishable from a test that sent none, and I briefly believed the wrong one.

The review pass

An external code review of the three source files, taken seriously enough to answer each point on the wire.

The structural item was the shared ABI header. The BPF map value structs had been living as three hand-synchronised copies, one per file that touched them, which works right up until one side gains a field the others do not and the two halves start reading different offsets out of the same bytes. They became a single included definition.

Two of the flagged edges turned out to be real, and both are the same kind of mistake.

The first is IP options. Options push the UDP header to a variable offset, and the parser was reading the GTSM and discriminator checks from fixed positions. A packet carrying options therefore walked straight past both guards from the previous section, the two that exist specifically to stop that packet, and arrived in userspace unvalidated.

/* IP options (ihl != 5) on a UDP packet: a single-hop BFD
 * control packet never carries them. Passing would skip the
 * GTSM/your_disc checks below (UDP header sits at a variable
 * offset with options) and leak the packet to the userspace
 * socket unvalidated - the same bypass class as an XDP_PASS
 * reject. Drop it. */
if (iph->ihl != 5) {
        count(3);
        return XDP_DROP;
}
udp = (void *)(iph + 1);

The fix is three lines, and the last line is the reason they are needed: udp = (void *)(iph + 1) assumes a 20-byte header. That assumption is fine, and it is load-bearing, and nothing had been enforcing it. A single-hop BFD control packet never legitimately carries options, so refusing them outright is cheaper and safer than learning to parse them.

The second is bfddp framing. On a framing error the reader reset its buffer and carried on reading the same stream, which means it can resync onto arbitrary mid-stream bytes and start interpreting them as messages. It now drops the connection cleanly, and because --dp-hold already exists that drop turns into a hitless reconnect rather than an outage.

Two RFC-correctness fixes rode along: the s6.8.7 jitter cap at 90% when detect_mult is 1, and trimming an over-length echoed frame back to 24 BFD bytes with a recomputed checksum.

Every change was checked the same way as everything else here, with an injection harness and a capture. None touched the steady-state path, so the resilience numbers from part 5 stand unchanged.

The rejected suggestions, and the reasoning for rejecting them, are in the repo under docs/refactor-abi. Writing down why you did not take a suggestion is worth as much as the changes you did take, and it is the part people skip.

What I took from it

The two review comments I was most inclined to wave away were the two that were real. Both were of the form “what if the header is not where you think it is”, which is exactly the class of assumption that a fast path is built on and a parser has to defend.

Next

Sixty-four sessions, which is the next item on that list and the one that found the most bugs.