Commit Graph

4560 Commits

Author SHA1 Message Date
Mark Stapp
cbd492990d
Merge pull request #7194 from qlyoung/tracing
Tracing
2020-10-24 16:50:35 -04:00
Renato Westphal
7281a37c8b
Merge pull request #7377 from donaldsharp/bgp_static_leak
bgpd: Bgp static routes memory leak
2020-10-24 15:29:34 -03:00
Quentin Young
fc3ee1e041 bgpd: clean up trace header style a bit
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
2020-10-23 15:23:12 -04:00
Quentin Young
0c3436aa22 bgpd: move packet read tracepoint out of mutex
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
2020-10-23 15:13:51 -04:00
Quentin Young
c7bb4f006b lib, bgpd: convert lttng tracepoints to frrtrace()
- tracepoint() -> frrtrace()
- tracelog() -> frrtracelog()
- tracepoint_enabled() -> frrtrace_enabled()

Also removes copypasta'd #ifdefs for those LTTng macros, those are
handled in lib/trace.h

Signed-off-by: Quentin Young <qlyoung@nvidia.com>
2020-10-23 15:13:51 -04:00
Quentin Young
6401252f7f bgpd: route processing tracepoints
Add tracepoints for route process, input filter and output filter.

Signed-off-by: Quentin Young <qlyoung@nvidia.com>
2020-10-23 15:13:51 -04:00
Quentin Young
d9a03c5736 bgpd: add basic packet-related tracepoints
Add tracepoints for:
- packet pushed to internal rx queue
- packet dequeued from rx queue and processed

Signed-off-by: Quentin Young <qlyoung@nvidia.com>
2020-10-23 15:13:51 -04:00
Mark Stapp
5047884528 *: unify thread/event cancel macros
Replace all lib/thread cancel macros, use thread_cancel()
everywhere. Only the THREAD_OFF macro and thread_cancel() api are
supported. Also adjust thread_cancel_async() to NULL caller's pointer (if
present).

Signed-off-by: Mark Stapp <mjs@voltanet.io>
2020-10-23 12:16:52 -04:00
Donald Sharp
811c6797b6 bgpd: Bgp static routes memory leak
When using MPLS_VPN/EVPN ( or really any two level table/route data structure setup )
FRR is leaking memory on shutdown:

eva# conf
eva(config)# router bgp 329
eva(config-router)# address-family ipv4 vpn
eva(config-router-af)# network 5.6.7.8/32 rd 44:55 label 3293
eva(config-router-af)# end
eva# exit
sharpd@eva ~/frr_coverity (master)> ps -ef | grep frr
root     1186423   10793  0 07:51 pts/1    00:00:00 sudo /usr/lib/frr/zebra --log stdout --log-level debug
frr      1186425 1186423  0 07:51 pts/1    00:00:00 /usr/lib/frr/zebra --log stdout --log-level debug
root     1263168  491694  0 11:10 pts/20   00:00:00 sudo valgrind --leak-check=full /usr/lib/frr/bgpd --log stdout --log-level debug
frr      1263169 1263168 22 11:10 pts/20   00:00:04 /usr/bin/valgrind.bin --leak-check=full /usr/lib/frr/bgpd --log stdout --log-level debug
sharpd   1263214  845829  0 11:10 pts/9    00:00:00 grep --color=auto frr
sharpd@eva ~/frr_coverity (master)> sudo kill -SIGTERM 1263169
sharpd@eva ~/frr_coverity (master)>

gives us this:

==1263169== 304 (40 direct, 264 indirect) bytes in 1 blocks are definitely lost in loss record 61 of 78
==1263169==    at 0x483AB65: calloc (vg_replace_malloc.c:760)
==1263169==    by 0x48DD878: qcalloc (memory.c:110)
==1263169==    by 0x5116D5: bgp_table_init (bgp_table.c:110)
==1263169==    by 0x4EB5C4: bgp_static_set_safi (bgp_route.c:5927)
==1263169==    by 0x4C3382: vpnv4_network (bgp_mplsvpn.c:1911)
==1263169==    by 0x489FBEC: cmd_execute_command_real (command.c:916)
==1263169==    by 0x489F7CB: cmd_execute_command (command.c:976)
==1263169==    by 0x489FD04: cmd_execute (command.c:1138)
==1263169==    by 0x493AF73: vty_command (vty.c:517)
==1263169==    by 0x493AA07: vty_execute (vty.c:1282)
==1263169==    by 0x4939B54: vtysh_read (vty.c:2115)
==1263169==    by 0x492E63C: thread_call (thread.c:1585)

