Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions protocols/luci-proto-bfd/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Copyright 2026 Wolf480pl <wolf480@interia.pl>
#
# Modified by Wolf480pl <wolf480@interia.pl>,
# based on luci-proto-gre, which was:
#
# Based on luci-proto-ipip.
# Credited author of luci-proto-ipip is Roger Pueyo Centelles <roger.pueyo@guifi.net>
# Copyright 2016 Roger Pueyo Centelles <roger.pueyo@guifi.net>
#
# Modified by Jan Betik <jan.betik@svine.su>
# Copyright 2020 Jan Betik <jan.betik@svine.su>
#
# This is free software, licensed under the Apache License, Version 2.0 .
#

include $(TOPDIR)/rules.mk

LUCI_TITLE:=Support for BFD (RFC5880)
LUCI_DEPENDS:=+bfdd

PKG_MAINTAINER:=Wolf480pl <wolf480@interia.pl>
PKG_LICENSE:=Apache-2.0

include ../../luci.mk

# call BuildPackage - OpenWrt buildroot signature
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
'use strict';
'require form';
'require network';
'require tools.widgets as widgets';

return network.registerProtocol('bfd', {
getI18n: function() {
return _('Bidirectional Forwarding Detection (BFD) Session');
},

getPackageName: function() {
return 'proto-bfd';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no proto-bfd package. getPackageName() is documented as "the name of the package to download, required for the protocol to function" (network.js:2464-2477),`` and in openwrt/packages#30364 the netifd handler (/lib/netifd/proto/bfd.sh) is installed by the `bfdd` package itself — which is also what `LUCI_DEPENDS:=+bfdd` in the Makefile refers to.

Suggested change
return 'proto-bfd';
return 'bfdd';

Generated by Claude Code

},

renderFormOptions: function(s) {
var o;
var proto = this;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: proto is never used in this function.

Suggested change
var proto = this;

Same for the tools.widgets as widgets import on line 4 — no widget from it is referenced (unlike luci-proto-gre, which uses widgets.NetworkSelect).


Generated by Claude Code


// -- general ---------------------------------------------------------------------

o = s.taboption('general', form.Value, 'peer_address', _("Remote IP address"), _("The IP address of the remote BFD peer."));
o.rmempty = false;
o.datatype = 'or(ip4addr("nomask"),ip6addr("nomask"))';

o = s.taboption('general', form.Flag, 'multihop', _("Multihop"), _("Allow BFD packats to traverse multiple hops. Use when BFD peer is not a direct neighbour of this router."));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo "packats" — this string goes into the translation catalogs, so it is worth fixing before merge.

Suggested change
o = s.taboption('general', form.Flag, 'multihop', _("Multihop"), _("Allow BFD packats to traverse multiple hops. Use when BFD peer is not a direct neighbour of this router."));
o = s.taboption('general', form.Flag, 'multihop', _("Multihop"), _("Allow BFD packets to traverse multiple hops. Use when BFD peer is not a direct neighbour of this router."));

Line 70 has a related one: "Minimum number of milliseconds between BFD packet we transmit" should be "packets".


Generated by Claude Code


o = s.taboption('general', form.Value, 'local_address', _("Local IPv4 address"), _("The local IP address used to communicate with the BFD peer (mandatory on multihop)."));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label says IPv4 but the datatype on line 29 accepts IPv6 as well, and the load() below populates the dropdown with getIP6Addrs() too. Looks like a leftover from luci-proto-gre, which is IPv4-only.

Suggested change
o = s.taboption('general', form.Value, 'local_address', _("Local IPv4 address"), _("The local IP address used to communicate with the BFD peer (mandatory on multihop)."));
o = s.taboption('general', form.Value, 'local_address', _("Local IP address"), _("The local IP address used to communicate with the BFD peer (mandatory on multihop)."));

Generated by Claude Code

o.rmempty = false;
o.datatype = 'or(ip4addr("nomask"),ip6addr("nomask"))';
o.depends('multihop', '1');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gating on multihop makes local_address unreachable for single-hop sessions, but the backend does not treat it that way: bfd.sh in openwrt/packages#30364 passes ${local_address:+-l "$local_address"} unconditionally, and bfdd's own bfdd.template.json documents local-address as "mandatory on multihop" — i.e. permitted, just not required, otherwise. The option's own help text says the same thing. Is hiding it for single-hop intentional? As written a user with several addresses on the bound device has no way to pin the source address for a single-hop session.

Same question for vrf_name on line 56 — bfd.sh emits vrf-name regardless of multihop.


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bfdd itself ignores vrf_name and local_address when multihop is is disabled https://github.com/rzalamena/bfdd/blob/921bfa05b0e0020eb910cffc2a6edf37543b84cf/bfd.c#L775-L792

It's an inconsistence between bfdd's documentation and implementation, though I guess I should pick one interpretation and stick to it instead of carrying the inconsistency into openwrt.

o.load = function(section_id) {
return network.getDevices().then(L.bind(function(devs) {
var addrs = devs.flatMap(d => {
var linkName = d.getName();
function addLinkName(addr) {
if (addr.startsWith("fe8")) {
addr = addr + "%" + linkName;
}
return addr;
}
Comment on lines +35 to +40

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zone-suffixed values this produces cannot pass the option's own datatype. ip6addr("nomask") matches against /^([0-9a-fA-F:.]+)$/ (validation.js:533-538),`` so % is rejected — picking any link-local entry from this dropdown fails validation with "valid IP address" and the form cannot be saved.

Either drop the %<link> suffix here, or replace the datatype on line 29 with a custom validate() that strips the zone before delegating to the address check.

Separately, the prefix test is too narrow: link-local is fe80::/10 (fe8, fe9, fea, feb), so startsWith("fe8") only covers fe80::/16.


Generated by Claude Code

var addrs4 = d.getIPAddrs().map(a => a.split('/')[0]);
var addrs6 = d.getIP6Addrs().map(a => a.split('/')[0]).map(addLinkName);
return addrs4.concat(addrs6)
}).sort()
Comment on lines +43 to +44

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing statement terminators; every other statement in this file is semicolon-terminated.

Suggested change
return addrs4.concat(addrs6)
}).sort()
return addrs4.concat(addrs6);
}).sort();

Generated by Claude Code


for (var i = 0; i < addrs.length; i++) {
this.value(addrs[i]);
}
return form.Value.prototype.load.apply(this, [section_id]);
}, this));
};

