Currently it is possible to configure the same peer with and without
interface name:
```
bfd
peer 1.1.1.1
!
peer 1.1.1.1 interface enp0s3
!
```
There are multiple problems with that:
1. Both nodes actually control the same BFD session. So the config is
either duplicated or, even worse, different - and there is no way to
say which one actually works.
2. When the user deletes both nodes, the session is not actually freed,
because its refcount is always greater than 1.
Such configuration must be forbidden. User should either have single
node with wildcard name or multiple nodes with actual names.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
If local-address is not supplied, then an incorrect xpath is generated
which is not expected by NB CLI.
Fixes#7465.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Valgrind is still reporting:
466020-==466020== by 0x11B9F4: main (bfdd.c:403)
466020-==466020== Address 0x5a7d544 is 84 bytes inside a block of size 272 free'd
466020:==466020== at 0x48399AB: free (vg_replace_malloc.c:538)
466020-==466020== by 0x490A947: qfree (memory.c:140)
466020-==466020== by 0x48F2AE8: if_delete (if.c:322)
466020-==466020== by 0x48F250D: if_destroy_via_zapi (if.c:195)
466020-==466020== by 0x497071E: zclient_interface_delete (zclient.c:2040)
466020-==466020== by 0x49745F6: zclient_read (zclient.c:3687)
466020-==466020== by 0x4955AEC: thread_call (thread.c:1684)
466020-==466020== by 0x48FF64E: frr_run (libfrr.c:1126)
466020-==466020== by 0x11B9F4: main (bfdd.c:403)
466020-==466020== Block was alloc'd at
466020:==466020== at 0x483AB65: calloc (vg_replace_malloc.c:760)
466020-==466020== by 0x490A805: qcalloc (memory.c:115)
466020-==466020== by 0x48F23D6: if_new (if.c:160)
466020-==466020== by 0x48F257F: if_create_name (if.c:214)
466020-==466020== by 0x48F3493: if_get_by_name (if.c:558)
466020-==466020== by 0x49705F2: zclient_interface_add (zclient.c:1989)
466020-==466020== by 0x49745E0: zclient_read (zclient.c:3684)
466020-==466020== by 0x4955AEC: thread_call (thread.c:1684)
466020-==466020== by 0x48FF64E: frr_run (libfrr.c:1126)
466020-==466020== by 0x11B9F4: main (bfdd.c:403)
Apparently the bs->ifp pointer is being set even in cases when
the bs->key.ifname is not being set. So go through and just
match the interface pointer and cut-to-the-chase.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
BFD key has optional fields "local" and "ifname" which can be empty when
the BFD session is created. In this case, the hash key will be calculated
with these fields filled with zeroes.
Later, when we're looking for the BFD session using the key with fields
"local" and "ifname" populated with actual values, the hash key will be
different. To work around this issue, we're doing multiple hash lookups,
first with full key, then with fields "local" and "ifname" filled with
zeroes.
But there may be another case when the initial key has the actual values
for "local" and "ifname", but the key we're using for lookup has empty
values. This case is covered for IPv4 by using additional hash walk with
bfd_key_lookup_ignore_partial_walker function but is not covered for IPv6.
Instead of introducing more hacks and workarounds, the following solution
is proposed:
- the hash key is always calculated in bfd_key_hash_do using only
required fields
- the hash data is compared in bfd_key_hash_cmp, taking into account the
fact that fields "local" and "ifname" may be empty
Using this solution, it's enough to make only one hash lookup.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
On shutdown, interfaces are deleted but if the bfd session
is down we retain the interface pointer. Remove the retained
pointer.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Valgrind reports:
2052866-==2052866==
2052866-==2052866== Syscall param sendmsg(msg.msg_name) points to uninitialised byte(s)
2052866:==2052866== at 0x49C8E13: sendmsg (sendmsg.c:28)
2052866-==2052866== by 0x11DC08: bp_udp_send (bfd_packet.c:823)
2052866-==2052866== by 0x11DD76: ptm_bfd_echo_snd (bfd_packet.c:179)
2052866-==2052866== by 0x114C2D: ptm_bfd_echo_xmt_TO (bfd.c:469)
2052866-==2052866== by 0x114C2D: ptm_bfd_echo_start (bfd.c:498)
2052866-==2052866== by 0x114C2D: bs_echo_timer_handler (bfd.c:1199)
2052866-==2052866== by 0x11E478: bfd_recv_cb (bfd_packet.c:702)
2052866-==2052866== by 0x4904846: thread_call (thread.c:1681)
2052866-==2052866== by 0x48CB4DF: frr_run (libfrr.c:1126)
2052866-==2052866== by 0x113044: main (bfdd.c:403)
2052866-==2052866== Address 0x1ffefff3e8 is on thread 1's stack
In ptm_bfd_echo_snd, for the v4 case we were memsetting the v6 memory
then setting the v4 memory. Just fix it.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
when receiving interface and address notifications, one may be puzzled
by the information since for example, the presence of an interface is
not enough to use it in a bfd session, simply because the interface is
in the wrong vrf. add VRF information on those traces.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
on vrf-lite environment, all incoming bfd packets are received by the
same socket on the default namespace. the vrfid is not relevant and
needs to be updated based on the incoming interface where traffic has
been received. If the traffic is received from an interface belonging to
a separate vrf, update the vrfid value accordingly.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The vrf interface notification and interface notifications are separated
on zapi interface between the system (zebra daemon) and other daemons
(bfd for instance). In the case of bfd, the initial code was waiting for
vrf notification to create the socket. Actually, in vrf-lite world, we
need to wait the vrf interface to be present, in order to create the
socket and bind to the vrf interface (this is the usual way to work with
vrf-lite).
On bfd, the changes consist in delaying the socket creation first, then
when interface is created, check the interface name presence instead of
checking the interface configuration.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When running in vrf-lite mode, the socket used in a vrf environment
should be bound to an interface belonging to the vrf. If no one is
selected, then the vrf interface itself should be bound to that socket,
so that outgoing packets are being applied routing rules for that vrf.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Replace the unclear error message:
```
% Failed to edit configuration.
YANG error(s):
Schema node not found.
YANG path: /frr-bfdd:bfdd/bfd/sessions/single-hop[dest-addr='192.168.253.6'][interface=''][vrf='default']/minimum-ttl
```
With:
```
frr(config-bfd-peer)# minimum-ttl 250
% Minimum TTL is only available for multi hop sessions.
! or
frr(config-bfd-peer)# echo
% Echo mode is only available for single hop sessions.
frr(config-bfd-peer)# echo-interval 300
% Echo mode is only available for single hop sessions.
```
Reported-by: Trae Santiago
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Don't reset interface/vrf pointer everytime a session is disabled
instead only do it when it was explicitly removed.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Show BFD sessions updated counters by asking the data plane for this
information and show data plane statistics.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Add hooks in the correct places so the BFD daemon uses the data plane
instead of the software packet sending implementation to monitor the
session.
This code also adds some handlers to support fallback to FRR BFD session
handling, however since this complicates the code it won't work at the
moment (the BFD sockets are disabled by default when using data plane).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The current distributed BFD implementantion doesn't support falling back
to software implementation in FRR, so to keep the code simple lets give
the data plane full control of the BFD packet handling (helps running a
software data plane for testing too otherwise it would fail with 'address
in use' error).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Distributed BFD is a term used for BFD implementations that do not run
on the routing engine, instead it is run on a data plane (software or
hardware based).
The current code implements the basic communication between FRR BFD
daemon with an external BFD data plane and defines the protocol format
in the file `bfddp_packet.h`.
To enable/use data plane you need to start BFD daemon with the command
line `--dplaneaddr <type>:<address>`, then a socket will be opened to
listen for incoming data plane connections.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The BFD data plane header has definitions for the data plane
communication protocol that will be used to implement the distributed
BFD feature.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Allows users with multiple links using same IPv6 address (same VRF) to
work.
Reported-by: Matti Suuronen
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The interface address delete callback from zebra was not
deleting the ifc that was created as per normal work methodologies
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Let the integration protocol always send the full configuration
instead of saving a few bytes. It will also allow protocols to specify
source address for IPv4 single hop connections and interface for multi
hop configuration.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Change thread_cancel to take a ** to an event, NULL-check
before dereferencing, and NULL the caller's pointer. Update
many callers to use the new signature.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Previously if there were two multihop peers created that had the same
peer address but different local addresses then the second peer to be
created would be merged with the first one and niether would be able to
be deleted. This was due to an issue in the function bfd_key_lookup().
When the second peer was created its key would be sent into the lookup
function and would reach the last section, even though it shouldn't
have. A check has been placed around the section so that it will not be
entered if a peer is multihop.
Signed-off-by: Tashana Mehta-Wilson <tashana.mehta-wilson@alliedtelesis.co.nz>
Experimental patch to allow us to discuss if we should
allow bfdd to work when v6 is turned off in the kernel.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Initial BFD protocol implementation had a hard coded value of maximum 5
hops, now we have a configurable hop amount with a safe default of 1
hop.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
The passive mode is briefly described in the RFC 5880 Bidirectional
Forwarding Detection (BFD), Section 6.1. Overview:
> A system may take either an Active role or a Passive role in session
> initialization. A system taking the Active role MUST send BFD
> Control packets for a particular session, regardless of whether it
> has received any BFD packets for that session. A system taking the
> Passive role MUST NOT begin sending BFD packets for a particular
> session until it has received a BFD packet for that session, and thus
> has learned the remote system's discriminator value. At least one
> system MUST take the Active role (possibly both). The role that a
> system takes is specific to the application of BFD, and is outside
> the scope of this specification.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
DEFPY_YANG will allow the CLI to identify which commands are
YANG-modeled or not before executing them. This is going to be
useful for the upcoming configuration back-off timer work that
needs to commit pending configuration changes before executing a
command that isn't YANG-modeled.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Remove mid-string line breaks, cf. workflow doc:
.. [#tool_style_conflicts] For example, lines over 80 characters are allowed
for text strings to make it possible to search the code for them: please
see `Linux kernel style (breaking long lines and strings)
<https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings>`_
and `Issue #1794 <https://github.com/FRRouting/frr/issues/1794>`_.
Scripted commit, idempotent to running:
```
python3 tools/stringmangle.py --unwrap `git ls-files | egrep '\.[ch]$'`
```
Signed-off-by: David Lamparter <equinox@diac24.net>
During the shutdown phase don't attempt to apply settings to peers
as it is useless and will crash if the peer hash is gone.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Implement the infrastructure for other protocols daemon (e.g. `bgpd`,
`ospfd`, `isisd` etc...) to communicate to BFD daemon which profile
they want to use with their peers.
It was also added the ability for protocols to change profile while
running (no need to remove the registration and then register again).
The protocols message building function was rewritten to support
multiple arguments through `struct bfd_session_arg`, so we can
implement new features without the need of changing function
prototypes. The old function was also rewritten to keep
compatibility.
The profile message part is only available for BFD daemon at the
moment.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Allow user to pre-configure peers with a profile. If a peer is using a
profile any configuration made to the peer will take precedence over
the profile configuration.
In order to track the peer configuration we have now an extra copy of
the peer configuration in `peer_profile` inside `struct bfd_session`.
This information will help the profile functions to detect user
configurations and avoid overriding what the user configured. This is
especially important for peers created via other protocols where the
default `shutdown` state is disabled (peers created manually are
`shutdown` by default).
Profiles can be used before they exist: if no profile exists then it
will use the default configuration.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Simplify and fix the code that handles session teardown on control
plane shutdown (either failure or graceful).
- Don't move the `NULL` check inside `free` functions that expect
data: it creates harder to understand flows.
- Add some new debug messages to aid visualizing session deletions.
- Add sanity check error message (if it ever happens).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Dealing with PRIu64 is unfortunately a bit hacky in the frr-format
plugin, as in, it works correctly with snprintfrr, but breaks on plain
snprintf. There's no good solution unfortunately :/.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
These are easy to get subtly wrong, and doing so can cause
nondeterministic failures when racing in parallel builds.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Having a fixed set of parameters for each northbound callback isn't a
good idea since it makes it difficult to add new parameters whenever
that becomes necessary, as several hundreds or thousands of existing
callbacks need to be updated accordingly.
To remediate this issue, this commit changes the signature of all
northbound callbacks to have a single parameter: a pointer to a
'nb_cb_x_args' structure (where x is different for each type
of callback). These structures encapsulate all real parameters
(both input and output) the callbacks need to have access to. And
adding a new parameter to a given callback is as simple as adding
a new field to the corresponding 'nb_cb_x_args' structure, without
needing to update any instance of that callback in any daemon.
This commit includes a .cocci semantic patch that can be used to
update old code to the new format automatically.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Replace all `random()` calls with a function called `frr_weak_random()`
and make it clear that it is only supposed to be used for weak random
applications.
Use the annotation described by the Coverity Scan documentation to
ignore `random()` call warnings.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
And again for the name. Why on earth would we centralize this, just so
people can forget to update it?
Signed-off-by: David Lamparter <equinox@diac24.net>
Same as before, instead of shoving this into a big central list we can
just put the parent node in cmd_node.
Signed-off-by: David Lamparter <equinox@diac24.net>
There is really no reason to not put this in the cmd_node.
And while we're add it, rename from pointless ".func" to ".config_write".
[v2: fix forgotten ldpd config_write]
Signed-off-by: David Lamparter <equinox@diac24.net>
The only nodes that have this as 0 don't have a "->func" anyway, so the
entire thing is really just pointless.
Signed-off-by: David Lamparter <equinox@diac24.net>
Don't attempt to handle out-of-memory situations: XMALLOC/XCALLOC will
`assert` if there is no memory left.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Move most of the log messages to debug guards so they only get activated
if the user configured the proper debug level.
Current debug levels:
- Peer events.
- Zebra events.
- Network layer debugs.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
According to the RFC 5880 the transmission time should be mandated by
the slowest system.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Lets avoid garbage data on packets by zeroing the packet before setting
the fields/flags.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
This is a full rewrite of the "back end" logging code. It now uses a
lock-free list to iterate over logging targets, and the targets
themselves are as lock-free as possible. (syslog() may have a hidden
internal mutex in the C library; the file/fd targets use a single
write() call which should ensure atomicity kernel-side.)
Note that some functionality is lost in this patch:
- Solaris printstack() backtraces are ditched (unlikely to come back)
- the `log-filter` machinery is gone (re-added in followup commit)
- `terminal monitor` is temporarily stubbed out. The old code had a
race condition with VTYs going away. It'll likely come back rewritten
and with vtysh support.
- The `zebra_ext_log` hook is gone. Instead, it's now much easier to
add a "proper" logging target.
v2: TLS buffer to get some actual performance
Signed-off-by: David Lamparter <equinox@diac24.net>
Some logging systems are, er, "allergic" to tabs in log messages.
(RFC5424: "The syslog application SHOULD avoid octet values below 32")
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
It's been a year search and destroy.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The vrrpd one conflicts with the standalone vrrpd package; also we're
installing daemons to /usr/lib/frr on some systems so they're not on
PATH.
Signed-off-by: David Lamparter <equinox@diac24.net>
Adding fields "detect-multiplier" and "remote-detect-multiplier"
for JSON to to reflect changes in "show bfd peers output"
Signed-off-by: Sayed Mohd Saquib sayed.saquib@broadcom.com
Adding new CLI clear bfd counters,
This CLI wil only reset Rx/Tx counters,
it will not reset session UP/DOWN and Zebra event count
Signed-off-by: Sayed Mohd Saquib <sayed.saquib@broadcom.com>
Use the interface VRF information instead of relying on the VRF specific
socket information.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
(cherry picked from commit c05c48621c)
Always bind the created sockets to their respective VRF devices. With
this it should be possible to run BFD on VRFs without needing to weaken
the security setting `net.ipv4.udp_l3mdev_accept=1`.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Rearrange the bfdd northbound callbacks as following:
* bfd_nb.h: prototypes of all northbound callbacks.
* bfd_nb.c: definition of all northbound callbacks and their
associated YANG data paths.
* bfd_nb_config.c: implementation of YANG configuration nodes.
* bfd_nb_state.c: implementation of YANG state nodes.
This should help to keep to code more organized and easier to
maintain.
No behavior changes intended.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Scenarios where this code change is required:
1. BFD is un-configured from BGP at remote end.
Neighbour BFD sends ADMIN_DOWN state, but BFD on local side will send
DOWN to BGP, resulting in BGP session DOWN.
Removing BFD session administratively shouldn't bring DOWN BGP session
at local or remote.
2. BFD is un-configured from BGP or shutdown locally.
BFD will send state DOWN to BGP resulting in BGP session DOWN.
(This is akin to saying do not use BFD for BGP)
Removing BFD session administratively shouldn't bring DOWN BGP session at
local or remote.
Signed-off-by: Sayed Mohd Saquib sayed.saquib@broadcom.com
When using link-local addresses we must provide scope-id to the
operating system so it knows where to send packets.
Spotted by Pavel Ivashchenko (@zays26).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Don't be selective about what to observe, always observe all possible
aspects of the session that may change on run-time (i.e. bind address,
interface and VRF existence).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
If a session is no longer able to send/receive packets, it is very
likely it will be down in a few milliseconds so lets speed up the
process and correctly mark it as down.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Otherwise the `thread_read` will keep waking us up to handle closing
sockets which are never unregistered.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Current autocompletion works only for simple "vrf NAME" case.
This commit expands it also for the following cases:
- "nexthop-vrf NAME" in staticd
- usage of $varname in many daemons
All daemons are updated to use single varname "$vrf_name".
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Switch the zclient->interface_add functionality to have everyone
use the interface create callback in lib/if.c
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Start the conversion to allow zapi interface callbacks to be
controlled like vrf creation/destruction/change callbacks.
This will allow us to consolidate control into the interface.c
instead of having each daemon read the stream and react accordingly.
This will hopefully reduce a bunch of cut-n-paste stuff
Create 4 new callback functions that will be controlled by
lib/if.c
create -> A upper level protocol receives an interface creation event
The ifp is brand spanking newly created in the system.
up -> A upper level protocol receives a interface up event
This means the interface is up and ready to go.
down -> A upper level protocol receives a interface down
destroy -> A upper level protocol receives a destroy event
This means to delete the pointers associated with it.
At this point this is just boilerplate setup for future commits.
There is no new functionality.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Adding a lock to protect the global running configuration doesn't
help much since the FRR daemons are not prepared to process
configuration changes in a pthread that is not the main one (a
whole lot of new protections would be necessary to prevent race
conditions).
This means the lock added by commit 83981138 only adds more
complexity for no benefit. Remove it now to simplify the code.
All northbound clients, including the gRPC one, should either run
in the main pthread or use synchronization primitives to process
configuration transactions in the main pthread.
This reverts commit 83981138fe.
upon vrf disable, an event informs bfd daemon that the vrf contexts
should be removed. in the case a vrf backend is netns based, all sockets
opened under that netns have to be closed. otherwise it is impossible
for the system to completely close the network namespace. that implies
that some interfaces may not be deleted, and may not be given back to
default vrf.
PR=65291
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Acked-by: Julien Floret <julien.floret@6wind.com>
bfd operational & config data may already applied and available, while
an external event requests for changing the vrf name. this change
updates the config and operational context of yang.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
if the bfd session is already enabled, then dynamically change the vrf
name if the vrf where bfd is executed changed its name.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Add source address to northbound when creating sessions with this
information. It is not possible to change source address after the
session was created, but we should be able to set it to make IPv6 work.
Spotted by Philippe Guibert.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
This helps northbound to create the `bfd` node on the configuration
output sooner than adding a peer.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Build will fail with `Werror` enabled with:
```
CC bfdd/bfdd_cli.o
In file included from ./lib/frratomic.h:21:0,
from ./lib/memory.h:22,
from ./lib/vector.h:25,
from ./lib/command.h:25,
from bfdd/bfdd_cli.c:23:
./config.h:665:0: error: "_FILE_OFFSET_BITS" redefined [-Werror]
#define _FILE_OFFSET_BITS 64
^
In file included from /usr/include/iso/stdlib_iso.h:49:0,
from /usr/include/stdlib.h:37,
from ./lib/memory.h:20,
from ./lib/vector.h:25,
from ./lib/command.h:25,
from bfdd/bfdd_cli.c:23:
/opt/gcc-5.1.0/lib/gcc/i386-pc-solaris2.11/5.1.0/include-fixed/sys/feature_tests.h:231:0: note: this is the location of the previous definition
#define _FILE_OFFSET_BITS 32
^
In file included from ./lib/thread.h:24:0,
from ./lib/vty.h:27,
from ./lib/command.h:26,
from bfdd/bfdd_cli.c:23:
./lib/zebra.h:271:2: error: #warning "assuming 4-byte alignment for CMSG_SPACE" [-Werror=cpp]
#warning "assuming 4-byte alignment for CMSG_SPACE"
^
./lib/zebra.h:277:2: error: #warning "assuming 4-byte alignment for CMSG_LEN" [-Werror=cpp]
#warning "assuming 4-byte alignment for CMSG_LEN"
^
cc1: all warnings being treated as errors
Makefile:6876: recipe for target 'bfdd/bfdd_cli.o' failed
gmake[1]: *** [bfdd/bfdd_cli.o] Error 1
```
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Lets allow specification to accept microseconds, but limit the timers
configuration in FRR to milliseconds (minimum is 10 ms and maximum is 60
seconds).
This matches the RFC 5880 and the IETF BFD YANG draft model.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Add command 'no bfd' to remove all BFD sessions configuration and fix
other daemon integration.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
```
warnings: BFD: [EC 100663317] YANG model "frr-bfdd@*" not embedded, trying external file
```
Embed the YANG model into the binary to avoid reading an external file.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When the remote discriminator hasn't been assigned yet, then we can't
return a value of 0. The value '0' is an invalid discriminator and the
yang handlers will complain about it.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Lets start using the new BFD yang model and translate the BFD session
configuration commands to use the northbound.
One important change: all sessions will default to use
`VRF_DEFAULT_NAME` (usually "default") when no VRF is configured. All
places which search for BFD sessions must now take this into account.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
These functions are going to be used by the future northbound
implementation to handle BFD sessions.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
when working with a standard vrf backend, bfdd ignores that and tries to
create and configure bfd sockets for each vrf, which will fail for the
second vrf discovered, since the network namespace used is the same, and
it is not possible to use same socket settings twice. Handle this case,
and avoids to reinitialise sockets.
This patch however does not leverage bfd support for vrf-lite.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
MTYPE definitions should be local to the file using them whereever
possible. Also remove some superfluous ;
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The `access-list ...` command was causing bfdd to return
'unknown commands'. Make bfdd at least cognizant of
access-lists enough to not create strange error messages
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When selecting to run bfdd with -N allow the namespace passed
in to be added to the $frr_statedir/<namespace name>/bfdd.sock
If --bfdctl is passed in that will override the -N option.
If neither --bfdctl or -N is passed in then the default
of $frr_statedir/bfdd.sock is used.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
It doesn't make much sense for a hash function to modify its argument,
so const the hash input.
BGP does it in a couple places, those cast away the const. Not great but
not any worse than it was.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
bfd cbit is a value carried out in bfd messages, that permit to keep or
not, the independence between control plane and dataplane. In other
words, while most of the cases plan to flush entries, when bfd goes
down, there are some cases where that bfd event should be ignored. this
is the case with non stop forwarding mechanisms where entries may be
kept. this is the case for BGP, when graceful restart capability is
used. If BFD event down happens, and bgp is in graceful restart mode, it
is wished to ignore the BFD event while waiting for the remote router to
restart.
The changes take into account the following:
- add a config flag across zebra layer so that daemon can set or not the
cbit capability.
- ability for daemons to read the remote bfd capability associated to a bfd
notification.
- in bfdd, according to the value, the cbit value is set
- in bfdd, the received value is retrived and stored in the bfd session
context.
- by default, the local cbit announced to remote is set to 1 while
preservation of the local path is not set.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
this structure contains the bfdd_privs structure in charge of the
privilege settings. The initialisation has moved a bit, in order that
the preinit settings are done.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
the vrf keyword is possible through show bfd command. However, there is
a change with previous version, since that show command was accepting
vrf keyword, only after peer keyword. Now, the vrf keyword is accepted,
but before peer keyword.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
when configuring bfd peers, some parameters may or may not be taken into
account to search for a previous context. This has as consequence that
the result is different with the order of vty commands:
bfd
peer 4.5.6.7 vrf AAAA local-address 1.2.3.5
end
peer 4.5.6.7 vrf AAAA <--- should not create new session
end
Similarly, the user thinks it has overwritten some key parameters like
local address, whereas it is wrong.
here, some informational message should be present.
bfd
peer 4.5.6.7 vrf AAAA
end
peer 4.5.6.7 vrf AAAA local-address 1.2.3.5
<--- should inform that the key lookup 1.2.3.5 is wrong
end
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
parse observer list, and update bs context if vrf pointer is not yet populated.
this is helpful for validation, but also will permit bfd to send
notification to remote daemon.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
messages from daemons to bfd daemons go through zebra. zebra reuses the
vrf identifier to send messages to bfd.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
vrf initialisation is done. hooks are installed. no specific action is
done, except the vrf initialisation.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
in the case vrf-lite is used, it is possible to call SO_BINDTODVICE, by
using vrf_socket() call.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
it is possible to configure both iface and vrfname. also, the
appropriate vrf is used, in case an iface is given.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
in order to be able to create sockets on separate namespaces, add the
privs setting needed.
the former capability is needed to use SO_BINDTODEVICE option.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
there is no specific constraints that should prevent from configuring a
multihop bfd session within a bfd session.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
this is a change to be more consistent with function naming convention
in bfd. a small change for 3 functions.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
zlog_debug is being replaced with log_debug, because all bfdd code uses
that way of logging information.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This macro:
- Marks ZAPI callbacks for readability
- Standardizes argument names
- Makes it simple to add ZAPI arguments in the future
- Ensures proper types
- Looks better
- Shortens function declarations
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
RFC 5881 Section 4 tells us that the BFD source port must be between
49152 and 65535 inclusive.
Spotted by Lucian Cristian.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
when a remote daemon wants to get rid of a session, a request is sent,
but the deletion of the bfd session was not done. The flush is done,
provided that there is not someone else that is using that session.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
there are cases where bfd sessions are created from remote daemons. in
that case, the bfd daemon were appearing in both operational and
configuration contexts of bfd. Change that by only keeping operational
contexts.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When we delete an interface, we need to set the interface
ifindex to an internal value so that we don't end up in
a state where the re-addition of the same ifindex, due to
a rename operation, causes an infinite loop.
Fixes:#4007
Fix-Suggested-by: Saravanan K
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Clang's SA is reporting that we have a assignment without
subsuquent use. Modify the code such that we no-longer
do this.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Running valgrind w/ bfdd and shut/no shuting interfaces
can result in this valgrind issue:
==20279== Conditional jump or move depends on uninitialised value(s)
==20279== at 0x115848: bfdd_sessions_enable_address (ptm_adapter.c:644)
==20279== by 0x115848: bfdd_interface_address_update (ptm_adapter.c:674)
==20279== by 0x48D8CAB: zclient_read (zclient.c:2698)
==20279== by 0x48CCEE3: thread_call (thread.c:1603)
==20279== by 0x48A84EF: frr_run (libfrr.c:1011)
==20279== by 0x10DAC3: main (bfdd.c:236)
==20279==
When creating the bso data structure set the bso_isaddress to false
as a default value.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Add the address family to the sockaddr structure otherwise `sendmsg`
will fail with `EAFNOSUPPORT`.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When the local-address configured by the peer doesn't exist, then we
must observe the session until the mentioned address comes up.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Use simplier data structure key to avoid having to do complex and
error-prone key building (e.g. avoid expecting caller to know IPv6
scope id, interface index, vrf index etc...).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Don't assume IPv6 will always be multi hop and handle the single hop
link-local address case.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
