Commit Graph

33585 Commits

Author SHA1 Message Date
Russ White
91c5a471a7
Merge pull request #14651 from opensourcerouting/fix/bgpd_coverity_fqdn_capability
bgpd: Drop unnecessary null-termination for fqdn
2023-10-25 07:24:04 -04:00
Russ White
7cdea4f5d0
Merge pull request #14645 from opensourcerouting/fix/crash_mp_reach_nlri
bgpd: A couple more bgpd crashes on malformed attributes
2023-10-25 07:21:25 -04:00
Donatas Abraitis
0df6464216 bgpd: Drop unnecessary null-termination for fqdn
str[len] is already null terminated before:

```
		if (len > BGP_MAX_HOSTNAME) {
			memcpy(&str, data, BGP_MAX_HOSTNAME);
			str[BGP_MAX_HOSTNAME] = '\0';
		} else if (len) {
			memcpy(&str, data, len);
			str[len] = '\0';
		}
```

CID: 1569357

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-25 07:52:38 +03:00
Russ White
f239b0f05a
Merge pull request #14639 from donaldsharp/zebra_nexthop_search_up
zebra: Allow longer prefix matches for nexthops
2023-10-24 16:13:20 -04:00
Donatas Abraitis
26373fbedd
Merge pull request #14333 from fdumontet6WIND/bgp4v2_snmp
bgpd: add support of traps for bgp4-mibv2
2023-10-24 21:34:47 +03:00
Francois Dumontet
ccef545122 bgpd: fix new Formatting / Styling warning
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-10-24 17:16:47 +02:00
Francois Dumontet
9d14c70b88 tests: add bgp4v2 MIB traps test
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-10-24 17:16:47 +02:00
Francois Dumontet
50c95b6be9 tests: add bgp4 MIB traps test
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-10-24 17:16:47 +02:00
Francois Dumontet
6261c2b994 tests: lib add support of snmptrapd daemon
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-10-24 17:16:47 +02:00
Francois Dumontet
7552dba274 doc: [no] bgp snmp traps <rfc4273|bgp4-mibv2>
update traps documentation

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
2023-10-24 17:16:47 +02:00
Francois Dumontet
e6de67f1e5 bgpd: add [no] bgp snmp traps bgp4-mibv2 command
There is no command to choose to send or not the bgp4-mibv2 traps.
Since the MIB bgp4-mibv2 notification are redundant with MIB RFC4273
we added a command:
- [no] bgp snmp traps bgp4-mibv2

By default, the bgp4-mibv2 traps will be disabled, to prevent from
redundancy.

Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-10-24 17:16:47 +02:00
Francois Dumontet
d78854f438 bgpd: add snmp traps for bgp4-mibv2
This commit add the support of traps for bgp4-mibv2.
It is conformant to draft-ietf-idr-bgp4-mibv2-11.
The following traps are supported:
- bgp4V2EstablishedNotification
- bgp4V2BackwardTransitionNotification

Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-10-24 17:16:47 +02:00
Francois Dumontet
83ca9a605e bgpd: add [no] bgp snmp traps rfc4273 command
There is no cli command to prevent the router to send traps
implemented in the rfc4273. If not done, when introducing
the traps from bgp4v2mib, traps will be send for each of
the two mibs: there will be redundancy in the sent information.

Add a new command:
- [no] bgp snmp traps rfc4273

Using this command will allow or not the notification of
the following traps:
- bgpEstablishedNotification
- bgpBackwardTransNotification

Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
2023-10-24 17:16:47 +02:00
Donatas Abraitis
d8482bf011 bgpd: Check mandatory attributes more carefully for UPDATE message
If we send a crafted BGP UPDATE message without mandatory attributes, we do
not check if the length of the path attributes is zero or not. We only check
if attr->flag is at least set or not. Imagine we send only unknown transit
attribute, then attr->flag is always 0. Also, this is true only if graceful-restart
capability is received.

A crash:

