Renato Westphal [Tue, 7 Feb 2017 11:47:37 +0000 (09:47 -0200)]
zebra: fix installation of LDP labels on static routes
If the ifindex of a nexthop is not zero, we can't assume that its type is
NEXTHOP_TYPE_IPV4_IFINDEX or NEXTHOP_TYPE_IPV6_IFINDEX. Nexthops of type
NEXTHOP_TYPE_IPV[46] can have their ifindex set by the nexthop_active()
function.
With that said, we need to me more flexible when comparing nexthops
on mpls_ftn_update() to make sure that we'll always find the correct
route/nexthop to update.
Regression introduced by commit 88d88a ("zebra/ldpd: allow MPLS ECMP on
unnumbered interfaces") and found by ANVL.
Renato Westphal [Fri, 3 Feb 2017 13:09:27 +0000 (11:09 -0200)]
ldpd: update local labels when necessary
ldpd allocates null labels for directly connected routes. If a connected
route is removed (interface goes down) and an IGP learned route takes its
place in the RIB, ldpd must update the local label of the associated FEC
entry with a non-null label. The same applies for the other way around
(an interface goes up and a connected route is selected in favour of an
IGP route). Labels should be dynamic and change when necessary.
Additionally, this patch fixes the processing of route delete messages
from zebra. Route delete messages don't contain any nexthop, meaning that
whenever we receive such messages we must delete all nexthop previously
received.
Based on a patch from Bingen Eguzkitza <bingen@voltanet.io>. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Renato Westphal [Fri, 27 Jan 2017 16:22:47 +0000 (14:22 -0200)]
ldpd: introduce advanced filtering capabilities
This patch introduces several new configuration commands to ldpd. These
commands should allow the operator to define advanced filtering policies
for things like label advertisement, label allocation, etc.
Renato Westphal [Wed, 1 Feb 2017 11:51:54 +0000 (09:51 -0200)]
ldpd: add synchronous IPC channels
By default all ldpd interprocess communication is asynchronous
(non-blocking socketpairs). Under some circumstances, however, we'll
need synchronous IPC as well. Examples:
* the lde child process requesting labels to zebra (through the parent
process);
* apply an access-list on a given IP prefix (ACLs are only available in
the parent process).
This patch only adds the necessary infrastructure to allow the child
processes to make synchronous requests to the parent process. Later
patches will make use of this new infrastructure.
Renato Westphal [Wed, 1 Feb 2017 15:03:53 +0000 (13:03 -0200)]
ldpd: fix a bug in the explicit-null command
Be more clever and update the label of the connected routes just once upon
a configuration change. This is not only more efficient but also fixes
a bug where no labels were updated when we didn't have any neighbors.
David Lamparter [Wed, 1 Feb 2017 12:40:00 +0000 (13:40 +0100)]
bgpd: clear bgp_master at exit to help valgrind
bgp_master can retain pointers keeping allocated structures "reachable"
in valgrind. Clear to 0 to tell valgrind that everything should've been
freed really.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Don Slice [Thu, 26 Jan 2017 20:49:00 +0000 (12:49 -0800)]
zebra: pass correct parameters to remove static routes with tags
Problem found in testing where certain "no ip route ... tag x" commands
would fail. This was due to a change in tag processing where the tag
value is validated and previously some of the parameters were passed
incorrectly. This caused the validation to fail. This change ensures
the correct parameters are passed for evaluation as tags. Manual testing
completed and the previously failing test now passes. bgp and ospf smoke
tests will also be performed before pushing.
Ticket: CM-14605 Signed-off-by: Don Slice <dslice@cumulusnetworks.com> Reviewed-by: CCR-5627
Dinesh G Dutt [Wed, 25 Jan 2017 19:55:02 +0000 (11:55 -0800)]
tools: Silly typo in regex for catching ip route syntax
Ticket: CM-14600
Reviewed By: CCR-5615
Testing Done: Verifying the issue with/without the fix
I had intended the regexp to catch both ip and ipv6 routes, but somewhere
along the way, I left out the grouping in the regexp to catch if it was
ip or ipv6 at the start. This caused all the rest of the matches and replaces
to be off causing the issue reported by the bug.
Don Slice [Fri, 13 Jan 2017 19:23:03 +0000 (11:23 -0800)]
zebra: send ipv4 singlepath delete messages to kernel without nexthop
Problem reported was stale routes left in the kernel in certain cases
when overlapping static routes were used and links were bounced. The
problem was determined to be an issue where the nexthop was changed
due to recursion as the link is going down, and the next-hop at the
time of deletion doesn't match what was previously installed by the
kernel. This caused the kernel to reject the deletion and the route
stuck around.
It was pointed out that the kernel doesn't actually require a next-hop
value on the netlink deletion call. In this fix, we are eliminating
the nexthop for RTM_DELROUTE messages to the kernel in the ipv4 singlepath
case. This approach could also be valid for other cases but the fix
as is resolved the reported failure case. More testing should be
performed before similar changes are made for other cases.
Testing included manual testing for the failure condition as well as
complete bgp-smoke and ospf-smoke tests with no new failures.
Ticket: CM-13328 Signed-off-by: Don Slice <dslice@cumulusnetworks.com> Reviewed-by: CCR-5562
radhika [Thu, 19 Jan 2017 22:23:21 +0000 (14:23 -0800)]
bgpd: Update BFD status when de-registering with PTM
Ticket: CM-14301
Reviewed By: Donald, Vivek
Testing Done: Unit, Min tests, PTM and BFD Smoke, BGP Smoke
Issue: BFD status is not changed from up to down for bgp peer even after the BFD session has been de-registered by bgpd.
Root Cause: This issue happens when bgpd detects peer down before getting the BFD down from PTM. bgpd will send the de-reg bfd session message to ptm after detecting peer down. If ptm receives the de-reg message before it detects the peer down, then the down message will not be sent to Quagga. This causes the bfd status in bgpd to remain unchanged.
Fix: Update the BFD status to down in bgpd before de-registering the session from PTM.
The JSON output of 'bgp neighbor show' lacked the decoded strings for
the last notification error code/subcode. Decoding these strings outside
quagga is painful, and then needs to match with any updates to the codes
from RFCs/drafts. Further, all apps that look to understanding this need
to then add their own decoders for these strings.
Just add the decoded strings to the JSON output as well. JSON key name
for this is 'lastNotificationReason'.
'ip route show' displays all routes as belonging to protocol zebra.
The user has to run an additional command (in vtysh) to get the actual
source of a route (bgp/ospf/static etc.). This patch addresses that by
pushing the appropriate protocol string into the protocol field of the
netlink route update message. Now you can see routes with the correct
origin as well as filter on them (ip route show proto ospf).
'ospf' is used for both IPv4 and IPv6 routes, even though the OSPF
version is different in both cases.
Sample output (old):
9.9.12.13 via 69.254.2.38 dev swp3.2 proto zebra metric 20
9.9.13.3 proto zebra metric 20
nexthop via 69.254.2.30 dev swp1.2 weight 1
nexthop via 69.254.2.34 dev swp2.2 weight 1
nexthop via 69.254.2.38 dev swp3.2 weight 1
Sample output (new):
9.9.12.13 via 69.254.2.38 dev swp3.2 proto bgp metric 20
9.9.13.3 proto bgp metric 20
nexthop via 69.254.2.30 dev swp1.2 weight 1
nexthop via 69.254.2.34 dev swp2.2 weight 1
nexthop via 69.254.2.38 dev swp3.2 weight 1
If the user specifies a network statement such as "network 1.1.1.1/24",
the running config shows this as "network 1.1.1.0/24" which causes
unnecessary withdrawl of the prefix and re-advertisement causing
perturbations. The same thing applies to prefix-lists and of course, IPv6
addresses.
IPv6 addresses were being normalized already, and so we use that same
function to handle the IPv6 portion of the issue. Interestingly community
strings were also getting ensnared in the normalized IPv6 function due to
the presence of ':', but thats OK.
quagga's running config changes 'null0' and 'blackhole' keywords into 'Null0'.
For example: ip route 10.1.1.0/24 blackhole' is displayed as
'ip route 10.1.1.0/24 Null0'. Reload mistakes this and issues a delete of the
Null0 route followed by an add of the "blackhole" route. Unnecessary, and
results in unexpected routing perturabations.
Also fix prefix-list's le/ge behavior: It always prints ge first even if the
user has specified le followed by ge, and it doesn't print l3 32/128 if ge
is also specified, else it prints them.
Don Slice [Thu, 5 Jan 2017 18:49:23 +0000 (10:49 -0800)]
bgpd: do not allow prefix length on update-source
A common misconfiguration that was silently accepted but wouldn't work
was including the prefix length on the update-source; i.e. 10.1.1.1/32.
This commit rejects this config and notifies the user.
Ticket: CM-13485 Signed-off-by: Don Slice <dslice@cumulusnetworks.com. Reviewed-by: Donald Sharp
Quagga reload handles deletes of entire context by issuing a "no.." on the
entire context itself instead of deleting each individual line of the context.
However, this doesn't work when the context is "interface" since its not
allowed to do "no interface <ifname>" inside quagga. So special case this by
deleting each individual line of the context.
If users specify "ip prefix-list FOO permit 1.2.3.4/24", running config
displays that line as "ip prefix-list FOO seq 5 permit 1.2.3.4/24", which
causes reload to delete the running config line and add back the one in
the config.
Dinesh G Dutt [Fri, 6 Jan 2017 02:49:13 +0000 (18:49 -0800)]
tools: Don't overwrite Quagga.conf on reload, unless user specified.
Ticket: CM-14059
Reviewed By: CCR-5524
Testing Done: the usual
At some point in the 3.x release cycle, it was decided to overwrite the
user's Quagga.conf configuration file with the output of running config
when the user did a quagga reload. This is problematic for several reasons
such as: losing user-specified comments, upsetting network automation
scripts which think some thing has changed all the time from the specified
config etc.
This patch fixes this issue by not overwriting the Quagga.conf file unless
the user specifies it via an additional option, or the file being used as
input to quagga reload isn't the default configuration file (incl. path).
radhika [Fri, 6 Jan 2017 20:54:25 +0000 (12:54 -0800)]
bgpd, zebra: Fix for ignored non-default VRF single-hop BFD status messages in Quagga
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.
Dinesh G Dutt [Tue, 13 Dec 2016 10:46:52 +0000 (02:46 -0800)]
tools: Fix unnecessary routing perturbations due to old style config
Ticket: CM-14060
Reviewed By:
Testing Done:
There are two harmful problems (cause routing changes in the network) with the
2.5.x style config:
one with the old style specification of "multipath as-relax", and
the other with ip import-table, used by redistribute neighbor
In 2.5, we had the user specify 'no-as-set' as the suffix to
'bgp bestpath as-path multipath relax' to avoid quagga's default behavior
which'd cause weird routing problems. However, in 3.x, we made 'no-as-set' as
the default, and so its neither required to specify it nor is it shown in the
running config. This means when we do quagga reload, we remove the multipath
as-relax line and add it back with the no-as-set line. This causes all BGP
sessions to be reset.
The problem with the "ip import-table" is that it causes us to unimport the
routes and then add it back again, causing routing prefix changes throughout
the network, potentially causing blackholing of traffic.
This fix addresses both these issues and avoids the unnecessary routing blips.
Donald Sharp [Tue, 24 Jan 2017 21:11:40 +0000 (16:11 -0500)]
pimd: Fix handling of *,G on RP to allow prune of S,G to go out
When on the RP we received a prune *,G for an established S,G
If join_desired is no longer true we need to prune and
reset some timers, in addition to removing the inherited
interface from the olist.
This was not happening because we were just removing
the inherited oif from the *,G.
Ticket: CM-14561 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 12 Dec 2016 00:28:08 +0000 (19:28 -0500)]
pimd: Cleanup uninitialized memory access
Valgrind noticed that we have a read of uninitialized memory:
Conditional jump or move depends on uninitialised value(s)
==13749== at 0x428067: pim_ifassert_winner_set (pim_assert.c:57)
==13749== by 0x4266F0: pim_ifchannel_add (pim_ifchannel.c:535)
==13749== by 0x426CC1: pim_ifchannel_join_add (pim_ifchannel.c:730)
==13749== by 0x427B5B: recv_join (pim_join.c:95)
==13749== by 0x427B5B: pim_joinprune_recv (pim_join.c:270)
==13749== by 0x42354F: pim_pim_packet (pim_pim.c:249)
==13749== by 0x4236C0: pim_sock_read (pim_pim.c:349)
==13749== by 0x4E60587: thread_call (thread.c:1462)
==13749== by 0x40C75E: main (pim_main.c:266)
==13749==
This commit fixes that issue.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Fri, 27 Jan 2017 00:14:06 +0000 (19:14 -0500)]
pimd: Modify pimreg creation
We were creating the pimreg device with a
created ifindex of 255. This was causing
issues when a interface was assigned a ifindex
of 255 by the kernel. Subsuquently pim
would stay in a hosed up state.
Modify the ifindex used for the pimreg device
to be 0.
Ticket: CM-14625 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 25 Jan 2017 19:47:04 +0000 (14:47 -0500)]
pimd: Use correct flag to add an oif
When we are creating the igmp ifchannel we were creating
it with both a P and a I flag. This was causing
it to not be cleaned up properly when the interface
was shut down. Subsuquently when the interface
came back up we would attempt to add it back in
but it would fail.
Ticket: CM-14586 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Sat, 21 Jan 2017 09:54:10 +0000 (04:54 -0500)]
pimd: Handle assignment of vif index better
PIM was handling vif creation deletion poorly
for interface down and up events. Fix this
issue by keeping track of which vif index'es
we have issued and allow the wholes to be
filled in.
Ticket: CM-14556 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Thu, 19 Jan 2017 17:09:26 +0000 (12:09 -0500)]
pimd: Fixup tracking of where we got OIF's from.
This commit does these three things:
1) Add code to 'show ip pim state' to show where OIF's got their
decision to include that interface
2) Add code in pim_mroute_[add|del] to display what we think we are
adding to the kernel
3) Add code to properly track where we got the incoming request from and
to appropriately not remove a OIL if we have state still
Ticket: CM-14034 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com> Reviewed-by: Don Slice <dslice@cumulusnetworks.com> Reviewed-by: Chirag Shah <chirag@cumulusnetworks.com>
Donald Sharp [Mon, 12 Dec 2016 17:16:21 +0000 (12:16 -0500)]
zebra: Refactor Error codes to proper place
These error codes have ended up only being used
for socket type interfaces to the kernel(*bsd),
yet we were exposing the #defines to the entirety
of the project.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
[DL: v2: fix missing slash in tests/Makefile.am] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
David Lamparter [Thu, 24 Apr 2014 17:29:07 +0000 (19:29 +0200)]
lib: add second-level route_table for srcdest
IPv6 srcdest routes need to be keyed by both destination and source
prefix. Since the lookup order is destination first, the simplest thing
to do here is to add a second route_table to destination entries, which
then contain source entries. Sadly, the result is somewhat confusing
since a route_node might now be either a source node or a destination
node.
There are helper functions to get source and destination prefix from a
given route node (which can be either a destination or a source route).
The following bits have been added by Christian Franke
<chris@opensourcerouting.org>:
- make srcdest routing table reusable by moving it into lib
- make the srcdest routing table structure more opaque
- implement a srcdest routing table iterator
- fix a refcounting issue in src_node_lookup
- match route_node_lookup behavior with srcdest_rnode_lookup
- add accessor for the route_node table and table_info
- add string formatter srcdest_rnode2str
Signed-off-by: David Lamparter <equinox@opensourcerouting.org> Signed-off-by: Christian Franke <chris@opensourcerouting.org>
[v3: adapted for cmaster-next as of 2016-12-05]
David Lamparter [Tue, 6 May 2014 17:21:51 +0000 (19:21 +0200)]
lib: add route_node_lookup_maynull
The sourcedest code needs to get the route_node even if its info pointer
is NULL (which occurs when there are srcdest routes, but no general
destination route.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>