ospfNbrStateChange is generated when the state of neighbor regresses or
it progresses to a terminal state. When transitioning to or from Full
state on non-broadcast multi-access and broadcast networks the trap
should be sent by the designated router. This last condition was not
taken into account when checking for the conditions of generating the
trap.
Fixesvolta/volta-stack#1811
Signed-off-by: Babis Chalios <mail@bchalios.io>
Use to-string functions for GR message codes instead of raw
string array indexing; the values used can come in packets
and are not validated.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
For TI-LFA, it is necessay to known the Adjacency SID advetise by the nieghbor
routers. However, the current Segment Routing code skip neighbor Adjacency SID
and thus, don't store them into the Segment Routing database.
This PR takes care of neighbor Adjacency SID by allowing to store them in the
Segment Routing database. Corresponding MPLS table entry is only configured if
the advertised Adjacency SID is global i.e. with L-Flag unset.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Currently, only default area parameter is cleared, but we should do the
same for all area parameters with interface addresses.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently, only default area parameter is used for initialization, but
we should use area parameters with interfaces address as well.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Move ospf initialization to the actual place where it is created.
We don't need to do that every time "router ospf" is entered.
Also remove a couple of useless checks that can never be true.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
We should check for existing networks configuration before creating
if_params structure, or it leads to the memory leak.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
ospf_router_id_update checks for ospf->oi_running, but it'll never be 1
right after creation by ospf_new.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
If we enter:
int eth0
ip ospf area 0
ip ospf 10 area 0
!
This will crash ospf. Prevent this from happening.
OSPF instances:
a) Cannot be mixed with non-instance
b) Are their own process.
Since in multi-instance world ospf instances are their own process,
when an ospf processes receives an instance command we must remove
our config( if present ) and allow the new config to be active
in the new process. The problem here is that if you have not
done a `router ospf` above the lookup of the ospf pointer will
fail and we will just crash. Put some code in to prevent a crash
in this case.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This commit fixes the following behavior:
```
nfware(config)# interface enp2s0
nfware(config-if)# ip ospf area 0
nfware(config-if)# no ip ospf area 0
% [ospfd]: command ignored as it targets an instance that is not running
```
We should be able to use the command without configuring the instance.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
We have this pattern in the code base:
if (thread)
THREAD_OFF(thread);
If we look at THREAD_OFF we check to see if thread
is non-null too. So we have a double check.
This is unnecessary. Convert to just using THREAD_OFF
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Instead of closing the socket, just note the failure and
continue on. If we actually failed here so many other
things would not be working at all, that actually
closing the fd won't matter.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When the ASBR stops announcing a prefix into the NSSA area, the LSA
type 7 is removed from the area. However the ABR is refreshing the
type 5 in its LSDB while removing the Type 7 LSA. Routers outside
the area do not get an update.
With the following topology: r1---r2---r3, with r3 being the ASBR
announcing type 7 LSA:
r3 configuration
router ospf
redistribute static
network 10.0.23.0/24 area 1
area 1 nssa
!
We stop announcing prefix 3.3.3.3 in the ASBR
r3# conf
r3(config)# router ospf
r3(config-router)# no redistribute static
r3(config-router)#
r2 (ABR)
r2# sh ip os database
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 33.33.33.33 3600 0x8000002f 0x13be E2 3.3.3.3/32 [0x0] <-- flushed
AS External Link States
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 7 0x8000002f 0x73c7 E2 3.3.3.3/32 [0x0] <-- refreshed(?)
With PR#7086 the LSA type 5 is flushed from the LSDB in r2 and the change is
announced to routers outside the area (r1)
r2# sh ip os da
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 33.33.33.33 3600 0x80000002 0x6d91 E2 3.3.3.3/32 [0x0] <-- flushed
AS External Link States
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 3600 0x80000002 0xcd9a E2 3.3.3.3/32 [0x0] <-- flushed
r1# sh ip os da
AS External Link States
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 3600 0x80000002 0xcd9a E2 3.3.3.3/32 [0x0] <-- flushed
Unfortunately I just realized that with PR#7086 I'm introducing a new bug, as Type-5 LSA
are not being refreshed when reaching MaxAge
r2# sh ip os da
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 33.33.33.33 35 0x80000002 0x6d91 E2 3.3.3.3/32 [0x0] <--- refreshed
AS External Link States
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 3600 0x80000002 0xcd9a E2 3.3.3.3/32 [0x0] <--- not refreshed!
So this PR should fix the original issue and the bug introduced later, so when stopping
redistribution in the ASBR, both type 5 and type 7 are flushed:
r2# sh ip os da
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 33.33.33.33 3600 0x80000002 0x6d91 E2 3.3.3.3/32 [0x0]
AS External Link States
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 3600 0x80000002 0xcd9a E2 3.3.3.3/32 [0x0]
Routers outside the area are also notified
r1# sh ip os da
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 3600 0x80000002 0xcd9a E2 3.3.3.3/32 [0x0]
Re-enabling redistribution, both LSA will be advertised again
r3# conf
r3(config)# router ospf
r3(config-router)# no redistribute static
r3(config-router)# redistribute static
r3(config-router)#
r2# sh ip os da
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 33.33.33.33 19 0x80000001 0x6f90 E2 3.3.3.3/32 [0x0]
AS External Link States
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 11 0x80000001 0xcf99 E2 3.3.3.3/32 [0x0]
and they are refreshed when reaching MaxAge
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 33.33.33.33 10 0x80000002 0x6d91 E2 3.3.3.3/32 [0x0] <-- Seq 2
AS External Link States
Link ID ADV Router Age Seq# CkSum Route
3.3.3.3 10.0.25.2 2 0x80000002 0xcd9a E2 3.3.3.3/32 [0x0] <-- Seq 2
Signed-off-by: ckishimo <carles.kishimoto@gmail.com>
Having an NSSA ABR redistributing statics, the type-7 LSA are being
continuously refreshed (every ~14 secs). The LSA Seq number keeps
incrementing and the LSA age is going back to 0 when reaching ~14s.
This PR fixes the issue by not forcing the LSA update
However I ignore if the "force" parameter was used in purpose. With this
PR updates are sent in case the metric or metric type are changed
Sep 24 08:54:48 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:55:02 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:55:16 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:55:30 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:55:44 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:55:58 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:56:12 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:56:26 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
Sep 24 08:56:40 r2 ospfd[7137]: ospf_flood_through: LOCAL NSSA FLOOD of Type-7.
ip route 2.2.2.2/32 blackhole
router ospf
network 10.0.23.0/24 area 1
area 1 nssa
!
r2# conf t
r2(config)# router ospf
r2(config-router)# redistribute static
r2# sh ip os da
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
2.2.2.2 10.0.25.2 13 0x8000000f 0x3f17 E2 2.2.2.2/32 [0x0] <<< Seq: f, age 13
r2# sh ip os da
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
2.2.2.2 10.0.25.2 0 0x80000010 0x3d18 E2 2.2.2.2/32 [0x0] <<< Seq: 10, age 0
r2# sh ip os da
NSSA-external Link States (Area 0.0.0.1 [NSSA])
Link ID ADV Router Age Seq# CkSum Route
2.2.2.2 10.0.25.2 3 0x8000001b 0x2723 E2 2.2.2.2/32 [0x0] <<< Seq: 1b, age 3
Signed-off-by: ckishimo <carles.kishimoto@gmail.com>
Description:
Added the following debug commands to enable debugs
GR helper functionality.
[no] debug ospf gr helper
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
Description:
The following show commands are added to display helper specific
information.
1.show ip ospf graceful-restart helper [detail] [json]
--> displays user configurations and list of all helpers details.
2.show ip ospf neighbour detail
--> diplays helper details
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
Description:
The follwoing helper exit scenarios are handled.
1. Recv Max age grace LSA from RESTARTER.
2. Grace timer expiry.
3. Due to topo change if lsa check is enabled.
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
Description:
1. Skipping inactivity timer during graceful restart to make
the RESTARTER active even after dead timer expiry.
2. Handling HELPER on unplanned outages.
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
1. Ospf dead-interval will be set as 4 times of hello-interval, incase
if it is not set by using "ip ospf dead-interval <dead-val>".
2. On resetting hello-interval using "no ip ospf hello-interval" the
dead interval and hello due will be changed accordingly.
Signed-off-by: Kaushik <kaushik@niralnetworks.com>
In a topology like R1 -- R2 -- R5, with R2 being NSSA ABR and R5 being
ASBR redistributing external routes, the ABR R2 will translate type-7
LSA into type-5 and advertise to the backbone. In the current implementation
R2 is also advertising a type-4 LSA when there is no need.
RFC 3101: "...NSSA's border routers never originate Type-4 summary-LSAs
for the NSSA's AS boundary routers, since Type-7 AS-external-LSAs are
never flooded beyond the NSSA's border..."
With this PR a type-4 LSA will not be advertised
Signed-off-by: ckishimo <carles.kishimoto@gmail.com>
When the ASBR stops announcing a prefix into the NSSA area, the LSA
type 7 is removed from the area. However the ABR is refreshing the
type 5 in its LSDB while removing the Type 7 LSA. Routers outside
the area do not get an update.
With this change the LSA type 5 is flushed from the LSDB and the
change is announced to routers outside the area
Signed-off-by: ckishimo <carles.kishimoto@gmail.com>
1. Fix for duplicating calls of ospf_external_lsa_refresh()
in ospf_distribute_list_update_timer().
2. Added the comment and change from PR 6981.
Signed-off-by: Kaushik <kaushik@niralnetworks.com>
If NSSA is enabled before redistribution is configured, Type-7 LSA's
are installed. But if NSSA is enabled after redistribution is
configured, Type-7 LSAs are missing.
With this change, when NSSA is enabled, scan for external LSA's and
if they exist, install Type-7.
Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
1. Minor change in distribute-list update timer based on lsa flag.
Co-authored-by: harios <hari@niralnetworks.com>
Signed-off-by: Kaushik <kaushik@niralnetworks.com>
In the context of TI-LFA it is necessary to have multiple
representations of SPFs for so called P and Q spaces. Hence it makes
sense to start with fresh vertex lists, and only delete them when
the SPF calculation is not a 'dry run'.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
Following PR #6726, Coverity Scan detected some new errors in the OSPF Segment
Routing code. This patch corrects them.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
OSPFD sends ARP proactively to speed up convergence for /32 networks
on a p2p connection. It is only an optimization, so it can be disabled.
It is enabled by default.
Signed-off-by: Jakub Urbańczyk <xthaid@gmail.com>
PR #6416 that solves opsf crash when segment routing is restarted, introduce a
regression in Extended Prefix flooding: when segment routing prefix is modified
or removed, new segment routing prefix is no more flooded. This patch correct
this regression.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Add new option to `segment-routing prefix` command to set the
Explcit Null flag in addition to the No-PHP flag. MPLS LFIB configuration
has been also updated to take into account the Explicit Null flag.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
RFC 8665 defines a Segment Routing Local Block for Adjacency SID.
This patch provides the possibility to modify the SRLB as well as
reserved the block range from the Label Manager.
- Introduce new CLI 'segment-routing local-block'
- Add local block to SRDB structure
- Parse / Serialize SRLB in Router Information LSA
- Update OSPF-SR topotest
- Update documentation
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Segment Routing Global Block is now using the Label Manager to reserved
label range. Label Manager connection uses the synchronous mode and dedicated
thread timer is used to establish the connection in safe manner without
blocking OSPFd is the Label Manager is not available.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
In the case of P2P links it is possible to use IP unnumbered which
yields a strong dependency to the interface data for nexthop
resolution in the SPF calculations. While the SPF code strives to
be as independent of non-LSA data as possible there is no way
around here: one has to resolve the nexthop for such a special case
using the interface data.
For this purpose a new flag 'spf_root_node' is introduced to signal
that interface data can be used for P2P links. For now this flag is
always 'true' since the SPF currently always uses the calculating
node as the root node. This will change with the introduction of
TI-LFA where other nodes can be root nodes.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
in OSPF interface data is used for the nexthop resolution
during the SPF algorithm, see RFC2328 16.1.1. However, for
certain technologies like TI-LFA it is desirable to be able
to calculate SPFs for arbitrary root nodes, not just the
calculating node. Since interface data is not available for
other nodes it is necessary to remove this dependency and
make its usage optional, depending on the intent of
changing the RIB with the generated tree (or not).
To signal that a SPF run is used without the intent to
change the RIB an additional flag `spf_dry_run` is
introduced to the ospf_area struct. This flag is currently
only used within the pure SPF code but will be extended
to the SPF postprocessing later on.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
Just non-functional changes, cosmetics, removal of eye
cancer. The intention here is to make the SPF code more
approachable.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
Router Information are contained in opaque LSAs and when such a LSA
is received a new SR node for the advertising router is created.
However, the RI related data is currently not set when such a SR node
already exists. This can happen when e.g. link and prefix information
arrive before the RI and therefore an SR node is created.
This is now fixed by setting the data everytime the RI is received,
independent of the SR node already existing or not.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
The `INET_ADDRSTRLEN` is 16 and is only enough to format an IPv4 address.
So when there is a prefix (`/xx`), the debug output may get truncated.
Use `PREFIX2STR_BUFFER` macro instead to fix the issue.
Signed-off-by: Aaron LI <aly@aaronly.me>
If you configure an area range in decimal format, the running
configuration displays it in dotted format.
Call ospf_area_display_format_set() for area range command,
as it is done for other variants.
Signed-off-by: Duncan Eastoe <duncan.eastoe@att.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>
BFD profiles can now be used on the interface level like this:
interface eth1
ip router isis 1
isis bfd
isis bfd profile default
Here the 'default' profile needs to be specified as usual in the
bfdd configuration.
Signed-off-by: GalaxyGorilla <sascha@netdef.org>
On startup of both zebra and ospfd. If ospfd has not
received a valid router-id *but* has received interface
data, interfaces will not be turned on in the state
machine. When ospf finally receives a valid router-id
it would never actually kick the state machine into
action for those interfaces it has been configured for.
Modify ospf on router id changes, *if* the old
router id was INADDR_ANY *and* the interface is
operative *and* the oi->state is ISM_Down, give
it the old kick in the patooeys
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Default originate with route-map was broken. Routemap
was not getting applied for redistribute route but was
getting applied default route itself. No default route
was originated even when routemap was satisfied.
Signed-off-by: Santosh P K <sapk@vmware.com>
Fix a number of library and daemon issues so that daemons can
call frr_fini() during normal termination. Without this,
temporary logging files are left behind in /var/tmp/frr/.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
For coherency, move and rename functions that send MPLS labels configurations
from ospf_sr.c to ospf_zebra.c:
- ospf_zebra_update_prefix_sid()
- ospf_zebra_delete_prefix_sid()
- ospf_zebra_send_adjacency_sid()
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
SIDs are not uninstall in LFIB when ospf adjacenyi or loopback goes down as
self LSA flusing is not handle by ospf_ext_link_lsa_update(). The patch
introduces new functions ospf_sr_ext_itf_add() and ospf_sr_ext_itf_delete() in
ospf_sr.c to directly manage LFIB for SIDs when change is detected in
ospf_ext_link_ism_change() and ospf_ext_link_nsm_change().
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
- Improve parsing of Router Information, especially when a router
stops advertising Segment Routing capabilities
- Finish conversion to '%pFX' and '%pI4'
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
* Change sr_prefix structure in ospf_sr.h to add support to ECMP
* Add new Segment Routing information to ospf_paths in ospf_route.h
* Backport MPLS label configuration from IS-IS Segment Routing implementation
* Re-write log message in ospf_sr.c and ospf_ext.c
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Description:
OSPF uses an algo to generate unique LSIDs when route updates
exists with same adrress and different masklens. It genearates
the unique LSIDs by masking its hostbits.
Ex :
Rt1 : 10.0.0.0/32 - LSID : 10.0.0.0
Rt2 : 10.0.0.0/16 - LSID : 10.0.255.255
Rt3 : 10.0.0.0/24 - LSID : 10.0.0.255
Observed an issue with external LSAs when such routes originated.
If the first route (with actual address as LSID) is got deleted,
the routes with same addresss(different msaks) are failed
to get LSA pointers from LSDB due to current fetching API design.
api : ospf_external_info_find_lsa
Here , it is allowing to look up the LSA with address specific
unique LSID (address with host bits set ex: 10.0.255.255) only
if the LSA exists where LSID as actual address of the route
(ex: 10.0.0.0 ) which is not expected and cauing for other issues.
Fix:
Corrected this logic, by looking up the LSA with unique LSID first
if it doesn’t exist then It is allowing to look up the LSDB with LSID
as address of the route.
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
Issue number #6291 describes how OSPFd crashes after being deleted and then
added again with configuration when segment routing is used.
The problem occurs in ospf_ri.c because the OspfRI structures retains
the reference to the old area pointer which is mofified when ospfd is
reactivated by configuration. When segment routing is activated, the LSA Router
Information is sent with reference to the old area pointer, instead the new one,
which causes the crash. The same problem is also present in ospf_ext.c with
OspfEXT structure and Extended Link/Prefix structure.
This commit introduces Extended Link/Prefix and Router Information LSAs flusing
when OSPFd is stopped when configuration is removed and adds the correct
initialization to the area pointer in OspfRI and Extended Link/Prefix structure
when OSPFd is re-enabled with the configuration. Area pointer has been removed
from the OspfEXT structure as it is never used with this commit.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
The command `area ... virtual-link ... retransmit-interval` supports
1-65535 range and the documentation already said
`ip ospf retransmit-interval` supports that, lets make the DEFUN to
accept that value.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Description:
When a routemap applied to set a tag, all the permitted routes are
refreshed with new tag, but when a different route map applied with
a different action still the same tag persits in the external route.
The actual tag received from zebra is expected to be set back to the
routes here. Corrected this behaviour by restoring a original tag
received from zebra.
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
Description:
Route-tag is not set to external lsas originated by ospf when a routemap
applied by setting a specific tag. When applying a route-map on redistribution,
external lsas will be refreshed if there is any change in the route parametrs
after applying routemap. But changing tag is not handled here.
Added the apripriate fix to correct this.
Signed-off-by: Rajesh Girada <rgirada@vmware.com>
These are easy to get subtly wrong, and doing so can cause
nondeterministic failures when racing in parallel builds.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
It's possible(but unlikely) that a read of data from the
network will give us bogus data. Don't automatically
just trust the data size from the network and limit
the read to the size of the buffer we have in play.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
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>
Fix
- Modulo check on data length not inclusive enough
- Garbage heap read when bounds checking
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
ospf_opaque_self_originated_lsa_received decrements refcount which can
result in a free, this is followed by a call to ospf_ls_ack_send which
accesses the freed LSA
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
... Oops ...
(for context, the defaults code originally didn't have a dedicated
"bool" variant and just used long for bools... I derp'd this when
adding bool as a separate case :( )
Reported-by: Donald Sharp <sharpd@cumulusnetworks.com>
Signed-off-by: David Lamparter <equinox@diac24.net>
Line break at the end of the message is implicit for zlog_* and flog_*,
don't put it in the string. Mid-message line breaks are currently
unsupported. (LF is "end of message" in syslog.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Some logging systems are, er, "allergic" to tabs in log messages.
(RFC5424: "The syslog application SHOULD avoid octet values below 32")
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Use the zapi_nexthop struct with the mpls_labels
zapi messages instead of the special-purpose (and
more limited) nexthop struct that was being used.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Multi instance ospf support was broken due to PR #4564.
Adding fix back and extra checks to support multi instance
OSPF.
Fixes issues #5343 & #5741
Signed-off-by: Santosh P K <sapk@vmware.com>
The vrrpd one conflicts with the standalone vrrpd package; also we're
installing daemons to /usr/lib/frr on some systems so they're not on
PATH.
Signed-off-by: David Lamparter <equinox@diac24.net>
Address Sanitizer is reporting this issue:
==26177==ERROR: AddressSanitizer: heap-use-after-free on address 0x6120000238d8 at pc 0x7f88f7c4fa93 bp 0x7fff9a641830 sp 0x7fff9a641820
READ of size 8 at 0x6120000238d8 thread T0
#0 0x7f88f7c4fa92 in if_delete lib/if.c:290
#1 0x42192e in ospf_vl_if_delete ospfd/ospf_interface.c:912
#2 0x42192e in ospf_vl_delete ospfd/ospf_interface.c:990
#3 0x4a6208 in no_ospf_area_vlink ospfd/ospf_vty.c:1227
#4 0x7f88f7c1553d in cmd_execute_command_real lib/command.c:1073
#5 0x7f88f7c19b1e in cmd_execute_command lib/command.c:1132
#6 0x7f88f7c19e8e in cmd_execute lib/command.c:1288
#7 0x7f88f7cd7523 in vty_command lib/vty.c:516
#8 0x7f88f7cd79ff in vty_execute lib/vty.c:1285
#9 0x7f88f7cde4f9 in vtysh_read lib/vty.c:2119
#10 0x7f88f7ccb845 in thread_call lib/thread.c:1549
#11 0x7f88f7c5d6a7 in frr_run lib/libfrr.c:1093
#12 0x412976 in main ospfd/ospf_main.c:221
#13 0x7f88f73b082f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#14 0x413c78 in _start (/usr/local/master/sbin/ospfd+0x413c78)
Effectively we are in a shutdown phase and as part of shutdown we delete the
ospf interface pointer ( ifp->info ). The interface deletion code
was modified in the past year to pass in the address of operator
to allow us to NULL out the holding pointer. The catch here
is that we free the oi and then delete the interface passing
in the address of the oi->ifp pointer, causing a use after free.
Fixes: #5555
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We actually don't validate the IHL field, although it certainly looks
like we do at a casual glance.
This patch saves us from an assert in case we actually do get an IP
packet with an incorrect header length field.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Some preprocessor constants converted to enums to make the names usable
in the preprocessor.
v2: better isolation between core and vty code to make future northbound
conversion easier.
Signed-off-by: David Lamparter <equinox@diac24.net>
Well, "obviously" this condition block is pointless, since
ospf_router_id_update is called afterwards unconditionally...
(And even if it were needed, it would need to go in ospf_get too.)
Signed-off-by: David Lamparter <equinox@diac24.net>
Commit: ddbf3e6060
This commit modified the interface up handling code in
ZAPI such that the zclient handled the decoding for you.
Prior to this commit ospf assumed that it could use the
old ifp pointer to know state before reading the stream.
This lead to a situation where ospf would `smartly` track
and do the right thing in this situation. This commit
changed this assumption and in certain scenarios, say
a interface was changed after it was already up would
lead to situations where ospf would not properly handle
the new interface up.
Modify ospf to track data that is important to it in
it's interface->info pointer.
This code pattern was followed in both eigrp and pim.
In eigrp's case it was just behaving weirdly in any event
so fixing this pattern is not a big deal. In pim's
case it was not properly using this so it's a no-op
to fix.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We are only saving 20 bytes of string output for ospf neighbor
commands. Fixed output:
act-7326-05# show ip ospf vrf vrf1012 neighbor all
VRF Name: vrf1012
Neighbor ID Pri State Dead Time Address Interface RXmtL RqstL DBsmL
9.9.12.11 1 Full/DROther 39.973s 200.254.2.10 swp49s0.2:200.254.2.9 4 0 0
9.9.12.12 1 Full/DROther 39.995s 200.254.2.14 swp49s1.2:200.254.2.13 9 0 0
9.9.12.13 1 Exchange/DROthe 39.981s 200.254.2.18 swp49s2.2:200.254.2.17 157 0 0
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We test nbr->oi in a couple of places for null, but
in the majority of places of the nbr->oi data is being
used we just access it. Touch up code to trust this
assertion and make the code more consistent in others.
Found in Coverity.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The indentation level for ospf_read was starting to be pretty
extremene. Rework into 2 functions for improved readability.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Read in up to 20(ospf write-multipler X) packets, for handling of data.
This improves performance because we allow ospf to have a bit more data
to work on in one go for spf calculations instead of 1 packet at a time.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Turning on packet debugs and seeing a header dump that is 11
lines long is useless
2019/11/07 01:07:05.941798 OSPF: ip_v 4
2019/11/07 01:07:05.941806 OSPF: ip_hl 5
2019/11/07 01:07:05.941813 OSPF: ip_tos 192
2019/11/07 01:07:05.941821 OSPF: ip_len 68
2019/11/07 01:07:05.941831 OSPF: ip_id 48576
2019/11/07 01:07:05.941838 OSPF: ip_off 0
2019/11/07 01:07:05.941845 OSPF: ip_ttl 1
2019/11/07 01:07:05.941857 OSPF: ip_p 89
2019/11/07 01:07:05.941865 OSPF: ip_sum 0xcf33
2019/11/07 01:07:05.941873 OSPF: ip_src 200.254.30.14
2019/11/07 01:07:05.941882 OSPF: ip_dst 224.0.0.5
We already have this debugged, it's not going to change and the
end developer can stick this back in if needed by hand to debug
something that is not working properly.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This commit has:
The received packet path in ospf, had absolutely no debugs associated with
it. This makes it extremely hard to know when we receive packets for
consumption. Add some breadcrumbs to this end.
Large chunks of commands have no ability to debug what is happening
in what vrf. With ip overlap X vrf this becomes a bit of a problem
Add some breadcrumbs here.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Add a helper function to return the name of the vrf we are in
so it can be used as part of debug strings.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We have a bunch of places that look for ORIGINAL_CODING. There is
nothing in our configure system to define this value and a quick
git blame shows this code as being original to the import a very
very long time ago. This is dead code, removing.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Recently Lot of issues are seen in OSPF adjacnecy establishements,
sessions was tear down because of DD Sequence Number mismatch.
adding Debugs to capture Master & slave generated sequence numbers.
Signed-off-by: Satheesh Kumar K <sathk@cumulusnetworks.com>
Use a local variable to avoid trying to take the address
of a packed struct member - an address from the ip header
in these cases.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
The opaque lsa that we are storing is stored on various
lists depending on it's type. This removal from the
list was being done *after* the pointer was freed. This
is not a good idea. Since the use after free was just
removal from a linked list and the freeing function does
not do anything other than free data, than just switching the function
order should be sufficient.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Root Cause:
Lookup for the point-to-point neighbor was failing because the neighbor
lookup was based on neighbor interface IP address. But, for point-to-point
neighbor the key is router-id for lookup. Lookup failure was causing the
BFD updates from PTM to get dropped.
Fix:
Added walk of the neighbor list if the network type is point-to-point to
find the appropriate neighbor. The match is based on source IP address of
the neighbor since that’s the address registered with BFD for monitoring.
Ticket: CM-20411
Signed-off-by: Radhika Mahankali <radhika@cumulusnetworks.com>
Scenarios where this code change is required:
1. BFD is un-configured from BGP at remote end.
Neighbour BFD sends ADMIN_DOWN state, but BFD on local side will send
DOWN to BGP, resulting in BGP session DOWN.
Removing BFD session administratively shouldn't bring DOWN BGP session
at local or remote.
2. BFD is un-configured from BGP or shutdown locally.
BFD will send state DOWN to BGP resulting in BGP session DOWN.
(This is akin to saying do not use BFD for BGP)
Removing BFD session administratively shouldn't bring DOWN BGP session at
local or remote.
Signed-off-by: Sayed Mohd Saquib sayed.saquib@broadcom.com
Cleanup the interface creation apis to make it more
clear what they are doing.
Make it explicit that the creation via name/ifindex will
only add it to the appropriate list.
Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
For all the places we have a zclient->interface_up convert
them to use the interface ifp_up callback instead.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Switch the zclient->interface_add functionality to have everyone
use the interface create callback in lib/if.c
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Start the conversion to allow zapi interface callbacks to be
controlled like vrf creation/destruction/change callbacks.
This will allow us to consolidate control into the interface.c
instead of having each daemon read the stream and react accordingly.
This will hopefully reduce a bunch of cut-n-paste stuff
Create 4 new callback functions that will be controlled by
lib/if.c
create -> A upper level protocol receives an interface creation event
The ifp is brand spanking newly created in the system.
up -> A upper level protocol receives a interface up event
This means the interface is up and ready to go.
down -> A upper level protocol receives a interface down
destroy -> A upper level protocol receives a destroy event
This means to delete the pointers associated with it.
At this point this is just boilerplate setup for future commits.
There is no new functionality.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There's no need to install MPLS FTNs using the ZEBRA_ROUTE_ADD
message since the ZEBRA_MPLS_LABELS_ADD message already does that
(in addition to installing an MPLS LSP).
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This new message makes it possible to install/reinstall LSPs with
multiple nexthops using a single ZAPI message.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
* Add ability to specify the nexthop type;
* Add ability to install or not a FTN (in addition to an LSP).
These two additions will be useful to install local SR Prefix-SIDs
configured with the no-PHP option.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
SR support for IS-IS is coming so we need to be able to distinguish
OSPF and IS-IS LSPs.
While here, add missing case statement for LDP on
lsp_type_from_re_type().
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Use the route type and instance instead of the route distance
to identify MPLS FTNs. This is a more robust approach since the
routing daemons can modify the distance of their announced routes
via configuration, which can cause inconsistencies.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Do this for the following reasons:
* Improve modularity of the code by separating the decoding of the
ZAPI messages from their processing;
* Create an API that is easier to use by the client daemons.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When OSPF receives a Database description packet and is in
`Down`, `Attempt` or `2-Way` state we are creating a warning
for the end user.
rfc2328 states(10.6):
Down - The packet should be rejected
Attempt - The packet should be rejected
2-Way - The packet should be ignored
I cannot find any instructions in the rfc to state what the operational
difference is between rejected and ignored. Neither can I figure
out what FRR expects the end user to do with this information.
I can see this information being useful if we encounter a bug
down the line and we have gathered a bunch of data. As such
let's modify the code to remove the flog_warn and convert
the message to a debug level message that can be controlled by
appropriate debug statements.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
This looks like a finish up of the partial cleanup that
ocurred at some point in time in the past. When we
alloc oi also always alloc the oi->obuf. When we delete
oi always delete the oi->obuf right before.
This cleans up a bunch of code to be simpler and hopefully
easier to follow.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
I am rarely seeing this crash:
r2: ospfd crashed. Core file found - Backtrace follows:
[New LWP 32748]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".
Core was generated by `/usr/lib/frr/ospfd'.
Program terminated with signal SIGABRT, Aborted.
2019-08-29 15:59:36,149 ERROR: assert failed at "test_ospf_sr_topo1/test_memory_leak":
Which translates to this code:
node = listhead(ospf->oi_write_q);
assert(node);
oi = listgetdata(node);
assert(oi);
So if we get into ospf_write without anything on the oi_write_q
we are stopping the program.
This is happening because in ospf_ls_upd_queue_send we are calling
ospf_write. Imagine that we have a interface already on the on_write_q
and then ospf_write handles the packet send for all functions. We
are not clearing the t_write thread and we are popping and causing
a crash.
Additionally modify OSPF_ISM_WRITE_ON(O) to not just blindly
turn on the t_write thread. Only do so if we have data.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
ospfd: Remove redundant asserts
assert(oi) is impossible all listgetdata(node) directly proceeding
it already asserts here, besides a node cannot be created
with a null pointer!
If list_isempty is called directly before the listhead call
it is impossilbe that we do not have a valid pointer here.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
vrf hook handler associated to updating the vrf default name is added.
Then, when creating default ospf vrf instance, the vrf identifier will
be associated to the correct default vrf name.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
when an other name is given to default vrf, then there is case where 2
ospf instances are created, which is not wished. Also, it appears that
interface learning and ospf interface configuration is not lost when not
creating that default ospf instance. So removing it.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Debian packaging when run finds a bunch of spelling errors:
I: frr: spelling-error-in-binary usr/bin/vtysh occurences occurrences
I: frr: spelling-error-in-binary usr/lib/frr/bfdd Amount of times Number of times
I: frr: spelling-error-in-binary usr/lib/frr/bgpd occurences occurrences
I: frr: spelling-error-in-binary usr/lib/frr/bgpd recieved received
I: frr: spelling-error-in-binary usr/lib/frr/isisd betweeen between
I: frr: spelling-error-in-binary usr/lib/frr/ospf6d Infomation Information
I: frr: spelling-error-in-binary usr/lib/frr/ospfd missmatch mismatch
I: frr: spelling-error-in-binary usr/lib/frr/pimd bootsrap bootstrap
I: frr: spelling-error-in-binary usr/lib/frr/pimd Unknwon Unknown
I: frr: spelling-error-in-binary usr/lib/frr/zebra Requsted Requested
I: frr: spelling-error-in-binary usr/lib/frr/zebra uknown unknown
I: frr: spelling-error-in-binary usr/lib/x86_64-linux-gnu/frr/libfrr.so.0.0.0 overriden overridden
This commit fixes all of them except the bgp `recieved` issue due to
it being part of json output. That one will need to go through
a deprecation cycle.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The `destination` field of the connection structure was used to store
the broadcast address, if the connection was not p2p. This multipurpose
is not very evident and the benefits over calculating the bcast address
on the fly minimal.
Signed-off-by: Juergen Werner <juergen@opensourcerouting.org>
Need to clear out the external_info for the "always" default route that
we installed in ospf_redistribute_default_set().
Signed-off-by: David Lamparter <equinox@diac24.net>
This is a "workaround" for something broken in LSDB sync that has been
kept around since the beginning of our git history...
(It works correctly without this "workaround".)
Signed-off-by: David Lamparter <equinox@diac24.net>
ospf->external[DEFAULT_ROUTE] and zclient->default_information don't
line up with each other; the former is only used for "originate always".
Fixes: #4237
Signed-off-by: David Lamparter <equinox@diac24.net>
This reverts commit a6b4e1fded.
This fix is wrong too. The zclient->redist & ->mi_redist arrays are
accessed past their size for any external route that is not 0.0.0.0/0.
Also, it is incorrect to check default_information for DEFAULT_ROUTE
since that's "originate always".
Signed-off-by: David Lamparter <equinox@diac24.net>
This reverts commit 313919d6e3.
This is not the correct way to fix this.
- touching the LSDB to explicitly remove a MaxAge LSA is always wrong
and results in desynchronization of the entire routing domain
- the LSDB code correctly handles replacing a MaxAge LSA with a newly
issued one
- removing the old LSA resets the sequence numbers, which may cause
other routers to reject the new LSA as old
- the function was horribly misnamed
Signed-off-by: David Lamparter <equinox@diac24.net>
Neither ospf_external_lsa_originate_timer() nor
ospf_default_originate_timer() are actually timers. They're only
executed on router-ID changes to refresh a particular LSA type.
Signed-off-by: David Lamparter <equinox@diac24.net>
Introducing a 3rd state for route_map_apply library function: RMAP_NOOP
Traditionally route map MATCH rule apis were designed to return
a binary response, consisting of either RMAP_MATCH or RMAP_NOMATCH.
(Route-map SET rule apis return RMAP_OKAY or RMAP_ERROR).
Depending on this response, the following statemachine decided the
course of action:
State1:
If match cmd returns RMAP_MATCH then, keep existing behaviour.
If routemap type is PERMIT, execute set cmds or call cmds if applicable,
otherwise PERMIT!
Else If routemap type is DENY, we DENYMATCH right away
State2:
If match cmd returns RMAP_NOMATCH, continue on to next route-map. If there
are no other rules or if all the rules return RMAP_NOMATCH, return DENYMATCH
We require a 3rd state because of the following situation:
The issue - what if, the rule api needs to abort or ignore a rule?:
"match evpn vni xx" route-map filter can be applied to incoming routes
regardless of whether the tunnel type is vxlan or mpls.
This rule should be N/A for mpls based evpn route, but applicable to only
vxlan based evpn route.
Also, this rule should be applicable for routes with VNI label only, and
not for routes without labels. For example, type 3 and type 4 EVPN routes
do not have labels, so, this match cmd should let them through.
Today, the filter produces either a match or nomatch response regardless of
whether it is mpls/vxlan, resulting in either permitting or denying the
route.. So an mpls evpn route may get filtered out incorrectly.
Eg: "route-map RM1 permit 10 ; match evpn vni 20" or
"route-map RM2 deny 20 ; match vni 20"
With the introduction of the 3rd state, we can abort this rule check safely.
How? The rules api can now return RMAP_NOOP to indicate
that it encountered an invalid check, and needs to abort just that rule,
but continue with other rules.
As a result we have a 3rd state:
State3:
If match cmd returned RMAP_NOOP
Then, proceed to other route-map, otherwise if there are no more
rules or if all the rules return RMAP_NOOP, then, return RMAP_PERMITMATCH.
Signed-off-by: Lakshman Krishnamoorthy <lkrishnamoor@vmware.com>
no router ospf triggers to cancel all threads
including read/write (receive/send packets) threads,
cleans up resources fd, message queue and data.
Last job of write (packet) thread invoked where the
ospf instance is referenced is not running nor
the socket fd valid.
Write thread callback should check if fd is valid and
ospf instance is running before proceeding to send a
message over socket.
Ticket:CM-20095
Testing Done:
Performed the multiple 'no router ospf' with the fix
in topology where the crash was seen.
Post fix the crash is not observed.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Same ospf neigbor can be learnt via multiple
interfaces, ospf detail json only displayed
last instance only.
Fix json output format to contain "neighbors"
keyword, under which to display all neighbors
for a given vrf.
Fix
show ip ospf neighbor detail json
show ip ospf neighbor detail all json
show ip ospf neighbor <intf name> detail json
Ticket:CM-25528
Reviewed By:
Testing Done:
Run the output with JSON formatter and the output
has passed.
switch1# show ip ospf vrf all neighbor detail json
{
"default":{
"vrfName":"default",
"vrfId":0,
"neighbors":{
"0.0.0.2":[
{
"ifaceAddress":"14.0.0.22",
"areaId":"0.0.0.0",
"ifaceName":"Bridge1.510",
"nbrPriority":1,
"nbrState":"Full",
"stateChangeCounter":6,
"lastPrgrsvChangeMsec":82668,
"routerDesignatedId":"14.0.0.22",
"routerDesignatedBackupId":"14.0.0.21",
"optionsCounter":2,
"optionsList":"*|-|-|-|-|-|E|-",
"routerDeadIntervalTimerDueMsec":36195,
"databaseSummaryListCounter":0,
"linkStateRequestListCounter":0,
"linkStateRetransmissionListCounter":0,
"threadInactivityTimer":"on",
"threadLinkStateRequestRetransmission":"on",
"threadLinkStateUpdateRetransmission":"on",
"peerBfdInfo":{
"type":"single hop",
"detectMultiplier":4,
"rxMinInterval":600,
"txMinInterval":800,
"status":"Down",
"lastUpdate":"0:00:00:29"
}
},
{
"ifaceAddress":"14.0.0.26",
"areaId":"0.0.0.0",
"ifaceName":"Bridge1.511",
"nbrPriority":1,
"nbrState":"Full",
"stateChangeCounter":6,
"lastPrgrsvChangeMsec":82658,
"routerDesignatedId":"14.0.0.26",
"routerDesignatedBackupId":"14.0.0.25",
"optionsCounter":2,
"optionsList":"*|-|-|-|-|-|E|-",
"routerDeadIntervalTimerDueMsec":36196,
"databaseSummaryListCounter":0,
"linkStateRequestListCounter":0,
"linkStateRetransmissionListCounter":0,
"threadInactivityTimer":"on",
"threadLinkStateRequestRetransmission":"on",
"threadLinkStateUpdateRetransmission":"on"
},
]
}
}
}
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
The shutdown of ospf was causing crashes because the shutdown
was calling a ALL_LIST_ELEMENTS_RO macro and modifying the
underlying data structures. Switch to using ALL_LIST_ELEMENTS.
This is caused by this change:
commit f9e1501aea5d429be2ecda1a3e2bde17e6ad5e4b
Author: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Wed Feb 27 15:08:29 2019 -0500
ospfd: Cleanup ospf->redist and ospf->external on shutdown
Effectively my original testing for this only had one external
route and as such we would not have a crash here. It only
showed up after multiple externals have been introduced.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
These two data types were written to handle redistribute
and external data types. On shutdown cleanup the memory
allocated to these if we are doing redistribution.
This was found using valgrind.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.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>
vrf_id parameter is replaced with struct vrf * parameter. It is
needed to create vrf structure before entering in the fuction.
an error is generated in case the vrf parameter is missing.
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>
vrf pointer is used as reference when calling if_get_by_name() function.
this will permit to create interfaces with an unknown vrf_id, since it
is only necessary to get the vrf structure to store the interfaces.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Introducing a 3rd state for route_map_apply library function: RMAP_NOOP
Traditionally route map MATCH rule apis were designed to return
a binary response, consisting of either RMAP_MATCH or RMAP_NOMATCH.
(Route-map SET rule apis return RMAP_OKAY or RMAP_ERROR).
Depending on this response, the following statemachine decided the
course of action:
Action: Apply route-map match and return the result (RMAP_MATCH/RMAP_NOMATCH)
State1: Receveived RMAP_MATCH
THEN: If Routemap type is PERMIT, execute other rules if applicable,
otherwise we PERMIT!
Else: If Routemap type is DENY, we DENYMATCH right away
State2: Received RMAP_NOMATCH, continue on to next route-map, otherwise,
return DENYMATCH by default if nothing matched.
With reference to PR 4078 (https://github.com/FRRouting/frr/pull/4078),
we require a 3rd state because of the following situation:
The issue - what if, the rule api needs to abort or ignore a rule?:
"match evpn vni xx" route-map filter can be applied to incoming routes
regardless of whether the tunnel type is vxlan or mpls.
This rule should be N/A for mpls based evpn route, but applicable to only
vxlan based evpn route.
Today, the filter produces either a match or nomatch response regardless of
whether it is mpls/vxlan, resulting in either permitting or denying the
route.. So an mpls evpn route may get filtered out incorrectly.
Eg: "route-map RM1 permit 10 ; match evpn vni 20" or
"route-map RM2 deny 20 ; match vni 20"
With the introduction of the 3rd state, we can abort this rule check safely.
How? The rules api can now return RMAP_NOOP (or another enum) to indicate
that it encountered an invalid check, and needs to abort just that rule,
but continue with other rules.
Question: Do we repurpose an existing enum RMAP_OKAY or RMAP_ERROR
as the 3rd state (or create a new enum like RMAP_NOOP)?
RMAP_OKAY and RMAP_ERROR are used to return the result of set cmd.
We chose to go with RMAP_NOOP (but open to ideas),
as a way to bypass the rmap filter
As a result we have a 3rd state:
State3: Received RMAP_NOOP
Then, proceed to other route-map, otherwise return RMAP_PERMITMATCH by default.
Signed-off-by:Lakshman Krishnamoorthy <lkrishnamoor@vmware.com>
It doesn't make much sense for a hash function to modify its argument,
so const the hash input.
BGP does it in a couple places, those cast away the const. Not great but
not any worse than it was.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
bfd cbit is a value carried out in bfd messages, that permit to keep or
not, the independence between control plane and dataplane. In other
words, while most of the cases plan to flush entries, when bfd goes
down, there are some cases where that bfd event should be ignored. this
is the case with non stop forwarding mechanisms where entries may be
kept. this is the case for BGP, when graceful restart capability is
used. If BFD event down happens, and bgp is in graceful restart mode, it
is wished to ignore the BFD event while waiting for the remote router to
restart.
The changes take into account the following:
- add a config flag across zebra layer so that daemon can set or not the
cbit capability.
- ability for daemons to read the remote bfd capability associated to a bfd
notification.
- in bfdd, according to the value, the cbit value is set
- in bfdd, the received value is retrived and stored in the bfd session
context.
- by default, the local cbit announced to remote is set to 1 while
preservation of the local path is not set.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The route_map_event_hook callback was passing the `route_map_event_t`
to each individual interested party. No-one is ever using this data
so let's cut to the chase a bit and remove the pass through of data.
This is considered ok in that the routemap.c code came this way
originally and after 15+ years no-one is using this functionality.
Nor do I see any `easy` way to do anything useful with this data.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
vrf_id parameter is added to the api of bfd_client_sendmsg().
this permits being registered to bfd from a separate vrf.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This macro:
- Marks ZAPI callbacks for readability
- Standardizes argument names
- Makes it simple to add ZAPI arguments in the future
- Ensures proper types
- Looks better
- Shortens function declarations
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Solve issue #4198
Link-ID and Remote IP address must be set accordingly to the interface type
(Point-to-Point or Broadcast) from the neighbor information. However, this
information are only valid once the Network State Machine (NSM) is Full i.e.
when the adjacency is up. The original TE code only look to Interface State
Machine (ISM) change which not allow to collect valid neighbor information.
The patch move setup of Link-ID and Remote-IP TE parameters from
ospf_mpls_te_ism_change() to ospf_mpls_te_nsm_change() function.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
The order of ECMP nexthops currently depends on whatever order the
pqueue code returns the vertices in, which is essentially random since
they compare as equal. While this shouldn't cause issues normally, it
is nondeterministic and causes the ldp-topo1 test to fail when the
ordering comes up different. Also, nondeterministic behaviour is not a
nice thing to have here in general.
Just sort by nexthop address; realistic numbers of ECMP nexthops should
hopefully not make this a performance issue. (Also, nexthops should be
hot in the caches here.)
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
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>
rn is not set the first time through the do {} while (); loop
As such we need to protect against it from being null( although
highly unlikely to ever happen given the ospf code base.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
While fragmenting ospf ls packets, before appending the link state info,
wrong value is checked to see if current packet can fit in another ls info.
Because of this, when a lower mtu is configured, it couldn't fit in even 1
ls ack, which tries to send all the available ls ack in the list in loop.
This keeps allocating memory to send the packet and ends up putting the
packet buffer without ls-ack into deferred send que(ospf_ls_ack_send_delayed).
This infinite loop causes infinite memory being allocated in a loop causing
system to be unstable. This commit takes care of calculating the right value
to compare for checking oif this buffer can fit in more.
Signed-off-by: Saravanan K <saravanank@vmware.com>
Fix a few json output values: a few are in seconds, not msecs,
and one is a number-per-second, not a duration.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
When creating a ospf vrf based instance allow it to work
if the vrf has been created *before* we create the ospf
instance.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Made changes and updated the routemap applied counter in the following flows.
1.Increment the routemap applied counter when route map attached to a
redistribution list. The counter will be updated if the routemap exists.
2.Decrement when route map removed / modified from a redistribution list.
3.Increment/decrement when route map create/delete callback triggered.
Signed-off-by: RajeshGirada <rgirada@vmware.com>
Based on the vulnerability mentioned in 793496 an attacker can craft an
LSA with MaxSequence number wtih invalid links and not set age to MAX_AGE
so the lsa would not be flush from the database.
To address the issue, check incoming LSA is MaxSeq but Age is not set
to MAX_AGE 3600, discard the LSA from processing it.
Based on RFC-2328 , When a LSA update sequence reaches MaxSequence
number, it should be prematurely aged out from the database with age set
to MAX_AGE (3600).
Ticket:CM-18989
Reviewed By:
Testing Done:
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
- some target_CFLAGS that needed to include AM_CFLAGS didn't do so
- libyang/sysrepo/sqlite3/confd CFLAGS + LIBS weren't used at all
- consistently use $(FOO_CFLAGS) instead of @FOO_CFLAGS@
- 2 dependencies were missing for clippy
Signed-off-by: David Lamparter <equinox@diac24.net>
Ospfd cored because of an assert when we try to write more than the MTU
size to the ospf packet buffer stream. The problem is - we allocate only MTU
sized buffer. The expectation is that Hello packets are never large
enough to approach MTU. Instead of crashing, this fix discards hello and
logs an error. One should not have so many neighbors behind an
interface.
Ticket: CM-22380
Signed-off-by: Nitin Soni <nsoni@cumulusnetworks.com>
Reviewed-by: CCR-8204
the command was not checking
correctly in all cases whether the virtual link existed. This caused
bugs in some corner cases, e.g. when two virtual links were created,
one of them was deleted, and the second one was reset with no
authentication - this would instead create a new virtual link with
the area in decimal format.
Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
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>
Some daemons like ospfd and isisd have the ability to advertise a
default route to their peers only if one exists in the RIB. This
is what the "default-information originate" commands do when used
without the "always" parameter.
For that to work, these daemons use the ZEBRA_REDISTRIBUTE_DEFAULT_ADD
message to request default route information to zebra. The problem
is that this message didn't have an AFI parameter, so a default route
from any address-family would satisfy the requests from both daemons
(e.g. ::/0 would trigger ospfd to advertise a default route to its
peers, and 0.0.0.0/0 would trigger isisd to advertise a default route
to its IPv6 peers).
Fix this by adding an AFI parameter to the
ZEBRA_REDISTRIBUTE_DEFAULT_{ADD,DELETE} messages and making the
corresponding code changes.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
DEFPY commands are easier to maintain and less susceptible to
bugs. In the long term we should try to merge the plethora of
"show ip ospf neighbor" commands (total of 14) into a single DEFPY.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
cf. https://wiki.debian.org/NonFreeIETFDocuments
These MIBs were in our git purely for documentation purposes, they are
not installed and not needed for building SNMP support.
Signed-off-by: David Lamparter <equinox@diac24.net>
so as to isolate ospf contexts separately for each vrf, the interface
used is cornered to the passed vrf context.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This reverts commit 48944eb65e.
We're using GNU C, not ISO C - and this commit triggers new (real)
warnings about {0} instead of bogus ones about {}.
Signed-off-by: David Lamparter <equinox@diac24.net>
It's been a year since we added the new optional parameters
to instantiation. Let's switch over to the new name.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Router Information needs to specify the area ID when flooding scope is set to
AREA. However, this authorize only one AREA. Thus, Area Border Router (ABR) are
unable to flood Router Information Opaque LSA in all areas they are belongs to.
The path implies that the area ID is no more necessary for the command
'router-info area'. It remains suported for compatibility, but mark as
deprecated. Documentation has been updated accordingly.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
avoid counting twice the number of areas configured, when entering back
to router ospf config node.
PR=61288
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Acked-by: Emmanuel Vize <emmanuel.vize@6wind.com>
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>
FRR_DAEMON_INFO should now contain an array of 'frr_yang_module_info'
structures describing the YANG modules implemented by the daemon.
This array will be used by frr_init() function to load all YANG modules
and initialize the northbound callbacks during the daemon initialization.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
when adding/removing virtual links per interface, sometimes, the ospf
virtual link can not be removed, whereas the associated area is already
removed. Do not remove the area while a virtual link is yet configured.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Add a missing check to bail out earlier when SR is not configured. The
same command without the "no" prefix has the same check as it prevents
unexpected things (i.e. crashes) from happening.
Fixes the following segfaults:
ospfd aborted: vtysh -c "configure terminal" -c "router ospf" -c "no segment-routing prefix 1.1.1.1/32"
ospfd aborted: vtysh -c "configure terminal" -c "router ospf" -c "no segment-routing prefix 1.1.1.1/32 index 65535 no-php-flag"
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When the ospf->oi_write_q is not empty that means that ospf could
already have a thread scheduled for running. Just dropping
the pointer before resheduling does not stop the one currently
scheduled for running from running. The calling of thread_add_write
checks to see if we are already running and does the right thing here
so it is sufficient to just call thread_add_write.
This issue was tracked down from this stack trace:
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [EC 134217739] interface eth2.1032:172.16.4.110: ospf_check_md5 bad sequence 5333618 (expect 5333649)
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: message repeated 3 times: [ [EC 134217739] interface eth2.1032:172.16.4.110: ospf_check_md5 bad sequence 5333618 (expect 5333649)]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: Assertion `node’ failed in file ospfd/ospf_packet.c, line 666, function ospf_write
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: Backtrace for 8 stack frames:
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 0] /usr/lib/libfrr.so.0(zlog_backtrace+0x3a) [0x7fef3efe9f8a]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 1] /usr/lib/libfrr.so.0(_zlog_assert_failed+0x61) [0x7fef3efea501]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 2] /usr/lib/frr/ospfd(+0x2f15e) [0x562e0c91815e]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 3] /usr/lib/libfrr.so.0(thread_call+0x60) [0x7fef3f00d430]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 4] /usr/lib/libfrr.so.0(frr_run+0xd8) [0x7fef3efe7938]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 5] /usr/lib/frr/ospfd(main+0x153) [0x562e0c901753]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 6] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7fef3d83db45]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: [bt 7] /usr/lib/frr/ospfd(+0x190be) [0x562e0c9020be]
Oct 19 18:04:00 VYOS-R1 ospfd[1811]: Current thread function ospf_write, scheduled from file ospfd/ospf_packet.c, line 881
Oct 19 18:04:00 VYOS-R1 zebra[1771]: [EC 4043309116] Client ‘ospf’ encountered an error and is shutting down.
Oct 19 18:04:00 VYOS-R1 zebra[1771]: client 41 disconnected. 0 ospf routes removed from the rib
We had an assert(node) in ospf_write, which means that the list was empty. So I just
searched until I saw a code path that allowed multiple writes to the ospf_write function.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The ->hash_cmp and linked list ->cmp functions were sometimes
being used interchangeably and this really is not a good
thing. So let's modify the hash_cmp function pointer to return
a boolean and convert everything to use the new syntax.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Made changes such that message wont be sent to zebra to validate default
route existence if user configured with “always”.
Signed-off-by: rgirada <rgirada@vmware.com>
Default route type is not considered while processing lsa
refresh timer expiry which intern makes it flushed from lsdb.
Signed-off-by: rgirada <rgirada@vmware.com>
Issue: # https://github.com/FRRouting/frr/issues/1836
Issue 1: if the router ospf current configuration is "area 0.0.0.2
range 1.0.0.0/24 cost 23" and user try to configure "area 0.0.0.2
range 1.0.0.0/24 not-advertise", the existing o/p is "area 0.0.0.2
range 1.0.0.0/24 cost 23 not-advertise". The keywords "not-advertise"
& "cost" are multually exclusive, so they should not come together.
The vice versa way configuration is working fine.
Fix: When ospf area range "not-advertise", the cost should be initialized
to OSPF_AREA_RANGE_COST_UNSPEC.
Issue 2: if the router ospf current configuration "area 0.0.0.2 range
1.0.0.0/24 substitute 2.0.0.0/24" and user try to configure "area 0.0.0.2
range 1.0.0.0/24 not-advertise" the existing o/p is "area 0.0.0.2 range
1.0.0.0/24 not-advertise substitute 2.0.0.0/24". The keywords
"not-advertise" & "substiture" are multually exclusive, so they should
not come together. The vice versa way configuration is working fine.
Fix: When ospf area range "not-advertise" is configured,
ospf_area_range_substitute_unset() should be get called.
Issue 3: if the router ospf6 current configuration is "area 0.0.0.2
range 2001::/64 cost 23" and user try to configure "area 0.0.0.2 range
2001::/64 advertise", the existing o/p is area 0.0.0.2 range 2001::/64.
The keyword "cost 23" disappears.
Fix: When ospf area range "advertise" is configured and the range is not
NULL, the cost should not be modified.
Signed-off-by: Sarita Patra <saritap@vmware.com>
The head and tail pointers of linked lists should never be modified
manually, the linked list API guarantees that these pointers are always
valid and up-to-date.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
config.h (or, transitively, zebra.h) must be the first include file
listed for autoconf things like _GNU_SOURCE and _POSIX_C_SOURCE to work
correctly.
Signed-off-by: David Lamparter <equinox@diac24.net>
Since we're now building through one large Makefile, we can easily put
things with their daemons and crossreference nicely.
Signed-off-by: David Lamparter <equinox@diac24.net>
If we detect we already have a neighbor, no need to
re-add so no need to warn since we do not do anything with
the data.
Signed-off-by: Donald Sharp <sharpd@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>
Problem reported that some bgp and ospf json commands did not return
any json output at all if the bgp/ospf instance did not exist.
Additionally, some bgp and ospf json commands did not return any json
output if the instance existed but no neighbors were defined. This
fix makes these commands more consistent in returning empty braces for
json output and issue a message if not using json output. Additionally,
made the flag "use_json" a bool to make it consistent since previously,
it had been defined as an int, char, u_char, and bool at various places.
Ticket: CM-21040
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
The Vrf aliases can be known with a specific hook. That hook will then,
from zebra propagate the information to the relevant zapi clients.
The registration hook function is the same for all daemons.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The problem is seen where speed mismatch caused ECMP route
not being reflected with correct number paths (NHs).
During cold boot, some interface speed updated by zebra as
part of one shot timer and triggers interface add to clients.
In this case, ospf already have created interface (bond interface),
but speed was not updated, trigger to do interface speed change
as part of interface add, which will trigger all Router LSA to
use updated speed into cost calculation.
Ticket:CM-22170
Testing Done:
Bring up CLOS config with Spine and leafs. Leaf have CLAG pair,
with same VRR ip address.
At spine one of the bond connecting to leaf node was having
higher speed than the paired device, With this fix, at spine (DUT)
bond interface speed is equal from all peer nodes.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
The ospf_external_route_lookup function was not
being used so let's just remove it.
Unfortunately the removal was not quite so simple as
that ospf_asbr.h was being used to generate a reference
for the `struct ospf_route` data structure, so we
need to fix up the compile by fixing up header
inclusions so that ospf_route.h is actually included
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>
* Use the correct license header
* Stop headers from including themselves
* Use uniform relative include conventions
* Ensure that sources include what they use
* Turn off clang-format around struct array blocks
Signed-off-by: Quentin Young <qlyoung@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>
Don't show BFD commands with timers since it might confuse users
("show running-config" won't display timers in client daemons anymore),
but keep accepting this command from previous configurations.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When BFD timers are configured, don't show it anymore in the daemon
side. This will help us migrate the timers command from daemons to
`bfdd`.
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When calling route_map_finish, every place that we do we must
first set the deletion event to NULL, or we will create an infinite
loop, if we are using the delayed route-map application code.
As such we might as well just make the route_map_finish code
do this work, as that there is really no viable alternative here
and route_map_finish should only be called on shutdown.
This fixes an infinite loop in zebra on shutdown when there
are route-maps.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Details:
- INET_ADDRSTRLEN is 16, for xxx.xxx.xxx\0, so 15 is now passed
to the strncpy call instead of 16, ensuring ASCII-z output
Signed-off-by: F. Aragon <paco@voltanet.io>
We lookup the lp value and lookup_linkparams_by_instance
can return NULL if something has gone terribly wrong.
Make sure that lp is not NULL.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
When we first get a packet, we need to know if we are self
originated later to make correct decisions. Go ahead and
note that we do not plan to make any decision points
about our self origination here.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Programs that link to libnetsnmp must be compiled using a special set
of flags as specified by the "net-snmp-config --base-cflags" command
(whose output is stored in the SNMP_CFLAGS variable). The problem is
that "net-snmp-config --base-cflags" can output -std=c99 in addition to
other compiler flags in some platforms, and this breaks the build since
FRR souce code makes use of some GNU compiler extensions (e.g. allow
trailing commas in function parameter lists). In order to solve this
problem, append -std=gnu99 after SNMP_CFLAGS in all makefiles where this
variable is used. This way the -std=c99 flag will be overwritten when it's
present. Source files that don't link to libnetsnmp will be compiled using
either -std=gnu99 or -std=gnu11 depending on the compiler availability.
Fixes#1617.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Currently, interface packet transmit queue is created/deleted
as part of Interface UP/Down event. This results in
a rare condition where port came up but queue
was not created. The creation of queue occupies only few bytes.
Moving fifo queue creation to interface create
would add few bytes of fifo creation but at least it guaranteed
to be available during Up/down -->Up event.
Initialize ospf packet fifo queue during ospf
interface creation.
Drain queue during interface down event.
Drained and free the queue as part of the interface
delete/cleanup.
Ticket:CM-20744
Testing Done:
Bring up ospfv2 topology with multiple neighbors.
1) Trigger multiple shut/no shut events and validate
all queues are freed.
2) configure/deconfigure router ospf and validate
all ospf instance and interface underneath are freed.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Macro that expands to be wrapped in parentheses was being used as a raw
condition for an if statement, leading to some very weird and confusing
formatting...
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
OSPF_IF_PARAM_CONFIGURED(S, P) checks both the nullity of S and the
value of P; assuming either one from the value of this macro is
incorrect.
Signed-off-by: Quentin Young <qlyoung@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>
Otherwise if it is scheduled the thread pointer will be accessed after
the shutdown task finishes accessing, having deleted the structure that
owns said pointer, which causes a heap UAF.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
This capability, when used, is mapped over linux sys_admin capability.
This is necessary from the daemon perspective, in order to handle NETNS
based VRFs, because calling setns() requires sys admin capability.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When OSPF SR is not configured one would expect
to not see any output from it at all in the debug
log with no debugs turned on.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
So when a ospf SR is sending down routes to the kernel
ensure that the nexthop vrf_id is set appropriately.
Yes SR is in the default VRF. But for people who
run across this code in the future, they will know
to do the right thing from it.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
- To ease checking the Segment Routing conformity in topotest,
add json output to 'show ip ospf database segment-routing' CLI.
- Update ospfd user guide accordingly
- Update OSPF-SR documentation with supported features and interoperability
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
The change consists in taking into account of the VRF identifier upon
which the ospf socket is created. Moreover, if the VRF is a netns
backend, then it is not necessary to perform the bind operations to vrf
device.
Also, when a VRF instance is enabled, it informs ospf VRF, and automatically
OSPF VRF benefits from it. Reversely, when VRF instance is disabled,
then OSPF VRF will be disabled too.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
the vrf identifier in the ospf_vrf_enable routine is never read, then
does not need to be initialised.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
- Lan Adjacency TLVs was incorrectly formatted due to an error in
TLV size computation. Add new macro to fix this issue
- Update SR link nexthop when it corresponds to an LAN Adj SID. The nexthop
is set to the router id in the TLVi (as per draft), but we need the neighbor
IP address to set the corresponding MPLS LFIB entry
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Fix a || && mixup.
Add an assert for area to show we expect it to be non-null
going forward.
When memory is allocated if it fails we abort then
no need to check for null.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
- ospfd/ospf_opaque.c: Update issue #1652 by introducing a new
function 'free_opaque_info_owner()' to clean list of callback owner
and call this function in appropriate place where 'listdelete_and_null'
is not used.
- ospfd/ospf_packet.c: In case of crash, ospfd is not been able to
flush LSA. In case of self Opaque LSA, when restarting, ospfd crash
during the resynchronisation process with its neighbor due to an
empty list of LSA to flood. Just add a control on the list count
in 'ospf_ls_upd_queue_send()' to escape the function and avoid the
problem.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Turns out we had 3 different ways to define labels
all of them overlapping with the same meanings.
Consolidate to 1. This one choosen is consistent
naming wise with what the *bsd and linux kernels
use.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
show ip ospf [vrf all] interface json and
show ip ospf [vrf all] neighbor json to display
objects in dictionary output rather in array list.
Ticket:CM-19115,CM-19097
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Opaque LSA were incorrectly filtered. LSA Type 1 with a
router id set to 4.x.x.x or 7.x.x.x. or 8.x.x.x are not correctly
filtered and pass to Segment Routing as wrong Opaque LSA of type
Router Information, Extended Prefix respectively Extended Link.
- Add Opaque LSA check to the filter
The CLI command 'segment-routing prefix' didn't check if a same prefix
already exist in SRDB resulting to multiple entries in the SRDB for the
same prefix.
- Update prefix intead of adding a new one if already present in the SRDB
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
When preforming CI test, CLI command 'no router ospf' followed by a
'router ospf' is performed to clean up the previous configuration.
Ospfd crash when configuring 'netwoark area'.
This is due to opsf_opaque_term() introduce in previous commit that cause this
crash. It remove not only Opaque LSA but also the list through the call to
'list_delete_and_null()' function. Same take place in 'ospf_mpls_te_term()',
'ospf_router_info_term()' and 'ospf_ext_term()' function.
New set of 'ospf_XXX_finish()' has been introduced to solve this issue while
keeping the possiblity to terminate properly the Opaque LSA and remove MPLS
LFIB entries set by Segment Routing.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
- When Extended Prefix LSA need to be refresh, paramaters may be
taken from the wrong interface i.e. Extended Link instead of Prefix
resulting in producing an empty LSA body. Then, ospfd crash due to the
assert on LSA length in ospf_lsa_different() function: code check that
the LSA size is larger than LSA header i.e. LSA is not empty.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
- ospfd/ospf_te.c: Remove unregister function and call to
ospf_delete_opaque_functab() following the introduction of
ospf_opaque_term() function in ospfd.c for ospfd termination.
- ospfd/ospf_sr.c: Set initial index value for node-msd CLI to
avaoid crash when using this command
- ospfd/ospf_ext.c: Disable call to ospf_sr_update_prefix() if
Segment Routing, thus Extended Link/Prefix, is not enable
- ospfd/ospf_opaque.c: Correct scheduling of Opaque LSA flooding
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
- Line 865 in ospfd/ospf_ext.c cause compilation failure when
-werror is set. This line was not necessary and add only for debugging
purpose.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Fix default-information parsing of metric-type,
route-map.
show running to display metic-type 2.
metric, metric-type and route-map can be configured in
any order, running-config displays in specific order..
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Following various review, following files have been modfied:
- All: Change u_intXX_t typedef to standard uintXX_t types
- doc/OSPF-SR.rst: Update doc in particular the Linux Kernel configuration
section
- doc/ospfd.texi: Update CLI
- ospfd/ospf_dump.[c,h]: Add new 'debug ospf sr' when performing 'sh run'
- ospfd/ospf_ext.[c, h]: Various bug corrections notably to handle flooding of
Extended Prefix at startup. iFix TLVs size for LAN Adjacency.
Update Licence as per Community.md
- ospfd/ospf_opaque.c: Add proper termination function call to remove MPLS entries
- ospfd/ospf_ri.[c,h]: Bug corrections
- ospfd/ospf_sr.[c,h]: Various bug corrections, notably to determine the nexthop
SR Node. Add support to 'no-php-flag'.
Update Licence as per Community.md
- ospfd/ospfd.c: Add call to 'ospf_opaque_term()'
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Call Route-map, prefix-list clean up routines and
vrf clearnup during ospf daemon exit routine.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Issue 1652 was related to OSPF Crash on termination when ospf is
configured to flood self Opaque LSA e.g TE or RI Opaque LSA.
Analysis:
The problem resides in free_opaque_info_per_type() line 576 of
ospf_opaque.c. Once LSA flush, the function removes by calling
listnode_delete() function the opaque_info_per_type data structure.
However, this is also performed at the upper level function
ospf_opaque_type10lsa_term() which call list_delete_and_null() function.
This result into a double free pointer exception.
Solution:
Remove call to listnode_delete() calls in free_opaque_info_per_type()
function as it is done by list_delete_and_null(). Delete lines 592 - 615.
Remove also second call to ospf_opaque_type10lsa_term() in ospfd.c line 848.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
- Change all u_intXX_t typedef to standard type uintXX_t
- Correct removal of ZEBRA_OSPF_SR route in ospf_sr.c line 670
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
- Remove OSPD_SR route type
- Check that Segment Routing is enable only in default VRF
- Add comment for SRGB in lib/mpls.h
- Update documentation
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Current json output does not differentiate start of
neighbor ip object. Adding "neighbors" keyword at the
beginning of neighbor list. This is useful when
displaying vrf level output along with neighbors
list.
Ticket:CM-19097
Testing Done:
show ip ospf neighbor json
show ip ospf vrf all neighbor json
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
- ospf_ext.c: Correct deferred pointer raised by valgrind
- ospf_sr.c: Correct deffered pointer raised by valgrind. Modify
Segment Routing shutdown. This is due to the fact that RI LSA
4.0.0.0 is flush prior to 7.0.0.X & 8.0.0.X LSA. This trigger
SR-Node deletion which also remove all Extended Link / Prefix
leaving them unavailable when there are referenced by 7.0.0.X
& 8.0.0.X LSA flushing.
- doc/OSPF-SR.rst: Correct Restructured Text syntax and add
Configuration example as well as Credit section
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
This is an implementation of draft-ietf-ospf-segment-routing-extensions-24
and RFC7684 for Extended Link & Prefix Opaque LSA.
Look to doc/OSPF_SR.rst for implementation details & known limitations.
New files:
- ospfd/ospf_sr.h: Segment Routing structure definition (SubTLVs + SRDB)
- ospfd/ospf_sr.c: Main functions for Segment Routing support
- ospfd/ospf_ext.h: TLVs and SubTLVs definition for RFC7684
- ospfd/ospf_ext.c: RFC7684 Extended Link / Prefix implementation
- doc/OSPF-SRr.rst: Documentation
Modified Files:
- doc/ospfd.texi: Add new Segment Routing CLI command definition
- lib/command.h: Add new string command for Segment Routing CLI
- lib/mpls.h: Add default value for SRGB
- lib/route_types.txt: Add new OSPF Segment Routing route type
- ospfd/ospf_dump.[c,h]: Add OSPF SR debug
- ospfd/ospf_memory.[c,h]: Add new Segment Routing memory type
- ospfd/ospf_opaque.[c,h]: Add ospf_sr_init() starting function
- ospfd/ospf_ri.c: Add new functions to Set/Get Segment Routing TLVs
Add new ospf_router_info_lsa_upadte() to send Opaque LSA to ospf_sr.c()
- ospfd/ospf_ri.h: Add new Router Information SR SubTLVs
- ospfd/ospf_spf.c: Add new scheduler when running SPF to trigger
update of NHLFE
- ospfd/ospfd.h: Add new thread for Segment Routing scheduler
- ospfd/subdir.am: Add new files
- vtysh/Makefile.am: Add new ospf_sr.c file for vtysh
- zebra/kernel_netlink.c: Add new OSPF_SR route type
- zebra/rt_netlink.[c,h]: Add new OSPF_SR route type
- zebra/zebra_mpls.h: Add new OSPF_SR route type
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.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>
Current json output does not differentiate start of
interface objects. Adding "interfaces" keyword at the
beginning of the interface list. This is useful
when displaying vrf level output along with interface list.
Ticket:CM-19115
Testing Done:
show ip ospf vrf all interface json
show ip ospf vrf all interface <specific intf> json
show ip ospf interface json
show ip ospf interface <specific intf> json
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
Signed-off-by: Daniel Walton <dwalton@cumulusnetworks.com>
Before
------
cel-redxp-10(config)# router ospf vrf RED
cel-redxp-10(config-router)# network 1.1.1.1/32 area 0.0.0.0
cel-redxp-10(config-router)# network 1.1.1.1/32 area 0.0.0.0
There is already same network statement.
cel-redxp-10(config-router)#
When we see the "There is already same network statement." message
vtysh exits non-zero. This scenario breaks frr-reload because
the command took and it in the config, it should exit zero here.
After
-----
cel-redxp-10(config)# router ospf vrf RED
cel-redxp-10(config-router)# network 1.1.1.1/32 area 0.0.0.0
cel-redxp-10(config-router)# network 1.1.1.1/32 area 0.0.0.0
cel-redxp-10(config-router)# network 1.1.1.1/32 area 0.0.0.0
cel-redxp-10(config-router)# network 1.1.1.1/32 area 0
cel-redxp-10(config-router)#
cel-redxp-10(config-router)# network 1.1.1.1/32 area 0.0.0.1
There is already same network statement.
cel-redxp-10(config-router)#