o = s.taboption('general', form.Value, 'vrf_name', _("VRF Name"), _("Optional. The VRF used to communicate with the BFD peer."));
o.optional = true;
o.datatype = 'string';
o.depends('multihop', '1');

// -- advanced ---------------------------------------------------------------------

o = s.taboption('advanced', form.Value, 'detect_multiplier', _("Detect Multiplier"), _("Number of consecutive missed BFD packets after which the session is considered down."));
o.optional = true;
o.placeholder = 3;
o.datatype = 'uinteger';

o = s.taboption('advanced', form.Value, 'receive_interval', _("Receive Interval (ms)"), _("Minimum number of milliseconds between BFD packets we're willing to receive from the peer."));
o.optional = true;
o.placeholder = 300;
o.datatype = 'uinteger';

o = s.taboption('advanced', form.Value, 'transmit_interval', _("Transmit Interval (ms)"), _("Minimum number of milliseconds between BFD packet we transmit."));
o.optional = true;
o.placeholder = 300;
o.datatype = 'uinteger';

o = s.taboption('advanced', form.Flag, 'echo_mode', _("Echo Mode"), _("Echo mode sends packets with ourselves as destination, hoping the peer will reflect them without noticing."));

o = s.taboption('advanced', form.Value, 'echo_interval', _("Echo Interval (ms)"), _("Minimum number of milliseconds between BFD Echo packets, both for transmitting and receiving/reflecting. Set to zero to tell our peer we don't want incoming echos."));
o.optional = true;
o.placeholder = 50;
o.datatype = 'uinteger';
}
});