The bgp_static_delete function was not unlocking the right bgp_dest.  This
problem goes away after fixing this.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2020-10-23 11:22:01 -04:00
Mark Stapp
b3d6bc6ef0 * : update signature of thread_cancel api
Change thread_cancel to take a ** to an event, NULL-check
before dereferencing, and NULL the caller's pointer. Update
many callers to use the new signature.

Signed-off-by: Mark Stapp <mjs@voltanet.io>
2020-10-23 08:59:34 -04:00
Donatas Abraitis
90a65457d1
Merge pull request #7345 from opensourcerouting/bgp-aggr-suppress
bgpd: aggregate-address suppress-map
2020-10-23 15:02:57 +03:00
Rafael Zalamena
6ba6de7e17 bgpd,topotests: log route suppression messages
To see the messages activate the BGP debug: `debug bgp updates`.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
2020-10-22 13:52:06 -03:00
Rafael Zalamena
4056a5f6a5 bgpd: route suppression refactory
Instead of just counting the route suppressions, keep a reference for
all aggregations that are doing it. It should help the with the
following problems:

- Which aggregation suppressed the route.
- Double suppression
- Double unsuppression
- Avoids calling `bgp_process` if already suppressed/unsuppressed.
- Easier code maintenance and understanding

This also fixes a crash when modifying a route map that is
associated with a working aggregate-address.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
2020-10-22 13:52:00 -03:00
Emanuele Bovisio
36d4bb44c4 bgpd: fix information strings for vtysh
set correct information strings for vtysh.

Signed-off-by: Emanuele Bovisio <emanuele.bovisio@eolo.it>
2020-10-22 17:51:04 +02:00
Donald Sharp
84de5a245a
Merge pull request #7343 from ton31337/fix/prefix2str_to_pFX
:* Convert prefix2str to %pFX
2020-10-22 08:07:57 -04:00
Donatas Abraitis
2dbe669bdf :* Convert prefix2str to %pFX
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-10-22 09:07:41 +03:00
Rafael Zalamena
365ab2e74b bgpd: aggregate address suppress more specific
Add new aggregate-address option to selectively suppress routes based
on route map results.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
2020-10-21 21:31:49 -03:00
Donald Sharp
b6c386bbbd bgpd: Make the process_queue per bgp process
We currently have a global process queue for handling route
updates in bgp.  This is fine, in general, except there are
places and times where we plug the queue for no new work
during certain peer states of bgp update delay.  If we
happen to be processing multiple bgp instances on startup
why do we want to stop processing in vrf A when vrf B
is in a bit of a pickle?

Also this separation will allow us to start forward thinking
about how to fully integrate pthreads into route processing
in bgp.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2020-10-21 15:34:47 -04:00
Anuradha Karuppiah
45a859f1c3 bgpd: fix crash in the MH cleanup handling
The MH datastructures were being released before the paths that were
referencing them. Fix is to do the MH cleanup last.

The MH finish function has also been stripped down to only do a
datastructure cleanup i.e. avoid sending route updates etc.