```
bgpd[7834]: [TJ23Y-GY0RH] 127.0.0.1 Unknown attribute is received (type 31, length 16)
bgpd[7834]: [PCFFM-WMARW] 127.0.0.1(donatas-pc) rcvd UPDATE wlen 0 attrlen 20 alen 17
BGP[7834]: Received signal 11 at 1698089639 (si_addr 0x0, PC 0x55eefd375b4a); aborting...
BGP[7834]: /usr/local/lib/libfrr.so.0(zlog_backtrace_sigsafe+0x6d) [0x7f3205ca939d]
BGP[7834]: /usr/local/lib/libfrr.so.0(zlog_signal+0xf3) [0x7f3205ca9593]
BGP[7834]: /usr/local/lib/libfrr.so.0(+0xf5181) [0x7f3205cdd181]
BGP[7834]: /lib/x86_64-linux-gnu/libpthread.so.0(+0x12980) [0x7f3204ff3980]
BGP[7834]: /usr/lib/frr/bgpd(+0x18ab4a) [0x55eefd375b4a]
BGP[7834]: /usr/local/lib/libfrr.so.0(route_map_apply_ext+0x310) [0x7f3205cd1290]
BGP[7834]: /usr/lib/frr/bgpd(+0x163610) [0x55eefd34e610]
BGP[7834]: /usr/lib/frr/bgpd(bgp_update+0x9a5) [0x55eefd35c1d5]
BGP[7834]: /usr/lib/frr/bgpd(bgp_nlri_parse_ip+0xb7) [0x55eefd35e867]
BGP[7834]: /usr/lib/frr/bgpd(+0x1555e6) [0x55eefd3405e6]
BGP[7834]: /usr/lib/frr/bgpd(bgp_process_packet+0x747) [0x55eefd345597]
BGP[7834]: /usr/local/lib/libfrr.so.0(event_call+0x83) [0x7f3205cef4a3]
BGP[7834]: /usr/local/lib/libfrr.so.0(frr_run+0xc0) [0x7f3205ca10a0]
BGP[7834]: /usr/lib/frr/bgpd(main+0x409) [0x55eefd2dc979]
```

Sending:

```
import socket
import time

OPEN = (b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
b"\xff\xff\x00\x62\x01\x04\xfd\xea\x00\x5a\x0a\x00\x00\x01\x45\x02"
b"\x06\x01\x04\x00\x01\x00\x01\x02\x02\x02\x00\x02\x02\x46\x00\x02"
b"\x06\x41\x04\x00\x00\xfd\xea\x02\x02\x06\x00\x02\x06\x45\x04\x00"
b"\x01\x01\x03\x02\x0e\x49\x0c\x0a\x64\x6f\x6e\x61\x74\x61\x73\x2d"
b"\x70\x63\x00\x02\x04\x40\x02\x00\x78\x02\x09\x47\x07\x00\x01\x01"
b"\x80\x00\x00\x00")

KEEPALIVE = (b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"
b"\xff\xff\xff\xff\xff\xff\x00\x13\x04")

UPDATE = bytearray.fromhex("ffffffffffffffffffffffffffffffff003c0200000014ff1f001000040146464646460004464646464646664646f50d05800100010200ffff000000")

s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect(('127.0.0.2', 179))
s.send(OPEN)
data = s.recv(1024)
s.send(KEEPALIVE)
data = s.recv(1024)
s.send(UPDATE)
data = s.recv(1024)
time.sleep(1000)
s.close()
```

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-24 15:22:52 +03:00
Donatas Abraitis
b08afc81c6 bgpd: Handle MP_REACH_NLRI malformed packets with session reset
Avoid crashing bgpd.

```
(gdb)
bgp_mp_reach_parse (args=<optimized out>, mp_update=0x7fffffffe140) at bgpd/bgp_attr.c:2341
2341			stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
(gdb)
stream_get (dst=0x7fffffffe1ac, s=0x7ffff0006e80, size=16) at lib/stream.c:320
320	{
(gdb)
321		STREAM_VERIFY_SANE(s);
(gdb)
323		if (STREAM_READABLE(s) < size) {
(gdb)
34	  return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
(gdb)

Thread 1 "bgpd" received signal SIGSEGV, Segmentation fault.
0x00005555556e37be in route_set_aspath_prepend (rule=0x555555aac0d0, prefix=0x7fffffffe050,
    object=0x7fffffffdb00) at bgpd/bgp_routemap.c:2282
2282		if (path->attr->aspath->refcnt)
(gdb)
```

With the configuration:

