Donald Sharp [Wed, 31 Jul 2019 16:29:32 +0000 (12:29 -0400)]
tests: Do not use peerUptime as a measure of if a clear worked
The peerUptime data received from a `show bgp ipv4 uni summ json`
gives you the time in seconds since the peer has come up.
The clear_bgp_and_verify function is checking the peerUptime
for before and after and if they are the same then the test
was declaring a clear failure.
The problem with this is of course that the tests can run fast
enough that the peerUptime is the same for before and after the clear.
Modify the test case to use peerUptimeEstablishedEpoch.
This value is the seconds since the epoch since the establishment
of the peer. This will allow us to know that a clear happened.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Don Slice [Thu, 25 Jul 2019 15:35:06 +0000 (11:35 -0400)]
bgpd: stop removing and replacing private asn if it matches the peer
Problems reported that if multiple peers have "remove-private-AS
replace-AS" with each other and all are using private asns, the as-path
gets hosed and continues to grow when a prefix is removed. This fix
disallows removing and replacing the private asn if it matches the
peer's ASN so that normal as-path loop prevention will operate correctly.
Ticket: CM-25489 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Donald Sharp [Mon, 29 Jul 2019 15:36:03 +0000 (11:36 -0400)]
zebra: Remove repeated enqueueing of system routes for rethinking
The code as written before this code change point would enqueue
every system route type to be refigured when we have an
interface event. I believe this was to originally handle bugs
in the way nexthop tracking was handled, mainly that if you keep
asking the question you'll eventually get the right answer.
Modify the code to not do this, we have fixed nexthop tracking
to not be so brain dead and to know when it needs to refigure
a route that it is tracking.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 29 Jul 2019 15:10:25 +0000 (11:10 -0400)]
zebra: Fix route replace v4 semantics with new system route
When a new system route comes in and we have a pre-existing
non-system route we are not deleting the current system
route from the linux kernel.
Modify the code such that when a route replace is sent
to the kernel with a new route as a system route and
the old route as a non-system route do a delete of
the old route so it is no longer in the kernel.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Rafael Zalamena [Fri, 26 Jul 2019 20:32:48 +0000 (17:32 -0300)]
topotest: fix bgp-path-attributes-topology
The first RIB check wants to assert that we don't have the r7's routes
in r1, so right after that code the routers r2 and r3 are configured
then `verify_rib` is called again to check for those routes.
This test never passed, but it didn't cause failures because of the
`try`/`except`.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Don Slice [Mon, 1 Jul 2019 17:17:46 +0000 (17:17 +0000)]
zebra: skip queued entries when resolving nexthop
Problem reported where certain routes were not being passed on to
clients if they were operated on while still queued for kernel
installation. Changed it to defer working on entries that were
queued to dplane so we could operate on them after getting an
answer back from kernel installatino.
Ticket: CM-25480 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Don Slice [Wed, 24 Jul 2019 15:52:03 +0000 (15:52 +0000)]
bgpd: solve invalid error message when clearing interface peer
Problem reported that if "clear bgp swp1" is issued, an error
message is received saying the name or address is malformed. This
was because of a change in bgp_vty.c that removed the storing
and passing of the interface name for this command. Commit that
caused the problem was ac5dec7e88ce2f8cd2943bb61437046718fb34c2.
Ticket: CM-25737 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Rafael Zalamena [Wed, 17 Jul 2019 13:09:25 +0000 (10:09 -0300)]
frr-reload.py: fix reload with different settings
Add `allow-external-route-update` and `domainname` to the one line
context list, otherwise reload will fail when those commands show up in
the running configuration.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Don Slice [Wed, 17 Jul 2019 17:19:08 +0000 (13:19 -0400)]
bgpd: encode implicit next-hop-self in some vrf route leaking scenarios
Problem reported that when vrf route-leaking between an unnumbered
peer in one vrf to a numbered peer in another vrf, the nexthop
attribute was missing from the update, causing the session to fail.
determined that we needed to expand the mechanism for verifying if
the route has been learned in the other vrf without an ipv4 nexthop.
Ticket: CM-25610 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Rafael Zalamena [Wed, 17 Jul 2019 21:39:03 +0000 (18:39 -0300)]
topotest: improve reliability of `verify_rib`
Attempt to run the function multiple times to make sure we got the
result we expected. This is a middle ground between big sleeps and short
test intervals.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
bgpd: Route-map VNI in-filter filters out all the routes for EVPN
Issue1: When a vni in-filter eg:"neighbor X.X.X.X route-map RM-VNI-FILTER in"
is configured under evpn address-family, all the received routes are dropped
regardless of whether the route has a matching vni or not.
(Where RM-VNI-FILTER contains "match evpn vni 100")
Issue2: Routes with 2 labels are not filtered correctly
Issue3: This filter should not get applied for MPLS routes. For MPLS routes,
we need route-map to handle a 3rd state besides match/nomatch called: noop.
Fix1: The handler bgp_update() that services the received route ignored the
route's label while deciding whether to filter it or not.
As part of the fix, the handler now uses the label info to make the
decision about whether to filter the route or not.
Fix2: route_match_vni() now tries to match both the labels within the route
Fix3: route_match_vni() should return noop when it encounters an mpls based
route. For this, route_map library should handle this 3rd state: RMAP_NOOP.
Related fix : Extract tunnel type
This fix relies on PR 4314 #4314 to extract the tunnel type from bgp extended
communities. The information about the route's tunnel type (vxlan or mpls)
is needed to apply "match evpn vni xx" rule. This rule is applicable to
vxlan routes, and should exit safely for mpls based evpn routes.
lib: Introducing a 3rd state for route-map match cmd: RMAP_NOOP
Introducing a 3rd state for route_map_apply library function: RMAP_NOOP
Traditionally route map MATCH rule apis were designed to return
a binary response, consisting of either RMAP_MATCH or RMAP_NOMATCH.
(Route-map SET rule apis return RMAP_OKAY or RMAP_ERROR).
Depending on this response, the following statemachine decided the
course of action:
State1:
If match cmd returns RMAP_MATCH then, keep existing behaviour.
If routemap type is PERMIT, execute set cmds or call cmds if applicable,
otherwise PERMIT!
Else If routemap type is DENY, we DENYMATCH right away
State2:
If match cmd returns RMAP_NOMATCH, continue on to next route-map. If there
are no other rules or if all the rules return RMAP_NOMATCH, return DENYMATCH
We require a 3rd state because of the following situation:
The issue - what if, the rule api needs to abort or ignore a rule?:
"match evpn vni xx" route-map filter can be applied to incoming routes
regardless of whether the tunnel type is vxlan or mpls.
This rule should be N/A for mpls based evpn route, but applicable to only
vxlan based evpn route.
Also, this rule should be applicable for routes with VNI label only, and
not for routes without labels. For example, type 3 and type 4 EVPN routes
do not have labels, so, this match cmd should let them through.
Today, the filter produces either a match or nomatch response regardless of
whether it is mpls/vxlan, resulting in either permitting or denying the
route.. So an mpls evpn route may get filtered out incorrectly.
Eg: "route-map RM1 permit 10 ; match evpn vni 20" or
"route-map RM2 deny 20 ; match vni 20"
With the introduction of the 3rd state, we can abort this rule check safely.
How? The rules api can now return RMAP_NOOP to indicate
that it encountered an invalid check, and needs to abort just that rule,
but continue with other rules.
As a result we have a 3rd state:
State3:
If match cmd returned RMAP_NOOP
Then, proceed to other route-map, otherwise if there are no more
rules or if all the rules return RMAP_NOOP, then, return RMAP_PERMITMATCH.
Donald Sharp [Tue, 9 Jul 2019 00:00:43 +0000 (00:00 +0000)]
pimd: Always create upstream reference when adding channel oil
Modify the code to create an upstream reference whenever the code
creates an channel_oil via the pim_mroute.c code. This code also
starts a keep alive timer to clean up the reference if we do
nothing with it after the normal time.
I've left alone the source->channel_oil creation because these
are kept and tracked independently already.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 17 Jul 2019 00:41:01 +0000 (20:41 -0400)]
pbrd: Do not install nexthop group until we have nexthops
The creation of a nexthop group results in a callback with
just the nexthop group name. At this point in time we
do not have any nexthop information so there is nothing to
install.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Quentin Young [Mon, 15 Jul 2019 17:16:30 +0000 (17:16 +0000)]
tests: remove strlen assert in inet_ntop tests
Some platform libc's like to render some v6 addresses as v4 mapped where
others render the same addresses as v6 with leading zeroes. Binary
equivalence checks pass but strlen checks sometimes fail here. Remove
assert causing the failure.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Donald Sharp [Sat, 13 Jul 2019 19:37:02 +0000 (15:37 -0400)]
vtysh: Make nexthop-group submode line unique
Now that multiple daemons understand nexthop-groups, we need
to ensure that vtysh doesn't double print out the individual
nexthops that are part of a nexthop-group.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Effectively we had encountered situations on system startup
where the interface speed for a device was not properly setup
when zebra learns about the interface ( Imagine a bond being
brought up and the controlling software creating the bond
is not fast given system load, the bond's speed changes
upwards for each interface added ).
The initial workup on this was to allow a 15 second window
and then just reread the interface speed. We've since noticed
that under heavy system load on startup this is not always sufficient.
So modify the code to wait the 15 seconds and then check the interfaces
speed. If the interfaces speed is still MAX_UINT32T or it has changed
let's wait a bit longer and try again.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Rafael Zalamena [Fri, 12 Jul 2019 20:24:17 +0000 (17:24 -0300)]
isisd: improvements based on feedback
* Remove sanity checks that are already done by northbound;
* Show error message on circuit absence;
* Use a better idiom for the configuration display code;
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Donald Sharp [Thu, 11 Jul 2019 18:40:31 +0000 (14:40 -0400)]
tests: Topotests are not giving sufficient time to propagate data
A bunch of our current tests setup data and redistribute
it across some bgp connections and then test for it
being there. A delay of 2 seconds that was initially
there to ensure that the data has propagated does not
actually work all the time when you have a loaded
virtualized environment.
Make the sleep 10 seconds. I agree this is not the ideal
solution but I would rather get the damn tests up and running
again.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
LDP opens two sockets to zebra, one through ldpd (always using
instance 0) and another through lde (using whatever instance
was set through the -n command line parameter). If no instance
was set, both connections would use the same protocol and instance,
making it impossible to distinguish them through zserv_find_client.
This meant that a response to a lm connect would erroneously go to
the wrong process. Fix this by having a default instance value of 1,
in case the user does not specify a different one.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
in order to both streamline the code and allow users to
define their own specialized versions of the LM api handlers,
define hooks for the 4 main primitives offered by the label
manager (i.e. connect, disconnect, get_chunk and release_chunk),
and have the existing code be run in response to a hook_call.
Additionally, have the responses to the requesting daemon be
callable from an external API.
Note that the proxy version of the label manager was a source of
issues and hardly used in practice. With the new hooks, users with
more complex requirements can simply plug in their own code to
handle label distribution remotely, so there is no longer a reason
to maintain this code.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
This is not part of the make check tests and it has been broken
for a while, apparently. The way the label manager is coded makes
it very hard to code unit tests, and testing the relay of requests
to an external label manager is probably better done through
a topotest, so remove this.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
when requesting a specific label chunk (e.g. for the SRGB),
it might happen that we cannot get what we want. In this
event, we must be prepared to receive a response with no
label chunk. Without this fix, if the remote label manager
was not able to alloate the chunk we requested, we would
hang indefinitely trying to read data from the stream which
was not there.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
For SRGB, we need to support chunk requests starting at a
specific point in the label space, rather than just asking
for any sufficiently large chunk. To this purpose, we extend
the label manager api to request a chunk with a base value;
if the base is set to 0, the label manager will behave as it
currently does, i.e. fetching the first free chunk big enough
to satisfy the request.
update all the existing calls to get chunks from the label
manager so that they use MPLS_LABEL_BASE_ANY as the base
for the requested chunk
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Matthew Smith [Tue, 9 Jul 2019 17:59:44 +0000 (12:59 -0500)]
bgpd: honor max prefix timer on inbound sessions
When using the maximum-prefix restart option with a BGP peer,
if the peer exceeds the limit of prefixes, bgpd causes the
connection to be closed and sets a timer. It will not attempt
to connect to that peer until the timer expires. But if the
peer attempts to connect to it before the timer expires, it
accepts the connection and starts exchanging routes again.
When accepting a connection from a peer, reject the connection
if the max prefix restart timer is set.
Signed-off-by: Matthew Smith <mgsmith@netgate.com>
projects / matthieu / frr.git / log