this information is necessary for local information, because the
interface associated to the mac address is stored with its ifindex, and
the ifindex may not be enough to get to the right interface when it
comes with multiple network namespaces.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
when working with vrf netns backend, two bridges interfaces may have the
same bridge interface index, but not the same namespace. because in vrf
netns backend mode, a bridge slave always belong to the same network
namespace, then a check with the namespace id and the ns id of the
bridge interface permits to resolve correctly the interface pointer.
The problem could occur if a same index of two bridge interfaces can be
found on two different namespaces.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
when receiving a netlink API for an interface in a namespace, this
interface may come with LINK_NSID value, which means that the interface
has its link in an other namespace. Unfortunately, the link_nsid value
is self to that namespace, and there is a need to know what is its
associated nsid value from the default namespace point of view.
The information collected previously on each namespace, can then be
compared with that value to check if the link belongs to the default
namespace or not.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
to be able to retrieve the network namespace identifier for each
namespace, the ns id is stored in each ns context. For default
namespace, the netns id is the same as that value.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
as remind, the netns identifiers are local to a namespace. that is to
say that for instance, a vrf <vrfx> will have a netns id value in one
netns, and have an other netns id value in one other netns.
There is a need for zebra daemon to collect some cross information, like
the LINK_NETNSID information from interfaces having link layer in an
other network namespace. For that, it is needed to have a global
overview instead of a relative overview per namespace.
The first brick of this change is an API that sticks to netlink API,
that uses NETNSA_TARGET_NSID. from a given vrf vrfX, and a new vrf
created vrfY, the API returns the value of nsID from vrfX, inside the
new vrf vrfY.
The brick also gets the ns id value of default namespace in each other
namespace. An additional value in ns.h is offered, that permits to
retrieve the default namespace context.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
an incoming bridge index has been found, that is linked with vxlan
interface, and the search for that bridge interface is done. In
vrf-lite, the search is done across the same default namespace, because
bridge and vxlan may not be in the same vrf. But this behaviour is wrong
when using vrf netns backend, as the bridge and the vxlan have to be in
the same vrf ( hence in the same network namespace). To comply with
that, use the netnamespace of the vxlan interface. Like that, the
appropriate nsid is passed as parameter, and consequently, the search is
correct, and the mac address passed to BGP will be ok too.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
other network namespaces are parsed because bridge interface can be
bridged with vxlan interfaces with a link in the default vrf that hosts
l2vpn.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
With vrf-lite mechanisms, it is possible to create layer 3 vnis by
creating a bridge interface in default vr, by creating a vxlan interface
that is attached to that bridge interface, then by moving the vxlan
interface to the wished vrf.
With vrf-netns mechanism, it is slightly different since bridged
interfaces can not be separated in different network namespaces. To make
it work, the setup consists in :
- creating a vxlan interface on default vrf.
- move the vxlan interface to the wished vrf ( with an other netns)
- create a bridge interface in the wished vrf
- attach the vxlan interface to that bridged interface
from that point, if BGP is enabled to advertise vnis in default vrf,
then vxlan interfaces are discovered appropriately in other vrfs,
provided that the link interface still resides in the vrf where l2vpn is
advertised.
to import ipv4 entries from a separate vrf, into the l2vpn, the
configuration of vni in the dedicated vrf + the advertisement of ipv4
entries in bgp vrf will import the entries in the bgp l2vpn.
the modification consists in parsing the vxlan interfaces in all network
namespaces, where the link resides in the same network namespace as the
bgp core instance where bgp l2vpn is enabled.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
the link information of vxlan interface is populated in layer 2
information, as well as in layer 2 vxlan information. This information
will be used later to collect vnis that are in other network namespaces,
but where bgp evpn is enabled on main network namespaces, and those vnis
have the link information in that namespace.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When fetching the next route node in the RIB, skip the empty ones
to avoid calling other northbound callbacks later unnecessarily.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The motivation for this change is that IPv6 link-local routes don't
conform to the zebra YANG module since they all have the same prefix
(fe80::/64), but zebra's YANG module require each route to have
an unique prefix (the key of the "rib" list). This violation can
cause problems when iterating over the RIB asynchronously, so skip
those routes.
At the end of the day nobody cares about link-local routes anyway :)
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When checking if a nexthop is active, if it has been marked as onlink,
just check on the presence and status of the nexthop's interface. When
handling client request to create a route, if the client says that the
nexthop is onlink, trust it; when internally (in zebra) determining
that the nexthop is onlink, ensure it is only done in the case of an
interface with a /32 IP address which is the case for OSPF unnumbered.
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Stephen Worley <sworley@cumulusnetworks.com>
to make sure that c++ code can include them, avoid using reserved
keywords like 'delete' or 'new'.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
This commit implements:
RIB operational list create/destroy.
Walk over RIB tables using keys.
The first RIB table will be IPV4/unicast (table-id 254)
will be fetched.
Create a new api to fetch RIB table based on
afi-safi and table id as the keys.
remove mandatory true statement from the leaf which
is part of the list key.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
L2VPN PW are very hard to determine why they do not come up. The following
fixes expand the existing show commands in ldp and zebra to display a
reason why the PW is in the DOWN state and also display the labeled nexthop
route selected to reach the PW peer. By adding this information it will
provide the user some guidance on how to debug the PW issue. Also fixed an
assert if labels were changed for a PW that is between directly connected
peers.
Signed-off-by: Lynne Morrison <lynne@voltanet.io>
Loosen the ONLINK restrictions such that when an upper
level protocol sends us a nexthop with an ONLINK attribute
just ensure that interface is up and usable. ONLINK effectively
means we know what we are doing to the kernel.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
If you haven't migrated your FPM server to use next hop groups, it is
possible that you want to disable this feature. This commit implements
a toggle to enable/disable next hop groups usage (even if your Linux
kernel is not using it).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Implement the next hop group send on startup if you are using
them. Normally you will only have them if you are already using this
Linux kernel feature.
NOTE: to make sure all next hop groups exist, we send/enqueue all next
hop groups first and then we send routes. The RIB route walk start is
at the end of the function `fpm_nhg_send()`.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Force off kernel NHG install with netns-based VRFs for
now. There is not really a good solution for allowing
kernel nexthop groups in namespaced based vrfs.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
When installing a nexthop group, dump out the ifindex of the
nexthop being installed as a bit more data for the developer.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This is an implementation of the IS-IS SR draft [1] for FRR.
The following features are supported:
* IPv4 and IPv6 Prefix-SIDs;
* IPv4 and IPv6 Adj-SIDs and LAN-Adj-SIDs;
* Index and absolute labels;
* The no-php and explicit-null Prefix-SID flags;
* Full integration with the Label Manager.
Known limitations:
* No support for Anycast-SIDs;
* No support for the SID/Label Binding TLV (required for LDP interop).
* No support for persistent Adj-SIDs;
* No support for multiple SRGBs.
[1] draft-ietf-isis-segment-routing-extensions-25
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The netlink_vrf_change() function is called both when a VRF device
is created in the Linux kernel and when it is activated. This
commit changes this function to perform the VRF misconfiguration
detection only when the VRF device is created, as doing the check
twice would cause a false positive followed by a hard failure (not
to mention the double check is unnecessary since the VRF table ID
can't change once the device is created).
Fixes#6319.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Commit e93a6fbb4 from PR3908 changed every interface into an
'unnumbered' interface - even interfaces that do not have
ipv4 at all. Undo that.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
The function zebra_vxlan_print_neigh_vni_vtep does not create
a json object when json has been requested from the CLI and as a
result it prints out the information in normal CLI format.
Fix is to allocate the json object when required.
Signed-off-by: Pat Ruddy <pat@voltanet.io>
Reported by testing agency that rfc 4861 section 6.2.1 states
that all implementations must have a configuration knob to change
the setting of the advertised retransmit timer sent in RA packets.
This fix adds that capability.
Ticket: CM-29199
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Intermittently, there is a 30 second delay for a LDP pseudowire to become
operational.
One way to reproduce the issue is: Once PW is up, shutdown link to trigger
a change to the pseudowire's next hop, and then restore link to cause
pseudowire to return to original NH.
Problem Descripton:
The Zebra PW manager installs pseudowires in the data plane when the
following two conditions are met:
1. Pseudowire is labeled via LDP mapping messages
2. A labeled NH route exists to reach the remote pseudowire endpoint
The Zebra PW manager registers a NHT callback when a pseudowire is enabled.
This allows the Zebra PW manager to install or reinstall the pseudowire.
The Zebra PW manager deregisters for the NHT callback when the pseudowire is
disabled. When LDP learns the remote-pseudowire status is 'not forwarding',
LDP notifies Zebra that the pseudowire is disabled.
This creates a race condition where a new labeled NH can be resolved after the
Zebra PW manager deregistered for the NHT callback.
For static pseudowires, it makes sense for Zebra PW manager to deregister for
NHT callbacks for disabled pseudowires. Static pseudowires become disabled
via CLI configuration commands.
For LDP pseudowires, the Zebra PW manager should not deregister for NHT
callbacks for disabled pseudowires.
Overview of changes:
1. Zebra PW manager should not deregister for NHT callbacks when an LDP
pseudowire is disabled.
Zebra PW manager will register for NHT callbacks when the LDP pseudowire
is first enabled.
Zebra PW manager will deregister for NHT callbacks when the LDP
pseudowire is deleted.
2. Remove the 30 second timer that was added in PR4122.
PR4122 tried to fix this race condition with a timer.
Once we eliminate the race condition (by keeping the Zebra PW manager
registered for NHT callbacks), this timer can be removed.
3. Zebra PW manager handling of static pseudowires will remain as-is.
Zebra PW manager will register for NHT callbacks when the static
pseudowire is enabled.
Zebra PW manager will deregister for NHT callbacks when the static
pseudowire is disabled.
Signed-off-by: Lynne Morrison <lynne@voltanet.io>
Signed-off-by: Karen Schoener <karen@voltanet.io>
An async route notification can indicate that installation
has failed, but the handling code wasn't dealing with that
possibility correctly.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
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>
Currently the linux kernel allows you to specify the same
table id -> multiple vrf's. While I am arguing with
the kernel people about proper behavior here let's
just remove this as a possiblity from happening and
mark it a zebra stopable misconfiguration.
(Effectively we are preventing a crash down the line
as that all over FRR we assume it's a unique
mapping not a many to one).
Why fail hard? Because we hope to get the person
who misconfigured it to actually notice immediately
not hours or days down the line when shit hits the fan.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The function rt_netlink.c is using to lookup the vrf by
passed in table id.
I'm also going to pretend that this function is not
so awful to run when we have a large number of routes
incoming.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There are a couple of switch statements in netlink_route_info_encode
in zebra_fpm_netlink.c that had logically dead code. We have
a switch statement let's take actual advantage of it instead
of doing gyrations to what we want.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
- Fix 1 byte overflow when showing GR info in bgpd
- Use PATH_MAX for path buffers
- Use unsigned specifiers for uint16_t's in zebra pbr
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Replace sprintf with snprintf where straightforward to do so.
- sprintf's into local scope buffers of known size are replaced with the
equivalent snprintf call
- snprintf's into local scope buffers of known size that use the buffer
size expression now use sizeof(buffer)
- sprintf(buf + strlen(buf), ...) replaced with snprintf() into temp
buffer followed by strlcat
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
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>
Call the `dp_fini` callback twice: once at the beginning of the shutdown
and then again right before `exit()`ing zebra.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Coverity is complaining that we are looking beyond the end
of the pointer. Why not just use prefix_cmp here? Since
we are comparing to route_nodes.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Use the zapi client session id in the label manager apis;
use the client struct directly in some code. Assign a session
id to ldpd's sync LM zapi session.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Distinguish zapi sessions, for daemons who use more than one,
by adding a session id. The tuple of proto + instance is not
adequate to support clients who use multiple zapi sessions.
Include the id in the client show output if it's present. Add
a bit of info about this to the developer doc.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
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>
Reported by testing agency that rfc 4861 section 6.2.1 states
that all implementations must have a configuration knob to change
the setting of the advertised hop limit. This fix adds that
capability.
Ticket: CM-29200
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
The netlink_request function takes a `struct nlmsghdr *`
pointer from a common pattern that we use:
struct {
struct nlmsghdr n;
struct fib_rule_hdr frh;
char buf[NL_PKT_BUF_SIZE];
} req;
We were calling it `netlink_request(Socket, &req.n)`
The problem here is that coverity, rightly so, sees that
we access the data after the nlmsghdr in netlink_request and
tells us we have an read beyond end of the structure. While
we know we haven't mangled anything up here because of manual
inspection coverity doesn't have this knowledge implicitly.
So let's modify the code call to netlink_request to pass in the
void pointer of the req structure itself, cast to the appropriate
data structure in the function and do the right thing. Hopefully
the coverity SA will be happy and we can move on with our life.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Implement the fix made in `master` to the remain pieces of code in the
data plane FPM module.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
* Break lines longer than 80 columns.
* Remove space after '('.
* Use '%pIX' instead of 'inet_ntop'.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Store VNI information in the data plane context so we can use it to
build the FPM netlink update with that information later.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Changes:
* Let the package builder scripts know that we have a new module that
needs to be taken care of.
* Include the frr atomic header to avoid undeclared atomic operations.
* Disable build on *BSDs because the code is using some zebra netlink
functions only available for Linux.
* Move data plane FPM module outside old FPM automake definition.
* Fix atomic usage for Ubuntu 14.04 (always use explicit).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
FPM has a thread to encode and enqueue output buffer that might compete
with zebra RIB/RMAC walk on startup, so lets use atomic operations to
make sure we are not getting statistic/counters wrong.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Enqueue all contexts inside FPM to avoid losing updates and to move all
processing to the FPM thread.
This helps in situations with huge amount of routes (e.g. BGP peer
flapping with a million routes).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Add counters to debug the output buffer usage and pull down its data
when the remote receiver is slow (so we get more space for writes).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Implement the code that walks the RMAC to send routes that are already
inside installed in the OS.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Add a public reset api, so a context can be reset and reused;
add apis to init a context for a route or mac update.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Instead of retuning always `0`, lets return the amount of used bytes for
the message. This will be used by the new FPM interface to know how many
bytes we must reserve for the output buffer.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
* Use `inet_ntop` instead of `inet_ntoa`
* Replace function name with `__func__`
* Inline functions
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Generalize the netlink route message building function so it can be used
in the future by the netlink Forwarding Plane Manager (FPM) interface.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
In some places we log the interface but not the vfr the
interface is in. In others we only output the vrf id, which
can be difficult for human to read. This commit makes zebra
debugs more vrf aware.
Signed-off-by: Jakub Urbańczyk <xthaid@gmail.com>
Issue:
For consecutive messages such as
MAC1 -> VTEP1 add
MAC1 -> VTEP2 add
MAC1 -> VTEP1 add
Final state, i.e. (MAC1 -> VTEP1 add) should be sent via FPM.
But, with current code, FPM will send (MAC1 -> VTEP2 add)
RCA:
When FPM receives (MAC1, VTEP1), it stores it in the FPM processing queue and
hash table.
When FPM receives (MAC1, VTEP2), this entry is stored as another node as hash
table key is (mac, vtep and vni)
IF FPM again receives (MAC1, VTEP1), we fetch this node in the hash table
which is already enqueued.
When the FPM queue is processed, we will send FPM message for (MAC1, VTEP1)
first and then for (MAC1, VTEP2)
This sequencing issue happened because the key of the table is (MAC, VTEP, VNI)
Fix:
Change the key of the hash table to (MAC, VNI)
So, every time we receive a new update for (MAC1, VNI1), we will find a node in
the processing queue corresponding to MAC1 if present.
We will update this same node for every operation related to (MAC1, VNI1)
Thus, at the time when FPM processes this node, it will have latest MAC1 info.
Signed-off-by: Ameya Dharkar <adharkar@vmware.com>
On startup of zebra, read in all ipv4/ipv6 rules from
the kernel and remove any with the zebra proto.
If there are any, this means we failed to remove them
on shutdown due to a crash or something. Without this,
users have to manually remove them with iproute2 or some
such and its really annoying.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Define some explicit rule replace code paths into the dataplane
code and improve the handling around it/releasing the the old
rule from the hash table.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
When default route is requested from client, default
route is sent to client if present. When route gets
deleted then delete is sent to clients.
Signed-off-by: Santosh P K <sapk@vmware.com>
zebra should only check whether a get_chunk operation succeeded
when processing the response, rather than insde the get_chunk
call itself. Spllitting the request and response hooks was done
precisely to allow for asynchronous calls to an external label
manager; in this case, the requested chunk is not necessarily
going to be available at request time.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Free unhashable (duplicate NHEs from the kernel) via ID table
cleanup. Since the NHE ID hash table contains extra entries,
that's the one we need to be calling zebra_nhg_hash_free()
on, otherwise we will never free the unhashable NHEs.
This was found via a memleak:
==1478713== HEAP SUMMARY:
==1478713== in use at exit: 10,267 bytes in 46 blocks
==1478713== total heap usage: 76,810 allocs, 76,764 frees, 3,901,237 bytes allocated
==1478713==
==1478713== 208 (88 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 35 of 41
==1478713== at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==1478713== by 0x48E35E8: qcalloc (memory.c:110)
==1478713== by 0x451CCB: zebra_nhg_alloc (zebra_nhg.c:369)
==1478713== by 0x453DE3: zebra_nhg_copy (zebra_nhg.c:379)
==1478713== by 0x452670: nhg_ctx_process_new (zebra_nhg.c:1143)
==1478713== by 0x4523A8: nhg_ctx_process (zebra_nhg.c:1234)
==1478713== by 0x452A2D: zebra_nhg_kernel_find (zebra_nhg.c:1294)
==1478713== by 0x4326E0: netlink_nexthop_change (rt_netlink.c:2433)
==1478713== by 0x427320: netlink_parse_info (kernel_netlink.c:945)
==1478713== by 0x432DAD: netlink_nexthop_read (rt_netlink.c:2488)
==1478713== by 0x41B600: interface_list (if_netlink.c:1486)
==1478713== by 0x457275: zebra_ns_enable (zebra_ns.c:127)
Repro with:
ip next add id 1 blackhole
ip next add id 2 blackhole
valgrind /usr/lib/frr/zebra
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
The rtadv code has two types of sockets:
a) namespace -> Where each zvrf get's it's own socket
b) vrf lite -> Where we get 1 socket for everything
When we were terminating a vrf we were *always*
killing the (b) socket. This is a mistake in
that other vrf's may need to be communicating.
Modify the code on vrf shutdown to only disable
that vrf's event processing and when we actually
terminate we shut the socket.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We don't want to install backup nexthops - yet - as part of the
nexthop-id-based kernel interactions on netlink platforms. Avoid
mixing backup and primary nexthops in the tree of dependencies
in the ecmp cases.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Include backup nexthops in nhe processing; connect incoming
zapi route data with updated rib/nhg apis; add more debugs in
nhg processing.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Refactor the detailed route debugging so that the dump of nexthops
can be used for both normal/active nexthops and backups (if they
are present).
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Use a backup index in a nexthop directly (if it has a backup
nexthop); revise the zebra nhe/nhg code; revise zapi route
decoding to match; revise the dataplane route datastructs.
Refactor some of the rib_add_multipath code to be prepared to
be called with an nhe, carrying nexthop and (possibly) backup
info together.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Use const with some args to ipaddr, zebra vxlan, mpls
lsp, and nexthop apis; add some extra checks to some
nexthop-related apis.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
If we find that a nexthop is a duplicate, break immediately
rather than continuing to look through the rest of the list.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Properly set the NEXTHOP_GROUP_VALID flag and use it
as a conditional for installation decisions for individual
nexthop and groups containing it.
We set the NEXTHOP_GROUP_VALID flag it is:
1) A fully resolved active nexthop
or
2) Its a group that contains at least one VALID NHE
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
We were still doing a lookup on the nhe_id from before we
started referencing re->nhe directly.
Change set flag to just use re->nhe directly here since they
should always be the same at this point in the code anyway.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
When we find a nexthop ID thats a duplicate in the code that converts
NHG rb trees into a flat list of nexthop IDs for the dataplane,
output a debug message.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
When we transform the nexthop group rb trees into a flat
array of IDs to send into the dataplane code (zebra_nhg_nhe2grp),
don't put an ID in there that has not been in installed or is
not currently queued to be installed into the dataplane.
Otherwise, if some of the nexthops fail to install, we will
still try to create a group with them and then the entire group
will fail.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
We were not properly handling the case of a NHG inside of
another NHG when converting the rb tree of a multilevel NHG
into a flat list of IDs. When constructing, we call the function
zebra_nhg_nhe2grp_internal() recursively so that the rare
case of a group within a group is handled such that its
singleton nexthops are appended to the grp array of IDs
we send to the dataplane code.
Ex)
1:
-> 2:
-> 3
-> 4
->5:
->6
becomes this:
1:
->3
->4
->6
when its sent to the dataplane code for final kernel installation.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
In the netlink code for determining whether to set
a src on the route, we check if the cmd=NEW_ROUTE
but its not possible for this to ever be anything
but a new route since we do a goto skip further up
if its a DEL_ROUTE cmd.
So remove this unnecessary check.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Determine src based on nexthop data even when we are using
kernel nexthop objects.
Before, we were entirely skipping this step and just sending the
nexthop ID, ignoring src determination.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Abstraction the route src determination from a nexthop in the
netlink code into a function for both singlepath and mutlipath
to call.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Line break at the end of the message is implicit for zlog_* and flog_*,
don't put it in the string. Mid-message line breaks are currently
unsupported. (LF is "end of message" in syslog.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
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>
Zebra is currently sending messages on interface add/delete/update,
VRF add/delete, and interface address change - regardless of whether
its clients had requested them. This is problematic for lde and isis,
which only listens to label chunk messages, and only when it is
waiting for one (synchronous client). The effect is the that messages
accumulate on the lde synchronous message queue.
With this change:
- Zebra does not send unsolicited messages to synchronous clients.
- Synchronous clients send a ZEBRA_HELLO to zebra.
The ZEBRA_HELLO contains a new boolean field: sychronous.
- LDP and PIM have been updated to send a ZEBRA_HELLO for their
synchronous clients.
Signed-off-by: Karen Schoener <karen@voltanet.io>
We currently have netlink_neigh_update_ctx,
netlink_vxlan_flood_update_ctx and netlink_macfdb_update_ctx
all of which do slightly different RTM_NEWNEIGH calls into
the kernel. After this change, there will be one common
function.
Signed-off-by: Jakub Urbańczyk <xthaid@gmail.com>
1) When programming a nhg id to the kernel we had no debug of that
is what we are doing.
2) Add debugs to all nexthop information to allow us to follow
which prefix we are talking about. This is especially
useful when dealing with a large number of routes and
you want to grep out one or two too see what is going on.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ensure that any weight associated with the next hop is installed for
IPv4 routes with IPv6 next hops too.
Updates: lib, zebra: Allow for installation of a weighted nexthop
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Add to the ZEBRA_INTERFACE_BFD_DEST_UPDATE code path
in zebra_ptm_redistribute.c the missing c-bit data.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Problem: While zebra going down, rmap update thread is being called as part of
timer event. This make zebra to crash.
RCA: At this time route_map_master_hash is made to 0 by sig int handler.
This is causing Zebrad to crash while executing rmap update thread
Fix: As part of SIGINT handler, before calling routemap_finish,
thread off any routemap update scheduled at that point and make sure that
it wont get scheduled again by making the timeout as 0.
Signed-off-by: Saravanan K <saravanank@vmware.com>
The return type of is_selfroute function is changed from int to bool.
Also remove the redundant invoking of the is_selfroute function in the
calling function netlink_route_change_read_unicast
Fixes: https://github.com/FRRouting/frr/issues/5984
Signed-off-by: Naveen Naidu <naveennaidu479@gmail.com>
Readd the special MAC that represents the flood (head-end replication) entry
for EVPN-VxLAN upon getting a delete notification for it.
Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by: Chirag Shah <chirag@cumulusnetworks.com>
Ticket: CM-25797
Ticket: CM-26238
Testing Done:
1. evpn-min, evpn-smoke - results summarized in CM-25798
add debug trace in specific neigh request send api
to help debug an issue where synchronous response parse
returns with NLMSG_DONE where there is no ipv6 neigh received.
the count value is set to 1 because the request contained
a spcific neigh.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Memory leak found where ipv6 global prefixes added to the router
advertisement prefix lists were not deleted when the process was
killed.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Add a common api that formats a time interval into a string
with different output for short and longer intervals. We do
this in several places, for cli/ui output.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Upper level clients ask for default routes of a particular family
This change ensures that they only receive the family that they
have asked for.
Discovered when testing in ospf `default-information originate`
=================================================================
==246306==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffa2e8 at pc 0x7ffff73c44e2 bp 0x7fffffffa090 sp 0x7fffffffa088
READ of size 16 at 0x7fffffffa2e8 thread T0
#0 0x7ffff73c44e1 in prefix_copy lib/prefix.c:310
#1 0x7ffff741c0aa in route_node_lookup lib/table.c:255
#2 0x5555556cd263 in ospf_external_info_delete ospfd/ospf_asbr.c:178
#3 0x5555556a47cc in ospf_zebra_read_route ospfd/ospf_zebra.c:852
#4 0x7ffff746f5d8 in zclient_read lib/zclient.c:3028
#5 0x7ffff742fc91 in thread_call lib/thread.c:1549
#6 0x7ffff7374642 in frr_run lib/libfrr.c:1093
#7 0x5555555bfaef in main ospfd/ospf_main.c:235
#8 0x7ffff70a2bba in __libc_start_main ../csu/libc-start.c:308
#9 0x5555555bf499 in _start (/usr/lib/frr/ospfd+0x6b499)
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This patch does two things:
1) Ensure the decoding of stream data between pim <-> zebra is properly
decoded and we don't read beyond the end of the stream.
2) In zebra when we are freeing memory alloced ensure that we
actually have memory to delete before we do so.
Ticket: CM-27055
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This code is effectively dead code. SO_PEERCRED is a getsockopt
call not *setsockopt* call. Additionally we are not doing
anything with the failed setsockopt call at all.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There was some code missed during the upstreaming process
due to code squash. Identify and put into a commit
to keep code consistent and correct.
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
this flag can be used when one routing daemon wants to force his route
to be injected prioritary with other routes, including selected routes.
for that, do not forget to update the new_selected pointer in the zebra
nexthop tracking algorithm.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The handlers for a couple of the main LSP-oriented zapi
messages explicitly limited themselves to a single out-label.
Allow multiple labels if the sender ... sends them.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
We made the decision to explicitly trust kernel and system routes
of every other type with 058c16b7e2.
So, we should trust directly connected routes the same way, assuming
the interface exists.
Old Behavior:
K 2.2.2.1/32 [0/0] is directly connected, unknown inactive, 00:00:39
New Behavior:
K>* 2.2.2.1/32 [0/0] is directly connected, test1, 00:00:03
As a bonus, this fixes the issues we were seeing with not removing
directly connected routes of certain interface types when
those interfaces go down/are deleted.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
It's been a year search and destroy.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Embed nexthop-group, which is just a pointer, in the zebra
nexthop-hash-entry object, rather than mallocing one.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
When a client connects to zebra with GR capabilities and
then restarts, it might disconnect again even before hello is
sent leading zebra cores.
GR should be supported only for dynamic neighbor who are capable
of restarting.
Signed-off-by: Santosh P K <sapk@vmware.com>
Somewhat gnarly code flow here that might be leaking memory - can't tell
if it's a test artifact or not, but in any case this reduces the
situations in which we need to alloc a block.
And we don't need to check XCALLOC for success...
And we don't need to null check before XFREE...
Or set XFREE'd pointers to NULL...
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Add in a few missing stub route-advert functions; these are
needed to build frr with v6 route adverts disabled.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
As part of checksum calculation for a received packet we were
comparing the checksum returned from in_cksum. Typically
when we calculate the checksum the value stored in the checksum
must be all 0's. Store the received checksum and then set
the checksum to 0 and then compare.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
In several places we would send debug messages for failure situations
that really should be errors.
Signed-off-by: Donald Sharpd <sharpd@cumulusnetworks.com>
Using SO_BROADCAST, in the linux kernel, requires a uint32_t to be passed
in for all SOL_SOCKET calls. Modify code to use it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Use the zapi_nexthop struct with the mpls_labels
zapi messages instead of the special-purpose (and
more limited) nexthop struct that was being used.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
RFC 4861 states that ipv6 RA messages sent out an interface should
contain all global ipv6 addresses on that interface. This fix adds
that capability. To override the default flags and timer settings
for a particular prefix, the existing "ipv6 nd prefix ..." command
should be used via vtysh under the appropriate interface.
Ticket: CM-20363
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Today vtysh can show the ip/ip6 routes through several commands:
- show_route_cmd
- show_route_detail_cmd
- show_route_summary_cmd
- show_route_table_cmd
- show_route_table_vrf_cmd
- show_route_all_table_vrf_cmd
Each command has its own set of filter rules:
- show_route_cmd can filter by vrf, protocol, tag, ... but not by table
- show_route_table_cmd always filter by table
- show_route_table_vrf_cmd always filter by table and can filter by vrf
too
- show_route_all_table_vrf_cmd show all route in any table for a vrf (or
all)
To reduce the number of commands and provide a possibility to filter by
any key add possibility for the show_route_cmd to filter by table with a
specific value or all to get route in all tables.
Then the show_route_table_cmd, show_route_table_vrf_cmd and
show_route_all_table_vrf_cmd functions can be removed as they are covered
by the generic show_route_cmd function.
It is to be noted that when zebra is started by default, it is possible
to execute show ip route command with both vrf and table parameters,
whereas before the command was not displayed. This is due to the fact
that this combination is only permitted when zebra is launched with vrf
network namespace mode. There, if zebra is configured with vrf-lite
backend, then a vty error message informs the user that the combination
of both table and vrf is not possible.
Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
The existing behavior is when a remote VTEP is deleted,
its associatedneighbor (arp) and MAC entries are removed from
zebra database and do not wait for explicit type-2 route
withdraw from originating VTEP.
Remote type-2 route delete checks if VTEP is present before
removing the entry.
The behavior works fine when all evpn routes points to the
same nexthop as the VTEP IP.
In MLAG topology with advertise-pip, self type-2 and type-5 routes
are advertised with individual VTEP IP as nexthop ip for the route.
When a new VNI is created, it is assigned individual IP as tunnel-ip
then it transition to anycast IP (of the MLAG). During the transition,
type-3 route (VTEP delete) withdraw is sent for the individual IP.
The remote VTEP delete should not trigger to remove evpn routes pointing
to VTEP IP. Instead the route will be removed via explicit withdraw.
Ticket:CM-27752
Reviewed By:CCR-9722
Testing Done:
In evpn with MLAG deployment with advertise-pip and advertise-svi-ip
enabled, validated remote vtep delete does not remove self type-2 routes
from zebra DB. Upon explicit type-2 withdraw routes are removed.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Use a hash walker/iterator instead of a temporary list to
show zebra's nexthop-groups/nexthop-hash-entries.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
The top variable has already been derefed by the time we get
to the test to see if it is non-NULL. No need to check it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Nexthop groups as a whole do not make sense to have a vrf'ness
As that you can have a arbitrary number of nexthops that point
to separate vrf's.
Modify the code to make this distinction, by clearly delineating
the line between the nhg and the nexthop a bit better.
Nexthop groups having a vrf_id only make sense if you are using
network namespaces to represent them.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The zebra implementation of nexthop groups has
two types of nexthops groups currently. Singleton
objects which have afi's and combined nexthop groups
that do not. Specifically call this out in the code
to make this distinction.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Handling capability received from client. It may contain
GR enable/disable, Stale time changes, RIB update complete
for given AFi, ASAFI and instance. It also has changes for
stale route handling.
Signed-off-by: Santosh P K <sapk@vmware.com>
Add a null check in `handle_recursive_depend()` so it
doesn't try to add a NULL pointer to the RB tree.
This was found with clang SA.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
We were not resetting the nexthop pointer to NULL for each
new read of a nexthop from the zapi route. On the chance we
get a nexthop that does not have a proper type, we will not
create a new nexthop and update that pointer, thus it still
has the last valid one and will create a group with two
pointers to the same nexthop.
Then when it enters any code that iterates the group, it loops
endlessly.
This was found with zapi fuzzing.
```
0x00007f728891f1c3 in jhash2 (k=<optimized out>, length=<optimized out>, initval=12183506) at lib/jhash.c:138
0x00007f728896d92c in nexthop_hash (nexthop=<optimized out>) at lib/nexthop.c:563
0x00007f7288979ece in nexthop_group_hash (nhg=<optimized out>) at lib/nexthop_group.c:394
0x0000000000621036 in zebra_nhg_hash_key (arg=<optimized out>) at zebra/zebra_nhg.c:356
0x00007f72888ec0e1 in hash_get (hash=<optimized out>, data=0x7ffffb94aef0, alloc_func=0x0) at lib/hash.c:138
0x00007f72888ee118 in hash_lookup (hash=0x7f7288de2f10, data=0x7f728908e7fc) at lib/hash.c:183
0x0000000000626613 in zebra_nhg_find (nhe=0x7ffffb94b080, id=0, nhg=0x6020000032d0, nhg_depends=0x0, vrf_id=<optimized out>,
afi=<optimized out>, type=<optimized out>) at zebra/zebra_nhg.c:541
0x0000000000625f39 in zebra_nhg_rib_find (id=0, nhg=<optimized out>, rt_afi=AFI_IP) at zebra/zebra_nhg.c:1126
0x000000000065f953 in rib_add_multipath (afi=AFI_IP, safi=<optimized out>, p=0x7ffffb94b370, src_p=0x0, re=0x6070000013d0,
ng=0x7f728908e7fc) at zebra/zebra_rib.c:2616
0x0000000000768f90 in zread_route_add (client=0x61f000000080, hdr=<optimized out>, msg=<optimized out>, zvrf=<optimized out>)
at zebra/zapi_msg.c:1596
0x000000000077c135 in zserv_handle_commands (client=<optimized out>, msg=0x61b000000780) at zebra/zapi_msg.c:2636
0x0000000000575e1f in main (argc=<optimized out>, argv=<optimized out>) at zebra/main.c:309
```
```
(gdb) p *nhg->nexthop
$4 = {next = 0x5488e0, prev = 0x5488e0, vrf_id = 16843009, ifindex = 16843009, type = NEXTHOP_TYPE_IFINDEX, flags = 8 '\b', {gate = {ipv4 = {s_addr = 0},
ipv6 = {__in6_u = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}},
bh_type = BLACKHOLE_UNSPEC}, src = {ipv4 = {s_addr = 0}, ipv6 = {__in6_u = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0, 0, 0, 0, 0, 0,
0}, __u6_addr32 = {0, 0, 0, 0}}}}, rmap_src = {ipv4 = {s_addr = 0}, ipv6 = {__in6_u = {__u6_addr8 = '\000' <repeats 15 times>, __u6_addr16 = {0, 0,
0, 0, 0, 0, 0, 0}, __u6_addr32 = {0, 0, 0, 0}}}}, resolved = 0x0, rparent = 0x0, nh_label_type = ZEBRA_LSP_NONE, nh_label = 0x0, weight = 1 '\001'}
(gdb) quit
```
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Since we are using a UNIQUE RB tree, we need to handle the
case of adding in a duplicate entry into it.
The list API code returns NULL when a successfull add
occurs, so lets pull that handling further up into
the connected handlers. Then, free the allocated
connected struct if it is a duplicate.
This is a pretty unlikely situation to happen.
Also, pull up the RB handling of _del RB API as well.
This was found with the zapi fuzzing code.
```
==1052840==
==1052840== 200 bytes in 5 blocks are definitely lost in loss record 545 of 663
==1052840== at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==1052840== by 0x48E1008: qcalloc (memory.c:110)
==1052840== by 0x44D357: nhg_connected_new (zebra_nhg.c:73)
==1052840== by 0x44D300: nhg_connected_tree_add_nhe (zebra_nhg.c:123)
==1052840== by 0x44FBDC: depends_add (zebra_nhg.c:1077)
==1052840== by 0x44FD62: depends_find_add (zebra_nhg.c:1090)
==1052840== by 0x44E46D: zebra_nhg_find (zebra_nhg.c:567)
==1052840== by 0x44E1FE: zebra_nhg_rib_find (zebra_nhg.c:1126)
==1052840== by 0x45AD3D: rib_add_multipath (zebra_rib.c:2616)
==1052840== by 0x4977DC: zread_route_add (zapi_msg.c:1596)
==1052840== by 0x49ABB9: zserv_handle_commands (zapi_msg.c:2636)
==1052840== by 0x428B11: main (main.c:309)
```
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Zebra will have special handling for clients with GR enabled.
When client disconnects with GR enabled, then a stale client
will be created and its RIB will be retained till stale timer
or client comes up and updated its RIB.
Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: Soman K S <somanks@vmware.com>
Signed-off-by: Santosh P K <sapk@vmware.com>
Adding header files changes where structure to hold
received graceful restart info from client is defined.
Also there are changes for show commands where exisiting
commands are extended.
Co-authored-by: Santosh P K <sapk@vmware.com>
Co-authored-by: Soman K S <somanks@vmware.com>
Signed-off-by: Santosh P K <sapk@vmware.com>
Add a config that disables use of kernel-level nexthop ids.
Currently, zebra always uses nexthop ids if the kernel supports
them.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
When we are receiving a kernel route, with an admin distance
of 255 we are not marking it as installed. This route
should be marked as installed.
New behavior:
K>* 4.5.7.0/24 [255/8192] via 192.168.209.1, enp0s8, 00:10:14
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
commit: 0eb97b860d
Removed this chunk of code in zebra:
- if (ifp)
- if (connected_is_unnumbered(ifp))
- SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK);
Effectively if we had a NEXTHOP_TYPE_IPV4_IFINDEX we would
auto set the onlink flag. This commit dropped it for some reason.
Add it back in an intelligent manner.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
My previous patch to fix a memory leak, caused by not properly freeing
the iptable iface list on stream parse failure, created/exposed a heap
use after free because we were not doing a deep copy
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
With recent changes to the lib nexthop_group
APIs (e1f3a8eb19), we are making
new assumptions that this should be adding a single nexthop
to a group, not a list of nexthops.
This broke the case of a recursive nexthop resolving to a group:
```
D> 2.2.2.1/32 [150/0] via 1.1.1.1 (recursive), 00:00:09
* via 1.1.1.1, dummy1 onlink, 00:00:09
via 1.1.1.2 (recursive), 00:00:09
* via 1.1.1.2, dummy2 onlink, 00:00:09
D> 3.3.3.1/32 [150/0] via 2.2.2.1 (recursive), 00:00:04
* via 1.1.1.1, dummy1 onlink, 00:00:04
K * 10.0.0.0/8 [0/1] via 172.27.227.148, tun0, 00:00:21
```
This group can instead just directly point to the nh that was passed.
Its only being used for a lookup (the memory gets copied and used
elsewhere if the nexthop is not found).
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Make the nexthop_copy/nexthop_dup APIs more consistent by
adding a secondary, non-recursive, version of them. Before,
it was inconsistent whether the APIs were expected to copy
recursive info or not. Make it clear now that the default is
recursive info is copied unless the _no_recurse() version is
called. These APIs are not heavily used so it is fine to
change them for now.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
cb86eba3ab was causing zebra to crash
when handling a nexthop group that had a nexthop which was recursively resolved.
Steps to recreate:
!
nexthop-group red
nexthop 1.1.1.1
nexthop 1.1.1.2
!
sharp install routes 8.8.8.1 nexthop-group red 1
=========================================
==11898== Invalid write of size 8
==11898== at 0x48E53B4: _nexthop_add_sorted (nexthop_group.c:254)
==11898== by 0x48E5336: nexthop_group_add_sorted (nexthop_group.c:296)
==11898== by 0x453593: handle_recursive_depend (zebra_nhg.c:481)
==11898== by 0x451CA8: zebra_nhg_find (zebra_nhg.c:572)
==11898== by 0x4530FB: zebra_nhg_find_nexthop (zebra_nhg.c:597)
==11898== by 0x4536B4: depends_find (zebra_nhg.c:1065)
==11898== by 0x453526: depends_find_add (zebra_nhg.c:1087)
==11898== by 0x451C4D: zebra_nhg_find (zebra_nhg.c:567)
==11898== by 0x4519DE: zebra_nhg_rib_find (zebra_nhg.c:1126)
==11898== by 0x452268: nexthop_active_update (zebra_nhg.c:1729)
==11898== by 0x461517: rib_process (zebra_rib.c:1049)
==11898== by 0x4610C8: process_subq_route (zebra_rib.c:1967)
==11898== Address 0x0 is not stack'd, malloc'd or (recently) free'd
Zebra crashes because we weren't handling the case of the depend nexthop
being recursive.
For this case, we cannot make the function more efficient. A nexthop
could resolve to a group of any size, thus we need allocs/frees.
To solve this and retain the goal of the original patch, we separate out the
two cases so it will still be more efficient if the nexthop is not recursive.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
The only two safi's that are usable for zebra for installation
of routes into the rib are SAFI_UNICAST and SAFI_MULTICAST.
The acceptance of other safi's is causing a memory leak:
Direct leak of 56 byte(s) in 1 object(s) allocated from:
#0 0x5332f2 in calloc (/usr/lib/frr/zebra+0x5332f2)
#1 0x7f594adc29db in qcalloc /opt/build/frr/lib/memory.c:110:27
#2 0x686849 in zebra_vrf_get_table_with_table_id /opt/build/frr/zebra/zebra_vrf.c:390:11
#3 0x65a245 in rib_add_multipath /opt/build/frr/zebra/zebra_rib.c:2591:10
#4 0x7211bc in zread_route_add /opt/build/frr/zebra/zapi_msg.c:1616:8
#5 0x73063c in zserv_handle_commands /opt/build/frr/zebra/zapi_msg.c:2682:2
Collapse
Sequence of events:
Upon vrf creation there is a zvrf->table[afi][safi] data structure
that tables are auto created for. These tables only create SAFI_UNICAST
and SAFI_MULTICAST tables. Since these are the only safi types that
are zebra can actually work on. zvrf data structures also have a
zvrf->otable data structure that tracks in a RB tree other tables
that are created ( say you have routes stuck in any random table
in the 32bit route table space in linux ). This data structure is
only used if the lookup in zvrf->table[afi][safi] fails.
After creation if we pass a route down from an upper level protocol
that has non unicast or multicast safi *but* has the actual
tableid of the vrf we are in, the initial lookup will always
return NULL leaving us to look in the otable. This will create
a data structure to track this data.
If after this event you pass in a second route with the same
afi/safi/table_id, the otable will be created and attempted
to be stored, but the RB_TREE_UNIQ data structure when it sees
this will return the original otable returned and the lookup function
zebra_vrf_get_table_with_table_id will just drop the second otable.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
==25402==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x533302 in calloc (/usr/lib/frr/zebra+0x533302)
#1 0x7fee84cdc80b in qcalloc /home/qlyoung/frr/lib/memory.c:110:27
#2 0x5a3032 in create_label_chunk /home/qlyoung/frr/zebra/label_manager.c:188:3
#3 0x5a3c2b in assign_label_chunk /home/qlyoung/frr/zebra/label_manager.c:354:8
#4 0x5a2a38 in label_manager_get_chunk /home/qlyoung/frr/zebra/label_manager.c:424:9
#5 0x5a1412 in hook_call_lm_get_chunk /home/qlyoung/frr/zebra/label_manager.c:60:1
#6 0x5a1412 in lm_get_chunk_call /home/qlyoung/frr/zebra/label_manager.c:81:2
#7 0x72a234 in zread_get_label_chunk /home/qlyoung/frr/zebra/zapi_msg.c:2026:2
#8 0x72a234 in zread_label_manager_request /home/qlyoung/frr/zebra/zapi_msg.c:2073:4
#9 0x73150c in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2688:2
When creating label chunk that has a specified base, we eventually are
calling assign_specific_label_chunk. This function finds the appropriate
list node and deletes it from the lbl_mgr.lc_list but since
the function uses list_delete_node() the deletion function that is
specified for lbl_mgr.lc_list is not called thus dropping the memory.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Signed-off-by: Donald Sharp <sharpd@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>
Previous patches introduced various issues:
- Removal of stream_free() to fix double free caused memleak
- Patch for memleak was incomplete
This should fix it hopefully.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The existing usage of the rta_nest and addattr_nest
functions were not adding the NLA_F_NESTED flag
to the type. As such the new nexthop functionality was
actually looking for this flag, while apparently older
code did not.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Add error handling for top level failures (not able to
execute command, unable to find vrf for command, etc.)
With this error handling we add a new zapi message type
of ZEBRA_ERROR used when we are unable to properly handle
a zapi command and pass it down into the lower level code.
In the event of this, we reply with a message of type
enum zebra_error_types containing the error type.
The sent packet will look like so:
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Length | Marker | Version |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| VRF ID |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Command |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| ERROR TYPE |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Also add appropriate hooks for clients to subscribe to for
handling these types of errors.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
There's confusion between the nexthop-group configuration and a
zebra-specific show command. For now, make the zebra show
command string RIB-specific until we're able to unify these
paths.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
=================================================================
==3058==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000010 (pc 0x7f5bf3ef7477 bp 0x7ffdfaa20d40 sp 0x7ffdfaa204c8 T0)
==3058==The signal is caused by a READ memory access.
==3058==Hint: address points to the zero page.
#0 0x7f5bf3ef7476 in memcpy /build/glibc-OTsEL5/glibc-2.27/string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:134
#1 0x4d158a in __asan_memcpy (/usr/lib/frr/zebra+0x4d158a)
#2 0x7f5bf58da8ad in stream_put /home/qlyoung/frr/lib/stream.c:605:3
#3 0x67d428 in zsend_ipset_entry_notify_owner /home/qlyoung/frr/zebra/zapi_msg.c:851:2
#4 0x5c70b3 in zebra_pbr_add_ipset_entry /home/qlyoung/frr/zebra/zebra_pbr.c
#5 0x68e1bb in zread_ipset_entry /home/qlyoung/frr/zebra/zapi_msg.c:2465:4
#6 0x68f958 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
#7 0x55666d in main /home/qlyoung/frr/zebra/main.c:309:2
#8 0x7f5bf3e5db96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#9 0x4311d9 in _start (/usr/lib/frr/zebra+0x4311d9)
the ipset->backpointer was NULL as that the hash lookup failed to find
anything. Prevent this crash from happening.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The decoding of _add and _del functions is practically identical
do a bit of work and make them so.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
=================================================================
==13611==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe9e5c8694 at pc 0x0000004d18ac bp 0x7ffe9e5c8330 sp 0x7ffe9e5c7ae0
WRITE of size 17 at 0x7ffe9e5c8694 thread T0
#0 0x4d18ab in __asan_memcpy (/usr/lib/frr/zebra+0x4d18ab)
#1 0x7f16f04bd97f in stream_get2 /home/qlyoung/frr/lib/stream.c:277:2
#2 0x6410ec in zebra_vxlan_remote_macip_del /home/qlyoung/frr/zebra/zebra_vxlan.c:7718:4
#3 0x68fa98 in zserv_handle_commands /home/qlyoung/frr/zebra/zapi_msg.c:2611:3
#4 0x556add in main /home/qlyoung/frr/zebra/main.c:309:2
#5 0x7f16eea3bb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
#6 0x431249 in _start (/usr/lib/frr/zebra+0x431249)
This decode is the result of a buffer overflow because we are
not checking ipa_len.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The linux kernel will occassionally send RTM_GETNEIGH when
it expects user space to help in resolution of an ARP entry.
See linux kernel commit:
commit 3e25c65ed085b361cc91a8f02e028f1158c9f255
Author: Tim Gardner <tim.gardner@canonical.com>
Date: Thu Aug 29 06:38:47 2013 -0600
net: neighbour: Remove CONFIG_ARPD
Since we don't care about this, let's just safely ignore this
message for the moment. I imagine in the future we might
care when we implement neighbor managment in the system.
Reported By: Stefan Priebe <s.priebe@profihost.ag>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There may be logic to prevent this ever happening earlier in the network
read path, but it doesn't hurt to double check it here, because clearly
deeper paths rely on this being the case.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Whatever this BFD re-transmission function is had a few problems.
1. Used memcpy instead of the (more concise) stream APIs, which include
bounds checking.
2. Did not sufficiently check packet sizes.
Actually, 2) is mitigated but is still a problem, because the BFD header
is 2 bytes larger than the "normal" ZAPI header, while the overall
message size remains the same. So if the source message being duplicated
is actually right up against the ZAPI_MAX_PACKET_SIZ, you still can't
fit the whole message into your duplicated message. I have no idea what
the intent was here but at least there's a warning if it happens now.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
- Fix iptable freeing code to free malloc'd list
- malloc iptable in zapi handler and use those functions to free it when
done to fix a linked list memleak
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
We copy a fixed length buffer from the wire but don't ensure it is null
terminated. Then print it as a c-string. Lul.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
further down we hash the src & dst ip, which asserts that the afi is one
of the well known ones, given the field names i assume the correct afis
here are af_inet[6]
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
zebra can catch the kernel's route deletion by netlink.
but current FRR can't delete kernel-route on vrf(l3mdev)
when kernel operator delete the route on out-side of FRR.
It looks problem about kernel-route deletion.
This problem is caused around _nexthop_cmp_no_labels(nh1,nh2)
that checks the each nexthop's member 'vrf_id'.
And _nexthop_cmp_no_labels's caller doesn't set the vrf_id
of nexthop structure. This commit fix that case.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
router-id is buried deep in "show running-config", this new
command makes it easy to retrieve the user configured router-id.
Example:
# configure terminal
(config)# router-id 1.2.3.4
(config)# end
# show router-id
router-id 1.2.3.4
# configure terminal
(config)# no router-id 1.2.3.4
(config)# end
# show router-id
#
Signed-off-by: Jafar Al-Gharaibeh <jafar@atcorp.com>
We were not setting the RTNH_F_ONLINK flag where appropriate
when creating nexthop objects in the kernel.
Set it on the nhmsg.nh_flags netlink message.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
When we are doing a lookup on an individual nexthop,
we should still be passing along the type that gets passed
via the arguments. Otherwise, we will always think we own that
NHE when in reality anyone could have put that into the
kernel.
Before this patch, nexthops in the kernel will get swepped
out even if we didn't create them.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
We should be NULL checking the entire re->nhe struct, not
the group inside of it. When we get routes from the kernel
using a nexthop group (and future protocols) they will only
pass us an ID to use. Hence, this struct can (and will be)
NULL on first attach when only passed an ID.
There shouldn't be a situation where we have an re->nhe
and don't have an re->nhe->nhg anyway.
Before this patch you can easily make zebra crash by creating a
route in the kernel using a nexthop group and starting zebra.
`ip next add dev lo id 111`
`ip route add 1.1.1.1/32 nhid 111`
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Older versions of protobuf-c do not support version 3 of the
protocol. Add a check into the system to see if we have
version 3 available and if so, compile it in.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
If you compile FRR with no j factor zebra_mlag.c fails to
build because the vtysh extraction methodology runs first
before the protobuf compiler runs and that compilation does
not have the proper dependancy chain built for the inclusions
that zebra_mlag.c had. Moving the DEF* code into a zebra_mlag_vty.c
which can be included in the vtysh extraction code and has
no mlag.proto dependancies makes the compilation work better.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Handle the special case where a route update contains
no installed nexthops - that means the route is not
installed.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
This is pretty much just to get rid of the HAVE_CUMULUS. The
hook/module API is as "wtf" as it was before...
Signed-off-by: David Lamparter <equinox@diac24.net>
Add an api that creates a copy of a list of nexthops and
enforces the canonical sort ordering; consolidate some nhg
code to avoid copy-and-paste. The zebra dplane uses
that api when a plugin sets up a list of nexthops, ensuring
that the plugin's list is ordered when it's processed in
zebra.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
The processing of dataplane route notifications was a little
off-target after the nexthop-group re-work. This should allow
notifications to work better.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Linux has the idea of allowing a weight to be sent
down as part of a nexthop group to allow the kernel
to weight particular nexthop paths a bit more or less
than others.
See:
http://tldp.org/HOWTO/Adv-Routing-HOWTO/lartc.rpdb.multiple-links.html
Allow for installation into the kernel using the weight attribute
associated with the nexthop.
This code is foundational in that it just sets up the ability
to do this, we do not use it yet. Further commits will
allow for the pass through of this data from upper level protocols.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
