If we have the following configuration:
```
vrf red
smth
exit-vrf
!
interface red vrf red
smth
```
And we delete the VRF using "no vrf red" command, we end up with:
```
interface red
smth
```
Interface config is preserved but moved to the default VRF.
This is not an expected behavior. We should remove the interface config
when the VRF is deleted.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
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>
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>
The route_map_object_t was being used to track what protocol we were
being called against. But each protocol was only ever calling itself.
So we had a variable that was only ever being passed in from route_map_apply
that had to be carried against and everyone was testing if that variable
was for their own stack.
Clean up this route_map_object_t from the entire system. We should
speed some stuff up. Yes I know not a bunch but this will add up.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Replace all lib/thread cancel macros, use thread_cancel()
everywhere. Only the THREAD_OFF macro and thread_cancel() api are
supported. Also adjust thread_cancel_async() to NULL caller's pointer (if
present).
Signed-off-by: Mark Stapp <mjs@voltanet.io>
rip(ng)d_instance_disable unlinks the vrf from the instance which means
that rip(ng)_interfaces_clean never works, because rip(ng)->vrf is
always NULL there. This leads to the crash #6477.
Clean interfaces before disabling the instance to fix the issue.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.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>
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>
Replace all `random()` calls with a function called `frr_weak_random()`
and make it clear that it is only supposed to be used for weak random
applications.
Use the annotation described by the Coverity Scan documentation to
ignore `random()` call warnings.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
And again for the name. Why on earth would we centralize this, just so
people can forget to update it?
Signed-off-by: David Lamparter <equinox@diac24.net>
Same as before, instead of shoving this into a big central list we can
just put the parent node in cmd_node.
Signed-off-by: David Lamparter <equinox@diac24.net>
There is really no reason to not put this in the cmd_node.
And while we're add it, rename from pointless ".func" to ".config_write".
[v2: fix forgotten ldpd config_write]
Signed-off-by: David Lamparter <equinox@diac24.net>
The only nodes that have this as 0 don't have a "->func" anyway, so the
entire thing is really just pointless.
Signed-off-by: David Lamparter <equinox@diac24.net>
Rearrange the ripd northbound callbacks as following:
* rip_nb.h: prototypes of all northbound callbacks.
* rip_nb.c: definition of all northbound callbacks and their
associated YANG data paths.
* rip_nb_config.c: implementation of YANG configuration nodes.
* rip_nb_state.c: implementation of YANG state nodes.
* rip_nb_rpcs.c: implementation of YANG RPCs.
* rip_nb_notifications.c: implementation of YANG notifications.
This should help to keep to code more organized and easier to
maintain.
No behavior changes intended.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
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.
This reverts commit 83981138fe.
ripd operational & config data may already applied and available, while
an external event requests for changing the vrf name. this change
updates the config and operational context of yang.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
if default vrf name is updated, then rip contexts based on that
hypothetical vrfname, will be enabled.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Field vrf_id is replaced by the pointer of the struct vrf *.
For that all other code referencing to (interface)->vrf_id is replaced.
This work should not change the behaviour.
It is just a continuation work toward having an interface API handling
vrf pointer only.
some new generic functions are created in vrf:
vrf_to_id, vrf_to_name,
a zebra function is also created:
zvrf_info_lookup
an ospf function is also created:
ospf_lookup_by_vrf
it is to be noted that now that interface has a vrf pointer, some more
optimisations could be thought through all the rest of the code. as
example, many structure store the vrf_id. those structures could get
the exact vrf structure if inherited from an interface vrf context.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
the vrf_id parameter is replaced by struct vrf * parameter.
this impacts most of the daemons that look for an interface based on the
name and the vrf identifier.
Also, it fixes 2 lookup calls in zebra and sharpd, where the vrf_id was
ignored until now.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
We should never attempt to remove a list item in the "del" callback
of the list. This is already performed by the list_delete() function,
doing it twice leads to crashes or memory corruption.
Introduce the offset_list_free() function so that we can separate the
removal and deallocation of offset lists into separate functions,
without code duplication. offset_list_del() will be used by the
northbound callbacks to remove offset lists, while offset_list_free()
will be used by rip_clean() to clean up all RIP offset lists using
list_delete(). Do the same for ripngd.
This is a fallout from the ripd/ripngd northbound conversion.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Merge commit to solve a bunch of conflicts with other PRs that were
merged in the previous weeks.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
an interface rmap context can be created from a custom name string,
instead of a vrf. This ability permits to handle several instances of
interface route map in the same vrf. The naming convention will be
transparent on what the name is for in the daemon code.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
so as to handle ri/ripng/eigrp multiple instances, the need is to
encapsulate if_rmap hash table into a container context self to each
instance. This work then reviews the if_rmap api, mainly by adding a
if_rmap_ctx context, that is passed for each exchange between library
and the daemon.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
ripd and ripngd were leveraging the zclient code to keep track of
the redistribute configuration, which is what most daemons do. The
problem, however, is that the zclient code uses VRF IDs to identify
VRFs, and VRF IDs are unknown until a VRF is enabled (information
received from zebra). This means we can't configure a redistribute
command on a RIP instance when the corresponding VRF is disabled
(doing so leads to a null-dereference crash right now in both ripd
and ripngd).
To fix this, change the rip/ripng data structures so that they
keep track of the full redistribute configuration and not only the
route-map and metric associated to each command. This is similar
to what bgpd and ospfd are doing to solve the same problem. In the
future the zclient code and all daemons need to be refactored to
consolidate the handling of redistribute configuration in a single
place to reduce code duplication. One of the most important changes
to do is to use VRF names and not VRF IDs to identify VRFs.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
* Turn the "instance" YANG presence-container into a YANG list keyed
by the new "vrf" leaf. This is a backward incompatible change but
this should be ok for now.
* RIP VRF instances can be configured even when the corresponding
VRF doesn't exist. And a RIP VRF instance isn't deleted when
the corresponding VRF is deleted. For this to work, implement the
rip_instance_enable() and rip_instance_disable() functions that are
called to enable/disable RIP routing instances when necessary. A
RIP routing instance can be enabled only when the corresponding
VRF is enabled (this information comes from zebra and depends on
the underlying VRF backend). Routing instances are stored in the new
rip_instances rb-tree (global variable).
* Add a vrf pointer to the rip structure instead of storing vrf_id
only. This is much more convenient than using vrf_lookup_by_id()
every time we need to get the vrf pointer from the VRF ID. The
rip->vrf pointer is updated whenever the VRF enable/disable hooks
are called.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The ripd code can handle this error just fine (e.g. reject the
configuration transaction), there's no need to exit when a socket
fails to be created.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
For unknown reasons, the IP_MULTICAST_IF sockoption doesn't
seem to work when the given socket is bound to a VRF device on
Linux. Switch from sendto() to sendmsg() so that we can specify the
outgoing interface of multicast packets using IP_PKTINFO ancillary
data. This also makes ripd more consistent with ripngd, which also
uses sendmsg() to send packets on the wire.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This is the last step to make ripd ready for multi-instance support.
Remove the rip global variable and add a "rip" parameter to all
functions that need to know the RIP instance they are working
on. On some functions, retrieve the RIP instance from the interface
variable when it exists (this assumes interfaces can pertain to
one RIP instance at most, which is ok for VRF support).
In preparation for the next commits (VRF support), add a "vrd_id"
member to the rip structure, and use rip->vrf_id instead of
VRF_DEFAULT wherever possible.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This is an old standing bug where the list of RIP peers wasn't
cleared after deconfiguring RIP, which caused the existing peers
to still be present on a newly configured RIP instance (except when
the timed out after ~3 minutes). Fix this.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The only sideeffect of this change is that these counters will be
reset when RIP is deconfigured and then configured again, but this
shouldn't be a problem as the RIP MIB isn't specific about this.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
None of these variables or functions were being used since the
initial revision ~16 years ago. It's safe to say we can remove
them now.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
* Call rip_clean() only when RIP is configured, this way we can
remove one indentation level from this function.
* rip_redistribute_clean() is only called on shutdown, so there's
no need to call rip_redistribute_withdraw() there since the RIP
table is already cleaned up elsewhere.
* There's no need to clean up the "rip->neighbor" nodes manually before
calling route_table_finish().
* Deallocate the rip structure only at the end of the function. This
prepares the ground for the next commits where all global variables
will be moved to the rip structure.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
a distribute_ctx context pointer is returned after initialisation to the
calling daemon. this context pointer will be further used to do
discussion with distribute service. Today, there is no specific problem
with old api, since the pointer is the same in all the memory process.
but the pointer will be different if we have multiple instances. Right
now, this is not the case, but if that happens, that work will be used
for that.
distribute-list initialisation is split in two. the vty initialisation
is done at global level, while the context initialisation is done for
each routing daemon instance.
babel daemon is being equipped with a routing returning the main babel
instance.
also, a delete routine is available when the daemon routing instance is
suppressed.
a list of contexts is used inside distribute_list. This will permit
distribute_list utility to handle in the same daemon to handle more than
one context. This will be very useful in the vrf context.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Implement the 'authentication-failure' and 'authentication-type-failure'
notifications defined in the frr-ripd YANG module.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This command deletes all received routes from the RIP routing table.
It should be used with caution as it can create black holes in the
network until RIP reconverges. Very useful to make automated testing
(e.g. ANVL) more predictable, since the internal state of ripd can be
cleared after each test.
Implement the command using a YANG RPC so that it can be executed by
other northbound clients in addition to the CLI.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
We can now leverage the new northbound API to perform a full configuration
reload in ripd without the need for external help (i.e. frr-reload.py).
When vty_read_config() is called with the 'config' parameter set to
NULL, it performs a new configuration transaction where the running
configuration is *replaced* by the provided configuration file. With that
said, we don't need to do anything other than calling this function in
the SIGHUP handler of all FRR daemons. If a daemon hasn't been converted
to the new northbound model, vty_read_config() will simply *merge*
the configuration file into the running configuration.
The calls to rip_clean() and rip_reset() in the SIGUP handler were
changing configuration variables directly, bypassing the northbound
layer. Configuration variables should be changed only by the northbound
callbacks, and failure to respect that inevitably leads to inconsistencies
and crashes. Fix this.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Now that "router rip" and all underlying commands were converted to the
new northbound model, there's no need to use the qobj infrastructure to
keep track of the 'rip' global variable anymore.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Trivial conversion. Use the northbound 'apply_finish()' callback so
we'll call rip_event() only once even if we change the three RIP timers
at the same time.
Convert the timers to uint32_t to match their representation in the
YANG model.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Trivial conversion. Remove the rip->route routing table and associated
code because this variable was used only to show the running
configuration.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Trivial conversion. As usual, combine multiple DEFUNs into a single
DEFPY for simplicity.
As a bonus of the northbound conversion, this commit fixes the
redistribution of certain protocols into ripd. The 'redist_type' array
used by the "redistribute" commands was terribly outdated, which was
preventing the CLI to parse correctly certain protocols like isis
and babel.
Remove the route_map hooks installed by rip_route_map_init() since they
were redundant (rip_init() already takes care of that).
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
In ripd, the "passive-interface default" command has the following
behavior:
* All interfaces are converted to the passive mode;
* The "passive-interface IFNAME" command becomes a no-operation and
"passive-interface IFNAME" statements are removed from the running
configuration.
* The "no passive-interface IFNAME" can be used to remove interfaces
from the passive mode.
This command was modeled using the following YANG data nodes in the
frr-ripd module:
leaf passive-default {
type boolean;
default "false";
description
"Control whether interfaces are in the passive mode
by default or not.";
}
leaf-list passive-interface {
when "../passive-default = 'false'";
type string {
length "1..16";
}
description
"A list of interfaces where the sending of RIP packets
is disabled.";
}
leaf-list non-passive-interface {
when "../passive-default = 'true'";
type string {
length "1..16";
}
description
"A list of interfaces where the sending of RIP packets
is enabled.";
}
The 'when' statements guarantee that the list of passive interfaces
is cleared when the "passive-interface default" command is entered
(likewise, they guarantee that the list of non-passive interfaces is
cleared when the "passive-interface default" command is removed). This
matches exactly the behavior we want to model.
Finally, move the 'passive_default' global variable into the
'rip' structure where it belongs. This fixed the bug where the
"passive-interface default" command was being retained after a "no router
rip" + "router rip".
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Remove the rip_offset_list_set() and rip_offset_list_unset() functions
since they set/unset multiple configuration options at the same time. The
northbound callbacks need to set/unset configuration options individually.
The frr-ripd YANG module models the "offset-list" command using a list
keyed by the 'interface' and 'direction' leafs. One important detail is
that the IFNAME parameter is optional, and when it's not present it means
we want to match all interfaces. This is modeled using an interface name
of '*' since key lists are mandatory by definition in YANG.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The frr-ripd YANG module models the ripd "network" command using two
separate leaf-lists for simplicity: one leaf-list for interfaces and
another leaf-list for actual networks. In the 'cli_show' callbacks,
display the "network" command for entries of both leaf-lists.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The "distance (1-255) A.B.C.D/M [WORD]" command was modeled using a
YANG list, which makes it a little bit more complicated to convert to
the new northbound model.
The rip_distance_set() and rip_distance_unset() functions were removed
since they set/unset multiple configuration options at the same time. The
northbound callbacks need to set/unset configuration options individually.
When a distance list is created, use yang_dnode_set_entry() to store
a pointer in the configuration node, and retrieve this pointer in the
other callbacks using yang_dnode_get_entry().
The 'rip_distance' structure was moved to ripd.h so that it can be used
in the rip_northbound.c file.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Trivial conversion.
rip->default_metric was converted to an uint8_t to match the way it's
defined in the YANG module.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Trivial conversion.
'rip->default_information_route_map' was removed since it wasn't being
used anywhere.
'rip->default_information' was removed too because it was being used only
to display the running configuration and thus is not necessary anymore.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Trivial conversion. The rip->ecmp variable was converted to a boolean to
match the way it's defined in the YANG module.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
* Implement the northbound callbacks associated to the
'/frr-ripd:ripd/instance' YANG path (the code is mostly a copy and paste
from the original "router rip" DEFUNs);
* Move rip_create_socket() out of rip_create() since creating a socket
is an error-prone operation and thus needs to be performed separately
during the NB_EV_PREPARE phase;
* On rip_create(), fetch the defaults from the frr-ripd YANG model;
* Convert the "[no] router rip" CLI commands to be dumb wrappers around
the northbound callbacks;
* On config_write_rip(), write logic to call all 'cli_show' northbound
callbacks defined under the '/frr-ripd:ripd/instance' YANG path.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
There is no need to check for failure of a ALLOC call
as that any failure to do so will result in a assert
happening. So we can safely remove all of this code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
fix a bug when sending packets.
in authenticate mode but without any string,
no packet should send.
Signed-off-by: lyq140 <34637052+lyq140@users.noreply.github.com>
fix a bug when sending a rip packet.
in authenticate mode but without any string,
no packet should send.
Signed-off-by: lyq140 <34637052+lyq140@users.noreply.github.com>
Fix ripd crash of null pointer.
when authenticate a rip packet,
the key pointer or the key string pointer may be null,
the code have to return then.
Signed-off-by: lyq140 <34637052+lyq140@users.noreply.github.com>
This fix a crash of null pointer.
when we don't add a key string or delete it,
the key is not null but key string is null,
so the code have to return.
Signed-off-by: lyq140 <34637052+lyq140@users.noreply.github.com>
Coverity SA has noticed that we are not ignoring the return
codes from rip_send_packet in one case. Since we do not care
let the system know we don't.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
