In startup, zebra would dump interface information from Kernel in 3
steps w/o lock: step1, get interface information; step2, get interface
ipv4 address; step3, get interface ipv6 address.
If any interface gets added after step1, but before step2/3, zebra
would get extra interface addresses in step2/3 that has not been added
into zebra in step1. Returning error in the referenced interface lookup
would cause the startup interface retrieval to be incomplete.
David Lamparter [Wed, 22 Sep 2021 09:59:08 +0000 (11:59 +0200)]
doc/workflow: prefer typesafe containers
The typesafe containers have been around for quite a while now and
haven't gone up in a blaze of flames, so let's add a "strong
recommendation" to use them for new code and refactors.
For the nhrpd custom lists I'm already working to remove them; meanwhile
the old skiplists are primarily used in RFAPI (4 users outside of that),
so those could be next.
What remains are the old `list_*` and `hash_*`, which have >300 and >100
users respectively, making them a much harder problem to tackle. And
the new hash implementation doesn't have the same level of
debug/introspection yet (it's on my TODO.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
currently, has_valid_label is only used to check need to print debug,
but if route has normal nexthops and mpls nexthops, label information
will be printed even for normal nexthops.
ospfd: rename the "graceful-restart helper-only" command
Considering that both the GR helper mode and restarting mode can be
enabled at the same time, the "graceful-restart helper-only" command
can be a bit misleading since it implies that only the helper mode
is enabled. Rename the command to "graceful-restart helper enable"
to clarify what the command does.
Start a deprecation cycle of one year before removing the original
command
ospf6d: rename the "graceful-restart helper-only" command
Considering that both the GR helper mode and restarting mode can be
enabled at the same time, the "graceful-restart helper-only" command
can be a bit misleading since it implies that only the helper mode
is enabled. Rename the command to "graceful-restart helper enable"
to clarify what the command does.
After `<daemon_name> is not running` message vtysh does not return
error. For example if you disable ospf in `/etc/frr/daemons` and run
`vtysh -c configure -c "router ospf"` it prints the message to stderr,
but returns 0.
This commit will make vtysh return error when not in interractive mode.
But if you run commands from vtysh, you will still be able to enter
views and exit them if daemon is not running.
Donald Sharp [Fri, 17 Sep 2021 09:41:37 +0000 (05:41 -0400)]
pimd: Prevent uninited usage of nexthop
pim_msdp_peer_rpf_check creates an nexthop to do
a rpf search against and doesn't initialize it
sucht that the pim_nexthop_lookup function is
making decisions against the nexthop just
created that was uninitialized.
Donald Sharp [Tue, 21 Sep 2021 11:52:54 +0000 (07:52 -0400)]
ospf6d: Use appropriate integer size and more context for reason strings
The ospfv3 spf reason strings are just presented internally in the code
without any real context. Give a tiny bit more useful information for
the developer and convert the integer to a uint32_t
Igor Ryzhov [Wed, 15 Sep 2021 15:22:47 +0000 (18:22 +0300)]
vtysh: remove sorting of vrf node commands
A simple strcmp-based sorting done by `config_add_line_uniq` breaks the
correct advanced sorting of static routes done by staticd. We don't
actually need to check vrf node commands for uniqueness as all commands
are daemon specific, so let's use simple `config_add_line` that doesn't
sort commands.
ospf6d: rework filtering commands to be in line with ospfd
Issue #9535 describes how the export-list/import-list commands work
differently on ospfd and ospf6d.
In short:
* On ospfd, "area A.B.C.D export-list" filters which internal
routes an ABR exports to other areas. On ospf6d, instead, that
command filters which inter-area routes an ABR exports to the
configured area (which is quite counter-intuitive). In other words,
both commands do the same but in opposite directions.
* On ospfd, "area A.B.C.D import-list" filters which inter-area
routes an ABR imports into the configured area. On ospf6d, that
command filters which inter-area routes an interior router accepts.
* On both daemons, "area A.B.C.D filter-list prefix NAME <in|out>"
works exactly the same as import/export lists, but using prefix-lists
instead of ACLs.
The inconsistency on how those commands work is undesirable. This
PR proposes to adapt the ospf6d commands to behave like they do
in ospfd.
These changes are obviously backward incompatible and this PR doesn't
propose any mitigation strategy other than warning users about the
changes in the next release notes. Since these ospf6d commands are
undocumented and work in such a peculiar way, it's unlikely many
users will be affected (if any at all).
Philippe Guibert [Tue, 17 Aug 2021 08:56:32 +0000 (10:56 +0200)]
zebra: refresh vxlan evpn contexts, when bridge interface goes up
When using bgp evpn rt5 setup, after BGP configuration has been
loaded, if the user attempts to detach and reattach the bridged
vxlan interface from the bridge, then BGP loses its BGP EVPN
contexts, and a refresh of BGP configuration is necessary to
maintain consistency between linux configuration and BGP EVPN
contexts (RIB). The following command can lead to inconsistency:
ip netns exec cust1 ip link set dev vxlan1000 nomaster
ip netns exec cust1 ip link set dev vxlan1000 master br1000
consecutive to the, BGP l2vpn evpn RIB is empty, and the way to
solve this until now is to reconfigure EVPN like this:
vrf cust1
no vni 1000
vni 1000
exit-vrf
Actually, the link information is correctly handled. In fact,
at the time of link event, the lower link status of the bridge
interface was not yet up, thus preventing from establishing
BGP EVPN contexts. In fact, when a bridge interface does not
have any slave interface, the link status of the bridge interface
is down. That change of status comes a bit after, and is not
detected by slave interfaces, as this event is not intercepted.
This commit intercepts the bridge link up event, and triggers
a check on slaved vxlan interfaces.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Tue, 17 Aug 2021 08:42:51 +0000 (10:42 +0200)]
zebra: handle bridge mac address update in evpn contexts
when running bgp evpn rt5 setup, the Rmac sent in BGP updates
stands for the MAC address of the bridge interface. After
having loaded frr configuration, the Rmac address is not refreshed.
This issue can be easily reproduced by executing some commands:
ip netns exec cust1 ip link set dev br1000 address 2e:ab:45:aa:bb:cc
Actually, the BGP EVPN contexts are kept unchanged.
That commit proposes to fix this by intercepting the mac address
change, and refreshing the vxlan interfaces attached to te bridge
interface that changed its MAC address.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Donald Sharp [Thu, 16 Sep 2021 19:12:13 +0000 (15:12 -0400)]
tests: Do not do a initial_timeout of 2 seconds for verify_bgp_rib
A bunch of tests have this pattern:
a) Install a new prefix into bgp
b) Run this loop:
foreach (router in topology) {
verify_bgp_rib(router)
}
This is to ensure that the prefix is actually disseminated.
The problem with this, of course, is that a wait of 2 seconds
for every item in that loop makes no sense. As that the initial
router verification of it's bgp rib will wait 2 seconds and
all the remaining bgp routers in the topology will have gotten
the data. So we end up waiting a bunch of extra time.
Remove the initial_wait time for verify_bgp_rib. Also
increase the failure wait time to 30 seconds. This is
to give a bigger window for bgp to send it's data for
our test systems that could be under heavy load. In the
normal case tests will never hit this.
ospf6d: fix build problem with ancient json-c versions
Some CI VMs are using really old versions of json-c (pre 2013 [1])
that expect filenames to be passed as "char *" instead of "const char *".
Add some explicit casts to fix the resulting compiler errors on those
VMs (passing "char *" when the API expects "const char *" is fine).
Hopefully this commit should be reverted once the CI is updated to use
newer versions of json-c.
Renato Westphal [Wed, 30 Jun 2021 18:12:30 +0000 (15:12 -0300)]
tests: add OSPFv3 graceful restart topotest
Add a new topotest that features a topology with seven routers spread
across four OSPF areas:
* 1 backbone area;
* 1 regular non-backbone area (0.0.0.1);
* 1 stub area (0.0.0.2);
* 1 NSSA area (0.0.0.3).
All routers have both GR and GR helper functionality enabled in
the configuration. The test consists of restarting each router,
one at time, and checking that all forwarding planes (and LSDBs)
are kept intact during those restarts.
A successful run takes about three minutes to finish.
ospf6d: introduce support for Graceful Restart (restarting mode)
RFC 5187 specifies the Graceful Restart enhancement to the OSPFv3
routing protocol. This commit implements support for the GR
restarting mode.
Here's a quick summary of how the GR restarting mode works:
* GR can be enabled on a per-instance basis using the `graceful-restart
[grace-period (1-1800)]` command;
* To perform a graceful shutdown, the `graceful-restart prepare ipv6
ospf` EXEC-level command needs to be issued before restarting the
ospf6d daemon (there's no specific requirement on how the daemon
should be restarted);
* `graceful-restart prepare ospf` will initiate the graceful restart
for all GR-enabled instances by taking the following actions:
o Flooding Grace-LSAs over all interfaces
o Freezing the OSPF routes in the RIB
o Saving the end of the grace period in non-volatile memory (a JSON
file stored in `$frr_statedir`)
* Once ospf6d is started again, it will follow the procedures
described in RFC 3623 until it detects it's time to exit the graceful
restart (either successfully or unsuccessfully).
Testing done:
* New topotest featuring a multi-area OSPF topology (including stub
and NSSA areas);
* Successful interop tests against IOS-XR routers acting as helpers.
introduced the idea of v6 LL using interface up/down events
instead of nexthop resolution to know when a peering should
happen or not. This above commit left a hole where if the remote
peer connected to this bgp, the bgp code would still believe
the peering is down. Modify the code to double check and
ensure that we have proper v6 LL resolution flags set.
Donald Sharp [Thu, 16 Sep 2021 01:10:24 +0000 (21:10 -0400)]
tests: Fix make check builds on some bsd variants
Compilation is warning that a memcpy is only copying
the first (sizeof pointer) into memory. This is not
what we really want. Although it does beg the question about
why this memcpy is needed( or what it is doing ). I'm going
to just fix the memcpy and call it a day.
Igor Ryzhov [Wed, 15 Sep 2021 19:45:23 +0000 (22:45 +0300)]
bgpd: fix memory leaks when using route-maps
There are places where we use route-maps using duplicated attributes and
neither intern nor flush them after the usage. If a route-map has set
rules for aspath/communities, they will be allocated and never freed.
We should always flush unneeded duplicated attributes.
Donald Sharp [Tue, 14 Sep 2021 17:28:00 +0000 (13:28 -0400)]
zebra: Fix case default usage w/ enum's
We should not be using `case default` with an enumerated type
This prevents the developer of new cases from knowing where
they need to fix by just compiling.
Donald Sharp [Fri, 13 Aug 2021 14:51:22 +0000 (10:51 -0400)]
vtysh: Allow us to gather a bit more data when extract.pl dies
When extract.pl dies, it was dieing in a manner that provided
absolutely no useful data as to what went wrong. Let's add
a tiny bit of debug code. So we can see what is going wrong.
bgpd: Extend EVPN next hop tracking to type-1 and type-4 routes
NH tracking is already in use for type-1, type-3 and type-5 routes.
This change extends that tracking to EAD and ESR to eliminate the 9s
delay (BGP holdtimer) with ES/L2-NHG update seen when all the uplinks
are shutdown on a remote EVPN PE.
Mark Stapp [Wed, 14 Jul 2021 17:06:41 +0000 (13:06 -0400)]
zebra: use the dataplane to read netlink intf addr changes
Read incoming interface address change notifications in the
dplane pthread; enqueue the events to the main pthread
for processing. This is netlink-only for now - the bsd
kernel socket path remains unchanged.
Mark Stapp [Tue, 13 Jul 2021 20:59:46 +0000 (16:59 -0400)]
zebra: add interface address apis for dplane
Add new apis for dplane interface address handling, based on
the existing api. The existing api is basically split in two:
the first part processes an incoming netlink message in the
dplane pthread, creating a dplane context with info about
the event. The second part runs in the main pthread and uses
the context data to update an interface or connected object.