Ticket: 31376

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
2020-10-21 09:09:21 -07:00
Donald Sharp
cd7f9b1711
Merge pull request #7323 from ton31337/fix/inet_ntoa_to_pFX_master
bgpd: Convert inet_ntoa to %pI4
2020-10-20 09:10:24 -04:00
Donatas Abraitis
9072f5c89a
Merge pull request #7311 from donaldsharp/table_lock_count
Abstract rn->lock accessing and cleanup usage to %pFX and %pRN
2020-10-20 16:04:15 +03:00
Patrick Ruddy
cecc76cdc5
Merge pull request #7300 from sworleys/EVPN-RType-NUMs
lib,bgpd: more evpn route-type numerical command versions
2020-10-20 11:05:31 +01:00
Donatas Abraitis
23d0a75356 bgpd: Convert inet_ntoa to %pI4/inet_ntop
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-10-18 11:22:30 +03:00
Donald Sharp
c10e14e96d *: Create/Use accessor functions for lock count
Create appropriate accessor functions for the rn->lock
data.  We should be accessing this data through accessor
functions since it is private data to the data structure.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2020-10-17 13:39:10 -04:00
Donatas Abraitis
0dc8647094
Merge pull request #7306 from donaldsharp/bgp_dest_print
Bgp dest print
2020-10-17 20:21:52 +03:00
Donald Sharp
752eed47ef bgpd: Use bgp_dest_get_prefix accessor function
Use the appropriate bgp_dest_get_prefix accessor function

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2020-10-17 08:52:35 -04:00
Donald Sharp
09319b4e0f bgpd: More bgp_node -> bgp_dest cleanup
Some more of the bgp_node usage snuck in from big commits in
the past month or so from feature work.  Do some work
to put it back to bgp_dest for incoming future work.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2020-10-17 08:52:35 -04:00
Donald Sharp
8228a9a7a5 bgpd: Convert to %pFX or %pBD where possible
Search and destroy places where we used prefix2str
that could be replaced with %pFX or %pBD in bgpd.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2020-10-17 08:52:35 -04:00
David Lamparter
56ca3b5b3a bgpd: add %pBD for printing struct bgp_dest *
`%pRN` is not appropriate anymore.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
2020-10-17 08:52:35 -04:00
Pat Ruddy
f137734bb4 bgpd: replace bgp_evpn_route2str with prefix2str
Remove bgp_evpn_route2str and replace calls with prefix2str

Signed-off-by: Pat Ruddy <pat@voltanet.io>
2020-10-16 11:54:30 +01:00
Igor Ryzhov
17b4e8c439 bgpd: print error when as-path filter doesn't exist
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
2020-10-14 23:10:48 +03:00
Donatas Abraitis
9ed5b22e13
Merge pull request #7283 from donaldsharp/thread_off_semantics
*: Use proper semantics for turning off thread
2020-10-14 14:50:08 +03:00
Stephen Worley
38677473b2 bgpd: numerical evpn type support to route-map cmds
Add numberical evpn route-type support to route-map evpn
commands.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
2020-10-13 16:13:16 -04:00
Stephen Worley
15dc08a600 bgpd: numerical evpn type support to debug commands
Add number evpn route type support for debug commands.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
2020-10-13 16:13:16 -04:00
Stephen Worley
31782fea7d bgpd: make debug evpn help use string constants
Make the debug evpn help commands use evpn string constants
for evpn route type info.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
2020-10-13 16:13:16 -04:00
Stephen Worley
2b1364e1b7 bgpd: numerical evpn type support to more show cmds
Add numberical evpn route type support to some more
show commands.

Also, simplify some of the code there to call common type parsing
function. Some of the bounds checking there is also unncessary given
how our cli node matching works.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
2020-10-13 16:13:16 -04:00
Donatas Abraitis
451171c09d
Merge pull request #7277 from chiragshah6/yang_nb6
bgpd: fix crash in bgp instance creation
2020-10-13 11:12:28 +03:00
Chirag Shah
bdaadb91e8 bgpd: use common api in bgp_get
Use consolidate api in bgp_get and bgp northbound
create callback.

Signed-off-by: Chirag Shah <chirag@nvidia.com>
2020-10-12 16:41:39 -07:00
Chirag Shah
a5ab756f24 bgpd: fix crash in bgp instance creation
In bgp global commands northbound local-as modify callback
check for backend db for checking existing bgp instance.

In an instance where no router bgp with old ASN cleaned up
followed by new bgp instance with new AS is created,
the nb_running_get_entry in validation phase returns stale
bgp reference, which leads to rejection of the router bgp command.

Uncovered via:
toptotest evpn_type5_test_topo1/test_evpn_type5_topo1.py
test_bgp_attributes_for_evpn_address_family_p1