```
 neighbor 127.0.0.1 remote-as external
 neighbor 127.0.0.1 passive
 neighbor 127.0.0.1 ebgp-multihop
 neighbor 127.0.0.1 disable-connected-check
 neighbor 127.0.0.1 update-source 127.0.0.2
 neighbor 127.0.0.1 timers 3 90
 neighbor 127.0.0.1 timers connect 1
 address-family ipv4 unicast
  redistribute connected
  neighbor 127.0.0.1 default-originate
  neighbor 127.0.0.1 route-map RM_IN in
 exit-address-family
!
route-map RM_IN permit 10
 set as-path prepend 200
exit
```

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-24 15:22:52 +03:00
Russ White
b6b0001a4c
Merge pull request #14540 from opensourcerouting/feature/bgpd_handle_fqdn_capability_via_dynamic_capability
bgpd: Handle FQDN capability using dynamic capabilities
2023-10-24 06:23:32 -04:00
Russ White
27a78f80d8
Merge pull request #13979 from gpnaveen/bgp_unique_rid
tests: Adding a bgp router id chaos test case.
2023-10-24 06:09:58 -04:00
Donatas Abraitis
e8cdfa2761
Merge pull request #14629 from mjstapp/zebra_debug_netlink_ifname
zebra: debug ifname in netlink link debugs
2023-10-24 10:09:45 +03:00
Donatas Abraitis
614d7873d5
Merge pull request #14634 from LabNConsulting/chopps/gdb-use-emacs
tests: add --gdb-use-emacs option
2023-10-24 08:58:40 +03:00
Donald Sharp
a272a2b364 zebra: Allow longer prefix matches for nexthops
Zebra currently does a shortest prefix match for
resolving nexthops for a prefix.  This is typically
an ok thing to do but fails in several specific scenarios.
If a nexthop matches to a route that is not usable, nexthop
resolution just gives up and refuses to use that particular
route.  For example if zebra currently has a covering prefix
say a 10.0.0.0/8.  And about the same time it receives a
10.1.0.0/16 ( a more specific than the /8 ) and another
route A, who's nexthop is 10.1.1.1.  Imagine the 10.1.0.0/16
is processed enough to know we want to install it and the
prefix is sent to the dataplane for installation( it is queued )
and then route A is processed, nexthop resolution will fail
and the route A will be left in limbo as uninstallable.

Let's modify the nexthop resolution code in zebra such that
if a nexthop's most specific match is unusable, continue looking
up the table till we get to the 0.0.0.0/0 route( if it's even
installed ).  If we find a usable route for the nexthop accept
it and use it.

The bgp_default_originate topology test is frequently failing
with this exact problem:

B>* 0.0.0.0/0 [200/0] via 192.168.1.1, r2-r1-eth0, weight 1, 00:00:21
B   1.0.1.17/32 [200/0] via 192.168.0.1 inactive, weight 1, 00:00:21
B>* 1.0.2.17/32 [200/0] via 192.168.1.1, r2-r1-eth0, weight 1, 00:00:21
C>* 1.0.3.17/32 is directly connected, lo, 00:02:00
B>* 1.0.5.17/32 [20/0] via 192.168.2.2, r2-r3-eth1, weight 1, 00:00:32
B>* 192.168.0.0/24 [200/0] via 192.168.1.1, r2-r1-eth0, weight 1, 00:00:21
B   192.168.1.0/24 [200/0] via 192.168.1.1 inactive, weight 1, 00:00:21
C>* 192.168.1.0/24 is directly connected, r2-r1-eth0, 00:02:00
C>* 192.168.2.0/24 is directly connected, r2-r3-eth1, 00:02:00
B>* 192.168.3.0/24 [20/0] via 192.168.2.2, r2-r3-eth1, weight 1, 00:00:32
B   198.51.1.1/32 [200/0] via 192.168.0.1 inactive, weight 1, 00:00:21
B>* 198.51.1.2/32 [20/0] via 192.168.2.2, r2-r3-eth1, weight 1, 00:00:32

Notice that the 1.0.1.17/32 route is inactive but the nexthop
192.168.0.1 is covered by both the 192.168.0.0/24 prefix( shortest match )
*and* the 0.0.0.0/0 route ( longest match ).  When looking at the logs
the 1.0.1.17/32 route was not being installed because the matching
route was not in a usable state, which is because the 192.168.0.0/24
route was in the process of being installed.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-10-23 08:15:11 -04:00
Donald Sharp
01d84db046
Merge pull request #14628 from opensourcerouting/fix/bgpd_conditional_advertisement_static_routes_withdrawn
bgpd: Do not suppress conditional advertisement updates if triggered
2023-10-23 07:41:07 -04:00
Christian Hopps
a921202a85 tests: add --gdb-use-emacs option
When specified `--gdb-use-emacs` will launch the daemon with gdb inside a
running emacs server using `emacsclient --eval` commands.

