Add a function to find the VRF or the loopback interface: the loopback
interface for the default VRF and the VRF master interface otherwise.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Rather than running selected source files through the preprocessor and a
bunch of perl regex'ing to get the list of all DEFUNs, use the data
collected in frr.xref.
This not only eliminates issues we've been having with preprocessor
failures due to nonexistent header files, but is also much faster.
Where extract.pl would take 5s, this now finishes in 0.2s. And since
this is a non-parallelizable build step towards the end of the build
(dependent on a lot of other things being done already), the speedup is
actually noticeable.
Also files containing CLI no longer need to be listed in `vtysh_scan`
since the .xref data covers everything. `#ifndef VTYSH_EXTRACT_PL`
checks are equally obsolete.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
A given interface has no enabled link-params context. If a link-params
configuration command fails, the link-params is wrongly enabled:
> r4(config-link-params)# no enable
> r4(config-link-params)# delay
> (0-16777215) Average delay in micro-second as decimal (0...16777215)
> r4(config-link-params)# delay 50 min 300 max 500
> Average delay should be comprise between Min (300) and Max (500) delay
> r4(config-link-params)# do sh run zebra
> (...)
> interface eth-rt1
> link-params
> enable
> exit-link-params
link-params are enabled if and only if the interface structure has a
valid link_params pointer. Before checking the command validity,
if_link_params_get() is called to retrieve the link-params pointer.
However, this function initializes the pointer if it is NULL.
Only use if_link_params_get() to retrieve the pointer to avoid
confusion. In command setting functions, initialize the link_params
pointer if needed only after the validation of the command.
Fixes: 16f1b9e ("Update Traffic Engineering Support for OSPFD")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Using strtol() to compare two strings is a bad idea.
Before the patch, if_cmp_name_func() may confuse foo001 and foo1.
PR=79407
Fixes: 106d2fd572 ("2003-08-01 Cougar <cougar@random.ee>")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Tested-by: Aurélien Degeorges <aurelien.degeorges@6wind.com>
Acked-by: Philippe Guibert <philippe.guibert@6wind.com>
assert when if_lookup_address is passed with
a family that is not AF_INET or AF_INET6 as
that we are dead in the water and this is a
dev escape
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When link-param is enabled for a given interface, TE metric is automatically
assigned to the metric of the interface. However, the metric of the interface
could be unassigned and keep the default value equal to 0. Thus, if the TE
metric is not explicitely modified within the `link-param metric` statement,
TE metric remains set to 0 which is not a valid value especially when
computing constrainted path.
This patch changes the assignement of the default value of the TE metric.
It is set to the metric of the interface only if the latter is not equal to 0.
TE topotests for OSPF and IS-IS have been adjusted accordingly.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
VRF name should not be printed in the config since 574445ec. The update
was done for NB config output but I missed it for regular vty output.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This is needed for the following two reasons:
1. To be able to remove the northbound HACK in if_update_to_new_vrf. It
is totally wrong to rewrite the configuration datastore when some
operational state changes. It is a hard blocker for storing a
configuration data in a management daemon which knows nothing about
the operational state.
2. To allow changing the VRF of the interface using FRR CLI or any other
frontend in the future. If the VRF is a part of the key, it can't be
changed. If the VRF is a simple leaf, it becomes possible to change
it and thus move the interface between VRFs. For now I mark the leaf
as a "config false" as it's not yet possible to control it from FRR.
But we can't simply remove the VRF from the key, because it is needed to
distinguish interfaces when using netns based VRFs, as it is possible to
have multiple interfaces with the same name in different namespaces. To
handle this, I came up with an idea to store both VRF and an interface
name in the "name" leaf using the pattern "vrfname:ifname". For example,
if there's an interface "eth0" in VRF "red" then its "name" leaf will be
"red:eth0".
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Since f60a1188 we store a pointer to the VRF in the interface structure.
There's no need anymore to store a separate vrf_id field.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Most users of if_lookup_address_exact only cared about whether the
address is any local address. Split that off into a separate function.
For the users that actually need the ifp - which I'm about to add a few
of - change it to prefer returning interfaces that are UP.
(Function name changed due to slight change in behavior re. UP state, to
avoid possible bugs from this change.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
We should always treat the VRF interface as a loopback. Currently, this
is not the case, because in some old pre-VRF code we use if_is_loopback
instead of if_is_loopback_or_vrf. To avoid any future problems, the
proposal is to rename if_is_loopback_or_vrf to if_is_loopback and use it
everywhere. if_is_loopback is renamed to if_is_loopback_exact in case
it's ever needed, but currently it's not used anywhere.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently, we automatically delete an inactive VRF when its last
interface is deleted. This code introduces a couple of crashes because
of the following problems:
- vrf_delete is called before calling if_del hook, so daemons may try to
dereference an ifp->vrf pointer which is freed
- in if_terminate, we continue to use the VRF in the loop condition
after the last interface is deleted
This check is needed only when the interface is deleted by the user,
because if the interface is deleted by the system, VRF must still exist
in the system. Move the check to appropriate places to fix crashes.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This function doesn't work correctly with netns VRF backend as the same
index may be used in multiple netns simultaneously. So let's hide it
from the public API to reduce temptation to use it instead of writing
the correct code.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
When writing the config from the NB-converted daemon, we must not rely
on the operational data. This commit changes the output of the interface
configuration to use only config data. As the code is the same for all
daemons, move it to the lib and remove all the duplicated code.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
It allows FRR to read the interface config even when the necessary VRFs
are not yet created and interfaces are in "wrong" VRFs. Currently, such
config is rejected.
For VRF-lite backend, we don't care at all about the VRF of the inactive
interface. When the interface is created in the OS and becomes active,
we always use its actual VRF instead of the configured one. So there's
no need to reject the config.
For netns backend, we may have multiple interfaces with the same name in
different VRFs. So we care about the VRF of inactive interfaces. And we
must allow to preconfigure the interface in a VRF even before it is
moved to the corresponding netns. From now on, we allow to create
multiple configs for the same interface name in different VRFs and
the necessary config is applied once the OS interface is moved to the
corresponding netns.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This function doesn't work correctly with netns VRF backend as the same
ifname may be used in multiple netns simultaneously. So let's hide it
from the public API to reduce temptation to use it instead of writing
the correct code.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.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>
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>
There is a possibility that the same line can be matched as a command in
some node and its parent node. In this case, when reading the config,
this line is always executed as a command of the child node.
For example, with the following config:
```
router ospf
network 193.168.0.0/16 area 0
!
mpls ldp
discovery hello interval 111
!
```
Line `mpls ldp` is processed as command `mpls ldp-sync` inside the
`router ospf` node. This leads to a complete loss of `mpls ldp` node
configuration.
To eliminate this issue and all possible similar issues, let's print an
explicit "exit" at the end of every node config.
This commit also changes indentation for a couple of existing exit
commands so that all existing commands are on the same level as their
corresponding node-entering commands.
Fixes#9206.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
The only difference in daemons' interface node definition is the config
write function. No need to define the node in every daemon, just pass
the callback as an argument to a library function and define the node
there.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This commit fixes the following problem:
- enter the interface node
- move the interface to another VRF
- try to continue configuring the interface
It is not possible to continue configuration because the XPath stored in
the vty doesn't correspond with the actual state of the system anymore.
For example:
```
nfware# conf
nfware(config)# interface enp2s0
<-- move the enp2s0 to a different VRF -->
nfware(config-if)# ip router isis 1
% Failed to get iface dnode in candidate DB
```
To fix the issue, go through all connected vty shells and update the
stored XPath.
Suggested-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
The backoff code assumed that yang operations always completed quickly.
It checked for > 100 YANG modeled commands happening in under 1 second
to enable batching. If 100 yang modeled commands always take longer than
1 second batching is never enabled. This is the exact opposite of what
we want to happen since batching speeds the operations up.
Here are the results for libyang2 code without and with batching.
| action | 1K rts | 2K rts | 1K rts | 2K rts | 20k rts |
| | nobatch | nobatch | batch | batch | batch |
| Add IPv4 | .881 | 1.28 | .703 | 1.04 | 8.16 |
| Add Same IPv4 | 28.7 | 113 | .590 | .860 | 6.09 |
| Rem 1/2 IPv4 | .376 | .442 | .379 | .435 | 1.44 |
| Add Same IPv4 | 28.7 | 113 | .576 | .841 | 6.02 |
| Rem All IPv4 | 17.4 | 71.8 | .559 | .813 | 5.57 |
(IPv6 numbers are basically the same as iPv4, a couple percent slower)
Clearly we need this. Please note the growth (1K to 2K) w/o batching is
non-linear and 100 times slower than batched.
Notes on code: The use of the new `nb_cli_apply_changes_clear_pending`
is to commit any pending changes (including the current one). This is
done when the code would not correctly handle a single diff that
included the current changes with possible following changes. For
example, a "no" command followed by a new value to replace it would be
merged into a change, and the code would not deal well with that. A good
example of this is BGP neighbor peer-group changing. The other use is
after entering a router level (e.g., "router bgp") where the follow-on
command handlers expect that router object to now exists. The code
eventually needs to be cleaned up to not fail in these cases, but that
is for future NB cleanup.
Signed-off-by: Christian Hopps <chopps@labn.net>
Compile with v2.0.0 tag of `libyang2` branch of:
https://github.com/CESNET/libyang
staticd init load time of 10k routes now 6s vs ly1 time of 150s
Signed-off-by: Christian Hopps <chopps@labn.net>
The first change in this commit is the processing of the VRF termination.
When we terminate the VRF, we should not delete the underlying interfaces,
because there may be pointers to them in the northbound configuration. We
should move them to the default VRF instead.
Because of the first change, the VRF interface itself is also not deleted
when deleting the VRF. It should be handled in netlink_link_change. This
is done by the second change.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Back when I put this together in 2015, ISO C11 was still reasonably new
and we couldn't require it just yet. Without ISO C11, there is no
"good" way (only bad hacks) to require a semicolon after a macro that
ends with a function definition. And if you added one anyway, you'd get
"spurious semicolon" warnings on some compilers...
With C11, `_Static_assert()` at the end of a macro will make it so that
the semicolon is properly required, consumed, and not warned about.
Consistently requiring semicolons after "file-level" macros matches
Linux kernel coding style and helps some editors against mis-syntax'ing
these macros.
Signed-off-by: David Lamparter <equinox@diac24.net>
Add if_vrf_lookup_by_index_next to get the next ifindex in a vrf
given the previous ifindex or 0 for the first.
Signed-off-by: Pat Ruddy <pat@voltanet.io>
Convert over to using the %pFX and %pRN modifiers
to output strings to allow us to consolidate on
one standard for printing prefixes.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The Solaris code has gone through a deprecation cycle. No-one
has said anything to us and worse of all we don't have any test
systems running Solaris to know if we are making changes that
are breaking on Solaris. Remove it from the system so
we can clean up a bit.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
If we have an interface configured in a daemon on shutdown
store the old ifindex value for retrieval on when it is
possibly recreated.
This is especially important for nexthop groups as that we
had at one point in time the ability to restore the
configuration but it was lost when we started deleting
all deleted interfaces. We need the nexthop group subsystem
to also mark that it has configured an interface.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Revert "zebra: support for macvlan interfaces"
This reverts commit bf69e212fd.
Revert "doc: add some documentation about bgp evpn netns support"
This reverts commit 89b97c33d7.
Revert "zebra: dynamically detect vxlan link interfaces in other netns"
This reverts commit de0ebb2540.
Revert "bgpd: sanity check when updating nexthop from bgp to zebra"
This reverts commit ee9633ed87.
Revert "lib, zebra: reuse and adapt ns_list walk functionality"
This reverts commit c4d466c830.
Revert "zebra: local mac entries populated in correct netnamespace"
This reverts commit 4042454891.
Revert "zebra: when parsing local entry against dad, retrieve config"
This reverts commit 3acc394bc5.
Revert "bgpd: evpn nexthop can be changed by default"
This reverts commit a2342a2412.
Revert "zebra: zvni_map_to_vlan() adaptation for all namespaces"
This reverts commit db81d18647.
Revert "zebra: add ns_id attribute to mac structure"
This reverts commit 388d5b438e.
Revert "zebra: bridge layer2 information records ns_id where bridge is"
This reverts commit b5b453a2d6.
Revert "zebra, lib: new API to get absolute netns val from relative netns val"
This reverts commit b6ebab34f6.
Revert "zebra, lib: store relative default ns id in each namespace"
This reverts commit 9d3555e06c.
Revert "zebra, lib: add an internal API to get relative default nsid in other ns"
This reverts commit 97c9e7533b.
Revert "zebra: map vxlan interface to bridge interface with correct ns id"
This reverts commit 7c990878f2.
Revert "zebra: fdb and neighbor table are read for all zns"
This reverts commit f8ed2c5420.
Revert "zebra: zvni_map_to_svi() adaptation for other network namespaces"
This reverts commit 2a9dccb647.
Revert "zebra: display interface slave type"
This reverts commit fc3141393a.
Revert "zebra: zvni_from_svi() adaptation for other network namespaces"
This reverts commit 6fe516bd4b.
Revert "zebra: importation of bgp evpn rt5 from vni with other netns"
This reverts commit 28254125d0.
Revert "lib, zebra: update interface name at netlink creation"
This reverts commit 1f7a68a2ff.
Signed-off-by: Pat Ruddy <pat@voltanet.io>
When using the default CLI mode, the northbound layer needs to create
a separate transaction to process each YANG-modeled command since
they are supposed to be applied immediately (there's no candidate
configuration nor the "commit" command like in the transactional
CLI). The problem is that configuration transactions have an overhead
associated to them, in big part because of the use of some heavy
libyang functions like `lyd_validate()` and `lyd_diff()`. As of
now this overhead is substantial and doesn't scale well when large
numbers of transactions need to be performed in sequence.
As an example, loading 50k prefix-lists using a single transaction
takes about 2 seconds on a modern CPU. Loading the same 50k
prefix-lists using 50k transactions can take more than an hour
to complete (which is unacceptable by any standard). To fix this
problem, some heavy optimization work needs to be done on libyang and
on the FRR northbound itself too (e.g. perform partial configuration
diffs whenever possible). This, however, should be a long term
effort since these optimizations shouldn't be trivial to implement
and we're far from having the performance numbers we need.
In the meanwhile, this commit introduces a simple but efficient
workaround to alleviate the issue. In short, a new back-off timer
was introduced in the CLI to monitor and detect when too many
YANG-modeled commands are being received at the same time. When
a certain threshold is reached (100 YANG-modeled commands within
one second), the northbound starts to group all subsequent commands
into a single large transaction, which allows them to be processed
much faster (e.g. seconds and not hours). It's essentially a
protection mechanism that creates dynamically-sized transactions
when necessary to prevent performance issues from happening. This
mechanism is enabled both when parsing configuration files and when
reading commands from a terminal.
The downside of this optimization is that, if several YANG-modeled
commands are grouped into the same transaction and at least one of
them fails, the whole transaction is rejected. This is undesirable
since users don't expect transactional behavior when that's not
enabled explicitly. To minimize this issue, the CLI will log all
commands that were rejected whenever that happens, to make the
user aware of what happened and have enough information to fix
the problem. Commands that fail due to parsing errors or CLI-level
validations in general are rejected separately.
Again, this proposed workaround is intended to be temporary. The
goal is to provided a quick fix to issues like #6658 while we work
on better long-term solutions.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
DEFPY_YANG will allow the CLI to identify which commands are
YANG-modeled or not before executing them. This is going to be
useful for the upcoming configuration back-off timer work that
needs to commit pending configuration changes before executing a
command that isn't YANG-modeled.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
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>
