Donald Sharp [Fri, 25 May 2018 18:45:16 +0000 (14:45 -0400)]
zebra: Add a breadcrumb for when we ignore a route
When we receive a route that we think we own and we
are not in startup conditions, then add a small debug
to help debug the issue when this happens, instead
of silently just ignoring the route.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 25 May 2018 18:36:12 +0000 (14:36 -0400)]
tools, zebra: Use different protocol value for our statics
The re-use of RTPROT_STATIC has caused too many collisions
where other legitimate route sources are causing us to
believe we are the originator of the route. Modify
the code so that if another protocol inserts RTPROT_STATIC
we will assume it's a Kernel Route.
Fixes: #2293 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Pascal Mathis [Wed, 16 May 2018 19:55:55 +0000 (21:55 +0200)]
bgpd: Fix memleak, adapt adv- to recv-routes code
This commit tries to adapt a similar codeflow within the `show bgp [afi]
[safi] neighbor <neighbor> advertised-routes` command compared to its
`received-routes` and `filtered-routes` opponents. Some branching code
has been restructured to achieve this.
Additionally, this commit fixes a memory leak within `received-routes`
(and `filtered-routes`, although the issue has been present before the
previous commit!) where the previous implementation forgot to
deduplicate the BGP attributes.
When a user called `<...> received-routes route-map <RM-TEST>` and that
routemap changed any AS path or community parameters, the duplicated
memory for these parameters was never freed. This has been fixed by
ensuring to call `bgp_attr_undup()` accordingly.
Pascal Mathis [Wed, 16 May 2018 17:17:42 +0000 (19:17 +0200)]
bgpd: Implement new adjacent route show commands
This commit changes the behavior of `show bgp [afi] [safi] neighbor
<neighbor> received-routes [json]` to return all received prefixes
instead of filtering rejected/denied prefixes.
Compared to Cisco and Juniper products, this is the usual way how this
command is supposed to work, as `show bgp [afi] [safi] neighbor
<neighbor> routes` will already return all accepted prefixes.
Additionally, the new command `show bgp [afi] [safi] neighbor <neighbor>
filtered-routes` has been added, which returns a list of all prefixes
that got filtered away, so it can be roughly described as a subset of
"received prefixes - accepted prefixes".
As the already available `filtered_count` variable inside
`show_adj_route` has not been used before, the last output line
summarizing the amount of prefixes found was extended to also mention
the amount of filtered prefixes if present.
Pascal Mathis [Tue, 15 May 2018 17:22:25 +0000 (19:22 +0200)]
bgpd: Improve route-map matching for INET(6) AF
While the current implementation does pay attention to the AF
(inet/inet6) when comparing the IPv4/v6 address against an address-list
/ prefix-list inside a route-map, the AF check is being done rather
late, which leads to CPU cycles being wasted due to unnecessary list
lookups / address matching.
This commit checks the address family of a prefix right inside the
`route_match_ip(v6)_` functions before looking up any address- and/or
prefix-list, which should improve performance.
Pascal Mathis [Mon, 14 May 2018 20:52:31 +0000 (22:52 +0200)]
bgpd: Respect AFI/SAFI when hard-clearing a peer
The current implementation does not respect the AFI+SAFI combination of
a peer when executing a non-soft (hard) clear. An example would be the
command `clear bgp ipv4 unicast *`, which will clear all BGP peers, even
those that do not have IPv4-Unicast activated.
This commit fixes that behavior by applying the same rules to both soft
and hard clear commands, so that peers without a matching AFI+SAFI
combination will be no longer modified.
Additionally, this commit adds warning messages to all `clear bgp
[<afi>] [<safi>] <target>` commands when no matching peers with the given
AFI+SAFI combination could be found.
Both existing and new warning messages have been extended to also
mention the AFI+SAFI combination that is missing, which is more helpful
to the user than a generic expression 'No peer configured'.
Pascal Mathis [Sun, 13 May 2018 00:29:40 +0000 (02:29 +0200)]
bgpd: Improve JSON support for large communities
The current implementation of building JSON output is greatly different
for large communities compared to standard communities. This is mainly
noticeable by the missing 'list' attribute, which usually offers an
array of all communities present on a BGP route.
This commit adds the missing functionality of properly returning a
'list' attribute in JSON output and also tries a similar approach like
the standard communities are using to implement this feature.
Additionally, the 'format' specifier has been completely removed from
large communities string/JSON rendering, as the official RFC8092 specifies that
there is only one canonical representation:
> The canonical representation of BGP Large Communities is three
> separate unsigned integers in decimal notation in the following
> order: Global Administrator, Local Data 1, Local Data 2. Numbers
> MUST NOT contain leading zeros; a zero value MUST be represented with
> a single zero. Each number is separated from the next by a single
> colon. For example: 64496:4294967295:2, 64496:0:0.
As the 'format' specifier has not been used/checked and only one
canonical representation exists per today, there was no reason to keep
the 'format' parameter in the function signature.
Last but not least, the struct attribute 'community_entry.config' is no
longer being used for large communities and instead 'lcommunity_str' is
being called to maintain a similar approach to standard communities.
As a side effect, this also fixed a memory leak inside 'community_entry_free'
which did not free the allocated memory for the 'config' attribute when
dealing with a large community.
Pascal Mathis [Sun, 13 May 2018 17:11:43 +0000 (19:11 +0200)]
lib: Moved no-password warnings into header file
The warning string which appears when the users executes 'no (enable)
password' was moved into command.h and declared as a constant named
'NO_PASSWD_CMD_WARNING'.
This avoids duplicate code and makes it easy to change the warning
message in all places at once.
Pascal Mathis [Sat, 12 May 2018 18:19:49 +0000 (20:19 +0200)]
lib: Improved warnings for 'no (enable) password'
When the user executes one of the commands 'no password' or 'no enable
password', a warning message gets shown to inform the user of the
security implications.
While the current implementation works, a warning message gets printed
once for each daemon, which can lead to seeing the same message many
times. This does not affect functionality, but looks like an error to
the user as it can be seen within issue #1432.
This commit only prints the warning message inside lib when vtysh
dispatch is not being used. Additionally, the warning message was copied
into the vtysh command handlers, so that they get printed exactly once.
Pascal Mathis [Fri, 11 May 2018 00:54:30 +0000 (02:54 +0200)]
lib: Ported 'no (enable) password' from stable/3.0
The pull request #1545 from @donaldsharp introduced the command 'no
password' to remove an existing terminal connection password.
Additionally, warnings have been added to both 'no password' and 'no
enable password' to make the user aware of any security implications.
It seems that this specific pull request was never merged against master
and got lost. This commit is a cherry-pick of d4961273cb with fixed
conflicts and updated documentation.
Thanks to @donaldsharp and @pogojotz for the original PR.
Pascal Mathis [Thu, 10 May 2018 22:03:23 +0000 (00:03 +0200)]
debianpkg: improve VTYSH_PAGER environment check
The current post-installation scripts for all Debian packages execute
grep 'VTYSH_PAGER=/bin/cat' to check if the VTYSH_PAGER variable is
present within /etc/environment.
While presence of that environment variable should be checked, the
current implementation does not handle this line being a comment (and
therefor not active) or the user picking a different VTYSH_PAGER than
/bin/cat.
This commit ensures that the environment variable can be freely changed
by the user, while still guaranteeing that it is present in the file
without being a comment.
Pascal Mathis [Thu, 10 May 2018 21:35:37 +0000 (23:35 +0200)]
bgpd: fix and improve snmp peer lookups
The previous implementation of bgp_peer_lookup_next did not consider the
internal ordering of peers when using peer groups, which led to all
standalone peers being skipped that had a lower IP address than the
highest IP address of a peer belonging to a group.
As the ordering of peers can not be arbitrary due to SNMP requiring
increasing OIDs when walking an OID tree, this commit fixes the bug by
properly looping through all peers and detecting the next highest IP
address.
Additionally, this commit improved both bgp_peer_lookup_next and
peer_lookup_addr_ipv4 by using the socketunion stored within the peer
struct (peer->su) instead of calling inet_pton for each peer during
comparison.
Fredi Raspall [Wed, 2 May 2018 14:30:26 +0000 (16:30 +0200)]
bgpd/bgp_zebra.c: Fix process of label-chunk msg
All messages to/from the label manager include two additional
fields: protocol and instance. This patch fixes the parsing
of label chunks response used by BGPd, which did not consider
the two fields.
The current implementation did not consider multiple clients to
a label-manager acting as proxy, i.e. relaying messages to another
label manager. Specifically, upon a client's request, it checked
the socket & buffer from the actual label manager for pending
responses and directly copìed them to the client --currently--
being served. As a result, if two clients (e.g. ldpd and bgpd)
sent requests, it could happen that responses being 'on the wire'
from the real label manager towards the proxy, where relayed to
the wrong client. This patch, which requires all msgs to include
a a proto & instance pair, lookups up the zserv client that a
message (response) is to be relayed to.
Add client proto and instance number in all msg (request and
responses) to/form a label manager. This is required for a
label manager acting as 'proxy' (i.e. relaying messages towards
another label manager) to correctly deliver responses to the
requesting clients.
There are cases when switching from one netns to an other one, where the
if_table registration by index has not been flushed. This fix mitigates
the potential crashes, in case the ifp->node pointer is null, the value
is overwritten by the route_node obtained.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
zebra: avoid case where same interface pointer returned
When checking for a duplicate interface in an other NETNS, one may find
an interface in default VRF. That interface may have been moved to that
default VRF, for further action. Prevent from doing any action at this
point.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Tue, 24 Apr 2018 14:35:00 +0000 (16:35 +0200)]
bgpd: handle bgp pbr hash list destroy upon BGP destroy
Upon BGP destroy, the hash list related to PBR are removed.
The pbr_match entries, as well as the contained pbr_match_entries
entries.
Then the pbr_action entries. The order is important, since the former
are referencing pbr_action. So the references must be removed, prior to
remove pbr action.
Also, the zebra associated contexts are removed.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Wed, 28 Mar 2018 12:51:57 +0000 (14:51 +0200)]
bgpd: handle FS redirect IP rule in PBR
If a new rule is identified, a new table identifier is created.
In that table, add a default route when possible. If redirect IP rule is
identified, then add a default route to that IP address.
If redirect VRF is identified, nothing is done for now
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Mon, 12 Mar 2018 11:56:06 +0000 (12:56 +0100)]
bgpd: hash_lookup for iptables
This commit is reading the installed2 value from bgp_pbr_match hash set.
Once value matches with the one received, the walk stops and the last
bgp_pbr_match structure is stored in a static entry, so that the entry
is obtained.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Mon, 12 Mar 2018 08:38:53 +0000 (09:38 +0100)]
bgpd: handle configuration of iptables with zebra
The API for filling in an IPTABLE_ADD and IPTABLE_DELETE message.
Also, the API is handling the notification callback, so as to know if
zebra managed to add or delete the relevant iptable entry.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd: add function handling flowspec entries to pass to zebra
Add a policy-route API to handle flowspec entry.
The entry is analysed, converted, and
selected if it is possible to inject the flowspec entry in local policy
routing entries.
redirect IP and redirect VRF actions are handled. The former extracts
the IPv4 address to redirect traffic to. The latter calculates the
matching VRF to redirect traffic to.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd: add convert function from flowspec to pbr match
This utility function analyses flowspec nlri and converts it into
readable structures. The structure is based on bgp_pbr_match structure
previously defined.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This structure is the model exchange between some bgp services like
flowspec and the policy routing service. This structure reflects what
the nlri entry means. To handle that structure, a dump routine is made
available. Also, a validation function is here to cancel a policy route
installation, whenever it is not possible to install the requested
policy routing.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd: support for policy-routing context used with flowspec
BGP flowspec will be able to inject or remove policy-routing contexts,
thanks to some protocols like flowspec. This commit adds some the APIS
necessary to create/delete policy routing contexts that will be injected
then into zebra.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Donald Sharp [Wed, 2 May 2018 03:24:53 +0000 (23:24 -0400)]
zebra: Fix crash on *BSD
The zns->ns pointer is not created until we get a callback
from the kernel that a ns exists. This should potentially
fix a crash in the *BSD code path.
Fixes: #2152 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Sun, 29 Apr 2018 16:40:12 +0000 (12:40 -0400)]
lib: Convert internal sequence number to int64_t
With the usage of a 32 bit number as a integer, but storing
non-signed values in it, we have cases where numbers greater
than 2 billion are being read in and stored and used before
lower value numbers, which of course is awful and mean.
Fixes: #2126 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
G. Paul Ziemba [Mon, 9 Apr 2018 20:28:11 +0000 (13:28 -0700)]
bgpd: when showing routes, add nexthop vrf and announce-self flag
As part of recent vpn-vrf leaking changes, it is now possible for a
route to refer to a nexthop in a different vrf. There is also a new
route flag that means "when announcing this route, indicate myself
as the next-hop."
route_vty_out(): nexthops are appended with:
"@VRFID" (where VRFID is the numerical vrf id) when different from
the route's vrf;
"<" when the route's BGP_INFO_ANNC_NH_SELF is set
This change also shows the route table's vrf id in the table header.
route_vty_out_detail(): show nexthop's vrf and announce-nh-self flag if
appropriate.
Both functions are also augmented to add json elements nhVrfId, nhVrfName,
and announceNexthopSelf as appropriate.
The intent of these changes is to make it easier to understand/debug
the relationship between a route and its nexthops.
Donald Sharp [Fri, 27 Apr 2018 19:18:41 +0000 (15:18 -0400)]
*: Move sharpd from developmental build to have to explicity enable it
sharpd has started to see some use from our field engineers as
well as people attempting to build/test their environments
as a way of easily injecting a large number of routes.
Modify configure.ac to move sharpd from a development build
option to having to explicity enable it via `--enable-sharpd=yes`
in order for it to be built.
This will allow those who want to build it, to build it without
having to use the development build option.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 27 Apr 2018 18:53:46 +0000 (14:53 -0400)]
zebra: Add PBR and SHARP handling
We are missing some handling of PBR and SHARP protocols
for netlink operations w/ the linux kernel.
Additionally add a bread crumb for new developers( or existing )
to know to fixup the rt_netlink.c when we start handling new
route types to hand to the kernel.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Quentin Young [Wed, 25 Apr 2018 21:16:55 +0000 (17:16 -0400)]
lib: fix heap corruption in stream_fifo_free
When popping a stream from a stream_fifo, the stream->next pointer is
not NULL'd out. If this same stream is subsequently pushed onto a
stream_fifo (either the same one or a different one), because
stream_fifo's use tail insertion the ->next pointer is not updated and
thus will point to whatever the next stream in the first stream_fifo
was. stream_fifo_free does not check the count of the stream_fifo when
freeing its constituent elements, and instead walks the linked list.
Consequently it will continue walking into the first stream_fifo from
which the last stream was popped, freeing each stream contained there.
This leads to use-after-free errors.
This patch makes sure to set the ->next pointer to NULL when doing tail
insertion in stream_fifo_push and when popping a stream from a
stream_fifo.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Donald Sharp [Wed, 25 Apr 2018 14:23:22 +0000 (10:23 -0400)]
bgpd: Prevent vrf 2 vrf route leaking from going offbox.
The vrf 2 vrf route leaking auto-derives RD and RT and
installs the routes into the appropriate vpn table.
These routes when a operator configured ipv[4|6] vpn
neighbors were showing up off box. The RD and RT
values choosen are localy significant but globaly
useless and may cause confusion.
Put a special bit of code in to notice that we
should not be advertising these routes off box.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Tue, 17 Apr 2018 13:21:24 +0000 (09:21 -0400)]
bgpd: Eliminate loop over afi's for vpn vrf leaking commands
The loop over all afi's implies that these commands actually need
to loop over all afi's to check the vpn policy. We know the
appropriate afi based upon the node we are in. So just return
the correct afi to look at and then just apply it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>