Don't uninstall sessions if the address, interface, VRF or TTL didn't change.
Update the library documentation to make it clear to other developers.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Currently this flag is only helpful in an extremely rare situation when
the BFD session registration was unsuccessful and after that zebra is
restarted. Let's remove this flag to simplify the API. If we ever want
to solve the problem of unsuccessful registration/deregistration, this
can be done using internal flags, without API modification.
Also add the error log to help user understand why the BFD session is
not working.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
- Handle badly formatted messages (don't set `ifp` in that case)
- Handle unread messages (e.g. when family is not don't trigger `assert`)
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
New BFD protocol integration API with abstractions to fix most common
protocol integration errors:
- Set address family together with the source/destination addresses
- Set the TTL together with the single/multi hop option
- Set/unset profile/interface easily
- Keep the arguments so we don't have to rebuild them every time
- Install/uninstall functions that keep track of current state so the
daemon doesn't have to
- Reinstall when critical configuration changes (address, multi hop
etc...)
- Reconfigure BFD when the daemon restarts automatically
- Automatically calls the user defined callback for session update
- Shutdown handler for all protocols
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Back when I put this together in 2015, ISO C11 was still reasonably new
and we couldn't require it just yet. Without ISO C11, there is no
"good" way (only bad hacks) to require a semicolon after a macro that
ends with a function definition. And if you added one anyway, you'd get
"spurious semicolon" warnings on some compilers...
With C11, `_Static_assert()` at the end of a macro will make it so that
the semicolon is properly required, consumed, and not warned about.
Consistently requiring semicolons after "file-level" macros matches
Linux kernel coding style and helps some editors against mis-syntax'ing
these macros.
Signed-off-by: David Lamparter <equinox@diac24.net>
Valgrind reports:
469901-==469901==
469901-==469901== Conditional jump or move depends on uninitialised value(s)
469901:==469901== at 0x3A090D: bgp_bfd_dest_update (bgp_bfd.c:416)
469901-==469901== by 0x497469E: zclient_read (zclient.c:3701)
469901-==469901== by 0x4955AEC: thread_call (thread.c:1684)
469901-==469901== by 0x48FF64E: frr_run (libfrr.c:1126)
469901-==469901== by 0x213AB3: main (bgp_main.c:540)
469901-==469901== Uninitialised value was created by a stack allocation
469901:==469901== at 0x3A0725: bgp_bfd_dest_update (bgp_bfd.c:376)
469901-==469901==
469901-==469901== Conditional jump or move depends on uninitialised value(s)
469901:==469901== at 0x3A093C: bgp_bfd_dest_update (bgp_bfd.c:421)
469901-==469901== by 0x497469E: zclient_read (zclient.c:3701)
469901-==469901== by 0x4955AEC: thread_call (thread.c:1684)
469901-==469901== by 0x48FF64E: frr_run (libfrr.c:1126)
469901-==469901== by 0x213AB3: main (bgp_main.c:540)
469901-==469901== Uninitialised value was created by a stack allocation
469901:==469901== at 0x3A0725: bgp_bfd_dest_update (bgp_bfd.c:376)
On looking at bgp_bfd_dest_update the function call into bfd_get_peer_info
when it fails to lookup the ifindex ifp pointer just returns leaving
the dest and src prefix pointers pointing to whatever was passed in.
Let's do two things:
a) The src pointer was sometimes assumed to be passed in and sometimes not.
Forget that. Make it always be passed in
b) memset the src and dst pointers to be all zeros. Then when we look
at either of the pointers we are not making decisions based upon random
data in the pointers.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Add a `enum zclient_send_status` for appropriate handling
of return codes from zclient_send_message. Touch all the places
where we handle this.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When a BFD integrated session already exists setting the profile
doesn't cause a session update (or vice versa): fix this issue by
handling the other cases.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
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>
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>
BFD profiles can now be used on the interface level like this:
interface eth1
ip router isis 1
isis bfd
isis bfd profile default
Here the 'default' profile needs to be specified as usual in the
bfdd configuration.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
Add the proper handling for cases where user forgets or doesn't have the
pointer needed to call the library function.
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>
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
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>
vrf_id parameter is added to the api of bfd_client_sendmsg().
this permits being registered to bfd from a separate vrf.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Problem reported that some bgp and ospf json commands did not return
any json output at all if the bgp/ospf instance did not exist.
Additionally, some bgp and ospf json commands did not return any json
output if the instance existed but no neighbors were defined. This
fix makes these commands more consistent in returning empty braces for
json output and issue a message if not using json output. Additionally,
made the flag "use_json" a bool to make it consistent since previously,
it had been defined as an int, char, u_char, and bool at various places.
Ticket: CM-21040
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
The following types are nonstandard:
- u_char
- u_short
- u_int
- u_long
- u_int8_t
- u_int16_t
- u_int32_t
Replace them with the C99 standard types:
- uint8_t
- unsigned short
- unsigned int
- unsigned long
- uint8_t
- uint16_t
- uint32_t
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The word Multiplier has been abbreviated to 'Mul' in
the output. This apparently is causing people
angst. Write word out.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The FSF's address changed, and we had a mixture of comment styles for
the GPL file header. (The style with * at the beginning won out with
580 to 141 in existing files.)
Note: I've intentionally left intact other "variations" of the copyright
header, e.g. whether it says "Zebra", "Quagga", "FRR", or nothing.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This is a rather large mechanical commit that splits up the memory types
defined in lib/memtypes.c and distributes them into *_memory.[ch] files
in the individual daemons.
The zebra change is slightly annoying because there is no nice place to
put the #include "zebra_memory.h" statement.
bgpd, ospf6d, isisd and some tests were reusing MTYPEs defined in the
library for its own use. This is bad practice and would break when the
memtype are made static.
Acked-by: Vincent JARDIN <vincent.jardin@6wind.com>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
[CF: rebased for cmaster-next]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
CM-10680
Issue: When BGP daemon is stopped, all the BGP BFD sessions are not getting deleted from PTM.
Root cause: BGP daemon stop causes BFD de-register message to be sent for every peer on which BFD is enabled. But, all the de-register messages from bgpd to zebra are not processed before the socket close. This results in some stale BGP BFD sessions.
Fix: Support for client de-register message has been added in PTM/BFD. Changes in Quagga to support BFD client de-registrations:
− The BFD clients de-registration is sent directly from zebra daemon when zebra client (bgpd, ospfd and ospf6d) socket close is detected.
− Introduced a BFD flag for the zebra clients to prevent BFD de-registration messages from being sent to zebra daemon when the client is shutting down. This reduces the BFD messaging.
CM-10540
Issue: Invalid ptm status “fail” instead of “n/a” being displayed for VRF interfaces.
Root cause: ptm status is not being initialized to “unknown” status when VRF interface is added or changed. The uninitialized value is ‘0’ which is the value for “fail”
Fix: Initialized the ptm status to the correct value.
Signed-off-by: Radhika Mahankali <radhika@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Kanna Rajagopal <kanna@cumulusnetworks.com>
Ticket: CM-10680, CM-10540
Reviewed By: CCR-4653
Testing Done: PTM smoke, BGP smoke and ptmd_test.py:TestMultipleAddrsIntfOspfBgp
BFD debugs were being output without any
debug protection. Modify the code
to not output unless debugging is
turned on.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Radhika Mahankali <radhika@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>
CM-10435
Issue: IBGP BFD sessions are created as multi-hop even though peer is single-hop away. This is causing an interop issue with ICOS.
Root Cause: By design all IBGP peers are registered with BFD as multi-hop.
Fix:
• Changed the default behavior of always treating IBGP BFD sessions as mult-hop. shared_network variable is used to determine whether the IBGP peer is single hop or multi-hop away. The logic for determining whether EBGP peer is single hop or multi-hop has not been changed.
• Since the default behavior has been changed, it will cause interop issues between 2.5 and 3.0 IBGP BFD sessions. A new hidden command “bfd multihop/singlehop” has been introduced to overcome the interop issues.
dell-s6000-10(config-router)# neighbor 30.0.2.6 bfd
<2-255> Detect Multiplier
<cr>
dell-s6000-10(config-router)# neighbor 30.0.2.6 bfd multihop
dell-s6000-10(config-router)# no neighbor 30.0.2.6 bfd multihop
dell-s6000-10(config-router)#
dell-s6000-10(config-router)# neighbor 30.0.2.6 bfd multihop
dell-s6000-10(config-router)# do show running-config
!
router bgp 100
neighbor igroup peer-group
neighbor igroup bfd 5 500 500
neighbor igroup bfd multihop
neighbor 30.0.2.2 remote-as 100
neighbor 30.0.2.2 peer-group igroup
neighbor 3101🔤bcad::2 remote-as 100
neighbor 3101🔤bcad::2 peer-group igroup
neighbor 30.0.2.6 remote-as 200
neighbor 30.0.2.6 bfd multihop
neighbor 3102🔤bcad::6 remote-as 200
neighbor 3102🔤bcad::6 bfd
neighbor 3102🔤bcad::6 ebgp-multihop 255
!
CM-10260
Issue: “Unable to connect to socket” message keeps getting logged when ptmd process doesn’t exist.
Root Cause: BFD clients (bgpd, ospfd and ospf6d) during initialization try to register with BFD/PTM by default. This results in continuous logging If PTM does not exist since there is no max on number of retries.
Fix:
• Stop the retries to connect to PTM after max reconnect timer of 5 mins is reached.
• Added zebra debug event wrapper to message logging to prevent it from showing by default.
CM-4541
Issue: Addition of a new command "ptm-enable" or "no ptm-enable" per interface to enable/disable PTM link status checks for an interface.
Fix: Currently there is only one ptm-enable global command that enables/disables PTM status updates for all interfaces. This new command will give the handle to individually stop interface from reacting on the PTM status updates.
• by default interface uses the ptm-enable global configuration
• "no ptm-enable" on an interface will disable PTM status updates from taking affect for that interface. This can bring the interface up if it was brought down due to PTM status update.
• "ptm-enable" on an interface will cause the interface to fallback to the global ptm-enable configuration value and will bring the interface up or down based on the last stored PTM status update if global ptm is enabled.
Ticket: CM-10435, CM-10260 and CM-4541
Signed-off-by: Radhika Mahankali
Reviewed-by: Donald Sharp, Kanna Rajagopal
