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.
Fixes a couple off-by-ones introduced in previous commit.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
[cherry-picked from master d1e4a518e6f55ccf80c67b58d16eeb1cbf5923e3] Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Lou Berger [Sun, 15 Jan 2017 16:58:06 +0000 (11:58 -0500)]
bgpd: partial revert of vpn/encap safi show changes (Issue #14)
To remove commands duplicated in bgp_mplsvpn/_encap
Also until vpn/encap specific show pieces are merged into
generic afi/safi show.
Philippe Guibert [Tue, 17 Jan 2017 16:51:40 +0000 (17:51 +0100)]
bgpd: avoid recalculating as_val value in str2prefix_rd
This is an optimisation that consists in avoiding calling twice atol()
function when converting an ext. community to an AS4 byte or a standard AS byte value.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd: fix nexthop comparison for nexthop vpnv6 attribute
As nexthop attribute for vpnv6 also contains a blank RD and a blank tag,
the IPv6 address length increases from 16 to 24 bytes. The same was
observed for vpnv4 nexthop, but was missing for VPNv6.
The same is done for martian nexthop debugging.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
David Lamparter [Thu, 15 Dec 2016 23:14:55 +0000 (00:14 +0100)]
tests: fix mis-fixed format string
PRIu64 is "u", we need "x", so PRIx64...
Signed-off-by: David Lamparter <equinox@opensourcerouting.org> Cc: Donald Sharp <sharpd@cumulusnetworks.com>
[cherry-picked from master eed831e065694b3746896b126f6ad47548ea9dc0]
David Lamparter [Fri, 13 Jan 2017 10:23:18 +0000 (11:23 +0100)]
tests: ignore stderr for expect
expect redirects both stdout and stderr to its input. This causes
serious problems when libzebra prints log messages to stderr, which are
then asynchronously inserted in the middle of lines of stdout test
output. Ultimately this makes testbgpmpattr fail on FreeBSD.
(The stdio buffering seems to be slightly different on FreeBSD,
triggering the bug.)
Just ignore stderr for expect.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Lou Berger [Fri, 6 Jan 2017 15:50:31 +0000 (10:50 -0500)]
bgpd: restore some missing afi/safi commands (Issue #14)
Added defines and parse utility functions
Fix vty code that treated vpn&encap as AFIs
and some other related vty printing/handline issues
Note: Includes branch specific changes (due to new parser)
Donald Sharp [Thu, 5 Jan 2017 17:52:18 +0000 (12:52 -0500)]
ospfd: Stop attempt to read beyond end of argv
When unconfiguring certain commands in ospf, you can
run into situations where we attempt to read beyond
the end of a argv[] vector created. On certain platforms
this will crash it.
Ticket: CM-14090 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Thu, 5 Jan 2017 17:44:23 +0000 (12:44 -0500)]
lib: Stop potential uninitialized memory access
VRF_UNKNOWN = MAX_INT16_T
The vrf macros to determine where in the bitmap something belongs
assume that the valid values of a vrf are 0 - (MAX_INT16 - 1)
so when they attempt to determine where to look in the bitmap
for VRF_DEFAULT, we can get invalid reads of memory.
This happens because bgp can create vrf's with VRF_UNKNOWN
when we get configuration for a vrf before we've been actually
created in zebra.
Ticket: CM-14090 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
projects / mirror / frr.git / log