Current behavior is inconsistent. When the session is created by another
daemon, it is up by default. When we later configure peer in bfdd, the
session is still up, but the NB layer thinks that it is down.
More than that, even when the session is created in bfdd using peer
command, it is created in DOWN state, not ADM_DOWN. And it actually
starts sending and receiving packets. The sessions is marked with
SHUTDOWN flag only when we try to reconfigure some parameter. This
behavior is also very unexpected.
Fixes#7780.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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
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>
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>
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>
Field vrf_id is replaced by the pointer of the struct vrf *.
For that all other code referencing to (interface)->vrf_id is replaced.
This work should not change the behaviour.
It is just a continuation work toward having an interface API handling
vrf pointer only.
some new generic functions are created in vrf:
vrf_to_id, vrf_to_name,
a zebra function is also created:
zvrf_info_lookup
an ospf function is also created:
ospf_lookup_by_vrf
it is to be noted that now that interface has a vrf pointer, some more
optimisations could be thought through all the rest of the code. as
example, many structure store the vrf_id. those structures could get
the exact vrf structure if inherited from an interface vrf context.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
vrf_id parameter is replaced with struct vrf * parameter. It is
needed to create vrf structure before entering in the fuction.
an error is generated in case the vrf parameter is missing.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.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>
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>
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>
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>
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>
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>
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>
Allow `bfdd` to configure inexisting interfaces / VRF and only activate
them once the interface/VRF start existing. This implementation doesn't
handle dynamic VRFs yet.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Use internal data to lookup sessions. This approach has two main
advantages:
* it uses less memory because it doesn't use strings for interface /
vrf, it uses OS indexes instead;
* prepares code to support VRF;
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
