Link-state prefixes are only intended to be read for a link-state
consumer (i.e. a controler). They cannot be installed in Forwarding
Information Base (FIB).
Do not announce them to zebra.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
BGP link-state prefixes are displayed in the form of NLRI-TYPE /
Prefix-Length.
> r2# show bgp all
>
> For address family: Link State
> BGP table version is 8, local router ID is 192.0.2.2, vrf id 0
> Default local pref 100, local AS 65002
> Network Next Hop Metric LocPrf Weight Path
> *> Link/153 0 65001 i
> *> IPv6-Prefix/77 0 65001 i
> *> IPv4-Prefix/57 0 65001 i
> *> Node/49 0 65001 i
> *> Node/45 0 65001 i
Add a lib prefix display hook in bgpd to display properly all the details.
> r2# show bgp all
>
> For address family: Link State
> BGP table version is 8, local router ID is 192.0.2.2, vrf id 0
> Default local pref 100, local AS 65002
> Network Next Hop Metric LocPrf Weight Path
> *> Link OSPFv3 ID:0xffffffffffffffff {Local {AS:4294967295 ID:4294967295 Area:4294967295 Rtr:10.10.10.11:2.2.2.2} Remote {AS:4294967295 ID:4294967295 Area:4294967295 Rtr:10.10.10.10:1.1.1.1} IPv4:10.1.0.1 Neigh-IPv4:10.1.0.2 IPv6:2001::1 Neigh-IPv6:2001::2 MT:0,2}/153
> 0 65001 i
> *> IPv6-Prefix OSPFv3 ID:0x20 {Local {AS:65001 ID:0 Area:0 Rtr:10.10.10.10} MT:2 OSPF-Route-Type:1 IPv6:12:12::12:12/128}/77
> 0 65001 i
> *> IPv4-Prefix OSPFv2 ID:0x20 {Local {AS:65001 ID:0 Area:0 Rtr:10.10.10.10:1.1.1.1} IPv4:89.10.11.0/24}/57
> 0 65001 i
> *> Node OSPFv2 ID:0x20 {Local {AS:65001 ID:0 Area:0 Rtr:10.10.10.10:1.1.1.1}}/49
> 0 65001 i
> *> Node OSPFv2 ID:0x20 {Local {AS:65001 ID:0 Area:0 Rtr:10.10.10.10}}/45
> 0 65001 i
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
When displaying the link-state prefixes with "show bgp link-state
link-state" command, the following output headers are not needed:
> 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
> RPKI validation codes: V valid, I invalid, N Not found
Do not display these headers for link-state SAFI.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
BGP Link-State prefixes are special prefixes that contains a lot of
data.
Extend the length of the prefix string buffer in order to display
properly this type of prefixes with the next commits.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Add the ability to send link-state prefixes that are in the BGP table.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
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>
To show the TCP MSS value per neighbor you have to configure it, otherwise you
don't see the actual value.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
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>
following crash occurs:
at ./nptl/pthread_kill.c:44
at ./nptl/pthread_kill.c:78
at ./nptl/pthread_kill.c:89
context=0x7ffd06d3d300)
at /build/make-pkg/output/_packages/cp-routing/src/lib/sigevent.c:246
length=0x7ffd06d3da88, exact=1, var_len=0x7ffd06d3da90, write_method=<optimized out>)
at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_snmp_bgp4v2.c:364
vp=vp@entry=0x7f7c88b584c0 <bgpv2_variables>, vp_len=vp_len@entry=102,
ename=ename@entry=0x7f7c88b58440 <bgpv2_trap_oid>, enamelen=enamelen@entry=8,
name=name@entry=0x7f7c88b58480 <bgpv2_oid>, namelen=namelen@entry=7,
iname=0x7ffd06d3e7b0, index_len=1, trapobj=0x7f7c88b53b80 <bgpv2TrapBackListv6>,
trapobjlen=6, sptrap=2 '\002')
at /build/make-pkg/output/_packages/cp-routing/src/lib/agentx.c:382
vp_len=vp_len@entry=102, ename=ename@entry=0x7f7c88b58440 <bgpv2_trap_oid>,
enamelen=enamelen@entry=8, name=name@entry=0x7f7c88b58480 <bgpv2_oid>,
namelen=namelen@entry=7, iname=0x7ffd06d3ec30, inamelen=16,
trapobj=0x7f7c88b53b80 <bgpv2TrapBackListv6>, trapobjlen=6, sptrap=2 '\002')
at /build/make-pkg/output/_packages/cp-routing/src/lib/agentx.c:298
at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_snmp_bgp4v2.c:1496
at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_fsm.c:48
at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_fsm.c:1314
event=Receive_NOTIFICATION_message)
at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_fsm.c:2665
at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_packet.c:3129
at /build/make-pkg/output/_packages/cp-routing/src/lib/event.c:1979
at /build/make-pkg/output/_packages/cp-routing/src/lib/libfrr.c:1213
at /build/make-pkg/output/_packages/cp-routing/src/bgpd/bgp_main.c:510
it's due to function bgpv2PeerErrorsTable returning
return SNMP_STRING(msg_str);
with msg_str NULL rather the string ""
this commit avoid the issue.
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
If we modify as-path with route-map and prepend with private ASNs, then we
advertise a new as-path without stripping private ASNs. Let's fix this, and
remove private ASNs despite if they were sent by the origin or prepended locally.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Not sure why this is needed, because it's reset on bgp_connect_success(),
when the session is UP.
When the session is reset, it clears those variables, and we are not able to
see what remote address was before, etc.
hostLocal, hostRemote reports Unknown for `show bgp neighbor json`.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The 'redistribute table' command can be used by configuration on a
non default BGP instance, but this command does not work for multiple
reasons:
- The route entries configured on a given table are always configured
from the default vrf. This constraint prevents from redistributing a
prefix from the default vrf to an other non default bgp instance.
- The importation of route entries requires 'ip import-table' on vrfs
and this command is not available
Fix this by preventing from configuring this kind of redistribution
on non default bgp instances.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
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>
The BGP "no retain" VPN option avoids storing VPN prefixes that are not
imported in the incoming BGP table (aka. Adj RIB in). When a VPN import
policy is changed, BGP does a soft clear so that a prefix refresh is
requested from the peers. However, the import from local VPN prefixes
is never requested.
Fix this issue by requesting a local import refresh.
Fixes: a486300b26 ("bgpd: implement retain route-target all behaviour")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
https://www.rfc-editor.org/rfc/rfc2042.html
says: 255 reserved for development
In FRR, 255 is kinda used too BGP_ATTR_VNC, even more we allow setting 255 in CLI.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The pdest pointer is locked by the bgp_node_get so
unlocking it should be fine and it should still exist.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
dest could be freed by the first unlock, but should
not be due to our locking structure. Ensure coverity
understands this.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Unsetting a flag after the dest has been possibly been
freed is not a good thing to do. Ensure that this
is not possible.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This is incase it has been freed ( it wont due to locking )
and then we need to ensure that we can continue to use
the pointer.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Again coverity believes that dest could be freed by a call
into bgp_dest_unlock_node, and it can if the lock count
is wrong. Let's fix that assumption for coverity
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The bgp_cleanup_routes function holds the lock for dest
while walking it. Ensure that coverity understands this
proposition.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Again the dest pointer should be still locked by the table walk. Ensure
that coverity is happy that this is not happening.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Again coverity believes that dest may be freed but it should not
be because of how locking is done. Make coverity happy.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The dest pointer may be freed( but should not be
due to locking ). Let's ensure that this assumption
is true and make coverity happy.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
There exist two spots in this function where the dest could be
freed, but is not due to locking, but coverity thinks it might
so let's make the function happy.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
But never really does due to locking, but since it can
we need to treat it like it does and ensure that FRR
is not making a mistake, by using memory after it
has been freed.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
dest will not be freed due to lock but coverity does not know
that. Give it a hint. This change includes modifying bgp_dest_unlock_node
to return the dest pointer so that we can determine if we should
continue working on dest or not with an assert. Since this
is lock based we should be ok.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
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 usage of BGP_EVENT_FLUSH is unnecessarily abstracting the
call into event_cancel_event_ready and in addtion the macro
was not always being used! Just convert to using the actual
event_cancel_event_ready function directly.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Currently evpn passes into bgp_path_info_cmp the pfx_buf
uninitialized. The bgp_path_info_cmp functionality actually
expects this value to be initialized. Additionally the
evpn section of bgp_path_info_comp was resetting the
new_buf and exist_buf values that were already being
set above to the same values if !debug was on( which
precluded it ever from happening )
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
rfc7606 defines:
Attributes 17 (AS4_PATH), 18 (AS4_AGGREGATOR), 22 (PMSI_TUNNEL), 23 (Tunnel
Encapsulation Attribute), 26 (AIGP), 27 (PE Distinguisher Labels),
and 29 (BGP-LS Attribute) do have error handling consistent with
Section 8 and thus are not further discussed herein.
Section 8 defines:
The "treat-as-withdraw" approach is generally
preferred and the "session reset" approach is discouraged.
For any malformed attribute that is handled by the "attribute
discard" instead of the "treat-as-withdraw" approach, it is critical
to consider the potential impact of doing so.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
https://datatracker.ietf.org/doc/html/rfc6514#page-10 states:
A router that supports the PMSI Tunnel attribute considers this
attribute to be malformed if either (a) it contains an undefined
tunnel type in the Tunnel Type field of the attribute, or (b) the
router cannot parse the Tunnel Identifier field of the attribute as a
tunnel identifier of the tunnel types specified in the Tunnel Type
field of the attribute.
When a router that receives a BGP Update that contains the PMSI
Tunnel attribute with its Partial bit set determines that the
attribute is malformed, the router SHOULD treat this Update as though
all the routes contained in this Update had been withdrawn.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The command "show bgp all rpki notfound" includes not only RPKI
notfound routes but also RPKI valid and invalid routes in its results.
Fix the code to display only RPKI notfound routes.
Old output:
```
frr# show bgp all rpki notfound
For address family: IPv4 Unicast
BGP table version is 0, local router ID is 10.0.0.1, vrf id 0
Default local pref 100, local AS 64512
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
RPKI validation codes: V valid, I invalid, N Not found
Network Next Hop Metric LocPrf Weight Path
N x.x.x.0/18 a.a.a.a 100 0 64513 i
V y.y.y.0/19 a.a.a.a 200 0 64513 i
I z.z.z.0/16 a.a.a.a 10 0 64513 i
Displayed 3 routes and 3 total paths
```
New output:
```
frr# show bgp all rpki notfound
For address family: IPv4 Unicast
BGP table version is 0, local router ID is 10.0.0.1, vrf id 0
Default local pref 100, local AS 64512
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
RPKI validation codes: V valid, I invalid, N Not found
Network Next Hop Metric LocPrf Weight Path
N x.x.x.0/18 a.a.a.a 100 0 64513 i
Displayed 1 routes and 3 total paths
```
Signed-off-by: Ryo Nakano <ryo.z.nakano@gmail.com>
It was noticed that occassionally peering failed in a testbed
upon investigation it was found that the peer was not in the
peer hash and we saw these failure messages:
Aug 25 21:31:15 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: %NOTIFICATION: sent to neighbor 2001:cafe:1ead:4::4 4/0 (Hold Timer Expired) 0 bytes
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] Can't get remote address and port: Transport endpoint is not connected
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] %bgp_getsockname() failed for peer 2001:cafe:1ead:4::4 fd 27 (from_peer fd -1)
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 33554464] %Neighbor failed in xfer_conn
root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr# vtysh -c 'show bgp peerhash' | grep 2001:cafe:1ead:4::4
root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr#
Upon looking at the code the peer_xfer_conn function can fail
and the bgp_establish code will then return before adding the
peer back to the peerhash.
This is only part of the failure. The peer also appears to
be in a state where it is no longer initiating connection attempts
but that will be another commited fix when we figure that one out.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
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>
It was noticed that occassionally peering failed in a testbed
upon investigation it was found that the peer was not in the
peer hash and we saw these failure messages:
Aug 25 21:31:15 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: %NOTIFICATION: sent to neighbor 2001:cafe:1ead:4::4 4/0 (Hold Timer Expired) 0 bytes
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] Can't get remote address and port: Transport endpoint is not connected
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 100663299] %bgp_getsockname() failed for peer 2001:cafe:1ead:4::4 fd 27 (from_peer fd -1)
Aug 25 21:31:22 doca-hbn-service-bf3-s06-1-ipmi bgpd[3048]: [EC 33554464] %Neighbor failed in xfer_conn
root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr# vtysh -c 'show bgp peerhash' | grep 2001:cafe:1ead:4::4
root@doca-hbn-service-bf3-s06-1-ipmi:/var/log/hbn/frr#
Upon looking at the code the peer_xfer_conn function can fail
and the bgp_establish code will then return before adding the
peer back to the peerhash.
This is only part of the failure. The peer also appears to
be in a state where it is no longer initiating connection attempts
but that will be another commited fix when we figure that one out.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Capability was unset, but forgot to unset the role.
Fixes: 5ad080d37a ("bgpd: Handle Role capability via dynamic capabilities for SET/UNSET properly")
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Before this path we used session reset method, which is discouraged by rfc7606.
Handle this as rfc requires.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
```
donatas-pc(config-router)# timers bgp 8 12
% keeplive value 8 is larger than 1/3 of the holdtime, setting to 4
donatas-pc(config-router)# do sh run | include timers bgp
timers bgp 4 12
donatas-pc(config-router)#
```
Closes https://github.com/FRRouting/frr/issues/14287
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When the BGP 'redistribute table' command is used for a given route
table, and BGP configuration is flushed and rebuilt, the redistribution
does not work.
Actually, when flushing the BGP configuration with the 'no router bgp'
command, the BGP redistribute entries related to the 'redistribute table'
entries are not flushed. Actually, at BGP deletion, the table number is
not given as parameter in bgp_redistribute_unset() function, and the
redistribution entry is not removed in zebra.
Fix this by adding some code to flush all the redistribute table
instances.
Fixes: 7c8ff89e93 ("Multi-Instance OSPF Summary")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
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>
When bgp_stop finishes and it deletes the peer it is sending
back a return code stating that the peer was deleted, but
the code was operating like it was not deleted and continued
to access the data structure. Fix.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The return code from a event handling perspective
is an enum. Let's intentionally make it a switch
so that all cases are ensured to be covered now
and in the future.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Straighten out the code to not mix the two. Especially
since bgp was assigning non enum values to the enum.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Shouldn't validate the label after 'decode_label'. If we validate
the label after 'decode_label', even the 'MPLS_INVALID_LABEL' will
be valid then.
Signed-off-by: Valerian_He <1826906282@qq.com>
```
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>
This is based on @donaldsharp's work
The current code base is the struct bgp_node data structure.
The problem with this is that it creates a bunch of
extra data per route_node.
The table structure generates ‘holder’ nodes
that are never going to receive bgp routes,
and now the memory of those nodes is allocated
as if they are a full bgp_node.
After splitting up the bgp_node into bgp_dest and route_node,
the memory of ‘holder’ node which does not have any bgp data
will be allocated as the route_node, not the bgp_node,
and the memory usage is reduced.
The memory usage of BGP node will be reduced from 200B to 96B.
The total memory usage optimization of this part is ~16.00%.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Signed-off-by: Yuqing Zhao <xiaopanghu99@163.com>
For v4 nexthops, ifindex was being set. Modified the check to set
ifindex only for v6 nexthops. Also modified the check to set ifindex
only if the v6 nexthop matches peer's LL address.
Signed-off-by: Pooja Jagadeesh Doijode <pdoijode@nvidia.com>
Make sure we don't exceed the maximum of BGP_MAX_SOFT_VERSION.
The Capability Length SHOULD be no greater than 64.
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Found when fuzzing:
```
==3470861==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xffff77801ef7 at pc 0xaaaaba7b3dbc bp 0xffffcff0e760 sp 0xffffcff0df50
READ of size 2 at 0xffff77801ef7 thread T0
0 0xaaaaba7b3db8 in __asan_memcpy (/home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgpd+0x363db8) (BuildId: cc710a2356e31c7f4e4a17595b54de82145a6e21)
1 0xaaaaba81a8ac in ptr_get_be16 /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/./lib/stream.h:399:2
2 0xaaaaba819f2c in bgp_attr_aigp_valid /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgp_attr.c:504:3
3 0xaaaaba808c20 in bgp_attr_aigp /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgp_attr.c:3275:7
4 0xaaaaba7ff4e0 in bgp_attr_parse /home/ubuntu/frr_8_5_2/frr_8_5_2_fuzz_clang/bgpd/bgp_attr.c:3678:10
```
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Properly free the dynamically allocated memory held by `str` after its use.
The change also maintains the return value of `nb_cli_apply_changes` by using `ret` variable.
The ASan leak log for reference:
```
Direct leak of 55 byte(s) in 2 object(s) allocated from:
#0 0x7f16f285f867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x7f16f23fda11 in qmalloc ../lib/memory.c:100
#2 0x7f16f23a01a0 in frrstr_join ../lib/frrstr.c:89
#3 0x7f16f23418c7 in argv_concat ../lib/command.c:183
#4 0x55aba24731f2 in set_aspath_exclude_access_list_magic ../bgpd/bgp_routemap.c:6327
#5 0x55aba2455cf4 in set_aspath_exclude_access_list bgpd/bgp_routemap_clippy.c:836
#6 0x7f16f2345d61 in cmd_execute_command_real ../lib/command.c:993
#7 0x7f16f23460ee in cmd_execute_command ../lib/command.c:1052
#8 0x7f16f2346dc0 in cmd_execute ../lib/command.c:1218
#9 0x7f16f24f7197 in vty_command ../lib/vty.c:591
#10 0x7f16f24fc07c in vty_execute ../lib/vty.c:1354
#11 0x7f16f250247a in vtysh_read ../lib/vty.c:2362
#12 0x7f16f24e72f4 in event_call ../lib/event.c:1979
#13 0x7f16f23d1828 in frr_run ../lib/libfrr.c:1213
#14 0x55aba2269e52 in main ../bgpd/bgp_main.c:510
#15 0x7f16f1dbfd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
The BGP FSM was using the peer as the unit of work
but the FSM is connection focused. So let's switch
it over to using that.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
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 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>
Move PEER_THREAD_WRITES_ON and PEER_THREAD_READS_ON to
be a part of the `struct peer_connection` since this is
a connection oriented bit of data.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Move the peer->t_write and peer->t_read into `struct peer_connection`
as that these are properties of the connection.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
P# Please enter the commit message for your changes. Lines starting
BGP tracks connections based upon the peer. But the problem
with this is that the doppelganger structure for it is being
created. This has introduced a bunch of fragileness in that
the peer exists independently of the connections to it.
The whole point of the doppelganger structure was to allow
BGP to both accept and initiate tcp connections and then
when we get one to a `good` state we collapse into the
appropriate one. The problem with this is that having
2 peer structures for this creates a situation where
we have to make sure we are configing the `right` one
and also make sure that we collapse the two independent
peer structures into 1 acting peer. This makes no sense
let's abstract out the peer into having 2 connection
one for incoming connections and one for outgoing connections
then we can easily collapse down without having to do crazy
stuff. In addition people adding new features don't need
to have to go touch a million places in the code.
This is the start of this abstraction. In this commit
we'll just pull out the fd and input/output buffers
into a connection data structure. Future commits
will abstract further.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Properly free the dynamically allocated memory held by `str` after its use.
The change also maintains the return value of `nb_cli_apply_changes` by using 'ret' variable.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in bgp_set_aspath_replace.test_bgp_set_aspath_replace/r1.asan.bgpd.11586
=================================================================
==11586==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 92 byte(s) in 3 object(s) allocated from:
#0 0x7f4e2951db40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f4e28f19ea2 in qmalloc lib/memory.c:100
#2 0x7f4e28edbb08 in frrstr_join lib/frrstr.c:89
#3 0x7f4e28e9a601 in argv_concat lib/command.c:183
#4 0x56519adf8413 in set_aspath_replace_access_list_magic bgpd/bgp_routemap.c:6174
#5 0x56519adf8942 in set_aspath_replace_access_list bgpd/bgp_routemap_clippy.c:683
#6 0x7f4e28e9d548 in cmd_execute_command_real lib/command.c:993
#7 0x7f4e28e9da0c in cmd_execute_command lib/command.c:1051
#8 0x7f4e28e9de8b in cmd_execute lib/command.c:1218
#9 0x7f4e28fc4f1c in vty_command lib/vty.c:591
#10 0x7f4e28fc53c7 in vty_execute lib/vty.c:1354
#11 0x7f4e28fcdc8d in vtysh_read lib/vty.c:2362
#12 0x7f4e28fb8c8b in event_call lib/event.c:1979
#13 0x7f4e28efd445 in frr_run lib/libfrr.c:1213
#14 0x56519ac85d81 in main bgpd/bgp_main.c:510
#15 0x7f4e27f40c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 92 byte(s) leaked in 3 allocation(s).
***********************************************************************************
***********************************************************************************
Address Sanitizer Error detected in bgp_set_aspath_exclude.test_bgp_set_aspath_exclude/r1.asan.bgpd.10385
=================================================================
==10385==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 55 byte(s) in 2 object(s) allocated from:
#0 0x7f6814fdab40 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
#1 0x7f68149d6ea2 in qmalloc lib/memory.c:100
#2 0x7f6814998b08 in frrstr_join lib/frrstr.c:89
#3 0x7f6814957601 in argv_concat lib/command.c:183
#4 0x5570e05117a1 in set_aspath_exclude_access_list_magic bgpd/bgp_routemap.c:6327
#5 0x5570e05119da in set_aspath_exclude_access_list bgpd/bgp_routemap_clippy.c:836
#6 0x7f681495a548 in cmd_execute_command_real lib/command.c:993
#7 0x7f681495aa0c in cmd_execute_command lib/command.c:1051
#8 0x7f681495ae8b in cmd_execute lib/command.c:1218
#9 0x7f6814a81f1c in vty_command lib/vty.c:591
#10 0x7f6814a823c7 in vty_execute lib/vty.c:1354
#11 0x7f6814a8ac8d in vtysh_read lib/vty.c:2362
#12 0x7f6814a75c8b in event_call lib/event.c:1979
#13 0x7f68149ba445 in frr_run lib/libfrr.c:1213
#14 0x5570e03a0d81 in main bgpd/bgp_main.c:510
#15 0x7f68139fdc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 55 byte(s) leaked in 2 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
Problem:
On GR helper, paths learnt from an interface based peer were linked
to bnc with ifindex=0. During restart of GR peer, BGP (unnumbered)
session (with GR restarter peer) goes down on GR helper but the routes
are retained. Later, when BGP receives an interface up event, it
will process all the paths associated with BNC whose ifindex matches the
ifindex of the interface for which UP event is received. However, paths
associated with bnc that has ifindex=0 were not being reinstalled since
ifindex=0 doesn't match ifindex of any interfaces. This results in
BGP routes not being reinstalled in zebra and kernel.
Fix:
For paths learnt from an interface based peer, set the
ifindex to peer's interface ifindex so that correct
peer based nexthop can be found and linked to the path.
Signed-off-by: Donald Sharp sharpd@nvidia.com
Signed-off-by: Pooja Jagadeesh Doijode <pdoijode@nvidia.com>
Relax this handling (RFC 7606) only for eBGP peers.
More details: https://datatracker.ietf.org/doc/html/rfc7606#section-4
There are two error cases in which the Total Attribute Length value
can be in conflict with the enclosed path attributes, which
themselves carry length values:
* In the first case, the length of the last encountered path
attribute would cause the Total Attribute Length to be exceeded
when parsing the enclosed path attributes.
* In the second case, fewer than three octets remain (or fewer than
four octets, if the Attribute Flags field has the Extended Length
bit set) when beginning to parse the attribute. That is, this
case exists if there remains unconsumed data in the path
attributes but yet insufficient data to encode a single minimum-
sized path attribute. <<<< HANDLING THIS CASE IN THIS COMMIT >>>>
In either of these cases, an error condition exists and the "treat-
as-withdraw" approach MUST be used (unless some other, more severe
error is encountered dictating a stronger approach), and the Total
Attribute Length MUST be relied upon to enable the beginning of the
NLRI field to be located.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Before the patch:
```
donatas-laptop(config-router)# bgp confederation
identifier AS number in plain <1-4294967295> or dotted <0-65535>.<0-65535> format
peers Peer ASs in BGP confederation
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
As per RFC7606 section 3g,
g. If the MP_REACH_NLRI attribute or the MP_UNREACH_NLRI [RFC4760]
attribute appears more than once in the UPDATE message, then a
NOTIFICATION message MUST be sent with the Error Subcode
"Malformed Attribute List". If any other attribute (whether
recognized or unrecognized) appears more than once in an UPDATE
message, then all the occurrences of the attribute other than the
first one SHALL be discarded and the UPDATE message will continue
to be processed.
However, notification is sent out currently for all the cases.
Fix:
For cases other than MP_REACH_NLRI & MP_UNREACH_NLRI, handling has been updated
to discard the occurrences other than the first one and proceed with further parsing.
Again, the handling is relaxed only for the EBGP case.
Also, since in case of error, the attribute is discarded &
stream pointer is being adjusted accordingly based on length,
the total attribute length sanity check case has been moved up in the function
to be checked before this case.
Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
As per RFC7606 section 4,
when the total attribute length value is in conflict with the
enclosed attribute length, treat-as-withdraw approach must be followed.
However, notification is being sent out for this case currently,
that leads to session reset.
Fix:
The handling has been updated to conform to treat-as-withdraw
approach only for EBGP case. For IBGP, since we are not following
treat-as-withdraw approach for any of the error handling cases,
the existing behavior is retained for the IBGP.
Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
Found some code where bgp was not unlocking the dest
and rd_dest when walking the tree attempting to
find something to install.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
RFC 3032 defines:
A value of 2 represents the "IPv6 Explicit NULL Label".
This label value is only legal at the bottom of the label
stack. It indicates that the label stack must be popped,
and the forwarding of the packet must then be based on the
IPv6 header.
Before this patch we set 128, but it was even more wrong, because it was sent
in host-byte order, not the network-byte.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
In the netlink-mediated kernel dataplane, each rule is stored
in either an IPv4-specific database or an IPv6-specific database.
PBRD opportunistically gleans each rule's address family value
from its source or destination IP address match value (if either
exists), or from its nexthop or nexthop-group (if it exists).
The 'family' value is particularly needed for netlink during
incremental rule deletion when none of the above fields remain set.
Before now, this address family has been encoded by occult means
in the (possibly otherwise unset) source/destination IP match
fields in ZAPI and zebra.
This commit documents the reasons for maintaining the 'family'
field in the PBRD rule structure, adds a 'family' field in the
common lib/pbr.h rule structure, and carries it explicitly in ZAPI.
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
Even if some of the attributes in bgp_path_info_extra are
not used, their memory is still allocated every time. It
cause a waste of memory.
This commit code deletes all unnecessary attributes and
changes the optional attributes to pointer storage. Memory
will only be allocated when they are actually used. After
optimization, extra info related memory is reduced by about
half(~400B -> ~200B).
Signed-off-by: Valerian_He <1826906282@qq.com>
The usage of bgp_vrf does not need to be tested
at this point since it's already been derefed in all
paths to this point.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
A route-map applied on incoming BGP updates is not able
to replace an unwanted as segments by another one.
unwanted as segment are based on an AS path access-list.
The below configuration illustrates the case:
router bgp 65001
address-family ipv4 unicast
neighbor 192.168.1.2 route-map rule_2 in
exit-address-family
bgp as-path access-list RULE permit ^65
route-map rule_2 permit 10
set as-path replace as-path-access-list RULE 6000
```
BGP routing table entry for 10.10.10.10/32, version 13
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
192.168.10.65
65000 1 2 3 123
192.168.10.65 from 192.168.10.65 (10.10.10.11)
Origin IGP, metric 0, valid, external, best (First path received)
```
After:
```
do show ip bgp 10.10.10.10/32
BGP routing table entry for 10.10.10.10/32, version 15
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
192.168.10.65
6000 1 2 3 123
192.168.10.65 from 192.168.10.65 (10.10.10.11)
Origin IGP, metric 0, valid, external, best (First path
received)
```
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
Add this logic inside bgp_capability_send() instead of repeating the whole
logic before calling bgp_capability_send().
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When setting local-role for the neighbor, force sending ROLE capability via
dynamic capability if it's enabled.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Gonna be covered later with further PRs. Now adding them to avoid compiler
errors due to uncovered switch/cases.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
We have dynamic capability support, but it handles only MP capability.
With this change, we can enable software version capability dynamicaly, without
resetting the session.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Add a `--v6-with-v4-nexthop` cli to bgp to allow it to peer with
neighbors in the configuration where the interface has no v6 addresses
at all and there is a v4 address that is usable as a v4 address
embedded in a v6 address.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Modify bgp to not allow a v6 peer to come up if the v6 afi is negotiated
and the outgoing interface has no v6 address as well as zebra does
not support the v6 with v4 nexthop capabilities that some dataplanes
allow.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
RCA:
On encountering any attribute error for core attributes in update message,
the error handling is set to 'treat as withdraw' and
further parsing of the remaining attributes is skipped.
But the stream pointer is not being correctly adjusted to
point to the next NLRI field skipping the rest of the attributes.
This leads to incorrect parsing of the NLRI field,
which causes BGP session to reset.
Fix:
The stream pointer offset is rightly adjusted to point to the NLRI field correctly
when the malformed attribute is encountered and remaining attribute parsing is skipped.
Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
When 'no neighbor .. update-source' is issued for a regular peer, that
peer is always reset. This is unnecessary if the peer is a member of a
peer-group and it inherits an identical update-source, so let's skip
the reset/Notification for that condition.
Config:
------------
router bgp 1
neighbor PG peer-group
neighbor PG remote-as internal
neighbor PG update-source 100.64.0.3
neighbor 192.168.122.99 peer-group PG
neighbor 192.168.122.99 update-source 100.64.0.3
Before:
------------
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99 4 1 36 34 0 0 0 00:00:17 0 0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 46083
ub20-2(config-router)# no neighbor 192.168.122.99 update-source
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99 4 1 36 35 0 0 0 00:00:01 Idle 0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 39847
After:
------------
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99 4 1 3 3 0 0 0 00:00:20 0 0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 39415
ub20-2(config-router)# no neighbor 192.168.122.99 update-source
ub20-2(config-router)# do show ip bgp sum | include .99
192.168.122.99 4 1 3 3 0 0 0 00:00:28 0 0 N/A
ub20-2(config-router)# do show ip bgp neighbors 192.168.122.99 | include Local host
Local host: 100.64.0.3, Local port: 39415
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
Include an event ptr-to-ptr in the event_execute() api
call, like the various schedule api calls. This allows the
execute() api to cancel an existing scheduled task if that
task is being executed inline.
Signed-off-by: Mark Stapp <mjs@labn.net>
The last_reset_cause is a plain old BGP_MAX_PACKET_SIZE buffer
that is really enlarging the peer data structure. Let's just
copy the stream that failed and only allocate how ever much
the packet size actually was. While it's likely that we have
a reset reason, the packet typically is not going to be 65k
in size. Let's save space.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Currently, json output of show BGP commands are no pretty format.
This is an extremely expensive operation for huge scale (lots of
routes with lots of paths).
BGP json non-pretty commands support added:
```
show bgp neighbors <nbr-id> advertised-routes json
show bgp neighbors <nbr-id> received-routes json
show bgp neighbors <nbr-id> advertised-routes detail json
show bgp neighbors <nbr-id> received-routes detail json
```
Ticket:#3513256
Issue:3513256
Testing: UT done
Signed-off-by: Sindhu Parvathi Gopinathan's <sgopinathan@nvidia.com>
The ringbuf is 650k in size. This is obscenely large and
in practical experimentation FRR never even approaches
that size at all. Let's reduce this to 1.5 max packet sizes.
If a BGP_MAX_PACKET_SIZE packet is ever received having a bit
of extra space ensures that we can read at least 1 packet.
This also will significantly reduce memory usage when the
operator has a lot of peers.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Commit: a0b937de42
Introduced the idea of a input Q packet limit. Say you read in
635000 bytes of data and the input Q is already at it's limit
(currently 1000) then when bgp_process_reads runs it will
assert because there is less then a BGP_MAX_PACKET_SIZE in ibuf_work.
Don't assert as that it's irrelevant. Even if we can't read a full packet
in let's let the whole system keep working as that as the input Q length
comes down we will start pulling down the ibuf_work and it will be ok.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The peer->ibuf_scratch was allocating 65535 * 10 bytes
for scratch space to hold data incoming from a read
from a peer. When you have 4k peers this is 262,1400,000
or 262 mb of data. Which is crazy large. Especially
since the i/o pthread is reading per peer without
any chance of having the data interfere with other reads.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Over time the bgp_zebra_announce_default function has gotten
slightly convoluted, clean it up so it's easier to read
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
bgpd, pbrd: use common pbr encoder
zebra: use common pbr decoder
tests: pbr_topo1: check more filter fields
Purpose:
1. Reduce likelihood of zapi format mismatches when adding
PBR fields due to multiple parallel encoder implementations
2. Encourage common PBR structure usage among various daemons
3. Reduce coding errors via explicit per-field enable flags
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
Subset: ZAPI changes to send the new data
Also adds filter_bm field; currently for PBR_FILTER_PCP, but in the
future to be used for all of the filter fields.
Changes by:
Josh Werner <joshuawerner@mitre.org>
Eli Baum <ebaum@mitre.org>
G. Paul Ziemba <paulz@labn.net>
Signed-off-by: G. Paul Ziemba <paulz@labn.net>
A route-map applied on incoming BGP updates is not able
to exclude the unwanted as segments, based on an AS path
access-list.
The below configuration illustrates the case:
router bgp 65001
address-family ipv4 unicast
neighbor 192.168.1.2 route-map rule_2 in
exit-address-family
bgp as-path access-list RULE permit ^65
route-map rule_2 permit 10
set as-path exclude as-path-access-list RULE
```
BGP routing table entry for 10.10.10.10/32, version 13
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
192.168.10.65
65000 1 2 3 123
192.168.10.65 from 192.168.10.65 (10.10.10.11)
Origin IGP, metric 0, valid, external, best (First path received)
```
After:
```
do show ip bgp 10.10.10.10/32
BGP routing table entry for 10.10.10.10/32, version 15
Paths: (1 available, best #1, table default)
Advertised to non peer-group peers:
192.168.10.65
2 3 123
192.168.10.65 from 192.168.10.65 (10.10.10.11)
Origin IGP, metric 0, valid, external, best (First path
received)
```
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
Assuming field 'ifindex_ipv6_ll' is not equal to field 'ifindex', then
nhop_found is just a garbage, let's avoid that.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When switching `router bgp`, `no router bgp` and doing redistributions, we should
ignore this action, otherwise memory leak happens:
```
Indirect leak of 400 byte(s) in 2 object(s) allocated from:
0 0x7f81b36b3a06 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:153
1 0x7f81b327bd2e in qcalloc lib/memory.c:105
2 0x55f301d28628 in bgp_node_create bgpd/bgp_table.c:92
3 0x7f81b3309d0b in route_node_new lib/table.c:52
4 0x7f81b3309d0b in route_node_set lib/table.c:61
5 0x7f81b330be0a in route_node_get lib/table.c:319
6 0x55f301ce89df in bgp_redistribute_add bgpd/bgp_route.c:8907
7 0x55f301dac182 in zebra_read_route bgpd/bgp_zebra.c:593
8 0x7f81b334dcd7 in zclient_read lib/zclient.c:4179
9 0x7f81b331d702 in event_call lib/event.c:1995
10 0x7f81b325d597 in frr_run lib/libfrr.c:1213
11 0x55f301b94b12 in main bgpd/bgp_main.c:505
12 0x7f81b2b57082 in __libc_start_main ../csu/libc-start.c:308
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
problem:
In CLI config codeflow there are memory leaks in failure scenario
Fix:
Code changes are done to free ecommunity
Signed-off-by: Abhinay Ramesh <rabhinay@vmware.com>
Useful to have it for datacenter profile only, disabled for traditional.
If the peer is not established or established, but has no description set,
we will show the FRR version instead, which is kinda handy to have instead of
nothing.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When running bgp_always_compare_med, I am frequently seeing a crash
After running with valgrind I am seeing this and a invalid write
immediately after this as well.
==311743== Invalid read of size 2
==311743== at 0x4992421: route_map_counter_decrement (routemap.c:3308)
==311743== by 0x35664D: peer_route_map_unset (bgpd.c:7259)
==311743== by 0x306546: peer_route_map_unset_vty (bgp_vty.c:8037)
==311743== by 0x3066AC: no_neighbor_route_map (bgp_vty.c:8081)
==311743== by 0x49078DE: cmd_execute_command_real (command.c:990)
==311743== by 0x4907A63: cmd_execute_command (command.c:1050)
==311743== by 0x490801F: cmd_execute (command.c:1217)
==311743== by 0x49C5535: vty_command (vty.c:551)
==311743== by 0x49C7459: vty_execute (vty.c:1314)
==311743== by 0x49C97D1: vtysh_read (vty.c:2223)
==311743== by 0x49BE5E2: event_call (event.c:1995)
==311743== by 0x494786C: frr_run (libfrr.c:1204)
==311743== by 0x1F7655: main (bgp_main.c:505)
==311743== Address 0x9ec2180 is 64 bytes inside a block of size 120 free'd
==311743== at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==311743== by 0x495A1BA: qfree (memory.c:130)
==311743== by 0x498D412: route_map_free_map (routemap.c:748)
==311743== by 0x498D176: route_map_add (routemap.c:672)
==311743== by 0x498D79B: route_map_get (routemap.c:857)
==311743== by 0x499C256: lib_route_map_create (routemap_northbound.c:102)
==311743== by 0x49702D8: nb_callback_create (northbound.c:1234)
==311743== by 0x497107F: nb_callback_configuration (northbound.c:1578)
==311743== by 0x4971693: nb_transaction_process (northbound.c:1709)
==311743== by 0x496FCF4: nb_candidate_commit_apply (northbound.c:1103)
==311743== by 0x496FE4E: nb_candidate_commit (northbound.c:1136)
==311743== by 0x497798F: nb_cli_classic_commit (northbound_cli.c:49)
==311743== by 0x4977B4F: nb_cli_pending_commit_check (northbound_cli.c:88)
==311743== by 0x49078C1: cmd_execute_command_real (command.c:987)
==311743== by 0x4907B44: cmd_execute_command (command.c:1068)
==311743== by 0x490801F: cmd_execute (command.c:1217)
==311743== by 0x49C5535: vty_command (vty.c:551)
==311743== by 0x49C7459: vty_execute (vty.c:1314)
==311743== by 0x49C97D1: vtysh_read (vty.c:2223)
==311743== by 0x49BE5E2: event_call (event.c:1995)
==311743== by 0x494786C: frr_run (libfrr.c:1204)
==311743== by 0x1F7655: main (bgp_main.c:505)
==311743== Block was alloc'd at
==311743== at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==311743== by 0x495A068: qcalloc (memory.c:105)
==311743== by 0x498D0C8: route_map_new (routemap.c:646)
==311743== by 0x498D128: route_map_add (routemap.c:658)
==311743== by 0x498D79B: route_map_get (routemap.c:857)
==311743== by 0x499C256: lib_route_map_create (routemap_northbound.c:102)
==311743== by 0x49702D8: nb_callback_create (northbound.c:1234)
==311743== by 0x497107F: nb_callback_configuration (northbound.c:1578)
==311743== by 0x4971693: nb_transaction_process (northbound.c:1709)
==311743== by 0x496FCF4: nb_candidate_commit_apply (northbound.c:1103)
==311743== by 0x496FE4E: nb_candidate_commit (northbound.c:1136)
==311743== by 0x497798F: nb_cli_classic_commit (northbound_cli.c:49)
==311743== by 0x4977B4F: nb_cli_pending_commit_check (northbound_cli.c:88)
==311743== by 0x49078C1: cmd_execute_command_real (command.c:987)
==311743== by 0x4907B44: cmd_execute_command (command.c:1068)
==311743== by 0x490801F: cmd_execute (command.c:1217)
==311743== by 0x49C5535: vty_command (vty.c:551)
==311743== by 0x49C7459: vty_execute (vty.c:1314)
==311743== by 0x49C97D1: vtysh_read (vty.c:2223)
==311743== by 0x49BE5E2: event_call (event.c:1995)
==311743== by 0x494786C: frr_run (libfrr.c:1204)
Effectively the route_map that is being stored has been freed already
but we have not cleaned up properly yet. Go through and clean the
code up by ensuring that the pointer actually exists instead of trusting
it does when doing the decrement operation.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
In BGP, when an interface event is detected or triggered,
the BNC that have a next-hop that matches the interface
are not evaluated.
The paths attached to the bnc context are evaluated in the
following situation:
- In the up event case, if at least one next-hop interface
matched the event interface.
- In the down event case, if there is no alternate next-hop
that does not use the event interface.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This commit changes the 'ifindex' name of the bnc structure.
As it is used only to handle ipv6 link local addresses, let
us use the 'ifindex_ipv6_ll' naming to avoid any confusions
with the ifindex value of the resolved next-hops of the bnc
structure.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
`bnc->ifindex` should not be with 0 ( IFINDEX_INTERNAL ), so we can ignore
the wrong interface to make it safe.
Signed-off-by: anlan_cs <vic.lan@pica8.com>
bgp_zebra_tm_connect calls bgp_zebra_get_table_range which
just used the global zclient. Which of course still had
us exposing the global zclient to read and drop important
data from zebra. This fixes commit 787c61e03c
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The total number of octets of the Sub-TLV Value field. The Sub-TLV Length field
contains 1 octet if the Sub-TLV Type field contains a value in the range from
0-127. The Sub-TLV Length field contains two octets if the Sub-TLV Type field
contains a value in the range from 128-255.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
More details: https://www.rfc-editor.org/rfc/rfc8810.html
Not sure if we want to maintain the old code more.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Also, this is visible only if `capability software-version` is enabled.
Example:
```
r1# show ip bgp summary
IPv4 Unicast Summary (VRF default):
BGP router identifier 192.168.1.1, local AS number 65001 vrf-id 0
BGP table version 0
RIB entries 0, using 0 bytes of memory
Peers 1, using 725 KiB of memory
Neighbor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd PfxSnt Desc
192.168.1.2 4 65002 54 54 0 0 0 00:00:52 0 0 FRRouting/9.1-dev-My
Total number of neighbors 1
r1#
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
`bmnc->nh` was not properly freed, leading to a memory leak.
The commit adds a check to ensure that the `bmnc->nh` member variable is freed if it exists.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in bgp_vpnv4_asbr.test_bgp_vpnv4_asbr/r2.asan.bgpd.6382
=================================================================
==6382==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 720 byte(s) in 5 object(s) allocated from:
#0 0x7f6a80d02d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x55c9afd7c81c in qcalloc lib/memory.c:105
#2 0x55c9afd9166b in nexthop_new lib/nexthop.c:358
#3 0x55c9afd93aaa in nexthop_dup lib/nexthop.c:843
#4 0x55c9afad39bb in bgp_mplsvpn_nh_label_bind_register_local_label bgpd/bgp_mplsvpn.c:4259
#5 0x55c9afb1c5e9 in bgp_mplsvpn_handle_label_allocation bgpd/bgp_route.c:3239
#6 0x55c9afb1c5e9 in bgp_process_main_one bgpd/bgp_route.c:3339
#7 0x55c9afb1d2c1 in bgp_process_wq bgpd/bgp_route.c:3591
#8 0x55c9afe33df9 in work_queue_run lib/workqueue.c:266
#9 0x55c9afe198e2 in event_call lib/event.c:1995
#10 0x55c9afd5fc6f in frr_run lib/libfrr.c:1213
#11 0x55c9af9f6f00 in main bgpd/bgp_main.c:505
#12 0x7f6a7f55ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
Indirect leak of 16 byte(s) in 2 object(s) allocated from:
#0 0x7f6a80d02d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x55c9afd7c81c in qcalloc lib/memory.c:105
#2 0x55c9afd91ce8 in nexthop_add_labels lib/nexthop.c:536
#3 0x55c9afd93754 in nexthop_copy_no_recurse lib/nexthop.c:802
#4 0x55c9afd939fb in nexthop_copy lib/nexthop.c:821
#5 0x55c9afd93abb in nexthop_dup lib/nexthop.c:845
#6 0x55c9afad39bb in bgp_mplsvpn_nh_label_bind_register_local_label bgpd/bgp_mplsvpn.c:4259
#7 0x55c9afb1c5e9 in bgp_mplsvpn_handle_label_allocation bgpd/bgp_route.c:3239
#8 0x55c9afb1c5e9 in bgp_process_main_one bgpd/bgp_route.c:3339
#9 0x55c9afb1d2c1 in bgp_process_wq bgpd/bgp_route.c:3591
#10 0x55c9afe33df9 in work_queue_run lib/workqueue.c:266
#11 0x55c9afe198e2 in event_call lib/event.c:1995
#12 0x55c9afd5fc6f in frr_run lib/libfrr.c:1213
#13 0x55c9af9f6f00 in main bgpd/bgp_main.c:505
#14 0x7f6a7f55ec86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 736 byte(s) leaked in 7 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
The `bgp_vrf->vrf_prd_pretty` string was not properly freed, leading to a memory leak.
This commit resolves the memory leak by freeing the memory allocated for `bgp_vrf->vrf_prd_pretty` before returning from the function.
The ASan leak log for reference:
```
***********************************************************************************
Address Sanitizer Error detected in evpn_type5_test_topo1.test_evpn_type5_topo1/e1.asan.bgpd.17689
=================================================================
==17689==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7fdd94fc0538 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x77538)
#1 0x55e28d9c4c6c in qstrdup lib/memory.c:117
#2 0x55e28d6c0d27 in evpn_configure_vrf_rd bgpd/bgp_evpn_vty.c:2297
#3 0x55e28d6c0d27 in bgp_evpn_vrf_rd bgpd/bgp_evpn_vty.c:6271
#4 0x55e28d94c155 in cmd_execute_command_real lib/command.c:994
#5 0x55e28d94c622 in cmd_execute_command lib/command.c:1053
#6 0x55e28d94ca99 in cmd_execute lib/command.c:1221
#7 0x55e28da6d7d4 in vty_command lib/vty.c:591
#8 0x55e28da6dc6e in vty_execute lib/vty.c:1354
#9 0x55e28da7644d in vtysh_read lib/vty.c:2362
#10 0x55e28da616e2 in event_call lib/event.c:1995
#11 0x55e28d9a7a65 in frr_run lib/libfrr.c:1213
#12 0x55e28d63ef00 in main bgpd/bgp_main.c:505
#13 0x7fdd93883c86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
SUMMARY: AddressSanitizer: 15 byte(s) leaked in 1 allocation(s).
***********************************************************************************
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
The bgp vpn policy had some attribute not free when the function bgp_free was called leading to memory leak as shown below.
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251:Direct leak of 592 byte(s) in 2 object(s) allocated from:
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #0 0x7f4b7ae92037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #1 0x7f4b7aa96e38 in qcalloc lib/memory.c:105
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #2 0x7f4b7aa9bec9 in srv6_locator_chunk_alloc lib/srv6.c:135
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #3 0x56396f8e56f8 in ensure_vrf_tovpn_sid_per_af bgpd/bgp_mplsvpn.c:752
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #4 0x56396f8e608a in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:846
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #5 0x56396f8e075d in vpn_leak_postchange bgpd/bgp_mplsvpn.h:259
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #6 0x56396f8f3e5b in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3397
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #7 0x56396fa920ef in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3238
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #8 0x7f4b7abb2913 in zclient_read lib/zclient.c:4134
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #9 0x7f4b7ab62010 in thread_call lib/thread.c:1991
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #10 0x7f4b7aa5a418 in frr_run lib/libfrr.c:1185
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #11 0x56396f7d756d in main bgpd/bgp_main.c:505
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #12 0x7f4b7a479d09 in __libc_start_main ../csu/libc-start.c:308
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251-
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251:Direct leak of 32 byte(s) in 2 object(s) allocated from:
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #0 0x7f4b7ae92037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #1 0x7f4b7aa96e38 in qcalloc lib/memory.c:105
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #2 0x56396f8e31b8 in vpn_leak_zebra_vrf_sid_update_per_af bgpd/bgp_mplsvpn.c:386
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #3 0x56396f8e3ae8 in vpn_leak_zebra_vrf_sid_update bgpd/bgp_mplsvpn.c:448
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #4 0x56396f8e09b0 in vpn_leak_postchange bgpd/bgp_mplsvpn.h:271
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #5 0x56396f8f3e5b in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3397
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #6 0x56396fa920ef in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3238
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #7 0x7f4b7abb2913 in zclient_read lib/zclient.c:4134
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #8 0x7f4b7ab62010 in thread_call lib/thread.c:1991
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #9 0x7f4b7aa5a418 in frr_run lib/libfrr.c:1185
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #10 0x56396f7d756d in main bgpd/bgp_main.c:505
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #11 0x7f4b7a479d09 in __libc_start_main ../csu/libc-start.c:308
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251-
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251:Direct leak of 32 byte(s) in 2 object(s) allocated from:
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #0 0x7f4b7ae92037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #1 0x7f4b7aa96e38 in qcalloc lib/memory.c:105
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #2 0x56396f8e5730 in ensure_vrf_tovpn_sid_per_af bgpd/bgp_mplsvpn.c:753
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #3 0x56396f8e608a in ensure_vrf_tovpn_sid bgpd/bgp_mplsvpn.c:846
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #4 0x56396f8e075d in vpn_leak_postchange bgpd/bgp_mplsvpn.h:259
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #5 0x56396f8f3e5b in vpn_leak_postchange_all bgpd/bgp_mplsvpn.c:3397
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #6 0x56396fa920ef in bgp_zebra_process_srv6_locator_chunk bgpd/bgp_zebra.c:3238
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #7 0x7f4b7abb2913 in zclient_read lib/zclient.c:4134
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #8 0x7f4b7ab62010 in thread_call lib/thread.c:1991
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #9 0x7f4b7aa5a418 in frr_run lib/libfrr.c:1185
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #10 0x56396f7d756d in main bgpd/bgp_main.c:505
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251- #11 0x7f4b7a479d09 in __libc_start_main ../csu/libc-start.c:308
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251-
./bgp_srv6l3vpn_to_bgp_vrf.test_bgp_srv6l3vpn_to_bgp_vrf/r2.bgpd.asan.603251-SUMMARY: AddressSanitizer: 656 byte(s) leaked in 6 allocation(s).
Signed-off-by: ryndia <dindyalsarvesh@gmail.com>
There is no route-map set action to replace any ASN,
or a part of an ASN, with a configured ASN.
The current commit adds a new command to use a configured
ASN as replacement, instead of using the local as number.
> set as-path replace any 65500
Update the 'bgp_set_aspath_replace' test.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
In case the full label stack is used, there may be
a table overrun happening. Avoid it by increasing the
size of the table.
Fixes: 27f4deed0a ("bgpd: update the mpls entry to handle return traffic")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The bmnc pointer is never null. Do not keep the test
on the pointer.
Fixes: 1069425868 ("bgpd: allocate label bound to received mpls vpn routes")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
With these changes,
the code ensures that the peer data-structures are accessed
only after it knows that BGPD is not terminating.
Authored-by: Naveen Thanikachalam <nthanikachal@vmware.com>
Signed-off-by: Iqra Siddiqui <imujeebsiddi@vmware.com>
Imagine the following scenario:
when a neighbor has an inbound policy set to modify the next hop, but no outbound route-map is configured.
In this case, if(!post_attr && (ROUTE_MAP_OUT_NAME(filter) || bgp_path_suppressed(pi))) returns false, causing rmap_in_change_flag to not be correctly cleared, and mistakenly identified as rmap_out_change_flag, leading to the failure of the subsequent neighbor-nexthop-self command.
Signed-off-by: Jack.zhang <hanyu.zly@alibaba-inc.com>
If this an error, we should use zlog_err, not zlog_info as this is literally
not an information, but an error.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When running all daemons with config for most of them, FRR has
sharpd@janelle:~/frr$ vtysh -c "show debug hashtable" | grep "VRF BIT HASH" | wc -l
3570
3570 hashes for bitmaps associated with the vrf. This is a very
large number of hashes. Let's do two things:
a) Reduce the created size of the actually created hashes to 2
instead of 32.
b) Delay generation of the hash *until* a set operation happens.
As that no hash directly implies a unset value if/when checked.
This reduces the number of hashes to 61 in my setup for normal
operation.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
add support of color extended community, conforming to RFC 9012.
This extended community will be added to the existing one, RT,SOO
and Node Target. The configuration will be made through the
route-map service.
find above a configuration example:
router bgp 65001
bgp router-id 192.168.1.1
no bgp ebgp-requires-policy
no bgp network import-check
neighbor 192.168.1.2 remote-as external
neighbor 192.168.1.3 remote-as external
neighbor 192.168.1.4 remote-as external
address-family ipv4 unicast
network 10.10.10.10/24 route-map rmap
exit-address-family
!
route-map rmap permit 10
set extcommunity color 55555 200
exit
Signed-off-by: Francois Dumontet <francois.dumontet@6wind.com>
Both the label manager and table manager zapi code send data requests via zapi
to zebra and then immediately listen for a response from zebra. The problem here
is of course that the listen part is throwing away any zapi command that is not
the one it is looking for.
ISIS/OSPF and PIM all have synchronous abilities via zapi, which they all
do through a special zapi connection to zebra. BGP needs to follow this model
as well. Additionally the new zclient_sync connection that should be created,
a once a second timer should wake up and read any data on the socket to
prevent problems too much data accumulating in the socket.
```
r3# sh bgp labelpool summary
Labelpool Summary
-----------------
Ledger: 3
InUse: 3
Requests: 0
LabelChunks: 1
Pending: 128
Reconnects: 1
r3# sh bgp labelpool inuse
Prefix Label
---------------------------
10.0.0.1/32 16
192.168.31.0/24 17
192.168.32.0/24 18
r3#
```
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
It is not possible to flush all the incoming as-path list
from a given BGP update.
Add a route-map set command to remove all as-paths
from a given AS path. Add the necessary tests.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
'{}' extra output may present JSON output from bgp_show_table_rd() when
no prefix are seen.
> {
> "vrfId": 0,
> "vrfName": "default",
> "tableVersion": 0,
> "routerId": "1.1.1.1",
> "defaultLocPrf": 100,
> "localAS": 65500,
> "routes": { "routeDistinguishers" : { "444:1" : { } } } }
> {}
Do not output '{}' when bgp_show_table() returns a valid JSON.
Tested without rd in config, bgp_show_table() returns nothing and the
JSON output is only '{}'
Fixes: 0224b3296c ("bgpd: Print empty JSON `{}` if no entries under `show bgp ipv4 vpn json`")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
After some VRF imports are removed and "no bgp retain route-target all"
is set, prefixes that are not imported anymore remain in the BGP table.
Parse the BGP table and remove un-imported prefixes in such a case.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
By default, bgpd stores all MPLS VPN SAFI prefixes unless the "no bgp
retain route-target all" option is used to store only prefixes that are
imported into local VRFs. The "no retain" option temporarily uses too
much memory, as all prefixes are stored in memory before the deletion of
non-imported prefixes is done.
Filter out non-imported prefixes before they are set into the BGP adj
RIB out.
Fixes: a486300b26 ("bgpd: implement retain route-target all behaviour")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Partially revert a486300b26 ("bgpd: implement retain route-target all
behaviour") in order to fix a memory consumption issue in the next
commit.
Fixes: a486300b26 ("bgpd: implement retain route-target all behaviour")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Remove the check about pi->peer value different from null.
Introducing this check introduces a SA warning on the value
of the from value (derived from pi->peer).
Actually, peer is set when bgp_path_info_make() call is
performed; peer is never null.
Fixes: 23bb4a9b5c64 ("bgpd: advertise mpls vpn routes with appropriate label")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When using the bgp-accept-own community, with the
'attribute-unchanged next-hop' command, the advertised
mpls vpn updates that are reflected by a route reflector
are received, but are not selected.
Once the accept-own community is detected, a new bgp_path
is created, in addition of the original one; then the
next-hop of the NLRI is checked, but fails for two reasons:
- the next-hop tracking returns the real IP reachability
status for prefixes that have the BGP_ROUTE_IMPORTED subtype.
This is what happens with bgp updates with the accept-own
community.
- as the next-hop was unchanged and was the peer IP in the VRF.
Consequently, the new bgp_path is considered inactive in the
default VRF, and is not selected.
The incoming bgp updates with the accept-own community should
not be checked against the next-hop tracking. As the bgp_path
subtype has been changed to BGP_ROUTE_IMPORTED, let us check
the bgp subtype before calling the 'bgp_find_or_add_nexthop()'
function in the 'bgp_update()' call.
Fixes: 46dbf9d0c0 bgpd: ("Implement ACCEPT_OWN extended community")
Fixes: 376797711f4d - bgpd: track mpls vpn nexthops
Fixes: e6110f755718 bgpd: ("fix use nexthop tracking for exported vpn paths")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When exporting redistributed prefixes from a given VRF
to an MPLS VPN network, the paths are always considered
as valid whereas it should not always be the case.
At exportation, a new MPLS VPN path is built in. Then
nexthop tracking is applied to the new path, and the
SAFI_MPLS_VPN parameter is used to tell the NHT code
to just check for the next-hop reachability. The previous
commit was wrongly considering that nexthop tracking was
never applied to mpls vpn networks. Ensure that nexthop
tracking for exported paths behaves as usual.
Fix this by not returning always 1 in the 'bgp_find_or_add_nexthop()'
function if the passed 'pi' parameter is a 'BGP_IMPORTED_ROUTE'
sub-type entry.
Fixes: 74be3f3ea9ec ("bgpd: track mpls vpn nexthops")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When advertising an mpls vpn entry with a new label,
the return traffic is redirected to the local machine,
but the MPLS traffic is dropped.
Add an MPLS entry to handle MPLS packets which have
the new label value. Traffic is swapped to the original
label value from the mpls vpn next-hop entry; then it is
sent to the resolved next-hop of the original next-hop
from the mpls vpn next-hop entry.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The advertised label value from mpls vpn routes is not modified
when the advertised next-hop is modified to next-hop-self.
Actually, the original label value received is redistributed as
is, whereas the new_label value bound in the nexthop label
bind entry should be used.
Only the VPN entries that contain MPLS information, and that
are redistributed between distinct peers, will have a label
value to advertise.
- no SRv6 attribute
- no local prefix
- no exported VPN prefixes from a VRF
If the advertisement to a given peer has the next-hop modified,
then the new label value will be picked up. The considered cases
are peers configured with 'next-hop-self' option, or ebgp peerings
without the 'next-hop-unchanged' option.
Note that the the NLRI format will follow the rfc3107 format, as
multiple label values for MPLS VPN NLRIs are not supported (the
rfc8277 is not supported).
Note also that the case where an outgoing route-map is applied to
the outgoing neighbor is not considered in this commit.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Current implementation does not offer a new label to bind
to a received VPN route entry to redistribute with that new
label.
This commit allocates a label for VPN entries that have
a valid label, and a reachable next-hop interface that is
configured as follows:
> interface eth0
> mpls bgp l3vpn-multi-domain-switching
> exit
An mplsvpn next-hop label binding entry is created in an mpls
vpn nexthop label bind hash table of the current BGP instance.
That mpls vpn next-hop label entry is indexed by the (next-hop,
orig_label) values provided by the incoming updates, and shared
with other updates having the same (next-hop, orig_label) values.
A new 'LP_TYPE_BGP_L3VPN_BIND' label value is picked up from the
zebra mpls label pool, and assigned to the new_label attribute.
The 'bgp_path_info' appends a 'bgp_mplsvpn_nh_label_bind' structure
to the 'mplsvpn' union structure. Both structures in the union are not
used at the same, as the paths are either VRF updates to export, or MPLS
VPN updates. Using an union gives a 24 bytes memory gain compared to if
the structures had not been in an union (24 bytes compared to 48 bytes).
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The label allocation mechanism is called implicitly for
labeled unicast paths. The check should be explicit, because
the current patch set will extend the mechanism for mpls vpn
paths, and the code should explicitly tell which safi calls
which code.
Fix this implicit call by checking the safi value. Move the
code to a specific function.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The label per nexthop attributes take 24 bytes per bgp path
entry on AMD64 platform, and are only used for unicast paths.
The current patch-set introduces a similar attributes, but that
will be used only for l3vpn paths. To gain some memory on the
bgp_path_info structure in the next commit, do some changes.
Create an 'mplsvpn' union structure that will either include the
label per nexthop structs for ipv4 paths, or the l3vpn paths
structures. The 'label_nexthop_cache' and the 'label_nh_thread'
attributes of the 'bgp_path_info' structure are moved into an
union under a new structure called 'bgp_mplsvpn_label_nh_blnc'.
The flags attribute of 'bgp_path_info' is increased from 16 bits
to 32 bits, and the BGP_PATH_MPLSVPN_LABEL_NH flag is added to
know the 'mplsvpn' usage.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
In the context of the ASBR facing an EBGP neighbor, or
facing an IBGP neighbor where the BGP updates received
are re-advertised with a modified next-hop, a new local
label will be re-advertised too, to replace the original
one.
Create a binding table, in the form of a hash list, from the
original labels to the new labels. Since labels can be the
same on several routers, set the next-hop and the label as
the keys. Add the needed API functions to manage the hash
list.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Redistributing mpls vpn prefixes with a new label
requires picking up unused MPLS local labels.
Today, there is an MPLS label pool which is owned
by the zebra daemon. BGP gets a chunk of labels
that are shared with the multiple usage of the BGP
daemon. A label type attribute is used whenever
BGP needs a new label value.
The 'LP_TYPE_BGP_L3VPN_BIND' label type will be used
to allocate the MPLS labels that will be bound to
the original next-hop, and label value of an L3VPN
update.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
There is no nexthop reachability information for
received MPLS VPN prefixes.
This information is necessary when BGP also acts
as LSR device, and is needed to create an MPLS entry
between two BGP speakers: the next-hop to pick-up
in the MPLS entry has to be connected.
The nexthop reachability information is available
for other non MPLS VPN prefixes, and is handled
by the bgp nexthop cache (bnc) contexts.
Extend the usage of the BNC contexts for L3VPN
prefixes.
Note that the MPLS VPN routes had to be redistributed
as before, to avoid breaking existing deployments
that use FRR as route reflectors. Because of this, the
nexthop reachability status has been maintained to OK
for MPLS VPN prefixes.
Note also that the label allocation per nexthop tracking
was wrongly using the MPLS VPN safi to get a valid BNC
context, when choosing which label to return in the
'vpn_leak_from_vrf_get_per_nexthop_label()' function.
Fix this by using SAFI_UNICAST instead.
Fixes: 577be36a41 ("bgpd: add support for l3vpn per-nexthop label")
Link: https://github.com/FRRouting/frr/pull/13380
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The label allocation per next-hop functionality is calling
the 'bgp_find_or_add_nexthop()' method using the SAFI_MPLS_VPN
safi parameter, whereas the call is supposed to apply to
unicast paths.
Fix this by using the SAFI_UNICAST safi parameter in the call.
Simplify the vpn_leak_from_vrf_get_per_nexthop_label() API by
removing the safi parameter from the function.
Fixes: 577be36a41 ("bgpd: add support for l3vpn per-nexthop label")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
When acting as intermediate device for BGP signaling, and
as transit device for data traffic, the device is not able
to modify the label value from incoming MPLS VPN updates:
- as BGP device, modifying the label value is necessary
when redistributing VPN prefixes with its own next-hop.
- as transit device that connects two ethernet segments
on separate interfaces, the return MPLS traffic must be
handled: the modified label value must be swapped with
the original label value and sent back to the original
next-hop.
The border router use case can be taken as example, when
it acts both as transit and as BGP device:
- When receiving updates from a border router peer, and where
interior traffic is expected to transit through the local
border router.
- When receiving updates from interior devices, and where
exterior traffic will transit through the local border router.
In those two situations, a new label is bound to the received
entry, and the entry is advertised to a new peer with the new
label. In the same time, an MPLS entry is created to handle
return traffic with the new mpls label: the traffic would be
swapped to the original MPLS label and the original next-hop.
This is the first commit of a series of patches, that address
the above mentioned issue.
The first commit introduces a new per-interface command:
> interface eth0
> [no] mpls bgp l3vpn-multi-domain-switching
> exit
This command will authorise mpls vpn updates to have a new
label value bound to the mpls vpn routes received over that
interface.
Link: https://www.rfc-editor.org/rfc/rfc3107.html#section-3
> When a BGP speaker redistributes a route, the label(s) assigned to
> that route must not be changed (except by omission), unless the
> speaker changes the value of the Next Hop attribute of the route.
Link: https://www.rfc-editor.org/rfc/rfc3031.html#section-4.6
Link: https://www.rfc-editor.org/rfc/rfc4364.html#section-10
sub-chapter b.
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Create a bgp_labels_same() function that does the
same operations as the static function labels_same from
bgp_mplsvpn.c.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
BGP cannot decide to disseminate the safi based upon the
bgp suppress-fib command. Modify the code to look at
the safi for the decision to communicate to a peer the
particular node.
Ticket: #3402926
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
```
bgpd/bgp_vty.c:865:5: warning: conflicting types for ‘bgp_vty_return’ due to enum/integer mismatch; have ‘int(struct vty *, enum bgp_create_error_code)’ [-Wenum-int-mismatch]
865 | int bgp_vty_return(struct vty *vty, enum bgp_create_error_code ret)
| ^~~~~~~~~~~~~~
In file included from ./bgpd/bgp_mplsvpn.h:15,
from bgpd/bgp_vty.c:48:
./bgpd/bgp_vty.h:148:12: note: previous declaration of ‘bgp_vty_return’ with type ‘int(struct vty *, int)’
148 | extern int bgp_vty_return(struct vty *vty, int ret);
| ^~~~~~~~~~~~~~
```
Fixing stuff regarding GCC13.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
We are seeing some frequent test failures with
setting the nexthop correctly. At this point
in time, I have no idea what is going wrong,
but I don't have a bunch of information either,
so let's add the local and remote values.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When trying to list advertised-routes for instance, it's not possible for now.
Relax this a bit, and allow doing this, instead of returning an error:
% Malformed address or name.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When using `addpath-tx-all` BGP announces all known paths instead of announcing
only an arbitrary number of best paths.
With this new command we can send N best paths to the neighbor. That means, we
send the best path, then send the second best path excluding the previous one,
and so on. In other words, we run best path selection algorithm N times before
we finish.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The only places entry->any could ever be set to true was
when str was NULL. Unfortunately with the way our CLI works
str is impossible to be NonNULL. The entry->any value *used*
to work prior to commit e961923c72
but it was changed back in 2016 and no-one has noticed the changed
ability.
Let's just admit that there are no users of this and remove this
dead code.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Consider the scenario of evpn, the box has some type-5 ECMP routes.
After one of its remote peers is removed ( or down ), `show evpn rmac vni all`
kept no change **sometimes**, it means the rmac of the removed peer maybe is
still in this box, and the traffic will be wrongly forwarded to the removed
peer.
The root cause is that the best path selection for type-5 routes maybe
keep no change and the best path is not routed to the removed peer, so `bgpd`
wrongly doesn't tell `zebra` to remove ( withdraw ) the type-5 routes owned
by the removed peer.
So, add a new flag to force the deletion.
Signed-off-by: anlan_cs <vic.lan@pica8.com>
When a state machine transition fails, bgpd would output
data about what happened, but not necessarily give the
reason why. Add that data to the output.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Let's account for vpn_policy_getafi having a failure
situation and back out of the whole thing gracefully.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
It is impossible for the blnc statement to ever be NULL at
line 1470 as that the if statement at 1453 guarantees it
to be set to something.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Not sure why it's here, but looks like it was since the beginning, let's see
if we can drop it.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When the remote peer is neither EBGP nor confed, aspath is the
shadow copy of attr->aspath in bgp_packet_attribute(). Striping
AS4_PATH should not be done on the aspath directly, since
that would lead to bgpd core dump when unintern the attr.
Signed-off-by: Yuan Yuan <yyuanam@amazon.com>
In CI, CLANG static analyzer started complaining about possible null
dereferences of pre-existing fields. Let's make it happy and do a null
check.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
Change CMD_WARNING -> CMD_WARNING_CONFIG_FAILED so that the rc is
non-zero and the caller can detect a failure.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
Adds a generalized martian reimport function used for triggering a
relearn/reimport of EVPN routes that were previously filtered/deleted
as a result of a "self" check (either during import or by a martian
change handler). The MAC-VRF SoO is the first consumer of this function,
but can be expanded for use with Martian Tunnel-IPs, Interface-IPs,
Interface-MACs, and RMACs.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
Currently we have a handler function that will walk the global EVPN
rib and unimport/remove routes matching a local IP/TIP. This generalizes
this function so that it can be re-used for other BGP Martian entry
types. Now this can be used to unimport routes when the MAC-VRF SoO is
reconfigured.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
For whatever reason, we were only updating tip_hash when we processed an
L2VNI add/del. This adds tip_hash updates to the L3VNI add/del codepaths
so that their VTEP-IPs are also used when when considering martian
addresses, e.g. bgp_nexthop_self().
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
All uses of struct bgp_addrv6 were removed in a prior commit that made
struct bgp_addr reusable for both v4 and v6. This cleans up the last
remnants of the old v6 struct.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
Initial support for configuring an SoO for all MAC-VRFs (EVIs/L2VNIs).
This provides a topology-independent method of preventing EVPN routes
from one MAC-VRF "site" (an L2 domain) from being imported by other PEs
in the same MAC-VRF "site", similar to how SoO is traditionally used in
L3VPN to identify and break loops for an L3/IP-VRF "site".
One example of where a MAC-VRF SoO can be used to avoid an L2 control
plane loop is with Active/Active MLAG VTEPs. For a given L2 site only
one control plane should be active. SoO can be used to ID/ignore entries
originated from the local MAC-VRF site so that EVPN will not attempt to
manage entries that are already handled by MLAG.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
bgp_create() and bgp_free() already call EVPN-specific handlers,
so there's no need to XCALLOC/XFREE BGP_EVPN_INFO directly. Let's move
all the references to MTYPE_BGP_EVPN_INFO into the EVPN specific files.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
When processing an interface up/create event from zebra, we insert that
interface's MAC address into the self_mac_hash used for dropping EVPN
routes carrying a 'self mac' (RMAC ext-comm or MAC in Type-2 NLRI).
However, we were unconditionally triggering a "rescan" of the EVPN RIB
to ensure we handle routes that match the MAC - even if the MAC already
existed in self_mac_hash (i.e. the change wasn't actionable).
This adds logic to only kick off a "rescan" if the MAC learned from
zebra is not already in the self_mac_hash.
Signed-off-by: Trey Aspelund <taspelund@nvidia.com>
This commit addresses a memory leak issue in the BGP Flowspec NLRI parsing function.
Previously when processing NLRI, dynamically allocated memory to `temp` was not being freed, leading to a memory leak.
The commit introduces the necessary code (XFREE) to properly free the temp memory after processing Flowspec NLRI.
The ASan leak log for reference:
```
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689:Direct leak of 56 byte(s) in 2 object(s) allocated from:
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #0 0x7fc9872b5037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #1 0x7fc986e5b1ee in qcalloc lib/memory.c:105
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #2 0x560421351bfe in bgp_nlri_parse_flowspec bgpd/bgp_flowspec.c:155
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #3 0x56042107d01c in bgp_nlri_parse bgpd/bgp_packet.c:350
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #4 0x560421086cf3 in bgp_update_receive bgpd/bgp_packet.c:2023
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #5 0x56042108deed in bgp_process_packet bgpd/bgp_packet.c:2933
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #6 0x7fc986f35bf7 in event_call lib/event.c:1995
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #7 0x7fc986e1e99d in frr_run lib/libfrr.c:1185
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #8 0x560420f3f59d in main bgpd/bgp_main.c:505
./bgp_flowspec.test_bgp_flowspec_topo/r1.bgpd.asan.687689- #9 0x7fc986805d09 in __libc_start_main ../csu/libc-start.c:308
```
Signed-off-by: Keelan Cannoo <keelan.cannoo@icloud.com>
In BGP, when an interface event is detected or triggered,
the BNC that have a next-hop that matches the interface
are not evaluated.
The paths attached to the bnc context are evaluated in the
following situation:
- In the up event case, if at least one next-hop interface
matched the event interface.
- In the down event case, if there is no alternate next-hop
that does not use the event interface.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This commit changes the 'ifindex' name of the bnc structure.
As it is used only to handle ipv6 link local addresses, let
us use the 'ifindex_ipv6_ll' naming to avoid any confusions
with the ifindex value of the resolved next-hops of the bnc
structure.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Reuse subgroup_process_announce_selected(). It does the same as we do here
duplicating the logic.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The output of show bgp all json is inconsistent across Address-families
i.e. ipv4/ipv6 is a no pretty format while l2vpn-evpn is in a pretty
format. For huge scale (lots of routes with lots of paths), it is better
to use no_pretty format.
Before fix:
torm-11# sh bgp all json
{
"ipv4Unicast":{
"vrfId": 0,
"vrfName": "default",
"tableVersion": 1,
"routerId": "27.0.0.15",
"defaultLocPrf": 100,
"localAS": 65000,
"routes": { } }
,
"l2VpnEvpn":{
"routes":{
"27.0.0.15:2":{
"rd":"27.0.0.15:2",
"[1]:[0]:[03:44:38:39:ff:ff:01:00:00:01]:[128]:[::]:[0]":{
"prefix":"[1]:[0]:[03:44:38:39:ff:ff:01:00:00:01]:[128]:[::]:[0]",
"prefixLen":352,
"paths":[
<SNIP>.............
After fix:
torm-11# sh bgp all json
{
"ipv4Unicast":{
"vrfId": 0,
"vrfName": "default",
"tableVersion": 1,
"routerId": "27.0.0.15",
"defaultLocPrf": 100,
"localAS": 65000,
"routes": { } }
,
"l2VpnEvpn":{
"routes":{"27.0.0.15:2":{"rd":"27.0.0.15:2","[1]:[0]:[03:44:38:39:ff:ff:01:00:00:01]:[128]:[::]:[0]":{"prefix":"[1]:[0]:[03:44:38:39:ff:ff:01:00:00:01]:[128]:[::]:[0]","prefixLen":352,"paths":[[{"valid":true,"bestpath":true,"selectionReason":"First path received","pathFrom":"external","routeType":1,"weight":32768,"peerId":"(unspec)","path":"","origin":"IGP","extendedCommunity"
<SNIP>.............
Issue: 3472865
Ticket:#3472865
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
In the json output of show bgp all json, the l2VpnEvpn afi-safi is
missing the 'routes' key making the json output format invalid.
Before Fix:
torm-11# sh bgp all json
{
<SNIP>....................
"l2VpnEvpn":{
{
"27.0.0.15:2":{
"rd":"27.0.0.15:2",
"[4]:[03:44:38:39:ff:ff:01:00:00:01]:[32]:[27.0.0.15]":{
"prefix":"[4]:[03:44:38:39:ff:ff:01:00:00:01]:[32]:[27.0.0.15]",
"prefixLen":352,
"paths":[
<SNIP>....................
After Fix:
torm-11# sh bgp all json
{
<SNIP>....................
"l2VpnEvpn":{
"routes":{
"27.0.0.15:2":{
"rd":"27.0.0.15:2",
"[1]:[0]:[03:44:38:39:ff:ff:01:00:00:01]:[128]:[::]:[0]":{
"prefix":"[1]:[0]:[03:44:38:39:ff:ff:01:00:00:01]:[128]:[::]:[0]",
"prefixLen":352,
"paths":[
Issue: 3472865
Ticket:#3472865
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
This can lead into some garbage outputs, that can't be decoded in utf-8 or so.
This was catched when testing 76b246aa1f.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Based on RFC-4760, if NEXT_HOP attribute is not
suppose to be set if MP_REACH_NLRI NLRI is used.
for IPv4 aggregate route only NEXT_HOP attribute
with ipv4 prefixlen needs to be set.
Testing Done:
Before fix:
----------
aggregate route:
*> 184.123.0.0/16 ::(TORC11) 0 32768 i
After fix:
---------
aggregate route:
*> 184.123.0.0/16 0.0.0.0(TORC11) 0 32768 i
* i peerlink-3 0 100 0 i
* uplink1 0 4435 5546 i
184.123.1.0/24 0.0.0.0(TORC11) 0 32768 i
s> 184.123.8.0/22 0.0.0.0(TORC11) 0 32768 i
Signed-off-by: Chirag Shah <chirag@nvidia.com>
In ebgp+ ibgp deployment aggregate summary-only route
selected path should always be locally originated
summary route.
When aggregate route summary-only config is removed
The selected path is iBGP peer as its lower cost
Upon reconfiguring aggregate route summary-only,
the locally originated is not selected due to
always choosing first path attribute and bailing
out as no change in route update.
Ticket:#3467890
Issue:3467890
Testing Done:
Config:
------
TORC11(config-router)#router bgp
TORC11(config-router)# address-family ipv4 unicast
TORC11(config-router-af)# aggregate-address 184.123.0.0/16
summary-only
TORC11(config-router-af)# no aggregate-address 184.123.0.0/16
summary-only
TORC11(config-router-af)# aggregate-address 184.123.0.0/16
summary-only
Before fix:
-----------
*> 184.123.0.0/16 ::(TORC11) 0 32768 i
* uplink1 0 4435 5546 i
* uplink2 0 4435 5546 i
* i peerlink-3 0 100 0 i
After fix:
----------
*> 184.123.0.0/16 ::(TORC11) 0 32768 i
* i peerlink-3 0 100 0 i
* uplink2 0 4435 5546 i
* uplink1 0 4435 5546 i
Signed-off-by: Chirag Shah <chirag@nvidia.com>
The main idea is to filter routes by matching source (originating) protocol
for outgoing direction. For instance, filter outgoing routes to an arbitrary
router that are static only. Or filter out only routes learned from RIP.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