Signed-off-by: Chirag Shah <chirag@nvidia.com>
2020-10-12 16:13:59 -07:00
Chirag Shah
feded4118b bgpd: rename router bgp callbacks
Avoid similar name function as bgp_crate()

Signed-off-by: Chirag Shah <chirag@nvidia.com>
2020-10-12 16:13:59 -07:00
Trey Aspelund
7daf25a3a8 bgpd: fix show bgp neighbor routes for labeled-unicast
bgp_show_neighbor_route() was rewriting safi from LU to uni
before checking if the peer was enabled for LU.  This resulted
in the peer's address-family check looking for unicast, which
would always fail for LU peers since unicast + LU are
mutually-exclusive AFIs.
This moves this safi reassignment after the peer AFI check,
ensuring that the peer's address-family check looks for LU
while the call to bgp_show() still uses uni.

-- highlights from manual testing

config:

router bgp 2
 neighbor 1.1.1.1 remote-as external
 neighbor 1.1.1.1 disable-connected-check
 neighbor 1.1.1.1 update-source 2.2.2.2
 !
 address-family ipv4 unicast
  no neighbor 1.1.1.1 activate
 exit-address-family
 !
 address-family ipv4 labeled-unicast
  neighbor 1.1.1.1 activate
 exit-address-family

before:

spine01# show bgp ipv4 unicast neighbors 1.1.1.1 routes
% No such neighbor or address family
spine01# show bgp ipv4 labeled-unicast neighbors 1.1.1.1 routes
% No such neighbor or address family

after:

spine01# show bgp ipv4 unicast neighbors 1.1.1.1 routes
% No such neighbor or address family
spine01# show bgp ipv4 label neighbors 1.1.1.1 routes
BGP table version is 1, local router ID is 2.2.2.2
Status codes: s suppressed, d damped, h history, * valid, > best, = multipath,
              i internal, r RIB-failure, S Stale, R Removed
Origin codes: i - IGP, e - EGP, ? - incomplete
   Network          Next Hop            Metric LocPrf Weight Path
*> 11.11.11.11/32   1.1.1.1                  0             0 1 i
Displayed  1 routes and 1 total paths

Signed-off-by: Trey Aspelund <taspelund@cumulusnetworks.com>
2020-10-12 16:19:44 -04:00
Donald Sharp
b1823b69f6 bgpd: Correctly calculate threshold being reached
if (pcout > (pcount * peer->max_threshold[afi][safi] / 100 ))
is always true.  So the very first route received will always
trigger the warning.  We actually want the warning to happen
when we hit the threshold.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2020-10-12 10:36:37 -04:00
Donald Sharp
28ef0ee121 *: Use proper semantics for turning off thread
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>
2020-10-12 08:35:18 -04:00
Donald Sharp
9f3f7d9a70
Merge pull request #7268 from ton31337/fix/dead_name_bgpd
bgpd: Use default VRF name if using `router bgp` command
2020-10-09 19:34:38 -04:00
Donatas Abraitis
7563bd3fd8
Merge pull request #7252 from sworleys/EVPN-RType-Help
lib,bgpd: EVPN help string cleanup
2020-10-09 09:19:18 +03:00
Donatas Abraitis
5ce106b72b bgpd: Use default VRF name if using router bgp command
When we enter `router bgp` it enters non-VRF instance which is default.

No need to check for VRF/VIEW name, kinda dead code.

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
2020-10-09 09:02:49 +03:00
Stephen Worley
f5dc0b1a42 bgpd: re-use HELP_STR evpn macros
Re-use some of the HELP_STR macros rather than copypastaing
strings everywhere.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>
2020-10-08 16:01:56 -04:00
Igor Ryzhov
1673a90dd3 bgpd: hide test commands
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
2020-10-08 11:03:25 +03:00
Donatas Abraitis
90c815bc99
Merge pull request #7246 from mjstapp/fix_bgp_nb_sa
bgpd: fix SA warnings in bgp northbound
2020-10-07 09:46:15 +03:00
Donatas Abraitis
64fe0feea3
Merge pull request #7239 from opensourcerouting/bgp-aggregate-med
bgpd: aggregate-address on matching MED only
2020-10-07 09:44:30 +03:00