Ticket: CM-15658
Reviewed By: CCR-6534
Testing Done: Unit
Issue: frr ptm-enable command not working for interfaces that have been created by frr as a place holder.
Root Cause: The ptm-enable on interface configuration was not getting stored when the interface was internally created by frr.
Fix: Store the ptm-enable configuration even if the interface is internally created.
Signed-off-by: Radhika Mahankali <radhika@cumulusnetworks.com>
* Rename client_connect and client_close hooks to zapi_client_connect
and zapi_client_close
* Remove some more unnecessary headers
* Fix a copy-paste error in zapi_msg.[ch] header comments
* Fix an inclusion comment in zserv.c
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
zserv.c was using hardcoded callbacks to clean up various components
when a client disconnected. Ergo zserv.c had to know about all these
unrelated components that it should not care about. We have hooks now,
let's use the proper thing instead.
Signed-off-by: Quentin Young <qlyoung@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>
Group send and receive functions together, change handlers to take a
message instead of looking at ->ibuf and ->obuf, allow zebra to read
multiple packets off the wire at a time.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
A lot of the handler functions that are called directly from the ZAPI
input processing code take different argument sets where they don't need
to. These functions are called from only one place and all have the same
fundamental information available to them to do their work. There is no
need to specialize what information is passed to them; it is cleaner and
easier to understand when they all accept the same base set of
information and extract what they need inline.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
All of the ZAPI message handlers return an integer that means different
things to each of them, but nobody ever reads these integers, so this is
technical debt that we can just eliminate outright.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
1) Add asserts in a couple of spots to show we
never expect prefix to be bad.
2) Fix some bfd code where out_ctxt will
always be NULL.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The code change to switch from stream_getX to STREAM_GETX added
a goto statement to be handled for a failure case. The failure
case was properly handled but the normal case was not tested
properly and there exists a situation where we would free
the out_ctxt 2 times. Prevent that from happening.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This is a preparatory work for configuring vrf/frr over netns
vrf structure is being changed to 32 bit, and the VRF will have the
possibility to have a backend made up of NETNS.
Let's put some history.
Initially the 32 bit was because one wanted to map on vrf_id both the
VRFLITE and the NSID.
Initially, one would have liked to make zebra configure at the same time
both vrf lite and vrf from netns in a flat way. From the show
running perspective, one would have had both kind of vrfs, thatone
would configure on the same way.
however, it leads to inconsistencies in concepts, because it mixes vrf
vrf with vrf, and vrf is not always mapped with netns.
For instance, logical-router could also be used with netns. In that
case, it would not be possible to map vrf with netns.
There was an other reason why 32 bit is proposed. this is because
some systems handle NSID to 32 bits. As vrf lite exists only on
Linux, there are other systems that would like to use an other vrf
backend than vrf lite. The netns backend for vrf will be used for that
too. for instance, for windows or freebsd, some similar
netns concept exists; so it will be easier to reuse netns
backend for vrf, than reusing vrflite backend for vrf.
This commit is here to extend vrf_id to 32 bits. Following commits in a
second step will help in enable a VRF backend.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When we get a STREAM_GET failure of some sort we
need to handle the failure case here and safely
free up stored memory/context and return gracefully.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This code modifies zebra to use the STREAM_GET functionality.
This will allow zebra to continue functioning in the case of
bad input data from higher level protocols instead of crashing.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The ptm code when it encountered an error situation
was not fully reading all the data in the stream
meant for it. Ensure that this is read.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This improves code readability and also future-proofs our codebase
against new changes in the data structure used to store interfaces.
The FOR_ALL_INTERFACES_ADDRESSES macro was also moved to lib/ but
for now only babeld is using it.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This is an important optimization for users running FRR on systems with
a large number of interfaces (e.g. thousands of tunnels). Red-black
trees scale much better than sorted linked-lists and also store the
elements in an ordered way (contrary to hash tables).
This is a big patch but the interesting bits are all in lib/if.[ch].
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This fixes the broken indentation of several foreach loops throughout
the code.
From clang's documentation[1]:
ForEachMacros: A vector of macros that should be interpreted as foreach
loops instead of as function calls.
[1] http://clang.llvm.org/docs/ClangFormatStyleOptions.html
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
1) Various socket close issues
2) Ensure afi passed is usable
3) Fix some reads beyond buffer and reads after free
4) Ensure some failure modes are handled properly
5) Memory Leak(s) fix
6) There is no 6.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This reverts commit c14777c6bf.
clang 5 is not widely available enough for people to indent with. This
is particularly problematic when rebasing/adjusting branches.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
All PIM Neighbors for a given pim interface is registered with
BFD.
Upon receiving BFD status down event, PIM Neighbor with BFD info is deleted.
Add pim bfd configuraiton (CLI) per interface, '[no] ip pim bfd'
Testing Done:
Configure BFD under PIM interface on all neighbor routers,
check bfd sessions up, remote end unconfigure BFD, results in BFD session down.
Previous state was UP to New state DOWN, results in PIM neighbor delete behind
that particular pim interface.
Pim-smoke Results:
Ran 94 tests in 7409.680s
FAILED (SKIP=8, failures=2)
Signed-off-by: Chirag Shah <chirag@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>
Pass pointer to pointer instead of assigning by return value. See
previous commit message.
To ensure that the behavior stays functionally correct, any assignments
with the result of a thread_add* function have been transformed to set
the pointer to null before passing it. These can be removed wherever the
pointer is known to already be null.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The way thread.c is written, a caller who wishes to be able to cancel a
thread or avoid scheduling it twice must keep a reference to the thread.
Typically this is done with a long lived pointer whose value is checked
for null in order to know if the thread is currently scheduled. The
check-and-schedule idiom is so common that several wrapper macros in
thread.h existed solely to provide it.
This patch removes those macros and adds a new parameter to all
thread_add_* functions which is a pointer to the struct thread * to
store the result of a scheduling call. If the value passed is non-null,
the thread will only be scheduled if the value is null. This helps with
consistency.
A Coccinelle spatch has been used to transform code of the form:
if (t == NULL)
t = thread_add_* (...)
to the form
thread_add_* (..., &t)
The THREAD_ON macros have also been transformed to the underlying
thread.c calls.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Don't leak a socket when we are unable to set it
as non-blocking and warn the user as appropriate.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-13425
Reviewed By: Donald, Kanna
Testing Done: Unit, Min tests, PTM Smoke and Nightly, BGP Smoke
Issue: BFD status up/down not reflected in the Quagga for non-default VRF single-hop BFD sessions.
Root Cause: PTM doesn’t keep track of VRF for Single hop BFD sessions since they are interface-based sessions. The status up/down messages to the quagga for single hop sessions do not have VRF information. In zebra daemon, the interface search based on the interface name extracted from the BFD status message is done across all VRFs. So, the search does not fail in zebra daemon. But, the interface search in bgpd/ospd is done per vrf and default VRF is used for search if no VRF is sent in the status message. So, the search fails and the BFD status changes are ignored.
Fix: The VRF information is extracted from the interface if VRF is not sent in the BFD status messages in zebra daemon and passed to bgpd/ospfd. The interface search will not fail since the appropriate VRF is passed to bgpd/ospfd and BFD satus changes are not ignored.
Signed-off-by: Radhika Mahankali <radhika@cumulusnetworks.com>
Cleanup 2 spots in the code:
1) In if_netlink.c -> combine multi-line if statement into 1 line
2) In zebra_ptm.c -> only handle code that needs to be inside the if statement
in the if statement.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Several places have paths and names that can change hardcoded, e.g. the
package name and the /var/run path. This fixes a few of them, there's
still some to do.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
There's no need to duplicate the 'vrf_id' and 'name' fields from the 'vrf'
structure into the 'zebra_vrf' structure. Instead of that, add a back
pointer in 'zebra_vrf' that should point to the associated 'vrf' structure.
Additionally, modify the vrf callbacks to pass the whole vrf structure
as a parameter. This allow us to make further simplifications in the code.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This puts all the proper VTY_DECLVAR_CONTEXT calls in place, removing
all vty->index uses in the process. With that, vty_config_lockless can
be enabled in zebra.
[v2: fix ordering screw-up in "interface XXX" command]
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Ticket: CM-11256
Signed-off-by: Radhika Mahankali <radhika@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Kanna Rajagopal <kanna@cumulusnetworks.com>
Testing: Unit, PTM smoke, OSPF smoke, BGP Smoke
Issue:
BFD client registrations are not being sent to PTM from BGP/OSPF clients when the quagga clients have no BFD configuration. This can create stale BFD sessions in PTM when BFD is removed from quagga configuration before quagga is restarted.
BFD client de-registrations from BGP/OSPF also go missing sometimes when quagga is restarted. This also will cause stale BFD sessions in PTM.
Root Cause:
BFD client registrations were being sent at the time of BGP/OSPF daemon initialization. But, they were being sent to zebra before the socket connection between zebra and BGP/OSPF was established. This causes the missing BFD client registrations.
BFD client de-registrations are sent from zebra when zebra detects socket close for BGP/OSPF daemons. Based on the timing, the de-registrations may happen after socket between PTM and zebra is closed. This will result in missing de-registrations.
Fix:
Moved sending of BFD client registration messages to zebra connected callback to make sure that they are sent after the BGP/OSPF daemons connect with zebra.
Added BFD client de-registrations for BGP/OSPF to be also sent when zebra daemon gets restart signal. They are sent from the signal handler only if it was not already handled in zebra client socket close callback.
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
The default vrf name choosen in quagga has no meaning.
Don't pass it to the ptm process as it gets dizzy.
Ticket: CM-10581
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Kanna Rajagopal <kanna@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
Zebra is crashing inside of zserv_rnh_register when it attempts to
lookup a vrf that was just deleted:
Imagine this series of events:
Pre (A): ifdown -a -X eth0
(A) zebra notification from kernel that vrf is goneroo
(B) zebra notifies all daemons
(C) bgpd is churning because we have been removing interfaces and we
have an new path and it hasn't handled the vrf goneroo event yet from
zebra so it sends to zebra a new rnh with an old vrf_id.
(D) zebra attempts to lookup the zvrf and crashes because of pointer
dereference.
zebra handles all callbacks in one function. Convert that function to
check to see if we have a valid zvrf. If so make the callback
Ticket: CM-10482
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Move zebra_vrf_XXX functionality into it's own
file so that we can isolate a bit the api edges
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
We were including 'extern struct zebra_t zebrad;' all
over the place. This made no sense. Refactor
into zserv.h where the definition was and remove resulting
unnecessary code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
The file if.c has a iflist that had the list of interfaces
in the default vrf. Remove this variable and replace
with a vrf_iflist lookup on the default vrf where it
was used.
Additionally, modify ptm code to iterate over all vrf's
when enabling ptm.
Ticket: CM-10338
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: Radhika Mahankali <radhika@cumulusnetworks.com>
There are cases where we get an interface name but do not have a
corresponding vrf. We care about getting an interface pointer
so just provide a function that searches all vrf's for the ifp.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Radhicak Mahankali <radhika@cumulusnetworks.com>
Reviewed-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
