| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
| |
When sending the packets over the network (dynamic capability) it reports 6 bytes
instead of 5 bytes, and causes some issues between little/big endian machines.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
| |
If we enter `bgp graceful-restart-disable`, make sure we disable the capabilities.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
| |
When using dynamic capabilities, do not forget to unset advertised capabilities.
Otherwise, it's kept as advertised.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is required by the current (latest/-02 draft).
IANA has registered code 8 for "Send Hold Timer Expired" in the "BGP
Error (Notification) Codes" sub-registry under the "Border Gateway
Protocol (BGP) Parameters" registry.
https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-sendholdtimer
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
| |
https://datatracker.ietf.org/doc/html/draft-abraitis-idr-addpath-paths-limit
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
| |
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
| |
Since we have a knob to disable sending FQDN capability, it MUST be checked
before sending it using dynamic capabilities.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
| |
I've kept the assignment in a comment because I am concerned
about new code being added later that the data pointer would
not be set correctly. Next coder can see the commented
out line and uncomment it.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When filtering with `debug bgp updates in x.x.x.x prefix-list plist`, we want
to filter out unnecessary messages like:
```
127.0.0.1(Unknown) rcvd UPDATE wlen 0 attrlen 20 alen 5
```
Such a line as above will be repeated for all the paths received and it's useless
without knowing the prefix (because NLRIs are not parsed yet).
But want to see only relevant ones:
```
127.0.0.1(Unknown) rcvd UPDATE w/ attr: nexthop 127.0.0.1, origin i, path 65002
127.0.0.1(Unknown) rcvd 10.255.255.1/32 IPv4 unicast
```
With `debug bgp updates detail` we can combine this to something like:
```
127.0.0.1(Unknown) rcvd UPDATE w/ attr: nexthop 127.0.0.1, origin i, path 65002
127.0.0.1(Unknown) rcvd UPDATE wlen 0 attrlen 20 alen 5
127.0.0.1(Unknown) rcvd 10.255.255.1/32 IPv4 unicast
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Send/Receive:
This field indicates whether the sender is (a) able to receive
multiple paths from its peer (value 1), (b) able to send
multiple paths to its peer (value 2), or (c) both (value 3) for
the <AFI, SAFI>.
If any other value is received, then the capability SHOULD be
treated as not understood and ignored [RFC5492].
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
| |
This is already handled above, no need to do here, because we could have an
overrun situation where len > 64 and we do out-of-bound actions.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If we receive MP_UNREACH_NLRI, we should stop handling remaining NLRIs if
no mandatory path attributes received.
In other words, if MP_UNREACH_NLRI received, the remaining NLRIs should be handled
as a new data, but without mandatory attributes, it's a malformed packet.
In normal case, this MUST not happen at all, but to avoid crashing bgpd, we MUST
handle that.
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|\
| |
| |
| |
| | |
opensourcerouting/fix/bgpd_coverity_fqdn_capability
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>
|
|\ \
| |/
|/| |
bgpd: A couple more bgpd crashes on malformed attributes
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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>
|
|/
|
|
| |
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
| |
Add an ability to enable/disable ORF capability dynamically without tearing
down the session.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|\
| |
| |
| |
| | |
opensourcerouting/feature/bgpd_handle_addpath_capability_via_dynamic_capability
bgpd: Handle Addpath capability using dynamic capabilities
|
| |
| |
| |
| |
| |
| |
| | |
Changing Addpath type, and or disabling RX (receiving) flag, we can do this
without tearing down the session, and using dynamic capabilities.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
| |
| |
| |
| | |
This reverts commit 67fe40676eb4e2ca78a41ddd70887af09b29fd9d.
|
|/
|
|
| |
This reverts commit 39a8d354c11f6f063fa5154f5807e7a0c9b04b46.
|
|
|
|
|
|
|
| |
We should not allow exceeding the stream's length, and also software version
can't be larger than 64 bytes.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|\
| |
| | |
bgpd: add basic support of BGP Link-State RFC7752
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Add the ability to store link-state prefixes in the BGP table.
Store a raw copy of the BGP link state NLRI TLVs as received in the
packet in 'p.u.prefix_linkstate.ptr'.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
|
| |
| |
| |
| |
| |
| |
| |
| | |
Accept the BGP Link-State AFI/SAFI capability when received from a peer
OPEN message.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
|
| |
| |
| |
| |
| |
| | |
Clear to defaults if receiving dynamic capability with UNSET action.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
| |
| |
| |
| |
| |
| | |
We flushed the main capability received flag, but missed flushing per AFI/SAFI.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|/
|
|
|
|
|
|
|
|
|
| |
LLGR stale time is exchanged using OPEN messages. In order to
reduce stal time before doing an actual graceful restart + LLGR, it might be useful
to increase the time, but this is not possible without resetting the session.
With this change, it's possible to send dynamic capability with a new value, and
GR will respect a new reset time value when LLGR kicks in.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
| |
Pass through a bunch of BGP_EVENT_ADD's and make
the code use a proper connection instead of a
peer->connection. There still are a bunch
of places where peer->connection is used and
later commits will probably go through and
clean these up more.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
|
|
| |
Modify all the receive functions to pass around the actual
connection being acted upon. Modify the collision detection
function to look at the possible two connections.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
|
|
|
| |
Modify bgp_fsm_change_status to be connection oriented and
also make the BGP_TIMER_ON and BGP_EVENT_ADD macros connection
oriented as well. Attempt to make peer_xfer_conn a bit more
understandable because, frankly it was/is confusing.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
| |
The peer_established function should be connection oriented.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
|
| |
The bgp_open_send function should use a connection oriented
pointer for it's basis.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
|
| |
The bgp_timer_set function should use a peer_connection pointer
instead.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
| |
The bgp_notify_send function should use a peer_connection
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
|
| |
The t_generate_updgrp_packets event pointer belongs in the
peer_connection pointer.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
| |
The t_routeadv belongs to the peer_connection data structure
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
|
|
|
|
|
| |
Just to be consistent with other zlog_ stuff for dynamic capabilities.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
| |
Just a safety check to avoid out of bound reading.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
| |
Graceful-Restart restart time is exchanged using OPEN messages. In order to
reduce restart time before doing an actual graceful restart, it might be useful
to increase the time, but this is not possible without resetting the session.
With this change, it's possible to send dynamic capability with a new value, and
GR will respect a new reset time value.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|\
| |
| |
| |
| | |
opensourcerouting/fix/set_role_as_undefined_when_capability_unset
bgpd: Unset role when receiving UNSET action for dynamic capability
|
| |
| |
| |
| |
| |
| |
| |
| | |
Capability was unset, but forgot to unset the role.
Fixes: 5ad080d37a26d72b56ecd0b796593bb7fc3aa6ad ("bgpd: Handle Role capability via dynamic capabilities for SET/UNSET properly")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|/
|
|
|
|
|
|
| |
capability
Also change the outputs a bit to be consistent and more detailed.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
| |
It was missed to handle UNSET Role capability using dynamic capabilities.
Also move length check before actually handling Role capability.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
```
3 0x00007f423aa42476 in __GI_raise (sig=sig@entry=11) at ../sysdeps/posix/raise.c:26
4 0x00007f423aef9740 in core_handler (signo=11, siginfo=0x7fffc414deb0, context=<optimized out>) at lib/sigevent.c:246
5 <signal handler called>
6 0x0000564dea2fc71e in route_set_aspath_prepend (rule=0x564debd66d50, prefix=0x7fffc414ea30, object=0x7fffc414e400)
at bgpd/bgp_routemap.c:2258
7 0x00007f423aeec7e0 in route_map_apply_ext (map=<optimized out>, prefix=prefix@entry=0x7fffc414ea30,
match_object=match_object@entry=0x7fffc414e400, set_object=set_object@entry=0x7fffc414e400, pref=pref@entry=0x0) at lib/routemap.c:2690
8 0x0000564dea2d277e in bgp_input_modifier (peer=peer@entry=0x7f4238f59010, p=p@entry=0x7fffc414ea30, attr=attr@entry=0x7fffc414e770,
afi=afi@entry=AFI_IP, safi=safi@entry=SAFI_UNICAST, rmap_name=rmap_name@entry=0x0, label=0x0, num_labels=0, dest=0x564debdd5130)
at bgpd/bgp_route.c:1772
9 0x0000564dea2df762 in bgp_update (peer=peer@entry=0x7f4238f59010, p=p@entry=0x7fffc414ea30, addpath_id=addpath_id@entry=0,
attr=0x7fffc414eb50, afi=afi@entry=AFI_IP, safi=<optimized out>, safi@entry=SAFI_UNICAST, type=9, sub_type=0, prd=0x0, label=0x0,
num_labels=0, soft_reconfig=0, evpn=0x0) at bgpd/bgp_route.c:4374
10 0x0000564dea2e2047 in bgp_nlri_parse_ip (peer=0x7f4238f59010, attr=attr@entry=0x7fffc414eb50, packet=0x7fffc414eaf0)
at bgpd/bgp_route.c:6249
11 0x0000564dea2c5a58 in bgp_nlri_parse (peer=peer@entry=0x7f4238f59010, attr=attr@entry=0x7fffc414eb50,
packet=packet@entry=0x7fffc414eaf0, mp_withdraw=mp_withdraw@entry=false) at bgpd/bgp_packet.c:339
12 0x0000564dea2c5d66 in bgp_update_receive (peer=peer@entry=0x7f4238f59010, size=size@entry=109) at bgpd/bgp_packet.c:2024
13 0x0000564dea2c901d in bgp_process_packet (thread=<optimized out>) at bgpd/bgp_packet.c:2933
14 0x00007f423af0bf71 in event_call (thread=thread@entry=0x7fffc414ee40) at lib/event.c:1995
15 0x00007f423aebb198 in frr_run (master=0x564deb73c670) at lib/libfrr.c:1213
16 0x0000564dea261b83 in main (argc=<optimized out>, argv=<optimized out>) at bgpd/bgp_main.c:505
```
With the configuration:
```
frr version 9.1-dev-MyOwnFRRVersion
frr defaults traditional
hostname ip-172-31-13-140
log file /tmp/debug.log
log syslog
service integrated-vtysh-config
!
debug bgp keepalives
debug bgp neighbor-events
debug bgp updates in
debug bgp updates out
!
router bgp 100
bgp router-id 9.9.9.9
no bgp ebgp-requires-policy
bgp bestpath aigp
neighbor 172.31.2.47 remote-as 200
!
address-family ipv4 unicast
neighbor 172.31.2.47 default-originate
neighbor 172.31.2.47 route-map RM_IN in
exit-address-family
exit
!
route-map RM_IN permit 10
set as-path prepend 200
exit
!
```
The issue is that we try to process NLRIs even if the attribute length is 0.
Later bgp_update() will handle route-maps and a crash occurs because all the
attributes are NULL, including aspath, where we dereference.
According to the RFC 4271:
A value of 0 indicates that neither the Network Layer
Reachability Information field nor the Path Attribute field is
present in this UPDATE message.
But with a fuzzed UPDATE message this can be faked. I think it's reasonable
to skip processing NLRIs if both update_len and attribute_len are 0.
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
|
|\
| |
| | |
Peer connection
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
As part of the conversion to a `struct peer_connection` it will
be desirable to have 2 pointers one for when we open a connection
and one for when we receive a connection. Start this actual
conversion over to this in `struct peer`. If this sounds confusing
take a look at the bgp state machine for connections and how
it resolves the processing of this router opening -vs- this
router receiving an open. At some point in time the state
machine decides that we are keeping one of the two connections.
Future commits will allow us to untangle the peer/doppelganger
duality with this abstraction.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
| |
| |
| |
| |
| |
| | |
The t_process_packet thread events should be managed by the connection.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|
| |
| |
| |
| |
| |
| |
| | |
The status and ostatus are a function of the `struct peer_connection`
move it into that data structure.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
|