The upcoming gRPC-based northbound plugin will run on a separate
pthread, and it will need to have access to the running configuration
global variable. Introduce a rw-lock to control concurrent access
to the running configuration. Add the lock inside the "nb_config"
structure so that it can be used to protect candidate configurations
as well (this might be necessary depending on the threading scheme
of future northbound plugins).
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Introduce a hash table to keep track of user pointers associated
to configuration entries. The previous strategy was to embed
the user pointers inside libyang data nodes, but this solution
incurred a substantial performance overhead. The user pointers
embedded in candidate configurations could be lost while the
configuration was being edited, so they needed to be regenerated
before the candidate could be committed. This was done by the
nb_candidate_restore_priv_pointers() function, which was extremely
expensive for large configurations. The new hash table solves this
performance problem.
The yang_dnode_[gs]et_entry() functions were renamed and moved from
yang.[ch] to northbound.[ch], which is a more appropriate place
for them. This patch also introduces the nb_running_unset_entry()
function, the counterpart of nb_running_set_entry() (unsetting
user pointers was done automatically before, now it needs to be
done manually).
As a consequence of these changes, we shouldn't need support for
libyang private pointers anymore (-DENABLE_LYD_PRIV=ON). But it's
probably a good idea to keep requiring this feature as we might
need it in the future for other things (e.g. disable configuration
settings without removing them).
Fixes#4136.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The IFF_OUT_LOG macro is using BUFSIZE, which is the sizeof(logbuf)
but for some reason 8.0 clang SA is not happy with it. Just
make it happy.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
PR #3622 renamed the "delete" northbound callback to "destroy" in
order to make the libfrr headers compatible with C++. This commit
renames a few functions that still use "delete" instead of "destroy"
in their names.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Change the northbound lib operation from DELETE to DESTROY;
make the required changes in the users of the northbound, in
the cli, rip, ripng, and isis.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Some misc changes to resolve some c++ compilation errors.
The goal is only to permit an external module - a plugin,
for example - to see frr headers, not to support or encourage
contributions in c++. The changes include: avoiding use
of keywords like 'new', 'delete'; cleaning up implicit
type-casting from 'void *' in several places.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
on interface search algorithm, at initialisation, when reading config
file, the vrf backend may not be yet known ( because zebra did not sync
yet with daemon). For that, avoid searching interface name in a separate
vrf. This change of behaviour is induced because the assumption is done
that at config startup, the user is not wrong with the interface
configuration to use. Every usage of vrf_get_backend() should then be
wisely adapted in order to handle that init state.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
other daemons need to sync with zebra to get to know which vrf backend
is available. in that time, there may be interface configuration
available. in that specific case, the vrf backend returned is not known.
A specific return value is sent back. This will be useful to know which
specific algorithm to apply.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When editing the candidate configuration, the northbound must ensure
that either all changes made by a command are accepted or none are.
This is done to prevent inconsistent states where only parts of a
command are applied in the event any error happens.
The previous API for converted commands, the nb_cli_cfg_change()
function, required callers to pass an array containing all changes
that needed to be applied in the candidate configuration. The
problem with this API is that it was very inconvenient for complex
commands, which change different configuration options depending
on several factors. This required users to manipulate the array
of configuration changes using low-level primitives, making it
complicated to implement some commands.
To solve this problem, introduce a new API based on the two following
functions:
- nb_cli_enqueue_change()
- nb_cli_apply_changes()
The first function is used to enqueue configuration changes, one
at time. Then the nb_cli_apply_changes() function is used to apply
all the enqueued configuration changes.
To implement this, a static-sized array was allocated in the "vty"
structure, along with a counter of enqueued changes. This eliminates
the need to declare an array of configuration changes in every
converted CLI command, simplifying things quite considerably.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Add the "abort_if_not_found" parameter to the yang_dnode_get_entry()
function instead of always aborting when an user pointer is not
found. This will make it possible, for example, to use this function
during the validation phase of a configuration transaction. Callers
will only need to check if the function returned NULL or not,
since new configuration objects (if any) won't be created until
the NB_EV_APPLY phase of the transaction.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The frr-interface YANG module models interfaces using a YANG list keyed
by the interface name and the interface VRF. Interfaces can't be keyed
only by their name since interface names might not be globally unique
when the netns VRF backend is in use. When using the VRF-Lite backend,
however, interface names *must* be globally unique. In this case, we need
to validate the uniqueness of interface names inside the appropriate
northbound callback since this constraint can't be expressed in the
YANG language. We must also ensure that only inactive interfaces can be
removed, among other things we need to validate in the northbound layer.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Introduce frr-interface.yang, which defines a model for managing FRR
interfaces.
Update the 'frr_yang_module_info' array of all daemons that will
implement this module.
Add automatically generated stub callbacks in if.c. These callbacks will
be implemented in the following commit.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When entering a interface name and you fat-finger it
actually display some useful information about the vrf
we are in.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Fix CLANG warning:
Report for if.c | 2 issues
===============================================
< WARNING: else is not generally useful after a break or return
< #390: FILE: /tmp/f1-28557/if.c:390:
Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
This crash occurs only with netns implementation.
vrf meaning is different regarging its implementation (netns or
vrf-lite)
- With vrf-lite implementation vrf is a property of the interface that
can be changed as the speed or the state (iproute2 command: "ip link
set dev IF_NAME master VRF_NAME"). All interfaces of the system are in
the same netns and so interface name is unique.
- With netns implementation vrf is a characteristic of the interface
that CANNOT be changed: it is the id of the netns where the interface
is located. To change the vrf of an interface (iproute2 command to
move an interface "ip netns exec VRF_NAME1 ip link set dev IF_NAME
netns VRF_NAME2") the interface is deleted from the old vrf and
created in the new vrf.
Interface name is not unique, the same name can be present in the
different netns (typically the lo interface) and search of interface
must be done by the tuple (interface name, netns id).
Current tests on the vrf implementation (vrf-lite or netns) are not
sufficient. In some cases (for example when an interface is moved from
a vrf X to the default vrf and then move back to VRF X) we can have a
corruption message and then a crash of zebra.
To avoid this corruption test on the vrf implementation, needed when an
interface changes, has been rewritten:
- For all interface changes except deletion the if_get_by_name function,
that checks if an interface exists and creates or updates it if
needed, is changed:
* The vrf-lite implementation is unchanged: search of the interface
is based only on the name and update the vrf-id if needed.
* The netns implementation search of the interface is based on the
(name, vrf-id) tuple and interface is created if not found, the
vrf-id is never updated.
- deletion of an interface (reception of a RTM_DELLINK netlink message):
* The vrf-lite implementation is unchanged: the interface
information are cleared and the interface is moved to the default
vrf if it does not belong to (to allow vrf deletion)
* The netns implementation is changed: only the interface
information are cleared and the interface stays in its vrf to
avoid conflict with interface with the same name in the default
vrf.
This implementation reverts (partially or totally):
commit 393ec5424e ("zebra: fix missing node attribute set in ifp")
commit e9e9b1150f ("lib: create interface even if name is the same")
commit 9373219c67 ("zebra: improve logs when replacing interface to an
other netns")
Fixes: b53686c52a ("zebra: delete interface that disappeared")
Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This debug should be moved to an error situation since it's a
developmental escape that needs to be fixed.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The definition of the interface commands in vtysh.c were outdated.
Currently, all daemons that call if_cmd_init() will have the "no interface
IFNAME" command and the "[no] description" commands as well, so there's
no need to define exceptions for these commands anymore.
To fix this, make extract.pl parse the if.c file so that vtysh can get the
interface commands from there automatically. Only the "interface IFNAME
[vrf NAME]" must be kept in vtysh.c because it changes the vty node and
thus needs special treatment.
Finally, make pimd and pbrd display interface descriptions on "sh run"
when they are configured.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When I did a show ip route with `json` on a vrf when it didn't exist,
frr would output invalid json.
Signed-off-by: Nathan Van Gheem <nathan@cumulusnetworks.com>
This function should be called with a known vrf_id. All other cases, the
other API should be called.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
For supporting vrf based on namespaces, it is possible that an interface
with the same index is present. This is the case for loopback
interfaces. For that, for each query, if the interface is not found
, matching the vrf identifier, then a new interface is created, when the
backens for VRF is NETNS.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Modify if_lookup_by_index to accept a VRF_UNKNOWN
as a vrf_id. This will cause it to look in all
vrf's for the interface pointer.
Subsequently all if_XXXX functions that call this function
will also get this behavior.
VRF_UNKNOWN *should* not be used for interface creation
as that this will break some core assumptions.
This work is part of allowing vrf route leaking. Currently
it is possible to create a route in the linux kernel that has
a nexthop across vrf boundaries.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This improves code readability and also future-proofs our codebase
against new changes in the data structure used to store interfaces.
The FOR_ALL_INTERFACES_ADDRESSES macro was also moved to lib/ but
for now only babeld is using it.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
If the p1 and p2 arguments pointed to identical strings ending with
a non-numeric character (e.g. "lo"), this function would return -1
instead of 0 as one would expect. This inconsistency didn't matter
for sorted linked-lists but for red-black trees it's a major source
of problems.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Performance tests showed that, when running on a system with a large
number of interfaces, some daemons would spend a considerable amount
of time in the if_lookup_by_index() function. Introduce a new rb-tree
to solve this problem.
With this change, we need to use the if_set_index() function whenever
we want to change the ifindex of an interface. This is necessary to
ensure that the 'ifaces_by_index' rb-tree is updated accordingly. The
return value of all insert/remove operations in the interface rb-trees
is checked to ensure that an error is logged if a corruption is
detected.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This is an important optimization for users running FRR on systems with
a large number of interfaces (e.g. thousands of tunnels). Red-black
trees scale much better than sorted linked-lists and also store the
elements in an ordered way (contrary to hash tables).
This is a big patch but the interesting bits are all in lib/if.[ch].
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Make use of strnlen() and strlcpy() so we can get rid of these
convoluted if_*_by_name_len() functions.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
list_free is occassionally being used to delete the
list and accidently not deleting all the nodes.
We keep running across this usage pattern. Let's
remove the temptation and only allow list_delete
to handle list deletion.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Convert the list_delete(struct list *) function to use
struct list **. This is to allow the list pointer to be nulled.
I keep running into uses of this list_delete function where we
forget to set the returned pointer to NULL and attempt to use
it and then experience a crash, usually after the developer
has long since left the building.
Let's make the api explicit in it setting the list pointer
to null.
Cynical Prediction: This code will expose a attempt
to use the NULL'ed list pointer in some obscure bit
of code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
