Convert a signed value to a time_t before addition
so that we can compare unsigned (time_t) to unsigned.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
We have this pattern in this test:
# Let's kill the interface on rt2 and see what happens with the RIB and BFD on rt1
tgen.gears["rt2"].link_enable("eth-rt1", enabled=False)
# By default BFD provides a recovery time of 900ms plus jitter, so let's wait
# initial 2 seconds to let the CI not suffer.
topotest.sleep(2, 'Wait for BFD down notification')
router_compare_json_output(
"rt1", "show ip route ospf json", "step3/show_ip_route_rt2_down.ref", 1, 0
)
Under a heavy CI load, interface down events and then reacting to them may not actually
happen within 2 seconds. Allow some more grace time in the test to ensure that we
react to it in an appropriate manner.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
To ensure this, add a const modifier to functions' arguments. Would be
great do this initially and avoid this large code change, but better
late than never.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
OSPF when it is deciding on whom it should elect for DR and backup
has a process that prioritizes network stabilty over the exact
same results of who is the DR / Backups.
Essentially if we have r1 ----- r2
Let's say r1 has a higher priority, but r2 comes up first, starts
sending hello packets and then decides that it is the DR. At some
point in time in the future, r1 comes up and then connects to r2
at that point it sees that r2 has elected itself DR and it keeps
it that way.
This is by design of the system. With our tight ospf timers as
well as high load being experienced on our test systems. There
exists a bunch of ospf tests that we cannot guarantee that a
consistent DR will be elected for the test. As such let's not
even pretend that we care a bunch and just look for `Full`.
If we care about `ordering` we need to spend more time getting
the tests to actually start routers, ensure that htey are up and
running in the right order so that priority can take place.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The `show ip ospf neighbor json` command was displaying
state:`Full\/DR`
Where state was both the role and whether or not the neigbhor
was converged. While from a OSPF perspective this is the state.
This state is a combination of two things.
This creates a problem in testing because we have no guarantee
that a particular ospf router will actually have a particular role
given how loaded our topotest systems are. So add a bit of json
output to display both the converged status as well as the
role this router is playing on this neighbor/interface.
The above becomes:
state:`Full\/DR`
converged:`Full`
role:`DR`
Tests can now be modified to look for `Full` and allow it to
continue. Most of the tests do not actually care if this
router is the DR or Backup.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The switch statement over `enum zebra_link_type` had a default
and FRR was missing a few of the pre-defined types we cared about.
Remove the default statement and add the missing values.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Commit 3551ee9e90304 introduced a regression that causes GR to fail
under certain circumstances. In short, while ISM events should
be ignored while acting as a helper for a restarting router, the
DR/BDR fields of the neighbor structure should still be updated
while processing a Hello packet. If that isn't done, it can cause
the helper to elect the wrong DR while exiting from the helper mode,
leading to a situation where there are two DRs for the same network
segment (and a failed GR by consequence). Fix this.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Before starting the graceful restart procedures, ospf_gr_prepare()
verifies for each configured OSPF instance whether it has the opaque
capability enabled (a pre-requisite for GR). If not, a warning is
emitted and GR isn't performed on that instance.
This PR introduces an additional opaque capability check that will
return a CLI error when the opaque capability isn't enabled. The
idea is to make it easier for the user to identify when the GR
activation has failed, instead of requiring him or her to check
the logs for errors.
The original opaque capability check from ospf_gr_prepare() was
retaining as it's possible that that function might be called from
other contexts in the future.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The ospfd opaque LSA infrastruture has an issue where it can't store
different versions of the same Type-9 LSA for different interfaces.
When flushing the self-originated Grace-LSAs upon exiting from the GR
mode, the code was looking up the single self-originated Grace-LSA
from the LSDB, setting its age to MaxAge and sending it out on all
interfaces.
The problem is that Grace-LSAs sent on broadcast interfaces have
their own unique "IP interface address" TLV that is used to identify
the restarting router. That way, just reusing the same Grace-LSA for
all interfaces doesn't work.
Fix this by generating a new Grace-LSA with its age manually set
to MaxAge whenever one needs to be flushed. This will allow the "IP
interface address" TLV to be set correctly and make GR work even in
the presence of multiple broadcast interfaces.
In the long term, the opaque LSA infrastructure should be updated
to support Type-9 link-local LSAs correctly so that we don't need to
resort to hacks like this.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When building clippy we must have python 3. Let's
ensure that we test for it and stop the auto-make
if it is not installed on the system.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
`bgp` pointer always exists and is used before this function call.
Calling `free` in `json` in this context will also cause a
use-after-free crash.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Fix a loop in the setup phase of isis_topo1_vrf: only configure
interfaces that each router actually has.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
Calling get_contexts() can't display as expected, it wrongly displays:
<__main__.Context object at 0x7fdee1d5ad50>
So make it display correct data by add __str__ in Context class.
Signed-off-by: anlan_cs <anlan_cs@tom.com>
Currently, the ll_type is set only in `netlink_interface` which is
executed only during startup. If the interface is created when the FRR
is already running, the type is not stored.
Fixes#1164.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Ensure GR helpers have received a Grace-LSA before killing the
ospfd/ospf6d process that is undergoing a graceful restart.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
There's no more difference between number-named and word-named access-lists.
This commit removes separate arguments for number-named ACLs from CLI.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
BGP LU will use implicit-null in more situations now; adjust
the original LU topotest to align with that. Node R2 uses
imp-null now, while R1 continues to allocate labels.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
Add a second BGP labelled-unicast (BGP-LU) test suite, with
an additional router and some additional tests.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
In some cases where bgp is at the mpls edge, where it has a BGP-LU
peer downstream but an IP peer upstream, it can advertise the
IMPLICIT_NULL label instead of a per-prefix label.
Signed-off-by: Mark Stapp <mstapp@nvidia.com>
Add a 'json' parameter to the 'show_opaque_info' callback definition,
and update all instances of that callback to not display plain-text
data when the user requested JSON data.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
2 things:
a) Each test was setting up for graceful restart with calls to
`graceful-restart prepare ip[v6] ospf`, then sleeping for
3 or 5 seconds. Then killing the ospf process. Under heavy
load there is no guarantee that zebra has received/processed
this signal. Write some code to ensure that this happens
b) Tests are issuing commands in this order:
1) issue gr prepare command
2) kill router
3) <ensure routes were still installed in zebra>
4) start router
5) <ensure routes were stil installed in zebra>
Imagine that the system is under some load and there is
a small amount of time before step 5 happens. In this
case ospf could have come up and started neighbor relations
and also started installing routes. If zebra receives
a new route before step 5 is issued then the route could
be in a state where it is not installed, because it is
being sent to the kernel for installation. This would
fail the test because it would only look 1 time. This
is fixed by giving time on restart for the routes to
be in the installed state.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
RFC 3623 says:
"If the restarting router determines that it was the Designated
Router on a given segment prior to the restart, it elects
itself as the Designated Router again. The restarting router
knows that it was the Designated Router if, while the
associated interface is in Waiting state, a Hello packet is
received from a neighbor listing the router as the Designated
Router".
Implement that logic when processing Hello messages to ensure DR
interfaces will preserve their DR status across a graceful restart.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Any command that uses `peer_lookup_in_view` crashes when "vrf all" is
used, because bgp is NULL in this case.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
FRR should only ever use the appropriate THREAD_ON/THREAD_OFF
semantics. This is espacially true for the functions we
end up calling the thread for.
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
FRR should only ever use the appropriate THREAD_ON/THREAD_OFF
semantics. This is espacially true for the functions we
end up calling the thread for.
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
