Donald Sharp [Wed, 18 Sep 2019 17:42:46 +0000 (13:42 -0400)]
lib, zebra: Allow for interface deletion when kernel event happens
When zebra gets a callback from the kernel that an interface has
actually been deleted *and* the end users has not configured
the interface, then allow for deletion of the interface from zebra.
This is especially important in a docker environment where containers
and their veth interfaces are treated as ephermeal. FRR can quickly
have an inordinate amount of interfaces sitting around that are
not in the kernel and we have no way to clean them up either.
My expectation is that this will cause a second order crashes
in upper level protocols, but I am not sure how to catch these
and fix them now ( suggestions welcome ). There are too many
use patterns and order based events that I cannot know for certain
that we are going to see any at all, until someone sees this problem
as a crash :( I do not recommend that this be put in the current
stabilization branch and allow this to soak in master for some time
first.
Testing:
sharpd@donna ~/frr4> sudo ip link add vethdj type veth peer name vethjd
sharpd@donna ~/frr4> sudo ip link add vethaa type veth peer name vethab
sharpd@donna ~/frr4> sudo vtysh -c "show int brief"
Interface Status VRF Addresses
--------- ------ --- ---------
dummy1 down default
enp0s3 up default 10.0.2.15/24
enp0s8 up default 192.168.209.2/24
enp0s9 up default 192.168.210.2/24
enp0s10 up default 192.168.212.4/24
lo up default 10.22.89.38/32
vethaa down default
vethab down default
vethdj down default
vethjd down default
virbr0 up default 192.168.122.1/24
virbr0-nic down default
sharpd@donna ~/frr4> sudo ip link set vethaa up
sharpd@donna ~/frr4> sudo ip link set vethab up
sharpd@donna ~/frr4> sudo ip link del vethdj
sharpd@donna ~/frr4> sudo vtysh -c "show int brief"
Interface Status VRF Addresses
--------- ------ --- ---------
dummy1 down default
enp0s3 up default 10.0.2.15/24
enp0s8 up default 192.168.209.2/24
enp0s9 up default 192.168.210.2/24
enp0s10 up default 192.168.212.4/24
lo up default 10.22.89.38/32
vethaa up default
vethab up default
virbr0 up default 192.168.122.1/24
virbr0-nic down default
sharpd@donna ~/frr4> sudo ip link del vethaa
sharpd@donna ~/frr4> sudo vtysh -c "show int brief"
Interface Status VRF Addresses
--------- ------ --- ---------
dummy1 down default
enp0s3 up default 10.0.2.15/24
enp0s8 up default 192.168.209.2/24
enp0s9 up default 192.168.210.2/24
enp0s10 up default 192.168.212.4/24
lo up default 10.22.89.38/32
virbr0 up default 192.168.122.1/24
virbr0-nic down default
sharpd@donna ~/frr4> sudo ip link add vethaa type veth peer name vethab
sharpd@donna ~/frr4> sudo vtysh -c "show int brief"
Interface Status VRF Addresses
--------- ------ --- ---------
dummy1 down default
enp0s3 up default 10.0.2.15/24
enp0s8 up default 192.168.209.2/24
enp0s9 up default 192.168.210.2/24
enp0s10 up default 192.168.212.4/24
lo up default 10.22.89.38/32
vethaa down default
vethab down default
virbr0 up default 192.168.122.1/24
virbr0-nic down default
sharpd@donna ~/frr4> sudo vtysh -c "show run"
Building configuration...
Current configuration:
!
frr version 7.2-dev
frr defaults datacenter
hostname donna.cumulusnetworks.com
log stdout
no ipv6 forwarding
!
ip route 192.168.3.0/24 192.168.209.1
ip route 192.168.4.0/24 blackhole
ip route 192.168.5.0/24 192.168.209.1
ip route 192.168.6.0/24 192.168.209.1
ip route 192.168.7.0/24 99.99.99.99 nexthop-vrf EVA
ip route 192.168.8.0/24 192.168.209.1
ip route 4.5.6.7/32 12.13.14.15
!
interface dummy1
ip address 12.13.14.15/32
!
interface vethaa
description FROO
!
line vty
!
end
sharpd@donna ~/frr4> sudo ip link del vethaa
sharpd@donna ~/frr4> sudo vtysh -c "show int brief"
Interface Status VRF Addresses
--------- ------ --- ---------
dummy1 down default
enp0s3 up default 10.0.2.15/24
enp0s8 up default 192.168.209.2/24
enp0s9 up default 192.168.210.2/24
enp0s10 up default 192.168.212.4/24
lo up default 10.22.89.38/32
vethaa down default
virbr0 up default 192.168.122.1/24
virbr0-nic down default
sharpd@donna ~/frr4> sudo vtysh -c "show run"
Building configuration...
Current configuration:
!
frr version 7.2-dev
frr defaults datacenter
hostname donna.cumulusnetworks.com
log stdout
no ipv6 forwarding
!
ip route 192.168.3.0/24 192.168.209.1
ip route 192.168.4.0/24 blackhole
ip route 192.168.5.0/24 192.168.209.1
ip route 192.168.6.0/24 192.168.209.1
ip route 192.168.7.0/24 99.99.99.99 nexthop-vrf EVA
ip route 192.168.8.0/24 192.168.209.1
ip route 4.5.6.7/32 12.13.14.15
!
interface dummy1
ip address 12.13.14.15/32
!
interface vethaa
description FROO
!
line vty
!
end
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
lib: add an exception in the northbound for operational data callbacks
During initialization, the northbound detects if any required
callback is missing (fatal error) or if any unneeded callback is
present (warning).
There are three callbacks, however, that should require special
handling: get_next(), get_keys() and lookup_entry().
These callbacks are normally unneeded for configuration lists. But,
if a configuration list is augmented with new state nodes by another
module, then the three callbacks mentioned above become required. In
this case, never log a warning when these callbacks are implemented
when they are not needed, since this depends on context (e.g. some
daemons might augment "frr-interface" while others don't).
Renato Westphal [Tue, 27 Aug 2019 01:31:21 +0000 (22:31 -0300)]
lib: fix ordering issues in the northbound
When a configuration transaction is being performed, the northbound
uses a red-black tree to store the configuration changes that need to
be processed. The problem is that we were sorting the configuration
changes based on their XPaths (and callback priorities). This means
the original order of the changes wasn't being respected, which is
a problem for lists that use the "ordered-by user" statement. To
fix this, add a new "seq" member to the "nb_config_cb" structure
so that we can preserve the order of the configuration changes as
told by libyang.
Since none of the FRR modules use "ordered-by user" lists so far,
no daemon was affected by this problem.
Reported-by: Martin Winter <mwinter@opensourcerouting.org> Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Revert "lib: introduce a read-write lock for northbound configurations"
Adding a lock to protect the global running configuration doesn't
help much since the FRR daemons are not prepared to process
configuration changes in a pthread that is not the main one (a
whole lot of new protections would be necessary to prevent race
conditions).
This means the lock added by commit 83981138 only adds more
complexity for no benefit. Remove it now to simplify the code.
All northbound clients, including the gRPC one, should either run
in the main pthread or use synchronization primitives to process
configuration transactions in the main pthread.
lib: introduce new 'pre_validate' northbound callback
This callback can be used to validate subsections of the
configuration being committed before validating the configuration
changes themselves. It's useful to perform more complex validations
that depend on the relationship between multiple nodes.
Only YANG-level validation (performed by libyang) and the
NB_EV_VALIDATE validation (that can be used to validate individual
configuration changes) proved to be insufficient in some cases.
when a client disconnects, we iterate over the routing table to
remove any label that originated from that client. However we
were erroneously passing the route type to the function, while
it was expecting the lsp type. As a result, for example, killing
ldpd would not remove the ldp labels from the routes.
Kudos to @rwestphal for pointing me to the source of the issue.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
In preparation to Segment Routing:
- Update the management of Traffic Engineering subTLVs to the new tlvs parser
- Add Router Capability TLV 242 as per RFC 4971 & 7981
- Add Segment Routing subTLVs as per draft-isis-segment-routing-extension-25
Modified files:
- isis_tlvs.h: add new structure to manage TE subTLVs, TLV 242 & SR subTLVs
- isis_tlvs.c: add new functions (pack, copy, free, unpack & print) to process
TE subTLVs, Router Capability TLV and SR subTLVs
- isis_circuit.[c,h] & isis_lsp.[c,h]: update to new subTLVs & TLV processing
- isis_te.[c,h]: remove all old TE structures and managment functions,
and add hook call to set local and remote IP addresses as wellas update TE
parameters
- isis_zebra.[c,h]: add hook call when new interface is up
- isis_mt.[c,h], isis_pdu.c & isis_northbound.c: adjust to new TE subTLVs
- tests/isisd/test_fuzz_isis_tlv_tests.h.gz: adapte fuuz tests to new parser
David Lamparter [Tue, 17 Sep 2019 14:32:26 +0000 (16:32 +0200)]
build: fix "make tags"
Anything we list in a xxx_SOURCES variable will be included for "make
tags", including filess marked as nodist_xxx_SOURCES. So if we don't
have Protobuf enabled, even though the entire library isn't built, "make
tags" will still try to process these files... which we can't
autogenerate because Protobuf is disabled. Same for gRPC.
Fixes: #3266 Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd: Remove trailing whitespaces from some header files
This is annoying when editing a file and saving the file. IDEs like
VSCode can automatically remove trailing whitespaces, hence it would be better
having a clean code before pushing other changes.
Renato Westphal [Fri, 9 Aug 2019 17:18:52 +0000 (14:18 -0300)]
isisd: fix validation of prefix-sid flags
The original check would always evaluate to false since
ISIS_PREFIX_SID_VALUE and ISIS_PREFIX_SID_LOCAL have different
values. Use !! to normalize the return value of the individual
checks to either 0 or 1, making the code do what was intended
(ensure the V/L flags are both 0 or 1).
Renato Westphal [Wed, 7 Aug 2019 22:44:11 +0000 (19:44 -0300)]
isisd: introduce new route update hook
This hook will be called whenever a route is added, updated or
deleted. It will be used, for instance, by the SR code to keep
Prefix-SIDs in sync with their associated routes.
Donald Sharp [Mon, 16 Sep 2019 18:25:55 +0000 (14:25 -0400)]
watchfrr: Convert `wtf` to a more meaningful message
There is a fairly common state we are seeing where watchfrr
has decided that something is not right and is printing out
a `wtf` message. At this point I am not sure what is going on
or how we are getting here, but let's add a bit more data dump
to the message so that we can figure out what is going on.
This is mainly being done because at this point in time I have no
clue the what/how of how we got here and I cannot reproduce.
Maybe by adding more useful information here I can figure out what is
going on.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com.
Donald Sharp [Mon, 16 Sep 2019 17:47:50 +0000 (13:47 -0400)]
watchfrr: Allow end users to turn off watchfrr for a particular daemon
Allow an end user who is debugging behavior, with say gdb, to turn
off watchfrr and it's attempts to keep control of a daemons up/responsiveness
With code change:
donna.cumulusnetworks.com# show watchfrr
watchfrr global phase: Idle
zebra Up
bgpd Up/Ignoring Timeout
staticd Up
Now grab bgpd with gdb:
sharpd@donna ~/frr4> date ; sudo gdb -p 27893
Mon 16 Sep 2019 01:44:57 PM EDT
GNU gdb (GDB) Fedora 8.3-6.fc30
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
Attaching to process 27893
[New LWP 27894]
[New LWP 27895]
[New LWP 27896]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
0x00007f1787a3e5c7 in poll () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.29-15.fc30.x86_64 gperftools-libs-2.7-5.fc30.x86_64 json-c-0.13.1-4.fc30.x86_64 libcap-2.26-5.fc30.x86_64 libgcc-9.1.1-1.fc30.x86_64 libgcrypt-1.8.4-3.fc30.x86_64 libgpg-error-1.33-2.fc30.x86_64 libstdc++-9.1.1-1.fc30.x86_64 libxcrypt-4.4.6-2.fc30.x86_64 libyang-0.16.105-1.fc30.x86_64 lua-libs-5.3.5-5.fc30.x86_64 lz4-libs-1.8.3-2.fc30.x86_64 pcre-8.43-2.fc30.x86_64 xz-libs-5.2.4-5.fc30.x86_64
(gdb)
In another window we can see when watchfrr thinks it's not responding:
donna.cumulusnetworks.com# show watchfrr
watchfrr global phase: Idle
zebra Up
bgpd Unresponsive/Ignoring Timeout
staticd Up
Finally exit gdb and watchfrr now believes bgpd is good to go again:
donna.cumulusnetworks.com# show watchfrr
watchfrr global phase: Idle
zebra Up
bgpd Up/Ignoring Timeout
staticd Up
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Quentin Young [Mon, 16 Sep 2019 15:33:49 +0000 (15:33 +0000)]
bgpd: do not send keepalives when KA timer is 0
RFC4271 specifies behavior when the hold timer is sent to zero - we
should not send keepalives or run a hold timer. But FRR, and other
vendors, allow the keepalive timer to be set to zero with a nonzero hold
timer. In this case we were sending keepalives constantly and maxing out
a pthread to do so. Instead behave similarly to other vendors and do not
send keepalives.
Unsure what the utility of this is, but blasting keepalives is
definitely the wrong thing to do.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
vdhingra [Tue, 27 Aug 2019 10:45:54 +0000 (03:45 -0700)]
lib: rmap dep table is not correct in case of exact-match clause
User pass the string match large-community 1 exact-match from CLI.
Now route map lib has got the string as "1 exact-match". It passes the string
to call back for compilation. BGP will parse this string and came to know
that for "1" it has to do exact match. Routemap lib has to save "1" in it’s
dependency table. Here routemap is saving this as a “1 exact-match”
which is wrong. The solution is used the compiled data.
Donald Sharp [Fri, 13 Sep 2019 20:43:16 +0000 (16:43 -0400)]
bgpd: Create `set distance XXX` command for routemaps
Allow bgp to set a local Administrative distance to use
for installing routes into the rib.
Example:
!
router bgp 9323
bgp router-id 1.2.3.4
neighbor enp0s8 interface remote-as external
!
address-family ipv4 unicast
neighbor enp0s8 route-map DISTANCE in
exit-address-family
!
route-map DISTANCE permit 10
set distance 153
!
line vty
!
end
eva# show ip route
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued route, r - rejected route
B 0.0.0.0/0 [153/0] via fe80::a00:27ff:fe84:c2d6, enp0s8, 00:00:06
K>* 0.0.0.0/0 [0/100] via 10.0.2.2, enp0s3, 00:06:31
B>* 1.1.1.1/32 [153/0] via fe80::a00:27ff:fe84:c2d6, enp0s8, 00:00:06
B>* 1.1.1.2/32 [153/0] via fe80::a00:27ff:fe84:c2d6, enp0s8, 00:00:06
B>* 1.1.1.3/32 [153/0] via fe80::a00:27ff:fe84:c2d6, enp0s8, 00:00:06
C>* 10.0.2.0/24 is directly connected, enp0s3, 00:06:31
K>* 169.254.0.0/16 [0/1000] is directly connected, enp0s3, 00:06:31
eva#
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Stephen Worley [Wed, 11 Sep 2019 18:22:55 +0000 (14:22 -0400)]
pbrd: Handle GATEWAY_IFINDEX nht conflicts
In pbrd we did not care if the nexthop interface nexthop tracking
sent us back did not match the one specified with `nexthop [GATEWAY]
[INTERFACE]`. This happened if the gateway was resolvable via a
different interface and the inteface we specified in the config was
unnumbered (no ipv4 address on it) since the default route gets forced
onlink when it gets into zebra.
This patch adds a check to not install the rule if the interface we got
back was different from the specified.
This patch also reworks the nexthop update path to make it a little more
clear what its doing.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
Mark Stapp [Wed, 14 Aug 2019 17:30:24 +0000 (13:30 -0400)]
zebra: revise redistribution delete to improve update case
When selecting a new best route, zebra sends a redist update
when the route is installed. There are cases where redist
clients may not see that redist add - clients who are not
subscribed to the new route type, e.g. In that case, attempt
to send a redist delete for the old/previous route type.
Revised the redist delete api to accomodate both cases;
also tightened up the const-ness of a few internal redist apis.
bgpd: prefer-global command not working on IPv4 peers
`set ipv6 next-hop prefer-global` is not working on IPv4 peers.
In MP-BGP, bgp routers can advertising IPv6 routes over IPv4 peers.
Remove the peer's remote address AFI type checking.
Donald Sharp [Tue, 10 Sep 2019 23:48:21 +0000 (19:48 -0400)]
tests: Add admin distance 255 static routes
Add a couple of test cases to ensure that admin distance of
255 actually causes the route to be accepted by zebra but
not installed into the linux kernel.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 11 Sep 2019 03:16:01 +0000 (23:16 -0400)]
tests: Fix topotests due to json error
Recent commit: 5fba22485b added a new topotest that used
an older version of FRR that referenced some json code
that was changed in between when the PR was submitted
and when it got in.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Quentin Young [Mon, 9 Sep 2019 16:59:09 +0000 (16:59 +0000)]
yang: create interface reference type
Instead of copy-pasting a 16 character string type for use as an
interface reference, create a new typedef that leafref's the name node
of an interface. This way the constraints change with the constraints on
an interface name itself, and it's self documenting.
Incidentally ripd and ripngd forgot the 16 character constraint in their
offset-list configs and IS-IS forgot it entirely, so this also fixes
minor bugs.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The Pim RFC does not appear to state any length requirements
of pim, other than the checksum must be correct.
Certain vendors are sending extra data at the end of a pim assert
message. This while not explicitly against the rules was a bit
of surprise to pim when we threw the assert message on the floor
for being too long.
Modify the test to see if length left will allow us to read
the 8 bytes of data that we need. If it is sufficient for
that allow the packet to be used.
Fixes: #4957 Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This change addresses the following:
1) Ensures logs under DEBUG macro checks are categorized
as zlog_debug instead of zlog_info.
2) Error logs are categorized as zlog_err instead of zlog_info.
3) Rephrasing certain logs to make them appear more intuitive.
There's no need to install MPLS FTNs using the ZEBRA_ROUTE_ADD
message since the ZEBRA_MPLS_LABELS_ADD message already does that
(in addition to installing an MPLS LSP).
projects / mirror / frr.git / log