summaryrefslogtreecommitdiffstats
path: root/bgpd/bgp_packet.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* bgpd: Avoid padding for bgp_paths_limit_capability structDonatas Abraitis2024-03-141-3/+4
| | | | | | | 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>
* bgpd: Allow dynamically disable graceful-restart/long-lived graceful-restartDonatas Abraitis2024-03-101-7/+0
| | | | | | If we enter `bgp graceful-restart-disable`, make sure we disable the capabilities. Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* bgpd: Unset advertised capabilities if capability is disabledDonatas Abraitis2024-03-091-45/+47
| | | | | | | | 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>
* bgpd: Send "Send Hold Timer Expired" on such events notificationDonatas Abraitis2024-02-291-1/+2
| | | | | | | | | | | | 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>
* bgpd: Implement Paths-Limit capabilityDonatas Abraitis2024-02-131-0/+126
| | | | | | https://datatracker.ietf.org/doc/html/draft-abraitis-idr-addpath-paths-limit Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* bgpd: Do not send dynamic capability if both peers do not have it exchangedDonatas Abraitis2024-02-131-1/+1
| | | | Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* bgpd: Send FQDN capability via dynamic capability if enabledDonatas Abraitis2024-02-051-1/+2
| | | | | | | 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>
* bgpd: data is set but never usedDonald Sharp2024-01-091-1/+1
| | | | | | | | | 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>
* bgpd: Add `debug bgp updates detail` commandDonatas Abraitis2024-01-071-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* bgpd: Validate Addpath capability flags per AFDonatas Abraitis2023-12-171-2/+14
| | | | | | | | | | | | | 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>
* bgpd: Do not null-terminate the domainname when receiving FQDN capabilityDonatas Abraitis2023-12-011-2/+0
| | | | | | | 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>
* bgpd: Ignore handling NLRIs if we received MP_UNREACH_NLRIDonatas Abraitis2023-10-311-1/+6
| | | | | | | | | | | | | | 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>
* Merge pull request #14651 from ↵Russ White2023-10-251-2/+0
|\ | | | | | | | | opensourcerouting/fix/bgpd_coverity_fqdn_capability bgpd: Drop unnecessary null-termination for fqdn
| * bgpd: Drop unnecessary null-termination for fqdnDonatas Abraitis2023-10-251-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | Merge pull request #14645 from opensourcerouting/fix/crash_mp_reach_nlriRuss White2023-10-251-5/+1
|\ \ | |/ |/| bgpd: A couple more bgpd crashes on malformed attributes
| * bgpd: Handle MP_REACH_NLRI malformed packets with session resetDonatas Abraitis2023-10-241-5/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | bgpd: Handle FQDN capability using dynamic capabilitiesDonatas Abraitis2023-10-201-2/+126
|/ | | | Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* bgpd: Handle ORF capability using dynamic capabilitiesDonatas Abraitis2023-10-181-2/+193
| | | | | | | Add an ability to enable/disable ORF capability dynamically without tearing down the session. Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* Merge pull request #14528 from ↵Russ White2023-10-111-2/+177
|\ | | | | | | | | opensourcerouting/feature/bgpd_handle_addpath_capability_via_dynamic_capability bgpd: Handle Addpath capability using dynamic capabilities
| * bgpd: Handle Addpath capability using dynamic capabilitiesDonatas Abraitis2023-10-031-2/+177
| | | | | | | | | | | | | | 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>
* | Revert "bgpd: accept bgp link-state capability"Donald Sharp2023-10-101-2/+0
| | | | | | | | This reverts commit 67fe40676eb4e2ca78a41ddd70887af09b29fd9d.
* | Revert "bgpd: store bgp link-state prefixes"Donald Sharp2023-10-101-5/+0
|/ | | | This reverts commit 39a8d354c11f6f063fa5154f5807e7a0c9b04b46.
* bgpd: Validate maximum length of software version when handling via dynamic capsDonatas Abraitis2023-09-291-17/+36
| | | | | | | 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>
* Merge pull request #12649 from louis-6wind/bgp-link-stateRuss White2023-09-261-0/+7
|\ | | | | bgpd: add basic support of BGP Link-State RFC7752
| * bgpd: store bgp link-state prefixesLouis Scalbert2023-09-181-0/+5
| | | | | | | | | | | | | | | | | | 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>
| * bgpd: accept bgp link-state capabilityLouis Scalbert2023-09-181-0/+2
| | | | | | | | | | | | | | | | 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>
* | bgpd: Flush per AFI/SAFI capabilities flags, stale_time for LLGR capDonatas Abraitis2023-09-221-0/+11
| | | | | | | | | | | | Clear to defaults if receiving dynamic capability with UNSET action. Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* | bgpd: Clear graceful-restart per AFI/SAFI capability flags when receiving unsetDonatas Abraitis2023-09-221-0/+11
| | | | | | | | | | | | We flushed the main capability received flag, but missed flushing per AFI/SAFI. Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* | bgpd: Handle LLGR capability using dynamic capabilitiesDonatas Abraitis2023-09-131-13/+131
|/ | | | | | | | | | | 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>
* bgpd: First pass of BGP_EVENT_ADDDonald Sharp2023-09-101-18/+20
| | | | | | | | | | | 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>
* bgpd: bgp_packet pass connection aroundDonald Sharp2023-09-101-60/+68
| | | | | | | | 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>
* bgpd: bgp_fsm_change_status/BGP_TIMER_ON and BGP_EVENT_ADDDonald Sharp2023-09-101-14/+14
| | | | | | | | | 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>
* bgpd: peer_established should be connection orientedDonald Sharp2023-09-101-11/+12
| | | | | | The peer_established function should be connection oriented. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bgpd: bgp_open_send is connection oriented not peer orientedDonald Sharp2023-09-101-2/+3
| | | | | | | The bgp_open_send function should use a connection oriented pointer for it's basis. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bgpd: make bgp_timer_set use a peer_connection insteadDonald Sharp2023-09-101-1/+1
| | | | | | | The bgp_timer_set function should use a peer_connection pointer instead. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bgpd: bgp_notify_send use peer_connection instead of peerDonald Sharp2023-09-091-55/+67
| | | | | | The bgp_notify_send function should use a peer_connection Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bgpd: move t_generate_updgrp_packets into peer_connectionDonald Sharp2023-09-091-3/+4
| | | | | | | The t_generate_updgrp_packets event pointer belongs in the peer_connection pointer. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bgpd: move t_routeadv to peer_connectionDonald Sharp2023-09-091-1/+1
| | | | | | The t_routeadv belongs to the peer_connection data structure Signed-off-by: Donald Sharp <sharpd@nvidia.com>
* bgpd: Print a hostname also for GR logs under dynamic capabilityDonatas Abraitis2023-08-301-8/+6
| | | | | | Just to be consistent with other zlog_ stuff for dynamic capabilities. Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* bgpd: Make sure we have enough data to read restart time and flags for GR capDonatas Abraitis2023-08-301-0/+7
| | | | | | Just a safety check to avoid out of bound reading. Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* bgpd: Handle Graceful-Restart capability with dynamic capabilityDonatas Abraitis2023-08-301-2/+151
| | | | | | | | | | | 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>
* Merge pull request #14300 from ↵Donald Sharp2023-08-301-0/+1
|\ | | | | | | | | opensourcerouting/fix/set_role_as_undefined_when_capability_unset bgpd: Unset role when receiving UNSET action for dynamic capability
| * bgpd: Unset role when receiving UNSET action for dynamic capabilityDonatas Abraitis2023-08-301-0/+1
| | | | | | | | | | | | | | | | 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>
* | bgpd: Use zlog_err and not zlog_info when we have an error for dynamic ↵Donatas Abraitis2023-08-291-31/+29
|/ | | | | | | | capability Also change the outputs a bit to be consistent and more detailed. Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
* bgpd: Handle Role capability via dynamic capabilities for SET/UNSET properlyDonatas Abraitis2023-08-291-3/+8
| | | | | | | | 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>
* bgpd: Do not process NLRIs if the attribute length is zeroDonatas Abraitis2023-08-221-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ``` 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>
* Merge pull request #8790 from donaldsharp/peer_connectionDonatas Abraitis2023-08-211-50/+56
|\ | | | | Peer connection
| * bgpd: Convert `struct peer_connection` to dynamically allocatedDonald Sharp2023-08-181-32/+31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * bgpd: Move t_process_packet and t_process_packet_error to connectionDonald Sharp2023-08-181-1/+1
| | | | | | | | | | | | The t_process_packet thread events should be managed by the connection. Signed-off-by: Donald Sharp <sharpd@nvidia.com>
| * bgpd: Move status and ostatus to `struct peer_connection`Donald Sharp2023-08-181-18/+21
| | | | | | | | | | | | | | 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>