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>
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>
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
If the user configures some command that is already in the config we
should return CMD_WARNING instead of CMD_WARNING_CONFIG_FAILED
This reverts commit c14777c6bf.
clang 5 is not widely available enough for people to indent with. This
is particularly problematic when rebasing/adjusting branches.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
This allows frr-reload.py (or anything else that scripts via vtysh)
to know if the vtysh command worked or hit an error.
log.c provides functionality for associating a constant (typically a
protocol constant) with a string and finding the string given the
constant. However this is highly delicate code that is extremely prone
to stack overflows and off-by-one's due to requiring the developer to
always remember to update the array size constant and to do so correctly
which, as shown by example, is never a good idea.b
The original goal of this code was to try to implement lookups in O(1)
time without a linear search through the message array. Since this code
is used 99% of the time for debugs, it's worth the 5-6 additional cmp's
worst case if it means we avoid explitable bugs due to oversights...
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The FSF's address changed, and we had a mixture of comment styles for
the GPL file header. (The style with * at the beginning won out with
580 to 141 in existing files.)
Note: I've intentionally left intact other "variations" of the copyright
header, e.g. whether it says "Zebra", "Quagga", "FRR", or nothing.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Pass pointer to pointer instead of assigning by return value. See
previous commit message.
To ensure that the behavior stays functionally correct, any assignments
with the result of a thread_add* function have been transformed to set
the pointer to null before passing it. These can be removed wherever the
pointer is known to already be null.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The way thread.c is written, a caller who wishes to be able to cancel a
thread or avoid scheduling it twice must keep a reference to the thread.
Typically this is done with a long lived pointer whose value is checked
for null in order to know if the thread is currently scheduled. The
check-and-schedule idiom is so common that several wrapper macros in
thread.h existed solely to provide it.
This patch removes those macros and adds a new parameter to all
thread_add_* functions which is a pointer to the struct thread * to
store the result of a scheduling call. If the value passed is non-null,
the thread will only be scheduled if the value is null. This helps with
consistency.
A Coccinelle spatch has been used to transform code of the form:
if (t == NULL)
t = thread_add_* (...)
to the form
thread_add_* (..., &t)
The THREAD_ON macros have also been transformed to the underlying
thread.c calls.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
These have copies in vtysh that do the node-switch locally and are
listed in extract.pl's ignore list. The ignore list however is
redundant since DEFUN_NOSH does the same thing...
ldpd is a bit hacky, but Renato is reworking this anyway.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Start centralising startup & option parsing into the library.
FRR_DAEMON_INFO is a bit weird, but it will become useful later (e.g.
for killing the ZLOG_* enum, and having the daemon name available)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* Simplify the RIP_TIMER_OFF macro and use it on more places;
* Be more explicit when creating the RIP UDP socket - cosmetic change
since socket(AF_INET,SOCK_DGRAM,0) defaults to UDP on every known
UNIX/Linux platform.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
We still need to check for self-generated packets on rip_read() because
ripd may also send broadcast packets. But using IP_MULTICAST_LOOP on the
ripd socket will at least prevent us from receiving a lot unnecessary
multicast packets when RIPv2 is being used, thus improving performance.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
In the early days of ripd, we supported running RIP on secondary IP
addresses. To do that, everytime we needed to send a multicast packet,
we would create a new temporary socket for each of the interface's
addresses and call bind() to change the source IP of the outgoing packets.
The problem with these temporary sockets is that they are more specific
than the global RIP socket (bound to INADDR_ANY). Then, even though these
sockets only exist for a short amount of time, they can receive some RIP
packets that were supposed to be received on the global RIP socket. And
since we never read from the temporary sockets, these packets are dropped.
Since we don't support secondary addresses anymore, the simplest way to
fix this problem is to stop using temporary sockets for sending multicast
packets. We are already setting IP_MULTICAST_IF before sending each
multicast packet, and in this case the primary address of the selected
interface is used as the source IP of the outgoing packets, which is
exactly what we want.
If we decide to reintroduce support for secondary addresses in the future,
we should try one of the following:
* Use IP_SENDSRCADDR/IP_PKTINFO to set the source address of the outgoing
multicast packets;
* Create one permanent UDP socket for each possible interface address,
and enable reading on all sockets.
Fixes the following IxANVL RIP tests: 7.10 and 14.1.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
We can't use if_lookup_prefix() in rip_update_process() because this
function uses prefix_cmp() internally to try matching an interface
address to a static neighbor's address.
Since prefix_cmp() tries to match exact prefixes, if_lookup_prefix()
was always returning NULL.
What we really need here is to use prefix_match(), which checks if
one prefix includes the other (e.g. one /24 interface including a /32
static neighbor's address). The fix then is to replace the call to
if_lookup_prefix() and use if_lookup_address() instead, which uses
prefix_match() internally.
Fixes IxANVL RIP test 17.1
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
RFC 2453 says (section 5.1):
"(...) For completeness, routers should also implement a receive control
switch which would determine whether to accept, RIP-1 only, RIP-2 only,
both, or none. It should also be configurable on a per-interface basis".
For the "ip rip send version" command, we don't need to implement the
"none" option because there's already the "passive-interface" command
for that.
Fixes IxANVL RIP test 16.8.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This command allows ripd to send v2 updates as broadcast packets instead
of multicast packets. Useful as a technique to help with RIPv1/v2
interop issues.
Fixes IxANVL RIP test 16.2
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
RFC 2453 says:
"If the password is under 16 octets, it must be left-justified and padded
to the right with nulls (0x00)".
Fixes IxANVL RIP test 10.3.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The dump of the md5 hash was missing one byte of the hash.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
If a command is put into the VIEW_NODE, it is going into the
ENABLE_NODE as well. This is especially true for show commands.
As such if a command is in both consolidate it down to VIEW_NODE.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This patch improves zebra,ripd,ripngd,ospfd and bgpd so that they can
make use of 32-bit route tags in the case of zebra,ospf,bgp or 16-bit
route-tags in the case of ripd,ripngd.
It is based on the following patch:
commit d25764028829a3a30cdbabe85f32408a63cccadf
Author: Paul Jakma <paul.jakma@hpe.com>
Date: Fri Jul 1 14:23:45 2016 +0100
*: Widen width of Zserv routing tag field.
But also contains the changes which make this actually useful for all
the daemons.
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
distribute.c doesn't allow to manage both v4 and v6 distribute lists. This
patch fix this problem by having 4 DISTRIBUTE* values in the enumeration instead
of two. The code in all daemons using distribute.c is adapted.
* rip_interface.c: Default for split_horizon_default differed between
rip_interface_new and rip_interface_reset, causing at least some issues
after interface events. See patchwork #604. Fix, and consolidate code.
(rip_interface_{reset,clean}) rename these to 'interface', as that's more
appropriate. Spin the ri specific bodies of these functions out to
rip_interface_{reset,clean} helpers. Factor out the overlaps, so
rip_interface_reset uses rip_interface_clean.
(rip_interface_new) just use rip_interface_reset.
* ripd.h: Update for (rip_interface_{reset,clean})
Reported by xufeng zhang, with a suggested fix on which this commit expands.
See patchwork #604. This commit addresses only the split-horizon
discrepency, issue #2. The other issue they reported, #1, is not addressed,
though suggested fix seems inappropriate.
Cc: xufeng.zhang@windriver.com
Quagga was using a mix of srand/rand and srandom/random.
Consolidate to use srandom/random which are the POSIX
versions of random number generators
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Introduce a new command "[no] allow-ecmp" to enable/disable the
ECMP feature in RIP. By default, ECMP is not allowed.
Once ECMP is disabled, only one route entry can exist in the list.
* rip_zebra.c: adjust a debugging information, which shows the number
of nexthops according to whether ECMP is enabled.
* ripd.c: rip_ecmp_add() will reject the new route if ECMP is not
allowed and some entry already exists.
A new configurable command "allow-ecmp" is added to control
whether ECMP is allowed.
When ECMP is disabled, rip_ecmp_disable() is called to
remove the multiple nexthops.
* ripd.h: Add a new member "ecmp" to "struct rip", indicating whether
ECMP is allowed or not.
Signed-off-by: Feng Lu <lu.feng@6wind.com>
Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 0b74a0a5db7bcf65bf68c44b547b02b1310b5cdb)
* Each node in the routing table is changed into a list, holding
the multiple equal-cost paths.
* If one of the multiple entries gets less-preferred (greater
metric or greater distance), it will be directly deleted instead
of starting a garbage-collection timer for it.
The garbage-collection timer is started only when the last entry
in the list gets INFINITY.
* Some new functions are used to maintain the ECMP list. And hence
rip_rte_process(), rip_redistribute_add() and rip_timeout() are
significantly simplified.
* rip_zebra_ipv4_add() and rip_zebra_ipv4_delete() now can share
the common code. The common part is moved to rip_zebra_ipv4_send().
Signed-off-by: Feng Lu <lu.feng@6wind.com>
Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit b397cf4f0fc484c5ebfc8a680090055c8e6cbe32)
Conflicts:
ripd/rip_zebra.c
RIP_MAX_RTE is defined in ripd.h as 25 but is in fact the
result of a formula. More over it is not used in the code:
the code itself includes the fomula. This makes it un-clear
for maintenance.
Signed-off-by: Feng Lu <lu.feng@6wind.com>
Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
(cherry picked from commit 342a31bfda)
The connected_lookup_address function should really
be a connected_lookup_prefix function. Refactor
the code to use it.
Ticket: CM-10890
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>
The file if.c has a iflist that had the list of interfaces
in the default vrf. Remove this variable and replace
with a vrf_iflist lookup on the default vrf where it
was used.
Additionally, modify ptm code to iterate over all vrf's
when enabling ptm.
Ticket: CM-10338
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed-by: Radhika Mahankali <radhika@cumulusnetworks.com>
These issues have been found by running buildtest.sh
using GCC 5.2.0 and Clang 3.7.0
Fixes pointer checks that can never be null
Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Tested-by: NetDEF CI System <cisystem@netdef.org>
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This commit fixes these warnings:
1) bgpd/bgp_nexthop.c - dereferencing pointer 'X' does break strict-aliasing rules
3) ripd/ripd.c - 'ifaddr.prefixlen' may be used uninitialized in this function
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ticket:
Reviewed By: CCR-3335
Testing Done: bgpsmoke, ENHE tests etc.
Add support for filtering routes from upper layer protocols to zebra
via route-maps for IPv6. The same functionality already existed for
IPv4.
In addition, add support for setting source of routes via IPv6 protocol
map.
Signed-off-by: Dinesh G Dutt <ddutt@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Vivek Venkataraman <vivek@cumulusnetworks.com>
Reviewed-by: Vipin Kumar <vipin@cumulusnetworks.com>
RIP_MAX_RTE is defined in ripd.h as 25 but is in fact the
result of a formula. More over it is not used in the code:
the code itself includes the fomula. This makes it un-clear
for maintenance.
Signed-off-by: Feng Lu <lu.feng@6wind.com>
Reviewed-by: Alain Ritoux <alain.ritoux@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Quagga sources have inherited a slew of Page Feed (^L, \xC) characters
from ancient history. Among other things, these break patchwork's
XML-RPC API because \xC is not a valid character in XML documents.
Nuke them from high orbit.
Patches can be adapted simply by:
sed -e 's%^L%%' -i filename.patch
(you can type page feeds in some environments with Ctrl-V Ctrl-L)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The interface metric is initialized to 0 in the commit db19c85:
zebra: set metric for directly connected routes via netlink to 0
Ripd and ripngd must be aware of it and avoid increase the
route metric by 0.
Signed-off-by: Feng Lu <lu.feng@6wind.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ripd had a check to restrict metric 0 to only directly connected routes.
This check was implemented by checking against Connected as route type.
This is, however, incorrect -- all routes that directly use an interface
without a nexthop should be treated as directly connected and passed off
with metric 0.
ripngd does not posess such a check and was not touched.
Reported-by: Sean Fulton <sean@gcnpublishing.com>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
ripd_create_socket() failed in bind() on Mac OS X 10.7 since there was garbage
in unused fields of struct sockaddr_in.
* ripd/ripd.c: zero out struct sockaddr_sin from before filling.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* ripd/rip_interface.c
* rip_request_neighbor(): comment out, unused
* rip_request_neighbor_all(): idem
* rip_interface_up(): Cast flags otherwise compiler complains
about %lld not matching uint64_t on 64 bit x86. Print in hex
since flags are bit field.
* rip_interface_add(): idem
* rip_interface_delete(): idem
* ripd/rip_zebra.c
* rip_redistribute_set(): comment out, unused
* ripd/ripd.h
* rip_redistribute_check(): move prototype here so compiler
can check function against prototype
* ripd/ripd.c
* rip_update_default_metric(): comment out, unused
