Fixing FRR’s BFD daemon · 3 of 3

My fixes to FRR’s BFD daemon

While building xdp-bfd, I ran FRR’s bfdd with xdp-bfd as its data plane. Not many people use that part of bfdd, and I kept running into bugs in it. Later on, adding demand mode and authentication turned up a few more. So far eleven of my fixes have been merged, and this post goes through them in roughly the order I hit them.

All of them are on FRR’s master branch, and none are in a release yet. Where there’s a workaround for released versions, I’ve put it in. Also, if you want to use a data plane at all, you need FRR 10.1.2 or newer, because older versions keep dropping the connection to it.

Data plane plumbing

The UNIX socket transport never worked

#22621. This was the very first one. bfdd and xdp-bfd were running on the same box, so I wanted them to talk over a UNIX socket, but every connect failed with EINVAL. The same setup over TCP worked fine.

strace showed the socket path was right, but the address length bfdd passed in was 112 bytes, and a UNIX socket address is only 110. It turns out bfdd keeps addresses in a union big enough for every address type, and it was passing the size of the whole union instead of the size of the address inside it. TCP sockets don’t mind a length that’s too long, but UNIX sockets check it and refuse it. Going by the git history, as far as I can tell unixc: never worked on Linux. The fix passes the real length. On released versions, use TCP.

Sessions lost to an 8KB buffer, twice

#22645 and #22692. When I went from 16 sessions to 64, bfdd silently lost every session after the 40th when it connected to xdp-bfd. On shutdown it had the same problem the other way round and lost the last few delete messages. This one has its own post.

A zero-byte read that looked like a closed socket

#22694. Running show bfd peers counters dropped the connection to xdp-bfd every second time I ran it. This one has its own post too.

Echo

Echo interval

#22805. BFD peers tell each other how often they’re willing to receive echo packets, and you’re not supposed to send faster than that. While I was adding echo to xdp-bfd, I had a peer asking for echoes every 200ms, and bfdd was telling xdp-bfd to send them every 50ms, which was just my local setting.

bfdd does work out the right interval, but only in the code where it handles packets itself. For a session handed to a data plane that code never runs, and the one other place that could have done it skips data plane sessions on purpose. The confusing part was that bfdd had the peer’s 200ms stored, and show bfd peer displayed it, so everything looked right in the CLI. I only noticed because I was measuring how often echoes actually went out. The fix uses whichever of the two intervals is slower, and tells the data plane whenever that changes.

IPv6 echo from the wrong address

#22920. With IPv6, turning on echo mode could take a perfectly good session down.

An earlier change in bfdd made it only bounce back echo packets whose source address matches a session it knows, which is a sensible safety check. But bfdd’s own IPv6 echo socket isn’t tied to an address, so the kernel picks the source for it. If the interface has more than one address in the same network, the kernel can pick one the peer doesn’t know about. The peer drops the echo, and the session goes down. The fix sets the source address on each echo packet. It has to be per packet, because the same socket sends echoes for every session.

Writing the test took longer than the fix. I needed Linux to pick the wrong address on purpose, and adding a second address didn’t do it, because Linux kept using the older one. What worked in the end was marking the right address as deprecated, so the kernel avoids it. The test also checks that the kernel really did pick the wrong address, so it can’t quietly pass if that behaviour ever changes.

Demand mode

#23023. Demand mode lets two BFD peers stop sending regular packets once both sides are up, and only check the path when they need to. bfdd had the option defined, but it didn’t do anything. The flag was set to 0 in three places and never to 1, and there were TODO: demand mode comments sitting in the code. I talked the design through with Rafael Zalamena first, then implemented it, including passing it through to data planes.

Zero discriminator checked against the wrong state

#23284. While I was testing demand mode, I restarted one side and it never came back. The side that stayed up just kept refusing the restarted peer’s packets.

A peer that has restarted sends a zero where the other side’s discriminator should be, and the RFC says to accept that as long as the packet says the peer is Down. bfdd was checking whether its own session was Down instead. Normally nobody notices, because the detection timer runs out, the session goes Down, and the next packet gets through. In demand mode that timer doesn’t run, so the session just stayed stuck. The fix checks the state in the packet, like the RFC says.

Auth

The replay check never ran

#23281. With keyed SHA1, every packet carries a sequence number, and bfdd is supposed to reject old or repeated ones so nobody can replay captured packets. It turns out that check never ran. bfdd decided whether it had seen a sequence number yet by checking if the last one was zero, and the only code that ever updated it was inside that same check. So it stayed at zero forever, and every packet was accepted, replays included.

FRR’s existing auth test never caught this, because two well-behaved BFD implementations never send a bad sequence number. So the test I added uses a small BFD speaker that sends its sequence numbers out of order on purpose.

No usable key meant no auth at all

#23282. With a 20-byte SHA1 key, bfdd sent plain unauthenticated packets, while show bfd peer still showed the keychain.

bfdd limited every key to 16 bytes, which is the limit for simple passwords, but SHA1 keys can be up to 20. With a longer key, bfdd found no usable key and quietly fell back to no authentication. A key with no algorithm set did the same thing. Now the length limit depends on the algorithm, and if a keychain has no usable key, the session doesn’t come up at all instead of running without auth. Anyone who was running without auth by accident will see their session go down after upgrading, so I pointed that out in the PR.

Auth keys never reached the data plane

#23331. Once xdp-bfd could sign packets itself, I found that bfdd had no way to give it the keys. A session with authentication got handed to the data plane and went out with no authentication at all, while show bfd peer still said auth was on. Now bfdd sends the keys that are usable now and the ones that will be later, with the times each one is valid, so the data plane can switch keys by itself. Part 6 of the xdp-bfd series has more on this.

Still in review

These six were still open in early October. Five of them are approved and waiting to be merged; #23465 hasn’t been reviewed yet.

  • #23453: don’t send an echo interval to the data plane when the peer says it can’t receive echo
  • #23463: let a data plane tell bfdd what it supports, so authenticated sessions only go to one that can handle them
  • #23464: forget the peer’s discriminator after a detection time, which fixes a case where a restarted peer stays down
  • #23465: don’t open echo sockets when a data plane handles the packets
  • #23495: send IPv6 echo with the same traffic class as the rest of BFD, so a congested switch doesn’t drop it first. Found in the flood tests in part 10.
  • #23496: open the echo sockets for a session that’s enabled after bfdd starts. Since a change in FRR 10.7.0, echo sessions set up that way got no sockets and kept flapping.

Thanks to Rafael Zalamena, Russ White, Donald Sharp and the other FRR reviewers for reviewing the ones that are merged.