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>
The following types are nonstandard:
- u_char
- u_short
- u_int
- u_long
- u_int8_t
- u_int16_t
- u_int32_t
Replace them with the C99 standard types:
- uint8_t
- unsigned short
- unsigned int
- unsigned long
- uint8_t
- uint16_t
- uint32_t
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
This commit fixes these three issues:
1) rinfo is used for rip packet sending not tmp_rinfo
2) With RIP_SPLIT_HORIZON and an interface with more than 1 ip addresses
we will not send the routes out an interface that they originate on
3) With RIP_SPLIT_HORIZON_POISONED_REVERSE and an interface with
more than 1 ip address we will not send out ipA with a metric of 16
and ipb with a metric of 1. Both will be 16 now.
Signed-off-by: lyq140 <34637052+lyq140@users.noreply.github.com>
The code was attempting to access a variable that would always be NULL.
In fact this code has been broken since the rip ECMP changes
were put into place a few years back.
I'm going to come straight out and say that I don't fully
understand this code. rinfo is the first item in the ecmp
list and tmp_rinfo is used to iterate over all the items
in the ecmp list. It sure looks like that the changes
made here were just hacked together. So I modified
the tmp_rinfo loop to just work on tmp_rinfo and
the check that was crashing I modified to just use
the rinfo since that what was checked originally
in code before the ECMP was added. So consider
this a hack job to stop the crashing.
I think worse case is that we might be sending some routes
back out interfaces it shouldn't be if you have
ip rip split-horizon poisoned-reverse configured but
that is less bad(tm) than crashing.
Fixes: #1717
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The rinfo variable was being set but never used.
We just need to call rip_ecmp_replace or rip_ecmp_add
this function does not care about the return values
because the rinfo returned is stored on the rip
route entry.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Allow rip_redistribute_add to receive and properly store
the nexthop type passed up from zebra.
Additionally display the different nexthop types appropriately.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
RIP is not using the nexthop data structure and as such when
it does not fully understand when it receives some of the
more exotic nexthop types what to do with it. This is the
start of a series of commits to allow RIP to start understanding
and properly displaying information about different nexthop
types.
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>
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>
