Philippe Guibert [Fri, 25 Nov 2022 16:51:33 +0000 (17:51 +0100)]
bgpd: add as notation support for bgp rpki service
Rpki service uses AS number:
- some show commands use the as number as paramter.
use asnotation support.
- the as number of entries is displayed based on the
asnotation mode of the chose bgp instance.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Fri, 25 Nov 2022 11:24:15 +0000 (12:24 +0100)]
bgpd: handle network rd parameter in network configuration
The bgp network command creates static routes with an optional
route-distinguisher parameter for VPN and EVPN address families.
Store the rd parameter in those static routes. This will be used
by the 'show running-config' later.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Thu, 24 Nov 2022 15:13:08 +0000 (16:13 +0100)]
bgpd: support for route-distinguisher format with 3 fields
The ietf proposes to define a RD with a 3 field separated by
the ':' character. The last 2 fields stands for the usual
fields, namely AS4B:NN, AS2B,NNNN, IP:NN. The first field
stands for the kind of route distinguisher used.
Today, except with the route-map, no other RD configuration
supports this mode. Handle the support for this in FRR.
Philippe Guibert [Thu, 24 Nov 2022 13:02:31 +0000 (14:02 +0100)]
yang: route-distinguisher typedef for route-maps reworked
Some route-distinguisher combinations were not possible under
route-maps:
route-map rmap permit 1
match evpn rd 65540:44
match evpn rd 1.2.3.4:44
match evpn rd 2000000:44
Do not use the ietf definition for route-distinguisher by overriding
a new definition in bgp-route-map.yang itself. When the BGP northbound
API will be done, this route-distinguisher definition will have to
be used too.
Fixes: ("48cb7ea99d10") bgpd: North-bound implementation for bgp rmaps Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Tue, 22 Nov 2022 10:20:51 +0000 (11:20 +0100)]
bgpd: store the route-distinguisher from config as a string
The route-distinguisher string can be expressed in different
ways when the AS number is part of the RD. And the configured
string value has to be kept intact.
The following vty commands store the string value internally:
- router bgp / address-family ipv4 unicast / rd vpn export <>
- router bgp / address-family l2vpn evpn / rd <>
- router bgp / address-family l2vpn evpn / vni <> / rd <>
The vty commands where RD is configured in the below places is
not considered:
- router bgp / rfapi related commands
- router bgp / address-family xxx xxx / network .. rd <>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Tue, 22 Nov 2022 08:57:10 +0000 (09:57 +0100)]
bgpd: support for as notation format for route distinguisher
RD may be built based on an AS number. Like for the AS, the RD
may use the AS notation. The two below examples can illustrate:
RD 1.1:20 stands for an AS4B:NN RD with AS4B=65536 in dot format.
RD 0.1:20 stands for an AS2B:NNNN RD with AS2B=0.1 in dot+ format.
This commit adds the asnotation mode to prefix_rd2str() API so as
to pick up the relevant display.
Two new printfrr extensions are available to display the RD with
the two above display methods.
- The pRDD extension stands for dot asnotation format
- The pRDE extension stands for dot+ asnotation format.
- The pRD extension has been renamed to pRDP extension
The code is changed each time '%pRD' printf extension is called.
Possibly, the asnotation may change the output, then a macro defines
the asnotation mode to use. A side effect of forging the mode to
use is that the string could not be concatenated with other strings
in vty_out and snprintfrr. Those functions have been called multiple
times. When zlog_debug needs to display the RD with some other string,
the prefix_rd2str() old API is used instead of the printf extension.
Some code has been kept untouched:
- code related to running-config. Actually, wherever an RD is displayed,
its configured name should be dumped.
- bgp rfapi code
- bgp evpn multihoming code (partially done), since the logic is
missing to get the asnotation of 'struct bgp_evpn_es'.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Fri, 18 Nov 2022 15:02:55 +0000 (16:02 +0100)]
bgpd: store the neighbor as identifier as a string
This identifier is used to display the peer configuration in
the running-config, like it has been configured.
The following commands are using a specific string attribute:
- neighbor .. remote-as ASN
- neighbor .. local-as ASN
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Fri, 18 Nov 2022 12:49:53 +0000 (13:49 +0100)]
bgpd: modify bgp as number output
A json AS number API is created in order to output a
given AS number. In order to keep backward compatibility,
if the as-notation uses a number, then the json is encoded
as an integer, otherwise the encoding will be a string.
For what is not relevant to running-configuration, the
as-notation mode is the one used for the BGP instance.
Also, the vty completion gets the configured 'as_pretty'
string value, when an user wants to get the available
BGP instances.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Mon, 14 Nov 2022 14:56:40 +0000 (15:56 +0100)]
bgpd: aspath list format binds on as-notation format
Each BGP prefix may have an as-path list attached. A forged
string is stored in the BGP attribute and shows the as-path
list output.
Before this commit, the as-path list output was expressed as
a list of AS values in plain format. Now, if a given BGP instance
uses a specific asnotation, then the output is changed:
ubuntu2004# do show bgp ipv4
BGP table version is 2, local router ID is 10.0.2.15, vrf id 0
Default local pref 100, local AS 1.1
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
The changes include:
- the aspath structure has a new field: asnotation type
The ashash list will differentiate 2 aspaths using a different
asnotation.
- 3 new printf extensions display the as number in the wished
format: pASP, pASD, pASE for plain, dot, or dot+ format (extended).
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd: add as-notation keyword to 'router bgp' vty command
A new keyword permits changing the BGP as-notation output:
- [no] router bgp <> [vrf BLABLA] [as-notation [<dot|plain|dot+>]]
At the BGP instance creation, the output will inherit the way the
BGP instance is declared. For instance, the 'router bgp 1.1'
command will configure the output in the dot format. However, if
the client wants to choose an alternate output, he will have to
add the extra command: 'router bgp 1.1 as-notation dot+'.
Also, if the user wants to have plain format, even if the BGP
instance is declared in dot format, the keyword can also be used
for that.
The as-notation output is only taken into account at the BGP
instance creation. In the case where VPN instances are used,
a separate instance may be dynamically created. In that case,
the real as-notation format will be taken into acccount at the
first configuration.
Linking the as-notation format with the BGP instance makes sense,
as the operators want to keep consistency of what they configure.
One technical reason why to link the as-notation output with the
BGP instance creation is that the as-path segment lists stored
in the BGP updates use a string representation to handle aspath
operations (by using regexp for instance). Changing on the fly
the output needs to regenerate this string representation to the
correct format. Linking the configuration to the BGP instance
creation avoids refreshing the BGP updates. A similar mechanism
is put in place in junos too.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
AS number can be defined as an unsigned long number, or
two uint16 values separated by a period (.). The possible
valus are:
- usual 32 bit values : [1;2^32 -1]
- <1.65535>.<0.65535> for dot notation
- <0.65535>.<0.65535> for dot+ notation.
The 0.0 value is forbidden when configuring BGP instances
or peer configurations.
A new ASN type is added for parsing in the vty.
The following commands use that new identifier:
- router bgp ..
- bgp confederation ..
- neighbor <> remote-as <>
- neighbor <> local-as <>
- clear ip bgp <>
- route-map / set as-path <>
An asn library is available in lib/ and provides some
services:
- convert an as string into an as number.
- parse an as path list string and extract a number.
- convert an as number into a string.
Also, the bgp tests forge an as_zero_path, and to do that,
an API to relax the possibility to have a 0 as value is
specifically called from the tests.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Philippe Guibert [Tue, 15 Nov 2022 15:42:40 +0000 (16:42 +0100)]
bgpd: store the bgp as identifier in the configured as-notation
This is a preliminary work to handle various ways to configure
a BGP Autonomous System. When creating a BGP instance, the
user may want to define the AS number as a dotted value,
instead of using an integer value.
To handle both cases, an as_pretty char attribute will store
the as number as it has been given to the vtysh command:
router bgp <as number>
Whenever the as integer of the BGP instance was dumped,
the as_pretty original format is used.
The json output reuses the integer value to keep backward
compatibility with old displays.
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
bgpd: Intern default-originate attributes to avoid use-after-free
When we receive a default route from a peer and we originate default route
using `neighbor default-originate`, we do not track of struct attr we use,
and when we do `no neighbor default-originate` we withdraw our generated
default route, but we announce default-route from the peer.
After we do this, we unintern aspath (which was used for default-originate),
BUT it was used also for peer's default route we received.
And here we have a use-after-free crash, because bgp_process_main_one()
reaps old paths that are marked as BGP_PATH_REMOVED with aspath->refcnt > 0,
but here it's 0.
```
0 0x55c24bbcd022 in aspath_key_make bgpd/bgp_aspath.c:2070
1 0x55c24b8f1140 in attrhash_key_make bgpd/bgp_attr.c:777
2 0x7f52322e66c9 in hash_release lib/hash.c:220
3 0x55c24b8f6017 in bgp_attr_unintern bgpd/bgp_attr.c:1271
4 0x55c24ba0acaa in bgp_path_info_free_with_caller bgpd/bgp_route.c:283
5 0x55c24ba0a7de in bgp_path_info_unlock bgpd/bgp_route.c:309
6 0x55c24ba0af6d in bgp_path_info_reap bgpd/bgp_route.c:426
7 0x55c24ba17b9a in bgp_process_main_one bgpd/bgp_route.c:3333
8 0x55c24ba18a1d in bgp_process_wq bgpd/bgp_route.c:3425
9 0x7f52323c2cd5 in work_queue_run lib/workqueue.c:282
10 0x7f52323aab92 in thread_call lib/thread.c:2006
11 0x7f5232300dc7 in frr_run lib/libfrr.c:1198
12 0x55c24b8ea792 in main bgpd/bgp_main.c:520
13 0x7f5231c3a082 in __libc_start_main ../csu/libc-start.c:308
14 0x55c24b8ef0bd in _start (/usr/lib/frr/bgpd+0x2c90bd)
```
tools: Fetch more commits for commitlint to validate the commit
Note: It's necessary that you specify the fetch-depth argument to
actions/checkout@v2 step. By default they fetch only latest commit of the
branch, but we need more commits since we validate a range of commit messages.
anlan_cs [Mon, 6 Feb 2023 01:27:05 +0000 (09:27 +0800)]
bgpd: fix use-after-free crash for evpn
```
anlan(config-router-af)# vni 33
anlan(config-router-af-vni)# route-target both 44:55
anlan(config-router-af-vni)# no route-target both 44:55
vtysh: error reading from bgpd: Resource temporarily unavailable (11)Warning: closing connection to bgpd because of an I/O error!
```
When `bgp_evpn_vni_rt_cmd` deals with "both" type, it wrongly created
only one node ( should be two nodes ) for lists of both `vpn->import_rtl` and
`vpn->export_rtl`. At this time, the two lists are already wrong.
In `no route-target both RT`, it will free the single node from lists of both
`vpn->import_rtl` and `vpn->export_rtl`. After freed from `vpn->import_rtl`,
it is "use-after-free" at the time of freeing it from `vpn->export_rtl`.
It causes crash sometimes, or other unexpected behaviours.
This issue is introduced by commit `3b7e8d`, which have adjusted both
`bgp_evpn_vni_rt_cmd` and `bgp_evpn_vrf_rt_cmd`.
Since `bgp_evpn_vrf_rt_cmd/no_bgp_evpn_vrf_rt_cmd` works well again
unintentionally with commit `7022da`, only `bgp_evpn_vni_rt_cmd` needs to
modify - add two nodes for "both" type and some explicit comments for this
special case of "both" type.
Donald Sharp [Thu, 2 Feb 2023 19:13:12 +0000 (14:13 -0500)]
bgpd: Don't try to recursively hold peer io mutex
BGP was modified in a0b937de428e14e869b8541f0b7810113d619c2e
to grab the peer->io_mtx before validating the header to ensure
that the input Queue was not being modified by anyone else at that
moment in time. Unfortunately validate_header can detect a problem
and attempt to relock the mutex, which deadlocks. This deadlock in
the bgp_io pthread is the lone deadlock at first, eventually though
bgp attempts to write another packet to the peer( say when the
it's time to send the next packet ) and the main pthread of bgpd
becomes deadlocked and then the whole bgpd process is stuck at that
point in time leaving us dead in the water.
The point of locking the mutex earlier was to ensure that the input
Queue wasn't being modified by anyone else, (Say reading off it )
as that we wanted to ensure that we don't hold more packets then necessary.
Let's grab the mutex long enough to look at the input Q size, this
ensure that we have room and then we can validate_header and do the right
thing from there. We'll need to lock the mutex when we actually move it
into the input Q as well.
Fixes: #12725 Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Donald Sharp [Thu, 2 Feb 2023 15:40:07 +0000 (10:40 -0500)]
bgpd: Convert evpn output to not pretty print json
Commit: 3cdb03fba7b40240fb38469a12b7b05a11043e09
changed the vty_json output to not be pretty printing.
The previous commit in the tree added vty_json_no_pretty
let's use that instead
Donald Sharp [Thu, 2 Feb 2023 15:28:19 +0000 (10:28 -0500)]
lib, bgpd: Add ability to specify that some json output should not be pretty
Initial commit: 23b2a7ef524c9fe083b217c7f6ebaec0effc8f52
changed the json output of `show bgp <afi> <safi> json` to
not have pretty print because when under a situation where
there are a bunch of routes with a large scale ecmp show
output was taking forever and this commit cut 2 minutes out
of vtysh run time.
When upgrading to latest version the long run time was noticed
due to testing. Let's add back this functionality such that
FRR can have reduced run times with vtysh when it's really
needed.
Louis Scalbert [Thu, 2 Feb 2023 14:44:31 +0000 (15:44 +0100)]
lib: fix traffic engineering extended mask
RFC7471 and RFC8570 have defined the Extended Traffic Engineering
metrics that are carried within TLV of 32 bits data length. Extended
metrics, excepting bandwidth ones, use the following format:
Data contains a flag/reserved of 8 bits and a 24 bits value.
The TE_EXT_MASK mask macro extracts a 28 bits value from a 32 bits
variable instead of 24 bits. It works in most of the case because
RESERVED bits are generally set to 0.
Fix the TE_EXT_MASK mask.
Fixes: 16f1b9ee29 ("Update Traffic Engineering Support for OSPFD") Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Donatas Abraitis [Tue, 24 Jan 2023 08:32:13 +0000 (10:32 +0200)]
bgpd: Set attr to NULL when passing NLRI_UPDATE with treat-as-withdraw
Before this patch, we always passed `struct attr` for NLRI_UPDATE, but if we
have a situation with treat-as-withdraw (for example: malformed attribute, or
using a command like `neighbor path-attribute treat-as-withdraw`) the route
MUST be withdrawn form the BGP table.
Hence, we MUST pass attr as NULL, in this case we already have this check
under NLRI_ATTR_ARG() macro, just reuse it properly.
Donald Sharp [Mon, 30 Jan 2023 15:12:06 +0000 (10:12 -0500)]
*: Add -Wswitch-enum to build
The -Wswitch-enum will allow the compiler to warn us
when a developer creates a switch over a enum and is
using `default:` when they should be iterating over
every enum
Donald Sharp [Fri, 27 Jan 2023 13:10:08 +0000 (08:10 -0500)]
bgpd: ecommunity_token_rt6 is not handled
The function ecommunity_str2com_internal appears to want to handle
the ecommunity_token_rt6 enum but skips over it. Commit 9a659715dfcb6c0b1e3ef8004b6c9d14c55f2081 tried to add this but I really
don't see how this is going to behave correctly. Add the
ecommunity_token_rt6 case to the switch statement so it is handled
appropriately?
projects / matthieu / frr.git / log