This is a first in a series of commits, whose goal is to rename
the thread system in FRR to an event system. There is a continual
problem where people are confusing `struct thread` with a true
pthread. In reality, our entire thread.c is an event system.
In this commit rename the thread.[ch] files to event.[ch].
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Don't directly use `time()` for generating sequence numbers for two
reasons:
1. `time()` can go backwards (due to NTP or time adjustments)
2. Coverity Scan warns every time we truncate a `time_t` variable for
good reason (verify that we are Y2K38 ready).
Found by Coverity Scan (CID 1519812, 1519786, 1519783 and 1519772)
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
When forming a neighbor relationship on an interface, ospf is
currently evaluating unnumbered as highest priority, without
any consideration for if you have /32's and non /32's on the
interface. Effectively if I have something like this:
int foo0
ip address 192.168.119.1/24
!
router ospf
network 0.0.0.0/0 area 0
!
ospf will form a neighbor on foo0 if it exists. Now
suppose someone does this:
int foo0
ip address 192.168.120.1/32
This will create the unnumbered interface on foo0 and
the peering will come down immediately.
The problem here is that the original designers of the unnumbered
code for ospf didn't envision end operators mixing and matching
addresses on an interface like this ( for perfectly legitimate
reasons I might add ).
So if ospf has both numbered and unnumbered let's match against
the numbered first and then unnumbered. This solves the problem
Fixes: #6823
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Let's just use THREAD_OFF consistently in the code base
instead of each daemon having a special macro that needs to
be looked at and remembered what it does.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Skip marking routes as changed in ospf_if_down if there's now
new_table present, which might be the case when the instance is
being finished
The backtrace for the core was:
raise (sig=sig@entry=11) at ../sysdeps/unix/sysv/linux/raise.c:50
core_handler (signo=11, siginfo=0x7fffffffe170, context=<optimized out>) at lib/sigevent.c:262
<signal handler called>
route_top (table=0x0) at lib/table.c:401
ospf_if_down (oi=oi@entry=0x555555999090) at ospfd/ospf_interface.c:849
ospf_if_free (oi=0x555555999090) at ospfd/ospf_interface.c:339
ospf_finish_final (ospf=0x55555599c830) at ospfd/ospfd.c:749
ospf_deferred_shutdown_finish (ospf=0x55555599c830) at ospfd/ospfd.c:578
ospf_deferred_shutdown_check (ospf=<optimized out>) at ospfd/ospfd.c:627
ospf_finish (ospf=<optimized out>) at ospfd/ospfd.c:683
ospf_terminate () at ospfd/ospfd.c:653
sigint () at ospfd/ospf_main.c:109
quagga_sigevent_process () at lib/sigevent.c:130
thread_fetch (m=m@entry=0x5555556e45e0, fetch=fetch@entry=0x7fffffffe9b0) at lib/thread.c:1709
frr_run (master=0x5555556e45e0) at lib/libfrr.c:1174
main (argc=9, argv=0x7fffffffecb8) at ospfd/ospf_main.c:254
Signed-off-by: Tomi Salminen <tsalminen@forcepoint.com>
Since f60a1188 we store a pointer to the VRF in the interface structure.
There's no need anymore to store a separate vrf_id field.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
We should always treat the VRF interface as a loopback. Currently, this
is not the case, because in some old pre-VRF code we use if_is_loopback
instead of if_is_loopback_or_vrf. To avoid any future problems, the
proposal is to rename if_is_loopback_or_vrf to if_is_loopback and use it
everywhere. if_is_loopback is renamed to if_is_loopback_exact in case
it's ever needed, but currently it's not used anywhere.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Running ospf_topo_vrf1 leads us to this valgrind issue:
==2386518== Invalid read of size 8
==2386518== at 0x4971520: route_top (table.c:401)
==2386518== by 0x181F08: ospf_interface_bfd_apply (ospf_bfd.c:126)
==2386518== by 0x182069: ospf_interface_disable_bfd (ospf_bfd.c:158)
==2386518== by 0x18BF51: ospf_del_if_params (ospf_interface.c:557)
==2386518== by 0x18C584: ospf_if_delete_hook (ospf_interface.c:712)
==2386518== by 0x490CA0B: hook_call_if_del (if.c:61)
==2386518== by 0x490D1F3: if_delete_retain (if.c:286)
==2386518== by 0x490D337: if_delete (if.c:309)
==2386518== by 0x490CDED: if_destroy_via_zapi (if.c:200)
==2386518== by 0x49940A9: zclient_interface_delete (zclient.c:2237)
==2386518== by 0x4998062: zclient_read (zclient.c:3969)
==2386518== by 0x4979529: thread_call (thread.c:1908)
==2386518== by 0x4919918: frr_run (libfrr.c:1164)
==2386518== by 0x181AC7: main (ospf_main.c:235)
==2386518== Address 0x5df39a0 is 0 bytes inside a block of size 56 free'd
==2386518== at 0x48399AB: free (vg_replace_malloc.c:538)
==2386518== by 0x492A03E: qfree (memory.c:141)
==2386518== by 0x4970C6F: route_table_free (table.c:141)
==2386518== by 0x4970A36: route_table_finish (table.c:61)
==2386518== by 0x18C543: ospf_if_delete_hook (ospf_interface.c:708)
==2386518== by 0x490CA0B: hook_call_if_del (if.c:61)
==2386518== by 0x490D1F3: if_delete_retain (if.c:286)
==2386518== by 0x490D337: if_delete (if.c:309)
==2386518== by 0x490CDED: if_destroy_via_zapi (if.c:200)
==2386518== by 0x49940A9: zclient_interface_delete (zclient.c:2237)
==2386518== by 0x4998062: zclient_read (zclient.c:3969)
==2386518== by 0x4979529: thread_call (thread.c:1908)
==2386518== by 0x4919918: frr_run (libfrr.c:1164)
==2386518== by 0x181AC7: main (ospf_main.c:235)
==2386518== Block was alloc'd at
==2386518== at 0x483AB65: calloc (vg_replace_malloc.c:760)
==2386518== by 0x4929EFC: qcalloc (memory.c:116)
==2386518== by 0x49709F8: route_table_init_with_delegate (table.c:53)
==2386518== by 0x49717F4: route_table_init (table.c:528)
==2386518== by 0x18C328: ospf_if_new_hook (ospf_interface.c:659)
==2386518== by 0x490C97D: hook_call_if_add (if.c:60)
==2386518== by 0x490CE85: if_create_name (if.c:223)
==2386518== by 0x490DF32: if_get_by_name (if.c:622)
==2386518== by 0x4993F73: zclient_interface_add (zclient.c:2186)
==2386518== by 0x4998062: zclient_read (zclient.c:3969)
==2386518== by 0x4979529: thread_call (thread.c:1908)
==2386518== by 0x4919918: frr_run (libfrr.c:1164)
==2386518== by 0x181AC7: main (ospf_main.c:235)
==2386518==
Fix the ordering to do the individual node tree cleanup after we delete
the data we care about.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Currently, we automatically delete an inactive VRF when its last
interface is deleted. This code introduces a couple of crashes because
of the following problems:
- vrf_delete is called before calling if_del hook, so daemons may try to
dereference an ifp->vrf pointer which is freed
- in if_terminate, we continue to use the VRF in the loop condition
after the last interface is deleted
This check is needed only when the interface is deleted by the user,
because if the interface is deleted by the system, VRF must still exist
in the system. Move the check to appropriate places to fix crashes.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
It allows FRR to read the interface config even when the necessary VRFs
are not yet created and interfaces are in "wrong" VRFs. Currently, such
config is rejected.
For VRF-lite backend, we don't care at all about the VRF of the inactive
interface. When the interface is created in the OS and becomes active,
we always use its actual VRF instead of the configured one. So there's
no need to reject the config.
For netns backend, we may have multiple interfaces with the same name in
different VRFs. So we care about the VRF of inactive interfaces. And we
must allow to preconfigure the interface in a VRF even before it is
moved to the corresponding netns. From now on, we allow to create
multiple configs for the same interface name in different VRFs and
the necessary config is applied once the OS interface is moved to the
corresponding netns.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
There's a helper function to check whether the interface is loopback or
VRF - if_is_loopback_or_vrf. Let's use it whenever we need to check that.
There's no functional change in this commit.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Adding defensive code to the interface_link_params zebra callback
to check if the link params changed before taking action.
Signed-off-by: Karen Schoener <karen@voltanet.io>
When we get this sequence of events:
- zebra receives interface up, sends to ospf
- ospf receives intf up, processes( including neighbor formation and spf )
and sends route to zebra for installation.
- zebra receives route for processing, schedules it too happen in the future
- zebra receives interface down event, sends to ospf
- zebra processes route X and marks it inactive because nexthop
interface is down
- zebra receives interface up event, sends to ospf
- ospf receives both events and processes the change and decides
that nothing has changed so it does not send any route change for X to zebra.
At this point zebra has a route from ospf that is marked as inactive, while
ospf believes that the route should be installed properly.
Modify the code such that on an interface down event, ospf marks the routes
as changed if the ifindex is being used for a nexthop, so that when ospf
is deciding if routes have changed post spf that it can just automatically
send that route down again if it still exists.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The interface parameters deletion must be called before
`route_table_finish` due to the usage of the route data structures to
search neighbors in the same interface. If the route info is removed
before that we get the following crash:
```
6 0x00007f5c6ed50394 in core_handler at lib/sigevent.c:255
7 <signal handler called>
8 ospf_interface_bfd_apply (ifp=<optimized out>) at ospfd/ospf_bfd.c:130
9 0x000055d4c306d076 in ospf_interface_disable_bfd at ospfd/ospf_bfd.c:159
10 0x000055d4c3071781 in ospf_del_if_params at ospfd/ospf_interface.c:553
11 0x000055d4c3071900 in ospf_if_delete_hook at ospfd/ospf_interface.c:704
12 0x00007f5c6ed17935 in hook_call_if_del at lib/if.c:59
13 if_delete_retain at lib/if.c:290
14 0x00007f5c6ed19bc5 in if_delete at lib/if.c:313
15 0x00007f5c6ed19d88 in if_terminate at lib/if.c:1067
16 0x00007f5c6ed63a04 in vrf_delete at lib/vrf.c:297
17 0x00007f5c6ed76784 in zclient_vrf_delete at lib/zclient.c:1974
18 zclient_read at lib/zclient.c:3686
19 0x00007f5c6ed60f85 in thread_call at lib/thread.c:1815
20 0x00007f5c6ed20228 in frr_run at lib/libfrr.c:1149
21 0x000055d4c306bc70 in main at ospfd/ospf_main.c:233
```
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Instead of trying to maintain if_ospf_cli_count, let's directly count
the number of configured interfaces when it is needed. Current approach
sometimes leads to an incorrect counter.
Fixes#8321.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently if you start ospfd, bring up neighbors and then issue
a tcpdump on a interface ospf is peering over, this causes the neighbor
relationship to be restarted:
root@spectrum301(mlx-4600c-01):mgmt:~# tcpdump -i vlan402
2020-11-13T21:25:38.059671+00:00 spectrum301 ospfd[29953]: AdjChg: Nbr 202.0.0.3(default) on vlan402:200.0.3.1: Full -> Deleted (KillNbr)
2020-11-13T21:25:38.065520+00:00 spectrum301 ospfd[29953]: ospfTrapNbrStateChange: trap sent: 200.0.3.2 now Deleted/DROther
2020-11-13T21:25:38.065922+00:00 spectrum301 ospfd[29953]: ospfTrapIfStateChange: trap sent: 200.0.3.1 now Down
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on vlan402, link-type EN10MB (Ethernet), capture size 262144 bytes
21:25:38.072330 IP 200.0.3.1 > igmp.mcast.net: igmp v3 report, 1 group record(s)
2020-11-13T21:25:38.080430+00:00 spectrum301 ospfd[29953]: ospfTrapIfStateChange: trap sent: 200.0.3.1 now Point-To-Point
2020-11-13T21:25:38.080654+00:00 spectrum301 ospfd[29953]: SPF Processing Time(usecs): 9734
2020-11-13T21:25:38.080829+00:00 spectrum301 ospfd[29953]: SPF Time: 6422
2020-11-13T21:25:38.080991+00:00 spectrum301 ospfd[29953]: InterArea: 1572
2020-11-13T21:25:38.081152+00:00 spectrum301 ospfd[29953]: Prune: 67
2020-11-13T21:25:38.081329+00:00 spectrum301 ospfd[29953]: RouteInstall: 1396
2020-11-13T21:25:38.081548+00:00 spectrum301 ospfd[29953]: Reason(s) for SPF: N, S, ABR, ASBR
21:25:38.092510 IP 200.0.3.1 > ospf-all.mcast.net: OSPFv2, Hello, length 44
This is happening because the curr_mtu is not being properly stored. It was being set
on interface creation( but we have not actually read in the mtu part of the interface data, so
it is still 0 ).
Modify the code to store the curr_mtu at a point in interface creation *After* we have read
in interface data.
Ticket: CM-32276
Signed-off-by: Donald Sharp <sharpd@nvidia.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>
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 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>
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>
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>
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>
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>
