feat: add partial BFD protocol implementation for BGP peer failure detection#3388
Open
Ivan-Pokhabov wants to merge 2 commits into
Open
feat: add partial BFD protocol implementation for BGP peer failure detection#3388Ivan-Pokhabov wants to merge 2 commits into
Ivan-Pokhabov wants to merge 2 commits into
Conversation
…tection Implements RFC 5880/5881 BFD (Bidirectional Forwarding Detection) for fast detection of BGP peer failures. BFD is configured per peer-group or neighbor and triggers a hard reset when a BFD session expires. Co-authored-by: Ivan-Pokhabov <vanek3372@gmail.com>
Contributor
Author
|
Hello, @fujita! Can you please review this PR? A lot of this changes is regen api and test. I think that we don't need to split it so its provided one feature |
Member
|
have you considered using openconfig-bfd? OpenConfig has a standard BFD model (openconfig-bfd). Could you share why you chose to define BfdConfig independently rather than importing it? Please split into separate PRs. This is too large.
|
Contributor
Author
We are not trying to model or implement BFD as a full standalone control plane. As we think GoBGP only needs a very small slice of BFD: enough to treat it as a liveness signal next to a BGP session (essentially “is the peer still reachable on the path we already use for BGP”), with a handful of knobs that map directly to that use case. The complete openconfig-bfd model describes a rich, general-purpose BFD subsystem (interfaces, peers, session details, micro-BFD, echo-related knobs, extensive operational state, and so on). That breadth is appropriate for a router that implements BFD end-to-end as its own product surface, but it is far beyond what GoBGP needs or intends to support in the near term. |
Contributor
Author
Ok, we will split it, thanks |
Member
|
"openconfig-bfd is too large" is not by itself a reason to avoid it — importing a YANG model doesn't mean implementing every leaf; you only map the fields you actually use. The more relevant question is whether the structure fits. openconfig-bfd is interface-scoped (/bfd/interfaces/interface/config/ holds timers; peers under it are read-only discovery state). Seems that your BfdConfig is peer-scoped (per-neighbor multiplier / rx_interval / tx_interval). Could you explain why peer-scoped configuration is needed? |
Contributor
Author
You're right that we don't have to implement every leaf. The structural mismatch is the issue: openconfig-bfd is interface-scoped — timers live at /bfd/interfaces/interface/config/, and discovered peers under it are read-only state. GoBGP has no concept of "interface" in its config model; the unit is always a BGP neighbor by address. Forcing that mapping would require a synthetic interface entry per neighbor, which is more confusing than an honest peer-scoped config. |
Member
|
I agree that using openconfig-bfd directly doesn't fit well here — its interface-scoped structure doesn't map naturally to GoBGP's neighbor-centric config model. That said, I'd like us to introduce BFD configuration in a way that's consistent with GoBGP's existing OpenConfig conventions: specifically, the config/state container split, using grouping and augment in the gobgp YANG module, and placing it under the neighbor and peer-group trees as we do for other gobgp-specific extensions. I'd also like to align the leaf names and units with openconfig-bfd (e.g., desired-minimum-tx-interval in microseconds rather than tx-interval in milliseconds), so that the schema stays close to the standard and remains easier to migrate or integrate in the future. https://github.com/fujita/gobgp/tree/bfd-config Could you take a look and let me know what you think? |
Contributor
Author
Could we keep a port leaf (default 3784) in the config to allow non-standard deployments? Everything else in your schema looks good. |
Member
Sure, a port leaf makes sense for non-standard deployments. Could you open a PR directly against master that incorporates my PoC schema and adds the port leaf on top? Please include a note in the YANG leaf description that setting a non-default port deviates from RFC 5881, which mandates destination UDP port 3784. |
Contributor
Author
What do you think about this PR, but with port leaf? Or something wrong with it? |
dantte-lp
added a commit
to dantte-lp/gobfd
that referenced
this pull request
May 8, 2026
* docs(structure): archive sprint logs and dated audits
Move S9-S11 sprint plans, implementation plan, and codebase consistency
audit to .archive/sprints/ with EN+RU mirrors. Add archive index README.
Drop sprint planning subgraph and table from canonical docs/{en,ru}/README.md.
Replace top-level docs/README.md badge counts with a description that
points to adr/ and reference/ subtrees.
Sprint planning records are no longer part of the user-facing
documentation surface in docs/{en,ru}/. Files remain git-blameable
under .archive/ for historical reference.
* docs(adr): introduce architecture decision records
Add docs/en/adr/ with three accepted records and EN+RU mirrors:
- ADR-0001: link-state monitoring via rtnetlink (was
linux-netlink-ebpf-research.md, sprint marker S4 dropped).
- ADR-0002: native OVSDB client for the OVS LAG backend (was
ovsdb-api-research.md, sprint impact section dropped).
- ADR-0003: Linux applicability of Micro-BFD, VXLAN BFD, and Geneve BFD
(was linux-advanced-bfd-applicability.md, sprint impact section dropped).
Each ADR uses a fixed template with Status, Date (immutable on Accepted),
Context, Decision, Consequences, References. The original research notes
under docs/{en,ru}/ are removed.
* docs(reference): promote dependency-risk to docs/{en,ru}/reference/
Introduce docs/{en,ru}/reference/ for living technical references and
move dependency-risk.md into it with EN+RU mirrors. Add reference/README.md
indices that describe the directory contract: living references maintained
as the codebase evolves, not point-in-time snapshots.
* docs(style): normalize canonical doc headers and tone
Apply unified template to docs/{en,ru}/01..16-*.md:
- Add badge row + declarative blockquote summary to 12-benchmarks,
13-competitive-analysis, 14-performance-analysis, 15-security,
16-production-runbooks (EN+RU).
- Promote Table of Contents heading from h3 to h2 across the canonical
series (EN+RU).
- Remove first-person voice ("we", "our", "us", "мы", "у нас") from
13 (5 hits EN, 1 RU) and 14 (2 hits EN+RU).
- Drop sprint markers from prose: S10.6 in 05-interop, S10 in 12-benchmarks,
S5b/S9 sprint references in 16-production-runbooks.
* refactor(bfd): consolidate package documentation in doc.go files
Move package-level documentation into dedicated doc.go files for the
four internal/ packages that previously kept the // Package comment in
a logic file:
- internal/bfd/doc.go (was at top of packet.go)
- internal/gobgp/doc.go (was at top of client.go)
- internal/sdnotify/doc.go (was at top of sdnotify.go)
- internal/version/doc.go (was at top of version.go)
Brings every internal/ package in line with the existing config/, metrics/,
netio/, and server/ packages, which already use this layout.
* fix(lint): zero golangci-lint and migrate gomodguard schema
- Migrate gomodguard to gomodguard_v2 in .golangci.yml; new module list
schema (top-level blocked array with module/recommendations/reason).
- Extract goconst literal "_uuid" into internal/netio.ovsdbUUIDColumn.
- Extract goconst literals "single-hop"/"multi-hop" via
cmd/gobfdctl/commands.shortSessionType reuse of new constants.
- Remove unused //nolint:gosec G115 directives in netio/sender.go,
netio/rawsock_linux.go, and test/interop-rfc/echo-reflector/main.go;
current Go versions handle the int conversions without complaint.
After this change, golangci-lint run reports 0 issues.
* docs(changelog): rewrite [Unreleased] for the v0.6.0 surface
Sweep sprint-internal records out of the [Unreleased] section in
CHANGELOG.md and CHANGELOG.ru.md. The new section reports user-facing
changes only:
- Added: end-to-end make targets (e2e-core/routing/rfc/overlay/linux/vendor),
Podman-only make interop-clab, public-image vendor profile set,
GoReleaser snapshot pipeline, vendored Protovalidate proto.
- Changed: documentation reorganization (sprint logs archived, ADRs and
reference/ subtrees introduced, header template unified, doc.go for
internal/ packages, gomodguard_v2 migration).
- Fixed: rtnetlink monitor shutdown, authenticated session cached packet,
goconst literals, stale nolint directives.
Drops 18 sprint-tagged "Added" lines and 9 sprint-tagged "Fixed" lines
that referenced internal harness work, archive S10/S11 plan documents,
and CI-only Dependabot fixes.
* refactor(bfd): split manager.go into per-concern files
Decompose internal/bfd/manager.go (1598 lines) into four files within
the same package:
- manager.go (514 lines): Manager errors, types (PacketMeta, sessionKey,
SessionSnapshot, SessionCounters), Manager struct, ManagerOption family,
NewManager, RunDispatch, broadcastStateChange, scheduleUnsolicitedCleanup,
cleanupUnsolicitedSession, DrainAllSessions, Close.
- manager_session.go: CreateSession, DestroySession, lookups, Demux,
DemuxWithWire, tryCreateUnsolicited, Sessions snapshot,
SubscribeStateChanges, ReconcileSessions.
- manager_echo.go: CreateEchoSession, DestroyEchoSession, DemuxEcho,
EchoSessions, ReconcileEchoSessions.
- manager_micro.go: CreateMicroBFDGroup, DestroyMicroBFDGroup,
MicroBFDGroups, LookupMicroBFDGroup, dispatchMicroBFD,
handleMicroBFDActuator, ReconcileMicroBFDGroups.
Pure file-level reorganization. No semantic changes; same package,
identical exported API. Tests pass without modification.
Also removes two stale //nolint:gosec G118 directives that lived on
the now-relocated session/echo cancel-context lines.
* refactor(bfd): split session.go into per-concern files
Decompose internal/bfd/session.go (1489 lines) into three files within
the same package:
- session.go (750 lines): SessionType, SessionRole, SessionConfig,
StateChange, PacketSender, options, Session struct, NewSession, getters,
RecvPacket, SetAdminDown, SetPathDown.
- session_loop.go: Run, runLoop, handleControlCommand, timer handlers
(handleTxTimer, handleDetectTimer), maybeSendControl, sendControl,
maybeResetAuthSeqKnown.
- session_packet.go: handleRecvPacket (RFC 5880 Section 6.8.6 steps 8-18),
recordValidReceivedPacket, checkAuthConsistency, FSM dispatch
(applyFSMEvent, executeFSMActions, executeAction, emitNotification,
logStateChange), interval/jitter helpers, microsecond conversions.
Pure file-level reorganization. Same package, identical exported API.
* refactor(cli): use cmd.Context() and centralise session-type literals
- Replace context.Background() with cmd.Context() in four cobra commands
(session list/show/add/delete). Lets the CLI propagate cancellation
from the cobra root signal handler instead of relying on the daemon
to time out long-running streaming RPCs.
- Drop the unused "context" import that the prior version required.
- Centralise CLI session-type literals "single-hop"/"multi-hop" into
package-level constants sessionTypeSingleHop/sessionTypeMultiHop and
reuse them across error messages, defaults, flag descriptions, and
the parseSessionType / shortSessionType switches.
* refactor(bfd): split cmd/gobfd/main.go into per-concern files
Decompose cmd/gobfd/main.go (2032 lines) into eight files within the
same package main:
- main.go: entry, run(), runServers, startHTTPServers,
startDaemonGoroutines, sd_notify (notifyReady/notifyStopping),
runWatchdog, newLoggerWithLevel.
- reload.go: handleSIGHUP, reloadConfig, reconcile{All,Sessions,Echo}
helpers, configEchoToBFD.
- factory.go: udpSenderFactory, sender lifecycle, newManager,
buildMicroBFDActuator, buildUnsolicitedPolicy, BFD config converters.
- listeners.go: startListenersAndReceivers, createListeners,
createControlListeners, createEchoListeners, closeListeners.
- shutdown.go: gracefulShutdown, startFlightRecorder, listenAndServe,
newMetricsServer, flightRecorderHandler, newGRPCServer.
- handlers.go: startGoBGPHandler, startInterfaceMonitor, loadConfig.
- micro_bfd.go: createMicroBFDListeners, micro-BFD reconcile family,
configMicroBFDToBFD.
- overlay.go: VXLAN/Geneve receivers, overlay tunnel reconcile family,
overlay session config.
Same package main, identical executable, same lifecycle. Tests pass
without modification. Imports trimmed per file via goimports.
* test(netio): add unit tests for Geneve decapsulation paths
Add table-driven tests for GeneveConn.decapGenevePacket and
GeneveConn.validateGeneveHeader without requiring a UDP socket:
- happy path (BuildGenevePacket -> decap -> matching VNI)
- packet shorter than GeneveHeaderMinSize -> ErrGeneveHeaderTooShort
- VNI mismatch -> ErrOverlayVNIMismatch
- O bit clear -> ErrGeneveOBitNotSet
- C bit set -> ErrGeneveCBitSet
- unexpected protocol type -> ErrGeneveUnexpectedProto
- packet too short for inner overhead -> ErrInnerPacketTooShort
The shared helper newTestGeneveConn() constructs a GeneveConn with a
nil-discarding logger and pre-allocated read buffer, avoiding socket
binding so the tests run with no privileges. Bumps internal/netio
coverage from 57.8% to 59.7%.
* test(cli): add golden tests for gobfdctl version and root help
Two regression tests:
- TestVersionCmd_Output verifies that versionCmd executes without error
with no arguments, locking in the contract that the version command
always runs cleanly even when build-info ldflags are unset.
- TestRootCmd_HelpListsCommands verifies that the root --help output
references every advertised top-level command (session, monitor,
version, shell), guarding against accidental command removal.
* docs(readme): add Why GoBFD section
Insert a five-bullet pitch above the Quick Start that frames the
project's niche for new readers landing from external links:
- standalone daemon decoupled from any control plane
- zero-allocation hot path with policy enforced by 28 micro-benchmarks
- explicit RFC coverage matrix
- production surfaces (ConnectRPC, Prometheus, sd_notify, flight recorder)
- verified interop set (FRR, BIRD3, aiobfd, Thoro/bfd, vendor NOS)
Quick Start gains a non-Podman first invocation so readers without
Podman can build and start the daemon directly with sudo.
* feat(api): add EchoService and MicroBFDService proto
S12.1-S12.3. Introduces typed CRUD services for two session families
that BfdService.AddSession previously rejected:
- EchoService: AddEchoSession, DeleteEchoSession, ListEchoSessions.
EchoSession message follows RFC 9747 Section 3.3 timer model
(locally provisioned TxInterval, no negotiated RequiredMinRx).
- MicroBFDService: AddMicroBFDGroup, DeleteMicroBFDGroup,
ListMicroBFDGroups. MicroBFDGroup message exposes per-member link
state (RFC 7130 Section 2) and aggregate count for LAG operational
monitoring.
Both services are additive on the wire: BfdService is unchanged;
existing AddSession / GetSession / WatchSessionEvents callers are not
affected. Proto-level validation enforces RFC bounds: detect_multiplier
in [1, 255], min_active_links >= 1, IP fields validated by
buf.validate.
Regenerated pkg/bfdpb/ via buf generate. Server handlers wiring and
gobfdctl subcommands ship in subsequent S12 commits.
Adds docs/en/roadmap.md describing the S12-S20 waterfall plan.
* feat(server): wire EchoService and MicroBFDService handlers
S12.4. Implement bfdv1connect.EchoServiceHandler and
MicroBFDServiceHandler in internal/server, then register them on the
daemon's HTTP/2 ConnectRPC mux next to the existing BfdService:
- internal/server/echo.go: EchoServer delegates AddEchoSession,
DeleteEchoSession, ListEchoSessions to bfd.Manager. RFC 9747 Section
3.3 timer model is enforced via echoSessionConfigFromProto:
TxInterval is locally provisioned (no negotiation), and
DetectMultiplier is range-checked into [1, 255].
- internal/server/micro.go: MicroBFDServer wraps Manager.CreateMicroBFDGroup,
DestroyMicroBFDGroup, MicroBFDGroups. microBFDConfigFromProto enforces
RFC 7130 invariants (LAG name required, member_links non-empty,
1 <= min_active_links <= len(member_links)).
- cmd/gobfd/shutdown.go: newGRPCServer registers both services on the
same mux and extends the grpc.health.v1 static checker to advertise
bfd.v1.EchoService and bfd.v1.MicroBFDService alongside BfdService.
* feat(cli): add gobfdctl echo and micro subcommands
S12.5. Adds typed CLI surfaces for the new ConnectRPC services:
- gobfdctl echo {add,list,delete} drives EchoService.
Flags: --peer, --local, --interface, --tx-interval, --detect-mult.
RFC 9747: TxInterval is locally provisioned; the CLI rejects
TxInterval <= 0 and DetectMultiplier == 0 before contacting the
daemon.
- gobfdctl micro {add,list,delete} drives MicroBFDService.
Flags: --lag, --members, --peer, --local, --tx-interval,
--rx-interval, --detect-mult, --min-active. The CLI enforces
RFC 7130 invariant 1 <= min-active <= len(members).
- format_advanced.go adds table/JSON/YAML formatters for EchoSession
and MicroBFDGroup that mirror the existing BfdSession formatters.
- root.go constructs the new ConnectRPC clients alongside the BFD
client and registers the echo / micro commands at the cobra root.
* test(server): cover EchoService and MicroBFDService handlers
S12.6. Adds 12 unit tests covering RFC-mandated invariants and the
basic CRUD paths of the new services:
EchoService (echo_test.go):
- AddEchoSession happy path with discriminator allocation.
- Reject nil tx_interval (RFC 9747 Section 3.3 requires positive TxInterval).
- Reject zero detect_multiplier.
- Reject invalid peer address.
- ListEchoSessions enumerates all sessions.
- DeleteEchoSession by discriminator.
MicroBFDService (micro_test.go):
- AddMicroBFDGroup happy path.
- Reject empty lag_interface.
- Reject empty member_links.
- Reject min_active_links > len(member_links) (RFC 7130 invariant).
- Reject zero detect_multiplier.
- DeleteMicroBFDGroup by LAG.
- Reject empty lag_interface on delete.
Both helpers register t.Cleanup(mgr.Close) so goleak.VerifyTestMain
does not flag the per-session goroutines that EchoService creates.
internal/server coverage holds at 81.0% with the new tests included.
* docs(cli): document echo and micro subcommands and roadmap
S12.7. Synchronize user-facing documentation with the new ConnectRPC
services and CLI surface introduced earlier in S12:
- docs/en/04-cli.md and docs/ru/04-cli.md gain Echo Commands and
Micro-BFD Commands sections under the same template (synopsis,
flag table, RFC reference, sample command). The pre-existing TOC
links Echo and Micro-BFD next to Session Commands.
- CHANGELOG.md and CHANGELOG.ru.md [Unreleased] now report the new
EchoService and MicroBFDService gRPC services, the gobfdctl echo
and gobfdctl micro subcommands, and the roadmap document.
Closes the S12 sprint. Quality and interop gates: golangci-lint 0
issues, go build / vet / -race tests green across 15 packages.
* fix(security): bump Go to 1.26.3 to close GO-2026-4986
govulncheck flagged GO-2026-4986 (CVE-2026-39820) in stdlib
html/template through transitive use by GoBGP-related dependencies.
Fixed in Go 1.26.3.
Updates:
- go.mod toolchain directive 1.26.2 -> 1.26.3.
- deployments/docker/Containerfile{,.dev,.debug,.exabgp,.haproxy-agent}
base image golang:1.26.2-trixie -> golang:1.26.3-trixie.
- .github/workflows/{build,ci,release,security}.yml setup-go and
container image fields aligned to 1.26.3.
After this change, make vulncheck reports zero unallowed
vulnerabilities under the existing scripts/vuln-audit.go allowlist.
* fix(build): add dev-ensure preflight to detect stale dev container
After a worktree teardown the localhost/gobfd_dev:latest container
keeps a bind-mount source pointing at a deleted path. crun then fails
every exec into the container with:
Error: crun: getcwd: Operation not permitted: OCI permission denied
The dev-ensure target inspects the container's mount source and
force-recreates the container when it does not match $(CURDIR), or
starts it when it is stopped. Wires build / test / test-v / lint-spell
to depend on dev-ensure so make verify is self-recovering.
Also bumps the dev container's golangci-lint from v2.11.4 to v2.12.1
(via Containerfile.dev ARG) so the gomodguard_v2 lint config introduced
on master matches the container toolchain.
* chore(lint): exclude local-only paths and extend cspell dictionary
- .yamllint.yaml: ignore .archive/ (sprint-log archive) and .serena/
(Serena MCP local cache) so lint-yaml does not error on
developer-side files.
- .gitignore: add .serena/ to keep the Serena cache out of commits.
- .cspell.json: add domain terms used in the new docs and changelog
entries (CURDIR, crun, getcwd, worktree, criticals, distros,
htmlhint, htmlreport, Pkgfile, recvmsg, sendmsg, REUSEPORT, RTNL,
sbfd, siderolabs, talosctl, goconst, gomodguard, rawsock, syscall).
* docs(changelog): record S12 follow-up build hygiene and Go 1.26.3 bump
Add Security and Fixed entries (EN+RU) describing the S12 follow-up
fixes that surfaced while tightening make verify in the dev container:
- Security: Go 1.26.3 closes GO-2026-4986 (CVE-2026-39820) in
html/template.
- Fixed: dev-ensure preflight target, lint-spell file list update,
lint-yaml ignore extensions, dev container golangci-lint bump.
* chore(promo): archive v0.6.0 community comment drafts
Stage drafts for upstream-engagement comments at osrg/gobgp#2563,
osrg/gobgp#2006, siderolabs/talos#11126, and cilium/cilium#22394.
soulwhisper/home-ops#966 is closed and skipped.
Drafts disclose authorship, reference v0.6.0 features, link to the
runnable demo paths under deployments/integrations/, and explicitly
position GoBFD relative to the in-process BFD direction taken by
osrg/gobgp#3388. Posting checklist included.
Stored under .archive/promo/ so the file does not pollute the canonical
documentation surface in docs/{en,ru}/.
* chore(release): prepare v0.6.0 changelog
Promote [Unreleased] entries to [0.6.0] - 2026-05-09 in both EN and RU
changelogs. Add comparison link rows. Reset [Unreleased] to point at
v0.6.0...HEAD per Keep a Changelog 1.1.0.
* chore(promo): rewrite community drafts with no-flattery tone
Replace the v1 drafts with versions that match each repo's actual
maintainer style after reading recent threads:
- osrg/gobgp: fujita's comments are short and structural; drafts now
open with a fact and a link, end with one concrete offer (gRPC verb
PR), no thanks-for-this filler.
- siderolabs/talos: smira asked for machine config / draft PR /
integration test; the rewrite mirrors that structure and proposes a
Talos extension in parallel with maxpain's GoBGP-as-extension idea.
- cilium/cilium: thread is tense after the BFD-is-enterprise news;
draft positions GoBFD as a stop-gap alongside the existing BIRD
workaround, control-plane only, no datapath claim.
- avelino/awesome-go: entry, PR body, and risk note (netio coverage).
Posting checklist now spaces the comments by 24-48h and explicitly
forbids cross-posting the same body across threads.
Member
|
@Ivan-Pokhabov Can we close this, right? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements RFC 5880/5881 BFD (Bidirectional Forwarding Detection) for
fast detection of BGP peer failures. BFD is configured per peer-group
or neighbor and triggers a hard reset when a BFD session expires.
Authored-by: Timur Aitov timonbl4@gmail.com
Co-authored-by: Ivan-Pokhabov vanek3372@gmail.com