Renato Westphal [Sat, 19 Jan 2019 00:58:58 +0000 (22:58 -0200)]
lib: reset the vty xpath index when entering the config mode
The CLI code uses the vty->xpath[] array and the vty->xpath_index
variables to keep track of where the user is in the configuration
hierarchy. As such, we were resetting vty->xpath_index to zero
whenever the user exited from the configuration mode in order to
keep the index valid. We weren't doing this in the vty_stop_input()
function however, which is called when the user types ^C in the
terminal. This was leading to bugs like this:
zebra> en
zebra# conf t
zebra(config)# interface eth0
zebra(config-if)# ^C
zebra# conf t
zebra(config)# interface eth0
% Configuration failed.
Schema node not found.
YANG path: /frr-interface:lib/interface[name='eth0'][vrf='default']/frr-interface:lib
To fix this, do something more clever: instead of resetting the
XPath index whenever the user exits from the configuration mode,
do that when the user enters in the configuration mode. This way
the XPath index needs to be reset in a single place only, not to
mention it's a more robust solution.
Renato Westphal [Fri, 18 Jan 2019 18:06:00 +0000 (16:06 -0200)]
zebra: send link parameters to zclients once they request interface info
We were sending ZEBRA_INTERFACE_LINK_PARAMS messages under the
following circumstances:
* New interface was created (via kernel or config);
* Interface went from down to up;
* Update in the link-params configuration.
Now also send ZEBRA_INTERFACE_LINK_PARAMS messages whenever a zclient
connects and sends a ZEBRA_INTERFACE_ADD request. Without this fix,
the client daemons don't receive interface link parameters if they
are configured in the zebra startup configuration.
Renato Westphal [Fri, 18 Jan 2019 18:06:00 +0000 (16:06 -0200)]
lib: fix checking of clients subscribed to receive default routes
In these two functions, we were using VRF_DEFAULT instead of the
VRF ID passed as a parameter when checking if the given client
subscribed to receive default routes or not. This prevented the
"default-originate" command from ospfd/isisd from working correctly
under specific circumstances.
Renato Westphal [Mon, 14 Jan 2019 18:29:18 +0000 (16:29 -0200)]
doc: update build instructions for freebsd on how to obtain libyang
Unfortunately the first version of the FreeBSD libyang port contained
a bug in which the libyang pkginfo file wasn't being installed
correctly in the system, and this prevented the FRR build system from
detecting the library. This bug was already fixed months ago but some
FreeBSD package repositories still have the old bugged version of the
port. This means we can't suggest people to install libyang using
"pkg install" since this causes problems for most people. In this
case, suggest FreeBSD users to build and install libyang manually
as we suggest for other BSD platforms.
This commit should be reverted once all FreeBSD package repositories
are updated with the new version of the libyang port.
Renato Westphal [Mon, 14 Jan 2019 18:29:18 +0000 (16:29 -0200)]
lib: update suggestions related to some northbound errors
Since commit 3a11599c, the FRR YANG modules are embedded inside the
binaries and no longer need to be loaded from the file system. This
way, it's impossible for the FRR binaries and YANG modules to be out
of sync anymore. As such, update the suggestions of the northbound
error codes.
Renato Westphal [Mon, 14 Jan 2019 18:29:18 +0000 (16:29 -0200)]
lib: don't abort when incomplete xpath is given by the user
Instead of aborting when an incomplete xpath is given to the
nb_oper_data_iterate() function, just return an error so that the
callers have a chance to treat this error. Aborting based on invalid
user input is never the right thing to do.
Renato Westphal [Mon, 14 Jan 2019 18:29:18 +0000 (16:29 -0200)]
lib: fix "use of uninitialised value" valgrind warning
When FRR is built without the --enable-config-rollbacks option,
the nb_db_transaction_save() function does nothing and the
"transaction_id" output parameter is left uninitialized. For
this reason, all northbound clients should initialize the
"transaction_id" argument before calling nb_candidate_commit() or
nb_candidate_commit_apply() (except when a NULL pointer is given,
which is the case of the confd and sysrepo plugins).
Renato Westphal [Mon, 14 Jan 2019 18:29:18 +0000 (16:29 -0200)]
lib: fix "may be used uninitialized" build warning
We are already handling all possible four cases from the "nb_event"
enumeration, so this problem can't happen in practice. Initialize the
"ref" variable to zero to silence the warning.
Renato Westphal [Fri, 11 Jan 2019 21:20:13 +0000 (19:20 -0200)]
lib, zebra: add AFI parameter to the ZEBRA_REDISTRIBUTE_DEFAULT_* messages
Some daemons like ospfd and isisd have the ability to advertise a
default route to their peers only if one exists in the RIB. This
is what the "default-information originate" commands do when used
without the "always" parameter.
For that to work, these daemons use the ZEBRA_REDISTRIBUTE_DEFAULT_ADD
message to request default route information to zebra. The problem
is that this message didn't have an AFI parameter, so a default route
from any address-family would satisfy the requests from both daemons
(e.g. ::/0 would trigger ospfd to advertise a default route to its
peers, and 0.0.0.0/0 would trigger isisd to advertise a default route
to its IPv6 peers).
Fix this by adding an AFI parameter to the
ZEBRA_REDISTRIBUTE_DEFAULT_{ADD,DELETE} messages and making the
corresponding code changes.
David Lamparter [Mon, 18 Feb 2019 20:34:06 +0000 (21:34 +0100)]
isisd: skip over deleted fragments
Since LSP fragments are also on our lspdb dict, lsp_tick() needs to skip
over them after calling lsp_destroy(). Otherwise it ends up accessing
free'd memory.
Fixes: #3533 Signed-off-by: David Lamparter <equinox@diac24.net>
Note that sysinit.target does not depend on any network* service or
target.
In other words, unless there is a service that requires
network-online.service, even if FRR is enabled it will not be started.
Therefore network-online.target is the wrong unit to have in WantedBy=,
as it is not always started.
This patch updates our service file so that it is properly started by
the system when enabled, delayed until networking is up, and if possible
delayed until after NetworkManager, systemd-networkd or any other
networking configuration manager has finished performing its tasks -
i.e. after network-online.target.
After these changes our new dependency graph looks like this:
David Lamparter [Mon, 4 Feb 2019 21:56:50 +0000 (22:56 +0100)]
lib: yang: use common yang_ctx_new_setup()
After creating a libyang context, we need to hook up our callback to use
embedded built-in modules. I hadn't added this to the yang translator
code.
Also, ly_ctx_new fails if the search directory doesn't exist. Since
that's not a hard error for us, work around that and ignore inaccessible
YANG_MODELS_DIR. (This is needed for snap packages.)
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Wed, 30 Jan 2019 17:11:54 +0000 (18:11 +0100)]
build: fix a whole bunch of *FLAGS
- some target_CFLAGS that needed to include AM_CFLAGS didn't do so
- libyang/sysrepo/sqlite3/confd CFLAGS + LIBS weren't used at all
- consistently use $(FOO_CFLAGS) instead of @FOO_CFLAGS@
- 2 dependencies were missing for clippy
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Fri, 30 Nov 2018 20:42:25 +0000 (21:42 +0100)]
build, lib/yang: bake in extensions if possible
Starting with libyang 0.16.74, we can load internally embedded yang
extensions instead of going through the file system/dlopen. Detect
support for this at build time and use if available.
NB: the fallback mechanism will go away in a short while.
Signed-off-by: David Lamparter <equinox@diac24.net>
Philippe Guibert [Mon, 28 Jan 2019 16:54:50 +0000 (17:54 +0100)]
bgpd: detach vrf labels allocated, when removing bgp instance
bgp instance is disabling the label allocated to reach vrf entity.
previously, only vrf disabling was removing the label. now, when bgp
leaves, bgp instance also frees the label used.
PR=62306 Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com> Acked-by: Julien Floret <julien.floret@6wind.com>
Rafael Zalamena [Wed, 23 Jan 2019 12:25:30 +0000 (10:25 -0200)]
ospf6d: keep track of the socket set thread
When using the timer to set the socket multicast options, keep track
of the thread pointer. If we lose the thread reference we might have
situations where multicast is enabled when it should be disabled and
vice versa.
Renato Westphal [Tue, 22 Jan 2019 19:50:33 +0000 (17:50 -0200)]
ldpd: fix corner case in which we wouldn't respect the max pdu length
The calculation to know when an LDP PDU went past the maximum
negotiated PDU length was wrong because it wasn't taking the
"Version" and "PDU Length" fields into account (total of four
bytes). Fix this.
Rafael Zalamena [Thu, 17 Jan 2019 22:46:11 +0000 (20:46 -0200)]
zebra: fix routing socket AF_LINK handling on NETMASK
IPv6 netmasks use AF_LINK family type and puts the correct amount of
set bits in the data structure. If we only copy the SDL header we
won't get all IPv6 address length, we must copy the whole extension of
the `sockaddr_in6` struct (which is provided in `destlen` parameter).
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
the command was not checking
correctly in all cases whether the virtual link existed. This caused
bugs in some corner cases, e.g. when two virtual links were created,
one of them was deleted, and the second one was reset with no
authentication - this would instead create a new virtual link with
the area in decimal format.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Philippe Guibert [Fri, 28 Dec 2018 13:27:45 +0000 (14:27 +0100)]
zebra: do not create vrf if name already set to default vrf at startup
if the default vrf name is manually set, by passing -o parameter to
zebra, then this should be detected when walking the list of netns
available in the system. If a netns called vrf0 is present, then it
should be ignored.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Fri, 21 Dec 2018 15:25:20 +0000 (16:25 +0100)]
zebra: start the netns notification mechanism after ns initialisation
when zebra is run, by using vrf netns backend mode, then the parser
detector of netns is run before forcing the default vrf to a possible
value. In that case, there is a possibility that the forced '-o' option
will create a second vrf with same name, whereas this option should be
there to uniquely have a default vrf with a value.
To make things consistent, the forced value will be priorised. Then, the
notifier will attempt to create vrf contexts. The expectation is that
the creation will fail, due to an already present vrf with same name.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd: flowspec redirect IP info is retrieved into nh tracking
redirect IP nh of flowspec entry is retrieved so that the nexthop
IP information is injected into the nexthop tracking, and is associated
to the bgp_path structure. This permits validating or unvalidating the
bgp_path for injection in zebra or not.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Tue, 18 Dec 2018 17:00:31 +0000 (18:00 +0100)]
zebra: handle multiple events for netns deletion event
When handling events from /var/run/netns folder, if several netns are
removed at the same time, only the first one is deleted in the frr. Fix
this behaviour by applying continue in the loop.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Donald Sharp [Wed, 9 Jan 2019 17:18:21 +0000 (12:18 -0500)]
lib: Convert RUSAGE_SELF to RUSAGE_THREAD where we can
When using getrusage, we have multiple choices about what
to call for data gathering about this particular thread of execution.
RUSAGE_SELF -> This means gather all cpu run time for all pthreads associated
with this process.
RUSAGE_THREAD -> This means gather all cpu run time for this particular
pthread.
Clearly with data gathering for slow thread as well as `show thread cpu`
it would be preferable to gather only data about the current running
pthread. This probably was the original behavior of using RUSAGE_SELF
when we didn't have multiple pthreads. So it didn't matter so much.
Prior to this change, 10 iterations of 1 million routes install/remove
from zebra would give us this cpu time for the dataplane pthread:
Showing statistics for pthread Zebra dplane thread
--------------------------------------------------
CPU (user+system): Real (wall-clock):
Active Runtime(ms) Invoked Avg uSec Max uSecs Avg uSec Max uSecs Type Thread
0 280902.149 326541 860 2609982 550 2468910 E dplane_thread_loop
After this change we are seeing this:
Showing statistics for pthread Zebra dplane thread
--------------------------------------------------
CPU (user+system): Real (wall-clock):
Active Runtime(ms) Invoked Avg uSec Max uSecs Avg uSec Max uSecs Type Thread
0 58045.560 334944 173 277226 539 2502268 E dplane_thread_loop
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 9 Jan 2019 19:59:22 +0000 (14:59 -0500)]
lib, bgpd: Convert frr_pthread_set_name to only cause it to set os name of the thread
The current invocation of frr_pthread_set_name was causing it reset the os_name.
There is no need for this, we now always create the pthread appropriately
to have both name and os_name. So convert this function to a simple
call through of the pthread call now.
Before(any of these changes):
sharpd@robot ~/frr1> ps -L -p 16895
PID LWP TTY TIME CMD
16895 16895 ? 00:01:39 bgpd
16895 16896 ? 00:00:54
16895 16897 ? 00:00:07 bgpd_ka
Donald Sharp [Wed, 9 Jan 2019 19:32:44 +0000 (14:32 -0500)]
lib: Cleanup thread name setting to happen at start
When we start a thread we always call fpt_run and since
the last commit we know os_name is filled with something,
therefore we can just set the name on startup.
I removed the abstraction to frr_pthread_set_name because
it was snprintf'ing into the same buffer which was the
real bug here( the first character of os_name became null).
In the next commit I'll remove that api because
it is unneeded and was a horrible hack to get
this to work for the one place it was wanted.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 9 Jan 2019 18:41:46 +0000 (13:41 -0500)]
lib: On frr_pthread_new save a os_name
On call of frr_pthread_new, save the os_name if given,
if not given use the name passed in( shortening to fit
in available space ) and finally if the name was not
passed in use the default value.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Wed, 9 Jan 2019 13:48:37 +0000 (08:48 -0500)]
bgpd: Do not send a label to zebra that it doesn't understand
When using an `import vrf` mechanism we are marking
the vrf label as BGP_PREVENT_VRF_2_VRF_LEAK, and then sending
this down to zebra. Since zebra knows nothing about this special
value, convert it to a value that it does know MPLS_LABEL_NONE.
And zebra kept the label as:
donna.cumulusnetworks.com# show mpls table
Inbound Outbound
Label Type Nexthop Label
-------- ------- --------------- --------
-2 BGP GREEN
-2 BGP BLUE
After this fix, neither the labels are stored in zebra nor do we see
the log error message.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Rafael Zalamena [Tue, 8 Jan 2019 12:32:28 +0000 (10:32 -0200)]
zebra: fix FreeBSD warning on fresh OS boot
Handle corner case where a warning log message is issued on interface
address netmask handling with sockaddr type AF_LINK: it may come empty
or with match all (all 0xFF).
In the first case all lengths are zero and we only need to copy the
first bytes, second case it comes with a zero index and all 0xFF bytes.
In any case we only need to figure out a few of the first bytes instead
of all data.
Rafael Zalamena [Tue, 8 Jan 2019 10:14:28 +0000 (08:14 -0200)]
zebra: implement FreeBSD route attr handling
When porting routing socket macro data handling to functions, the
attribute function was forgotten. The only difference between the
attribute and address handler is the family type check.
`isis network point-to-point` was being rejected from the configuration
file as it was being processed before the reception of the UP zebra
notification for the interface. This meant that the `circ_type` was set
at CIRCUIT_T_UNKNOWN, which led the northbound callback to fail. This
check was removed as it was not really necessary; when the zebra
notification is received, the correct circuit type will be enforced,
but now the point-to-point config will be saved and correctly applied
when zebra recognizes the interface as a broadcast one.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Renato Westphal [Wed, 2 Jan 2019 18:47:51 +0000 (16:47 -0200)]
zebra: silence harmless ioctl warning when retrieving interface speed
zebra uses the SIOCETHTOOL ioctl with the ETHTOOL_GSET command to
fetch the speed of interfaces from the kernel. The only problem is
that ETHTOOL_GSET returns EOPNOTSUPP when the given interface is a
virtual interface. This leads to zebra emitting warnings like this
at startup:
ZEBRA: IOCTL failure to read interface lo speed: 95 Operation not supported
ZEBRA: IOCTL failure to read interface dummy0 speed: 95 Operation not supported
ZEBRA: IOCTL failure to read interface ovs-system speed: 95 Operation not supported
Silence these warnings by ignoring EOPNOTSUPP errors, since we know
they are harmless. This is similar to how we handle EINVAL errors
from the BSD SIOCGIFMEDIA ioctl (commit c69f2c1ff).
Renato Westphal [Wed, 2 Jan 2019 15:05:53 +0000 (13:05 -0200)]
zebra, lib: fix the ZEBRA_INTERFACE_VRF_UPDATE zapi message
Unlike the other interface zapi messages, ZEBRA_INTERFACE_VRF_UPDATE
identifies interfaces using ifindexes and not interface names. This
is a problem because zebra always sends ZEBRA_INTERFACE_DOWN
and ZEBRA_INTERFACE_DELETE messages before sending
ZEBRA_INTERFACE_VRF_UPDATE, and the ZEBRA_INTERFACE_DELETE callback
from all daemons set the interface index to IFINDEX_INTERNAL. Hence,
when decoding a ZEBRA_INTERFACE_VRF_UPDATE message, the interface
lookup would always fail since the corresponding interface lost
its ifindex. Example (ospfd):
OSPF: Zebra: Interface[rt1-eth2] state change to down.
OSPF: Zebra: interface delete rt1-eth2 vrf default[0] index 8 flags 11143 metric 0 mtu 1500
OSPF: [EC 100663301] INTERFACE_VRF_UPDATE: Cannot find IF 8 in VRF 0
To fix this problem, use interface names instead of ifindexes to
indentify interfaces like the other interface zapi messages do.
Donald Sharp [Mon, 31 Dec 2018 22:49:47 +0000 (17:49 -0500)]
sharpd: Allow sharpd to accept nexthop group as part of route install
When installing routes via sharpd 'sharp install route... ' command
add the ability to specify a nexthop-group to use. This will
allow sharpd to create ECMP routes into zebra.
donna.cumulusnetworks.com# sharp install routes 10.0.50.0 nexthop-group JANELLE 10
donna.cumulusnetworks.com# end
donna.cumulusnetworks.com# 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
K>* 0.0.0.0/0 [0/106] via 10.0.2.2, enp0s3, 00:20:38
C>* 10.0.2.0/24 is directly connected, enp0s3, 00:20:38
D>* 10.0.50.0/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.1/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.2/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.3/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.4/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.5/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.6/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.7/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.8/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
D>* 10.0.50.9/32 [150/0] via 192.168.209.1, enp0s8, 00:00:02
* via 192.168.210.1, enp0s9, 00:00:02
C>* 192.168.209.0/24 is directly connected, enp0s8, 00:20:38
C>* 192.168.210.0/24 is directly connected, enp0s9, 00:20:38
donna.cumulusnetworks.com#
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>