The support bundle feature(tm) asks for some data
from zebra in the form of a command that has
never existed in FRR. Looks like some
cruft snuck in remove.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Drop the `-n` (`--noerror`) flag from the `vtysh -b` invocation called by the
init script responsible for starting FRR. This ensures that errors in the
configuration file is propagated to the administrator, and prevents a node from
entering a production network while running an essentially undefined
configuration (a behaviour that I can personally attest to has the potential to
cause disastrous network outages - documented in more detail in Cumulus
Networks CS#12791).
Silently ignoring errors also leads to the rather odd behaviour that starting
FRR will ostensibly succeed, while reloading it immediately after - without
changing the configuration - will fail. This is due to the fact that the `-n`
flag is not used while reloading.
The use of the `-n` flag appears to have been introduced without any
explanation in commit 858aa29c68 by @donaldsharp.
Looking at the commit message, I suspect that it was not an intentional change.
It seems more likely to me that it was just meant to be used during testing and
development, but ended up being committed to master by accident.
Ticket:CM-28003
Signed-off-by: Tore Anderson <tore@fud.no>
This adds -N and --netns options to watchfrr, allowing it to start
daemons with -N and switching network namespaces respectively.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Reported that in certain config changes, a static intended for the
default table would be duplicated into a vrf context. Determined
that we still weren't keeping or adding the exit-vrf command when
necessary to keep the contexts straight. Added logic to look for
the failing circumstances and add or remove the delete of the
exit-vrf command as needed.
Signed-off-by: Don Slice <dslice@nvidia.com>
In several instances a call to log.error() is preceded by a print()
for the same message. To prevent duplicate messages these print()
calls are removed.
To maintain (very) similar behaviour we add a StreamHandler to the
logger, when doing logging to a file (ie. --reload without --stdout),
which additionally sends error and above logs to STDOUT without any
metadata (exactly as they did before, with print()).
There is one subtle change - the log from Vtysh.is_config_available()
is now preceded with the "vtysh 'configure' returned" text, whereas
previously only the output from vtysh was sent to STDOUT.
Furthermore any error logs which weren't previously explicitly logged
to STDOUT will now be.
Signed-off-by: Duncan Eastoe <duncan.eastoe@att.com>
Add a "--log-level" option to frr-reload to set the maximum message
level to be logged. When the option is not used, the level is set to
info as before.
The existing --debug option is synonymous with --log-level=debug and
these options are therefore mutually exclusive.
Signed-off-by: Duncan Eastoe <duncan.eastoe@att.com>
Remove mid-string line breaks, cf. workflow doc:
.. [#tool_style_conflicts] For example, lines over 80 characters are allowed
for text strings to make it possible to search the code for them: please
see `Linux kernel style (breaking long lines and strings)
<https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings>`_
and `Issue #1794 <https://github.com/FRRouting/frr/issues/1794>`_.
Scripted commit, idempotent to running:
```
python3 tools/stringmangle.py --unwrap `git ls-files | egrep '\.[ch]$'`
```
Signed-off-by: David Lamparter <equinox@diac24.net>
When you have this configuration:
int foo
ipv6 address fd01:0:0:1::1/64
And issue a reload statement, FRR-reload
is reducing the code to a
`no ipv6 address fd01:0:0:1::/64`
and then issuing a:
`ipv6 address fd01:0:0:1::/64`
The end result is of course that the foo
interface now has two v6 addresses on it.
The brilliance of this is of course if you
happen to have two systems that are connected
over an interface, and you issue a reload command.
They both get fd01:0:0:1::/64 as an ipv6 address
and DAD detection kicks in and stomps on your stuff.
Put a special hey don't munch the v6 address line
in a reload situation.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
frr-reload.py needs to know about config-level commands, otherwise it
assumes they are contexts
Ticket: CM-30128
Ticket: CM-30077
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
the refactored frr-reload.py is adding 'no-header' to the
'show running' command of vtysh, but if a daemon is specified
the no-header option should only be added after the daemon name.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
After the cleanup, adding this doesn't require updating a zillion
locations in the code anymore, just one :)
Partially derived from 6a00e91d99f7f98d857c2056d0dcfeba48966581
Originally-by: Emanuele Di Pascale <emanuele@voltanet.io>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
- throw vtysh into a wrapper class
- ignore "username" commands
- use mark output on stdout
- some other random cleanups
Signed-off-by: David Lamparter <equinox@diac24.net>
Original start/stop of FRR prior to David's rewrite in
PR 3507, when configuring multi-instance would
only start multi-instance (-1 -2 -3 -4...) or
just the daemon, not both. If you happened
to start a ospfd instance of 1 then both
the default and instance 1 would react to cli.
Do not allow this, put it back to original behavior
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This reverts commit 3fa139a65b.
This is being reverted because this commit completely
breaks the invocation of frr-reload.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This only applies for split-config; the init script would create an
empty config file with default permissions.
Reported-by: Robert Scheck <robert@fedoraproject.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Problem reported that with certain configs, when the user
deleted a "neighbor x.x.x.x bfd 4 100 100" statement from
frr.conf and then reloaded, a traceback was seen and the
deletion did not succeed. Found that in some scenarios
it was possible to have something in lines_to_add that
was in a different context and when the re.search was
attempted, it found an empy line and was unhappy. This
fix avoids trying to search in the wrong context.
Ticket: CM-29145
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
This dumps call graph data from LLVM bitcode files into a JSON file.
Specifically for FRR, it understands thread_add_*(), hook_*() and
install_element() so it can provide extra information in these cases.
As a general feature, it tries to track down function pointers as far as
easily feasible.
Signed-off-by: David Lamparter <equinox@diac24.net>
when removing a whole address-family block from ldpd config
we were erroneously trying to also remove each of the interface
sub-sub-contexts that belonged to it; this would effectively
re-enable the AF we just removed. Work around this by ignoring
these sub-sub-contexts if we detect that we are already
removing the parent block.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Having a fixed set of parameters for each northbound callback isn't a
good idea since it makes it difficult to add new parameters whenever
that becomes necessary, as several hundreds or thousands of existing
callbacks need to be updated accordingly.
To remediate this issue, this commit changes the signature of all
northbound callbacks to have a single parameter: a pointer to a
'nb_cb_x_args' structure (where x is different for each type
of callback). These structures encapsulate all real parameters
(both input and output) the callbacks need to have access to. And
adding a new parameter to a given callback is as simple as adding
a new field to the corresponding 'nb_cb_x_args' structure, without
needing to update any instance of that callback in any daemon.
This commit includes a .cocci semantic patch that can be used to
update old code to the new format automatically.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
- Fix 1 byte overflow when showing GR info in bgpd
- Use PATH_MAX for path buffers
- Use unsigned specifiers for uint16_t's in zebra pbr
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Replace sprintf with snprintf where straightforward to do so.
- sprintf's into local scope buffers of known size are replaced with the
equivalent snprintf call
- snprintf's into local scope buffers of known size that use the buffer
size expression now use sizeof(buffer)
- sprintf(buf + strlen(buf), ...) replaced with snprintf() into temp
buffer followed by strlcat
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Coccinelle needs to know about complicated macros to understand certain
code paths, add some more macros there.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Our two northbound tools don't have embedded YANG modules like the
other FRR binaries. As such, ly_ctx_set_module_imp_clb() shouldn't be
called when the YANG subsystem it being initialized by a northbound
tool. To make that possible, add a new "embedded_modules" parameter
to the yang_init() function to control whether libyang should look
for embedded modules or not.
With this fix, "gen_northbound_callbacks" and "gen_yang_deviations"
won't emit "YANG model X not embedded, trying external file"
warnings anymore.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This provides the first reasonably-working version of the frr-format GCC
plugin. I've only tested it with gcc 9.3.0.
Signed-off-by: David Lamparter <equinox@diac24.net>
... remove everything we don't need (or can't use because GCC doesn't
export all of its internal classes & stuff.)
Signed-off-by: David Lamparter <equinox@diac24.net>
Problem seen when deleting many static routes or access-lists due
to frr-reload.py issuing individual vtysh -c commands for every
line. On slow switches, this can take long enough for systemd to
time out the reload process and restart frr. This fix uses add
logic for static routes, prefix-lists, and access-lists to gang
the changes together.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Ticket: CM-27856
These make no sense. stderr=subprocess.STDOUT means that vtysh's stdout
and stderr are combined and returned by check_output. We don't expect
errors in that, and we certainly don't log them.
Leaving vtysh's stderr as stderr is perfectly fine, it'll be captured
for logging just like stderr output from frr-reload.py.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Hopefully at some point we can get rid of the --enable-datacenter switch
and just have the init script do magic. Should already work for Cumulus
as it is.
NB: the profile name can't be baked into the package. The whole point
is to make the package profile-agnostic; in theory at some point the
exact same package files should work on both, say, a Cumulus switch and
a Linux software BGP DFZ router.
Signed-off-by: David Lamparter <equinox@diac24.net>
Found that while the previous fix solved the traceback and created
the correct configuration, it was doing a delete/add process rather
than just an add. This was due to an incorrectly created search
string. This commit fixes that search string and testing verifies
that the correct thing is now being done.
Ticket: CM-27233
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Problem reported with tracebacks seen when making multiple bfd timer
changes in frr.conf and applying via frr-reload.py. Found that when
multiple bfd timer changes are made, the same line can be added for
deletion more than once, causing the traceback when the deletion is
performed. This fix verifies the correct line is being appended for
deletion.
Ticket: CM-27233
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
instead of suppressing the 'exit' markers at the end of each
'interface XXX' clause in the mpls ldp configuration, mark
those with a special marker 'exit-ldp-if' and teach the
reload script to correctly recognize the new sub-subcontext
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Add a new '-s' option which controls whether the generated northbound
callbacks are declared with the 'static' specifier or not. If not
(the default), a prototype is generated for each callback before
their declarations.
It's suggested that daemons shouldn't use the '-s' option so that
their northbound callbacks can be implemented in different files
according to their class (config, state, rpc or notification).
libfrr commands, on the other hand, can use the '-s' option when
their associated YANG module is too small and putting all callbacks
in the same file is desirable.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
frr-reload.py has many special case rules that did not consider ldpd
at all. Specifically:
1. The bulk of ldp configuration comes in a big 'mpls ldp' context, which was
previously considered a single-line context as it started with 'mpls'. This
rule should only apply to labels and lsps.
2. ldp has a 'router-id' config line that fell into the same rule as the above
one. It should not be considered a single-line context as more ldp
configuration can follow.
3. enabled interfaces should not end their context. A better fix
would actually require popping a new context for each interface
in case there is any interface-specific config, but at least this
fix will address the most common use case.
4. when declaring pseudowires, any line with 'member pseudowire XXX' should
be considered a sub-context of the 'l2vpn YYY type ZZZ' context. Without
this fix, changes in the first psuedowire declared would not correctly
be processed (e.g. removing a 'control-word exclude' line would not
be picked up).
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
allow frr-reload.py to be invoked with a --daemon option to specify
an individual daemon for which the configuration diff should be
computed. This is useful when integrated config is not used and we
want to apply a patch to a single daemon config file.
No attempt to integrate this with 'service frr reload' has been done.
Making watchfrr work with per-daemon config is outside the scope of
this simple patch.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
allow command line parameters to specify different folder for
the vtysh binary, config file location and temporary file.
Keep the old hardcoded paths as default values for those options
to preserve current functionality.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
This commit is to copy the support bundle scripts to appropriate directories during installation
Signed-off-by: Sri Mohana Singamsetty <msingamsetty@vmware.com>
For frr_each, just fix some existing warnings; for frr_with_* add a
warning indicating that braces should always be used.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
frr_with_mutex(...) { ... } locks and automatically unlocks the listed
mutex(es) when the block is exited. This adds a bit of safety against
forgetting the unlock in error paths & co. and makes the code a slight
bit more readable.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The correct cast for these is (unsigned char), because "char" could be
signed and thus have some negative value. isalpha & co. expect an int
arg that is positive, i.e. 0-255. So we need to cast to (unsigned char)
when calling any of these.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Use the alternate struct instantiation that does not generates warning
on old compilers.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
This allows developer to set a temporary YANG model directory path for
generating northbound for models not yet installed.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
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>
Without this, we end up restarting watchfrr with the systemd watchdog
non-functional & tripped a bit later. Also, if watchfrr is in the
"control" cgroup, systemd 232 will kill it. (241 apparently doesn't.
Can't find anything about this in systemd's ChangeLog though.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Zebra already flushes routes on proper shutdown if you are not
using the -K option. If you are using the -K option then you
do not want the tools/frr script to flush routes.
If zebra crashes and we restart then load up will either delete
the routes or leave them depending on the -K option.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Place in the code the ability for end operators to know how
to modify MAX_FDS so that they can run large scale operations.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
It doesn't make much sense for a hash function to modify its argument,
so const the hash input.
BGP does it in a couple places, those cast away the const. Not great but
not any worse than it was.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Problem reported that if a bgp neighbor had a bfd timer change
made in frr.conf and systemctl reload frr performed, the neighbor
with the timer changed bounced. If the change is made in vtysh
by just adding the new timer values, no peer bounce occurs. This
fix skips the delete part of the delete/add process in frr-reload
so the peers stay up.
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Apparently, the default changed to use `/etc/frr/daemons` instead of
`/etc/frr/daemons.conf`. Therefore, we should ignore absence of the
latter file, because its absence is not an actuall error but will
cause a confusing error message like this:
/etc/init.d/frr: line 507: /etc/frr/daemons.conf: No such file or directory
The "declare -p watchfrr_options" call is just to support backwards
compatibility. If it fails, silently ignore that.
Signed-off-by: David Lamparter <equinox@diac24.net>
Add some Coccinelle semantic patches we can use to automatically
refactor code in the future.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
