Mark Stapp [Mon, 15 Feb 2021 18:59:02 +0000 (13:59 -0500)]
build: detect ICC, only try ICC options if ICC
Some ICC command-line options can cause confusion for other
compilers; test for ICC specifically, and only try to use those
options if ICC is being used.
Donald Sharp [Sun, 14 Feb 2021 21:08:07 +0000 (16:08 -0500)]
bgpd: Prevent store but never read of i
The new bgp_mplsvpn_snmp.c code increments i and
never uses that value. Comment the code out
to make the clang SA analyzer happy. Shouldn't
be a problem since this code will never probably
be touched again(ha!).
Donald Sharp [Sun, 14 Feb 2021 21:04:16 +0000 (16:04 -0500)]
nhrpd: Fix clang SA about null deref
Clang was complaining when running SA that the nhrpd_privs.change
function was null. It just does not fully understand how things
are setup. Add a assert to make it happy.
Donatas Abraitis [Sun, 14 Feb 2021 15:49:19 +0000 (17:49 +0200)]
bgpd: Check for peer->su_remote if not NULL when handling IPv6 nexthop
```
(gdb) bt
0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
1 0x00007fe57ca4a42a in __GI_abort () at abort.c:89
2 0x00007fe57ddd1935 in core_handler (signo=6, siginfo=0x7ffc81067570, context=<optimized out>) at lib/sigevent.c:255
3 <signal handler called>
4 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
5 0x00007fe57ca4a42a in __GI_abort () at abort.c:89
6 0x00007fe57ddd1935 in core_handler (signo=11, siginfo=0x7ffc81067e30, context=<optimized out>) at lib/sigevent.c:255
7 <signal handler called>
8 0x000055a7b25b923f in bgp_path_info_to_ipv6_nexthop (ifindex=ifindex@entry=0x7ffc810683c0, path=<optimized out>, path=<optimized out>) at bgpd/bgp_zebra.c:909
9 0x000055a7b25bb2e5 in bgp_zebra_announce (dest=dest@entry=0x55a7b5239c10, p=p@entry=0x55a7b5239c10, info=info@entry=0x55a7b5239cd0, bgp=bgp@entry=0x55a7b518b090, afi=afi@entry=AFI_IP6, safi=safi@entry=SAFI_UNICAST) at bgpd/bgp_zebra.c:1358
10 0x000055a7b256af6a in bgp_process_main_one (bgp=0x55a7b518b090, dest=0x55a7b5239c10, afi=AFI_IP6, safi=SAFI_UNICAST) at bgpd/bgp_route.c:2918
11 0x000055a7b256b0ee in bgp_process_wq (wq=<optimized out>, data=0x55a7b5221800) at bgpd/bgp_route.c:3027
12 0x00007fe57ddea2e0 in work_queue_run (thread=0x7ffc8106cd60) at lib/workqueue.c:291
13 0x00007fe57dde0781 in thread_call (thread=thread@entry=0x7ffc8106cd60) at lib/thread.c:1684
14 0x00007fe57dda84b8 in frr_run (master=0x55a7b48aaf00) at lib/libfrr.c:1126
15 0x000055a7b250a7da in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:540
(gdb)
```
This crashes with configs like:
```
router bgp 65534
no bgp ebgp-requires-policy
no bgp network import-check
!
address-family ipv6 unicast
import vrf donatas <<<<<< Crashes when entering this command
exit-address-family
!
router bgp 65534 vrf donatas
no bgp ebgp-requires-policy
no bgp network import-check
neighbor fe80::c15a:ddab:1689:db86 remote-as 65025
neighbor fe80::c15a:ddab:1689:db86 interface eth2
neighbor fe80::c15a:ddab:1689:db86 update-source eth2
neighbor fe80::c15a:ddab:1689:db86 capability extended-nexthop
!
address-family ipv6 unicast
network 2a02:face::/32 <<<<<< Crashes due to static networks
neighbor fe80::c15a:ddab:1689:db86 activate
exit-address-family
!
```
Locally configured routes do not have peer->su_remote.
```
exit1-debian-9# show bgp ipv6 unicast
BGP table version is 3, local router ID is 192.168.100.1, vrf id 0
Default local pref 100, local AS 65534
Status codes: s suppressed, d damped, h history, * valid, > best, = multipath,
i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes: i - IGP, e - EGP, ? - incomplete
Network Next Hop Metric LocPrf Weight Path
*> 2a02:abc::/64 fe80::c15a:ddab:1689:db86@5<
0 65025 i
2a02:face::/32 ::@5< 0 32768 i
Displayed 2 routes and 2 total paths
exit1-debian-9#
Trey Aspelund [Sat, 13 Feb 2021 05:08:19 +0000 (05:08 +0000)]
ospfd: cleanup ospf_flood debugs
Replaces some hard-coded function names with __func__,
adds some additional references to neighbor/interface,
and cleans up some debug strings to be more readable.
Quentin Young [Thu, 11 Feb 2021 23:54:27 +0000 (18:54 -0500)]
bgpd: send correct BMP down message when nht fails
When sending BMP messages for a status change event for a peer whose NHT
has failed, we were sending a Peer Down Reason Code of 1 (Local system
closed, NOTIFICATION follows) with no NOTIFICAION PDU (because there was
none). This is wrong. Also, the reason code of 1 is semantically off, it
should be 2 (Local system closed, FSM event follows).
This patch:
- adds definitions of all BGP FSM event codes per RFC4271
- changes the BMP reason code emitted when a peer changes state due to
NHT failure to 2 and encodes FSM event 18 (TcpConnectionFails)
- changes the catch-all case where we have not yet
implemented the appropriate BMP response to indicate reason code 2
with FSM event 0 (no relevant Event code is defined).
These changes ought to prevent the BMP session from being torn down due
to an improperly formatted message.
Signed-off-by: Quentin Young <qlyoung@qlyoung.net>
David Lamparter [Thu, 4 Feb 2021 21:27:11 +0000 (22:27 +0100)]
lib: de-uglify `-t` option + `log stdout` combo
The logging code writes log messages with a `\n` line ending, meanwhile
the VTY code switches it so you need `\r\n`...
And we don't flush the newline after executing a command either.
After this patch, starting daemons like `zebra/zebra -t` should provide
a nice development/debugging experience with a VTY open right there on
stdio and `log stdout` interspersed.
(This is already documented in the man pages, it just looked like sh*t
previously since the log messages didn't newline correctly.)
Signed-off-by: David Lamparter <equinox@diac24.net>
David Lamparter [Thu, 4 Feb 2021 21:25:49 +0000 (22:25 +0100)]
lib: memorize what fds were open at startup
... in case the user does something like `zebra 3>logfile`. Also useful
for some module purposes, maybe even feeding config at some point in the
future.
Signed-off-by: David Lamparter <equinox@diac24.net>
David Teach [Thu, 11 Feb 2021 20:44:00 +0000 (20:44 +0000)]
bgpd: Add Support for rfc 8050 MRT add-path
- Rfc 8050 adds support for BGP NLRI that carry path identifiers. this commit adds that support to FRR
- Updated bgp_dump.h to include new sub-type values
- Updated bgp_dump.c to check for add_path af_caps in the peer struct.
- Updated bgp_dump.c to use the proper sub-type values upon detection of add-path af_caps
- Updated bgp_dump.c to properly dump the path_id wen present.
Signed-off-by: David Teach <dteach@routeviews.org>
Trey Aspelund [Tue, 9 Feb 2021 01:10:03 +0000 (01:10 +0000)]
bgpd: fix empty advertised-routes for 2-tier safis
'show bgp ipv[46] vpn neighbors ... advertised-routes' was displaying
empty output due to new command syntax using show_adj_routes() which
assumed each bgp_table was single-tier (not nested). This fixes that
assumption for safis with a two-tier bgp_table (SAFI_MPLS_VPN,
SAFI_ENCAP, and SAFI_EVPN).
Before:
ub18# show bgp ipv6 vpn neighbors 2001:db8:cafe::2 advertised-routes
ub18#
After:
ub20# show bgp ipv6 vpn neighbors 2001:db8:cafe::1 advertised-routes
BGP table version is 2, local router ID is 100.64.0.222, vrf id 0
Default local pref 100, local AS 1
Status codes: s suppressed, d damped, h history, * valid, > best, = multipath,
i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes: i - IGP, e - EGP, ? - incomplete
Network Next Hop Metric LocPrf Weight Path
Route Distinguisher: 30:30
*> 2::2/128 :: 0 100 32768 i
*> 2::22/128 :: 0 100 32768 i
Route Distinguisher: 33:33
*> 2::2/128 :: 0 100 32768 i
*> 2::22/128 :: 0 100 32768 i
Olivier Dugeon [Wed, 10 Feb 2021 18:20:24 +0000 (19:20 +0100)]
ospfd: Debug race condition in Segment Routing
Issue #7926 hilight a race condition in Segment Routing processing.
The problem occurs when Router Information Opaque LSA is received late, in
particular after SPF run and after ospf_sr_nhlfe_update() was called. This
scenario is unfrequent and takes place due to a slow DR election.
In this particular case, SR Prefix are handle but not fully fill. In fact,
SRGB for the nexthop is not yet received and thus, output label could not
be computed.
When Router Information Opaque LSA is received and processed, if the
corresponding SR node is a direct neighbor of the self node, update_out_nhlfe()
is called against all SR nodes to adjust SR prefix if the next hop is the new
SR node. The function wrongly computes output label and configure a bad MPLS
LFIB entries.
Another way to hilight the problem is to change through CLI the SRGB of a node
and look to MPLS LFIB of direct neighbor, in particular those who announce
EXPLICIT NULL Prefix SID.
This patch correct the update_out_nhlfe() function by calling the appropriate
function (sr_prefix_out_label() instead of index2label()) to compute the output
label.
Some log debugs were adjusted and unused prefix route table was removed too.
Donald Sharp [Thu, 11 Feb 2021 12:31:05 +0000 (07:31 -0500)]
ospfd: Prevent duplicate packet read in certain vrf situations
Currently if the sysctl net.ipv4.raw_l3mdev_accept is 1, packets
destined to a specific vrf also end up being delivered to the default
vrf. We will see logs like this in ospf:
2021/02/10 21:17:05.245727 OSPF: ospf_recv_packet: fd 20(default) on interface 1265(swp1s1.26)
2021/02/10 21:17:05.245740 OSPF: Hello received from [9.9.36.12] via [swp1s1.26:200.254.26.13]
2021/02/10 21:17:05.245741 OSPF: src [200.254.26.14],
2021/02/10 21:17:05.245743 OSPF: dst [224.0.0.5]
2021/02/10 21:17:05.245769 OSPF: ospf_recv_packet: fd 45(vrf1036) on interface 1265(swp1s1.26)
2021/02/10 21:17:05.245774 OSPF: Hello received from [9.9.36.12] via [swp1s1.26:200.254.26.13]
2021/02/10 21:17:05.245775 OSPF: src [200.254.26.14],
2021/02/10 21:17:05.245777 OSPF: dst [224.0.0.5]
This really really makes ospf unhappy in the vrf we are running in.
I am approaching the problem by just dropping the packet if read in the
default vrf because of:
lib: Allow bgp to always create a listen socket for the vrf
Effectively if we have `router ospf vrf BLUE` but no ospf running
in the default vrf, we will not have a listener and that would
require a fundamental change in our approach to handle the ospf->fd
at a global level. I think this is less than ideal at the moment
but it will get us moving again and allow FRR to work with
a bunch of vrf's and ospf neighbors.
Philippe Guibert [Thu, 23 Jul 2020 06:57:05 +0000 (08:57 +0200)]
nhrpd: shortcut routes installed with nexthop.
Previously, when a shortcut entry was created, its associated route was
created on system, with no nexthop, only gre device. eg:
[..]
N>* 192.168.2.0/24 [10/0] is directly connected, gre1, 00:01:04 <--- can not be resolved
[..]
Type Prefix Via Identity
dynamic 192.168.2.0/24 10.255.255.2 <---- correct
This situation was forcing neighbor resolution on the first outgoing packet matching the route entry. for instance 192.168.2.1 could not be resolved at link layer, and was going to fail. Instead, nhrp nexthop should have been used.
This is what this commit intends to do, that is to say that when a
shortcut is installed by nhrp, the associated nexthop entry is used.
[..]
N>* 192.168.2.0/24 [10/0] via 10.255.255.2, gre1 onlink, 00:00:31
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Soman K S [Wed, 10 Feb 2021 11:15:22 +0000 (16:45 +0530)]
ospf6d : fix issue in ecmp inter area route
Issue: When a path in the inter area ecmp route is deleted, the route is removed
Fix: The fix is to remove the specific path from the inter area route using
ospf6_abr_old_route_remove() when abr route entry is not found.
In the function ospf6_abr_old_route_remove() the path to be removed needs
to match adv router and link state ID
Fixed memory leak in ospf6_intra_prefix_update_route_origin() caused by
route node lock not getting released.
Igor Ryzhov [Tue, 9 Feb 2021 18:38:45 +0000 (21:38 +0300)]
vrf: mark vrf as configured when entering vrf node
The VRF must be marked as configured when user enters "vrf NAME" command.
Otherwise, the following problem occurs:
`ip link add red type vrf table 1`
VRF structure is allocated.
`vtysh -c "conf t" -c "vrf red"`
`lib_vrf_create` is called, and pointer to the VRF structure is stored
to the nb_config_entry.
`ip link del red`
VRF structure is freed (because it is not marked as configured), but
the pointer is still stored in the nb_config_entry.
`vtysh -c "conf t" -c "no vrf red"`
Nothing happens, because VRF structure doesn't exist. It means that
`lib_vrf_destroy` is not called, and nb_config_entry still exists in
the running config with incorrect pointer.
`ip link add red type vrf table 1`
New VRF structure is allocated.
`vtysh -c "conf t" -c "vrf red"`
`lib_vrf_create` is NOT called, because the nb_config_entry for that
VRF name still exists in the running config.
After that all NB commands for this VRF will use incorrect pointer to
the freed VRF structure.
zyxwvu Shi [Mon, 8 Feb 2021 12:09:02 +0000 (20:09 +0800)]
bgpd: Do not compare attr again.
`same_attr` has been computed and `hook_call(bgp_process)` (calling
BMP module) would not change it. We could reuse the value to filter
same attribute updates, avoiding an extra comparison.
Quentin Young [Mon, 8 Feb 2021 01:15:24 +0000 (20:15 -0500)]
vtysh: disable bracketed paste in readline
GNU Readline 8.1 enables bracketed paste by default. This results in
newlines not ending the readline() call, which breaks the ability of
users to paste in configs to vtysh's interactive shell.
Disable bracketed paste.
Signed-off-by: Quentin Young <qlyoung@qlyoung.net>
Donald Sharp [Sun, 7 Feb 2021 20:03:51 +0000 (15:03 -0500)]
bfdd: Prevent use after free ( again )
Valgrind is still reporting:
466020-==466020== by 0x11B9F4: main (bfdd.c:403)
466020-==466020== Address 0x5a7d544 is 84 bytes inside a block of size 272 free'd
466020:==466020== at 0x48399AB: free (vg_replace_malloc.c:538)
466020-==466020== by 0x490A947: qfree (memory.c:140)
466020-==466020== by 0x48F2AE8: if_delete (if.c:322)
466020-==466020== by 0x48F250D: if_destroy_via_zapi (if.c:195)
466020-==466020== by 0x497071E: zclient_interface_delete (zclient.c:2040)
466020-==466020== by 0x49745F6: zclient_read (zclient.c:3687)
466020-==466020== by 0x4955AEC: thread_call (thread.c:1684)
466020-==466020== by 0x48FF64E: frr_run (libfrr.c:1126)
466020-==466020== by 0x11B9F4: main (bfdd.c:403)
466020-==466020== Block was alloc'd at
466020:==466020== at 0x483AB65: calloc (vg_replace_malloc.c:760)
466020-==466020== by 0x490A805: qcalloc (memory.c:115)
466020-==466020== by 0x48F23D6: if_new (if.c:160)
466020-==466020== by 0x48F257F: if_create_name (if.c:214)
466020-==466020== by 0x48F3493: if_get_by_name (if.c:558)
466020-==466020== by 0x49705F2: zclient_interface_add (zclient.c:1989)
466020-==466020== by 0x49745E0: zclient_read (zclient.c:3684)
466020-==466020== by 0x4955AEC: thread_call (thread.c:1684)
466020-==466020== by 0x48FF64E: frr_run (libfrr.c:1126)
466020-==466020== by 0x11B9F4: main (bfdd.c:403)
Apparently the bs->ifp pointer is being set even in cases when
the bs->key.ifname is not being set. So go through and just
match the interface pointer and cut-to-the-chase.
Donald Sharp [Sun, 7 Feb 2021 19:59:53 +0000 (14:59 -0500)]
*: Fix usage of bfd_adj_event
Valgrind reports:
469901-==469901==
469901-==469901== Conditional jump or move depends on uninitialised value(s)
469901:==469901== at 0x3A090D: bgp_bfd_dest_update (bgp_bfd.c:416)
469901-==469901== by 0x497469E: zclient_read (zclient.c:3701)
469901-==469901== by 0x4955AEC: thread_call (thread.c:1684)
469901-==469901== by 0x48FF64E: frr_run (libfrr.c:1126)
469901-==469901== by 0x213AB3: main (bgp_main.c:540)
469901-==469901== Uninitialised value was created by a stack allocation
469901:==469901== at 0x3A0725: bgp_bfd_dest_update (bgp_bfd.c:376)
469901-==469901==
469901-==469901== Conditional jump or move depends on uninitialised value(s)
469901:==469901== at 0x3A093C: bgp_bfd_dest_update (bgp_bfd.c:421)
469901-==469901== by 0x497469E: zclient_read (zclient.c:3701)
469901-==469901== by 0x4955AEC: thread_call (thread.c:1684)
469901-==469901== by 0x48FF64E: frr_run (libfrr.c:1126)
469901-==469901== by 0x213AB3: main (bgp_main.c:540)
469901-==469901== Uninitialised value was created by a stack allocation
469901:==469901== at 0x3A0725: bgp_bfd_dest_update (bgp_bfd.c:376)
On looking at bgp_bfd_dest_update the function call into bfd_get_peer_info
when it fails to lookup the ifindex ifp pointer just returns leaving
the dest and src prefix pointers pointing to whatever was passed in.
Let's do two things:
a) The src pointer was sometimes assumed to be passed in and sometimes not.
Forget that. Make it always be passed in
b) memset the src and dst pointers to be all zeros. Then when we look
at either of the pointers we are not making decisions based upon random
data in the pointers.
bgpd: Dump BGP attrs to check what's the actual prefix with aggr_as 0
Just for more debug information regarding malformed aggregator_as.
```
bgpd[5589]: [EC 33554434] 192.168.10.25: AGGREGATOR AS number is 0 for aspath: 65030
bgpd[5589]: bgp_attr_aggregator: attributes: nexthop 192.168.10.25, origin i, path 65030
```
Igor Ryzhov [Tue, 2 Feb 2021 22:02:15 +0000 (01:02 +0300)]
bfdd: fix session lookup
BFD key has optional fields "local" and "ifname" which can be empty when
the BFD session is created. In this case, the hash key will be calculated
with these fields filled with zeroes.
Later, when we're looking for the BFD session using the key with fields
"local" and "ifname" populated with actual values, the hash key will be
different. To work around this issue, we're doing multiple hash lookups,
first with full key, then with fields "local" and "ifname" filled with
zeroes.
But there may be another case when the initial key has the actual values
for "local" and "ifname", but the key we're using for lookup has empty
values. This case is covered for IPv4 by using additional hash walk with
bfd_key_lookup_ignore_partial_walker function but is not covered for IPv6.
Instead of introducing more hacks and workarounds, the following solution
is proposed:
- the hash key is always calculated in bfd_key_hash_do using only
required fields
- the hash data is compared in bfd_key_hash_cmp, taking into account the
fact that fields "local" and "ifname" may be empty
Using this solution, it's enough to make only one hash lookup.
projects / mirror_frr.git / log