OSPFD uses -1 as a sentinel value for uninitialized metrics. When
applying a route map with a +/-metric to redistributed routes, we were
using -1 as our base value to increment or decrement on, which meant
that if you set e.g. +10, you would end up with a redistributed route of
metric 9.
This patch also removes an off-by-one sanity check that would cause a
set metric +1 or set metric 0 to result in a metric value of 20 :-)
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
default-information originate does not work
if config is removed and re-added.
Ticket:CM-20026
Testing Done:
Validate default-information originate config
removed and re-added, check ospf lsa database, and peer
route cache entry for default route.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
1) stream allocation cannot fail
2) some warnings were removed when functions safely ignored
the calling parameters being wrong.
3) some warnings were removed when functions did not consider
the state as an error since we did not return an error code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
In all but one instance we were following this pattern
with ospf_lsa_new:
ospf_lsa_new()
ospf_lsa_data_new()
so let's create a ospf_lsa_new_and_data to abstract
this bit of fun and cleanup all the places where
it assumes these function calls can fail.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
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>
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>
Inform the .clang-format file about LSDB_LOOP and
put the proper indentation for this loop into the
code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Some of the deprecated stream.h macros see such little use that we may
as well just remove them and use the non-deprecated macros.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Currently, ospf external routers are part of struct
ospf_master which is not vrf aware ospf instance.
All ospf external routes are injected/leaked into all
vrfs.
Moved ospf external routes db to struct ospf to make
vrf aware, such one external routes learnt in one vrf
is not leaked into another vrf.
Ticket:CM-18855
Testing Done:
Inject external route in non-default vrf x, validated
ospf database across the vrf x, validated ospf routes
for vrf x.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Router-ID change or ospf instance going down,
send LS-Upd with MAXAGE to self origintated LSAs to
all ospf neighbors.
Ticket:CM-1576
Testing Done:
Bring R1 - R2, Change Router-ID on R2, restart frr on R2
Validated R1 ospf LSDB for max aged 3600 LSA from R2.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
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>
maxage_lsa using prefix structure to store in route_table.
The lsa pointer is 4-8 bytes depending on arch, but prefix member field
would be 1 byte. Use ptr field of prefix structure to store lsa pointer.
Also memset to initialize to avoid crash on ARM platform as same LSA is
referenced from multiple times during shutdown of ospf.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
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>
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>
This ditches tv_add, tv_sub, tv_cmp, etc. in favour of
monotime{,_since,_until}() which actually makes the code much more
readable in some locations.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
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>
HAVE_OPAQUE_LSA is used by default and you have to actively turn it off
except that OPAQUE_LSA is an industry standard and used pretty much
everywhere. There is no need to have special #defines for this anymore.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
(cherry picked from commit 36fef5708d074a3ef41f34d324c309c45bae119b)
ospf->lsa_refresher_started is only used in relative timing to itself;
replace with monotonic clock which is appropriate for this.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
When considering small networks that have extreme requirements on
availability and thus convergence delay, the timers given in the OSPF RFC
seem a little “conservative”, i.e., the delay between accepted LSAs and the
rate at which LSAs are sent. Cisco introduced two commands 'timers throttle
lsa all’ and 'timers lsa arrival’, which allow operators to tune these
parameters.
I have been writing a patch to also support 'timers lsa arrival’ fully and
‘timers throttle lsa all’ (without the throttling part) also in quagga.
On higher warning levels, compilers expect %p printf arguments to be
void *. Since format string / argument warnings can be useful
otherwise, let's get rid of this noise by sprinkling casts to void *
over printf calls.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
* ospf_spf.h: use an enum for the reason, and have it as a new argument to
ospf_spf_calculate_schedule, no need for additional call, and let compiler
do the checking.
* ospf_spf.c: format changes - Quagga coding style places function names
at the start of a new line, for easy grepping for definition.
(ospf_spf_calculate_timer) Change the log format of SPF execution time to
avoid ginormous line, and make logging conditional, as is the norm.
(cherry picked from commit b6eef003e1a79471addea0b01853b08aed812cc8)
Conflicts:
ospfd/ospf_spf.c
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Ticket: CM-10948
The previous formula for calculating delay was:
delay = OSPF_LS_REFRESH_TIME - LS_AGE (lsa) - OSPF_LS_REFRESH_JITTER
+ (random () % (2*OSPF_LS_REFRESH_JITTER));
If LS_AGE() is 0 we get:
delay = 1800 - 0 - 60 + (random() % 2*60)
So delay will be 1740 + 0..120, so we could easily pick a delay that is
greater than OSPF_LS_REFRESH_TIME (1800). For the normal case where
we hit this else the LS_AGE will be > 0 so that reduces the chances of
delay being greater than 1800.
Change-Id: Iff33d70089d1393bf3e9c757d9e9faf134699121
Signed-off-by: kitty <khiruthigai.balasubramanian@hpe.com>
Tested-by: NetDEF CI System <cisystem@netdef.org>
Acked-by: Donald Sharp <sharpd@cumulusnetworks.com>