Signed-off-by: Christian Hopps <chopps@labn.net>
2023-10-23 05:11:32 -04:00
Donatas Abraitis
571b403519
Merge pull request #14631 from idryzhov/nb-remove-comment
lib: remove incorrect comment from northbound
2023-10-22 11:21:40 +03:00
Igor Ryzhov
a041d3169b lib: remove incorrect comment from northbound
This was true when we had only a CLI for configuration. Now mgmtd has a
public frontend interface that can be used by external applications, and
they can send invalid requests that lead to errors.

This is still true for CLI though, so the same comment still stays in
`nb_cli_apply_changes_internal`.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
2023-10-21 13:09:00 +03:00
Mark Stapp
85dc2e85e0 zebra: debug ifname in netlink link debugs
Print the ifname with netlink LINK debug output.

Signed-off-by: Mark Stapp <mjs@labn.net>
2023-10-20 11:20:25 -04:00
Donatas Abraitis
3c94151258 tests: Check if BGP conditional advertisement works fine with static routes
If we modify the prefix-list that is used to define the routes to be
advertised, all of them MUST be advertised.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-20 12:58:33 +03:00
Donatas Abraitis
2d8e859585 bgpd: Do not suppress conditional advertisement updates if triggered
If we have a prefix-list with one entry, and after some time we append a prefix-list
with some more additional entries, conditional advertisement is triggered, and the
old entries are suppressed (because they look identical as sent before).

Hence, the old entries are sent as withdrawals and only new entries sent as updates.

Force re-sending all BGP updates for conditional advertisement. The same is done
for route-refresh, and/or soft clear operations.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-20 12:05:45 +03:00
Donatas Abraitis
49d1539a70 doc: Add a new command to resend dynamic capabilities
For now it includes only FQDN capability, because other capabilities can be
resend using specific knobs.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-20 09:36:33 +03:00
Donatas Abraitis
f90ea076da bgpd: Add clear bgp capabilities command to resend some dynamic capabilities
For instance, it's not possible to resend FQDN capability without resetting
the session, so let's create some more elegant way to do that.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-20 09:36:33 +03:00
Donatas Abraitis
03ee1cadd5 bgpd: Handle FQDN capability using dynamic capabilities
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-20 09:36:32 +03:00
Donald Sharp
627888864d
Merge pull request #14614 from opensourcerouting/feature/bgpd_handle_orf_capability_via_dynamic_capability
bgpd: Handle ORF capability using dynamic capabilities
2023-10-19 16:01:24 -04:00
Donatas Abraitis
2775d2263a
Merge pull request #14618 from donaldsharp/watchfrr_extend
watchfrr: Extend ignore option to daemon being killed
2023-10-19 18:48:37 +03:00
Donald Sharp
8f839353dc
Merge pull request #14615 from opensourcerouting/fix/rename_test_function_for_bgp_dynamic_capability
tests: Rename test_bgp_dynamic_capability_role
2023-10-19 08:15:15 -04:00
Donald Sharp
3f4bac66d8
Merge pull request #14616 from subsecond/patch-5
doc: add "enforce-first-as" to BGP documentation
2023-10-19 08:14:53 -04:00
Donald Sharp
c168244b99 watchfrr: Extend ignore option to daemon being killed
When testing GR features, it is desired to kill bgp
(or really any daemon )and not immediately have bgp start up again.
Modify the code to not attempt to restart the daemon
by hand to let us developers work when the `watchfrr ignore XXX`
command is issued.

Testing:
watchfrr ignore bgpd
kill -9 bgpd
start bgp by `/usr/lib/frr/watchfrr.sh start bgpd` at some point in time
in the future

leaf-1# show watchfrr
watchfrr global phase: Idle
 Restart Command: "/usr/lib/frr/watchfrr.sh restart %s"
 Start Command: "/usr/lib/frr/watchfrr.sh start %s"
 Stop Command: "/usr/lib/frr/watchfrr.sh stop %s"
 Min Restart Interval: 60
 Max Restart Interval: 600
 Restart Timeout: 90
  zebra                Up
  bgpd                 Up/Ignoring Timeout
  staticd              Up
leaf-1#

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-10-18 14:30:03 -04:00
Manuel Schweizer
3acc6ae932 doc: add "enforce-first-as" to BGP doc
With the deprecation of the global "bgp enforce-first-as" command back
in https://github.com/FRRouting/frr/pull/2259 the newly introduced
option to enable that setting on a specific peer was not documented.

This commit adds the necessary documentation and states the command's
default.

Signed-off-by: Manuel Schweizer <manuel.schweizer@cloudscale.ch>
2023-10-18 17:30:39 +02:00
Donatas Abraitis
2c0c11f3e8 bgpd: Handle ORF capability using dynamic capabilities
Add an ability to enable/disable ORF capability dynamically without tearing
down the session.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-18 16:56:02 +03:00
Donatas Abraitis
4b843e759b tests: Rename test_bgp_dynamic_capability_role
Was copied, but forgot to rename accordingly.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-18 12:31:22 +03:00
Donatas Abraitis
a681f525b9
Merge pull request #14607 from mobash-rasool/fixes2
pim6d: valgrind issue fixes
2023-10-17 17:34:11 +03:00
Donatas Abraitis
6ece98ecc1 bgpd: Reuse orf_type_str/orf_mode_str for dynamic capabilities code
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-17 16:01:00 +03:00
Donatas Abraitis
1fb08e91d7 tests: Check if ORF capability works with BGP dynamic capabilities
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-17 16:00:00 +03:00
Mobashshera Rasool
1064818645 pim6d: valgrind issue fixes
Problem Statement:
===================
Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
at 0x4975157: sendmsg (sendmsg.c:28)
==2263111==    by 0x1413BE: pim_msg_send_frame (pim_pim.c:629)
==2263111==    by 0x1413BE: pim_msg_send (pim_pim.c:743)
==2263111==    by 0x1425DC: pim_register_send (pim_register.c:332)
==2263111==    by 0x1427EE: pim_null_register_send (pim_register.c:443)
==2263111==    by 0x14D228: pim_upstream_register_stop_timer (pim_upstream.c:1608)
==2263111==    by 0x48CE6DF: thread_call (thread.c:1693)
==2263111==    by 0x4899EFF: frr_run (libfrr.c:1068)
==2263111==    by 0x11D035: main (pim6_main.c:190)
==2263111==  Address 0x1ffeffdcb1 is on thread 1's stack
==2263111==  in frame #2, created by pim_register_send (pim_register.c:273)
==2263111==  Uninitialised value was created by a stack allocation
==2263111==    at 0x142690: pim_null_register_send (pim_register.c:389)

RCA:
====================
1. All members of struct pim_msg_header were not initiliased while sending
null register packet. Therefore when the pointers are assigned while
sending the msg via sendmsg, it complains the pointer points to
uninitialised byte.
2. struct ipv6_ph ph was also not initialised.

Fix:
====================
Initialised all the members using memset.

Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>
2023-10-16 21:44:32 -07:00
Donald Sharp
c8d568487c
Merge pull request #14599 from opensourcerouting/fix/issue_14419
tests: Check if evpn route-map match by route type works
2023-10-16 10:20:23 -04:00
Donatas Abraitis
c7a9af861a tests: Check if evpn route-map match by route type works
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
2023-10-15 19:46:34 +03:00
Donatas Abraitis
c97c449e1f
Merge pull request #14585 from donaldsharp/send_capability
ldpd: Clarify error situation for different problems
2023-10-14 20:22:37 +03:00
Donald Sharp
50e6ba26a4
Merge pull request #14582 from cloudscale-ch/denis/topotest-for-14488
tests: Add OSPF test for issue 14488
2023-10-14 09:42:49 -04:00
Donald Sharp
914c45c8e8
Merge pull request #14590 from opensourcerouting/ospf6d-fwd-addr
ospf6d: fix setting of the forwarding address in AS-External LSAs
2023-10-13 16:27:30 -04:00
Donatas Abraitis
0a8ce5f3f3
Merge pull request #14553 from donaldsharp/zebra_weighted_ecmp
Zebra weighted ecmp
2023-10-13 23:09:18 +03:00
Donald Sharp
f024e4db71 ldpd: Clarify error situation for different problems
Clarify the fatal error message recorded when an error situation
happens.  Disambiguating the default case from the TLV_TYPE_DYNAMIC_CAP
case.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
2023-10-13 13:48:43 -04:00
Donald Sharp
02cbd97801
Merge pull request #14561 from idryzhov/implicit-fallthrough
build: add -Wimplicit-fallthrough
2023-10-13 11:51:11 -04:00