Currently, in `zebra_srte_client_close_cleanup` we use the `RB_FOREACH`
macro to traverse the SR policies tree. We remove the SR policies within
the loop. Removing elements from the tree and freeing them is not safe
and causes a use-after-free crash whenever the
`zebra_srte_client_close_cleanup` is called to perform cleanup.
This commit replaces the `RB_FOREACH` macro with its variant
`RB_FOREACH_SAFE`. Unlike `RB_FOREACH`, `RB_FOREACH_SAFE` permits both
the removal of tree elements as well as freeing them from within the
loop safely.
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
If you have this order in your configuration file:
no fpm use-next-hop-groups
fpm address 127.0.0.1
the dplane code was using the same event thread t_event and the second
add event in the code was going, you already have an event scheduled
and as such the second event does not overwrite it. Leaving
no code to actually start the whole processing. There are probably
other cli iterations that will cause this fun as well, but I'm
not going to spend the time sussing them out at the moment.
Fixes: #12314
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The API for configuring ES in zebra had a strict check for if_type
"isBond" that prevented the ES config from being created before the
interface.
Ticket: CM-29454
Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
Install a new command `behavior usid` into the `SRV6_LOC_NODE` CLI node.
This command allows the user to set/unset the `SRV6_LOCATOR_USID` flag
for an SRv6 locator. The `SRV6_LOCATOR_USID` flag indicates whether a
locator is a uSID locator or not. When the flag is set, the routing
daemons (e.g., bgpd) will install SRv6 behaviors with the uSID in the
dataplane.
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
In this commit, we add two helper functions
`zebra_notify_srv6_locator_add` and `zebra_notify_srv6_locator_delete`.
These functions are used to notify locator additions/deletions to
zclients.
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
Fix the following build failure raised since version 8.4 and
d53dc9bd81:
zebra/netconf_netlink.c: In function 'netlink_netconf_change':
zebra/netconf_netlink.c:109:32: error: 'AF_MPLS' undeclared (first use in this function)
109 | if (ncm->ncm_family == AF_MPLS)
| ^~~~~~~
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
In file included from /usr/include/net/ethernet.h:10,
from ./lib/prefix.h:26,
from zebra/tc_netlink.c:32:
/usr/include/netinet/if_ether.h:115:8: error: redefinition of 'struct ethhdr'
115 | struct ethhdr {
| ^~~~~~
In file included from zebra/tc_netlink.c:28:
/usr/include/linux/if_ether.h:169:8: note: originally defined here
169 | struct ethhdr {
| ^~~~~~
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
At this point add abilty for the encode/decode of the
resilience down ZAPI to zebra. Just hookup sharpd
at this point in time.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
FRR does not use the NLM_F_APPEND semantics ( in fact I would argue that
the NLM_F_APPEND semantics just introduce pain for all parties involved )
I would also argue that most people who use the kernel netlink api
have recognized that NLM_F_APPEND for a route is a recipe for disaster
that is well documented and as such it is not used as anything other
than a curiousity by operators.
See:
https://bugzilla.redhat.com/show_bug.cgi?id=1337855https://github.com/thom311/libnl/issues/226
Are 2 great examples of how confusing it is for anyone in user
space to know what the correct thing to do is. Given that
new fields can be added with no semantics to allow us to know
what has resulted in a change or not.
In an attempt to recognize this, let's note that FRR
believes it has gotten out of sync with the kernel.
Future commits will react to the desynchronized route
and request from the kernel a reload of that specific
route if possible.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Use "get" as the name for checking the status of the bgp
accept lower seq knob. This already has an equivalent "set"
so makes sense to keep it consistent.
Signed-off-by: Stephen Worley <sworley@nvidia.com>
In this commit, we extend the ZAPI to support encoding and decoding the
locator flags contained in the messages exchanged between zebra and the
routing daemons.
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
When zebra receives routes from upper level protocols it decodes the
zapi message and places the routes on the metaQ for processing. Suppose
we have a route A that is already installed by some routing protocol.
And there is a route B that has a nexthop that will be recursively
resolved through A. Imagine if a route replace operation for A is
going to happen from an upper level protocol at about the same time
the route B is going to be installed into zebra. If these routes
are received, and decoded, at about the same time there exists a
chance that the metaQ will contain both of them at the same time.
If the order of installation is [ B, A ]. B will be resolved
correctly through A and installed, A will be processed and
re-installed into the FIB. If the nexthops have changed for
A then the owner of B should be notified about the change( and B
can do the correct action here and decide to withdraw or re-install ).
Now imagine if the order of routes received for processing on the
metaQ is [ A, B ]. A will be received, processed and sent to the
dataplane for reinstall. B will then be pulled off the metaQ and
fail the install since A is in a `not Installed` state.
Let's loosen the restriction in nexthop resolution for B such
that if the route we are dependent on is a route replace operation
allow the resolution to suceed. This requires zebra to track a new
route state( ROUTE_ENTRY_ROUTE_REPLACING ) that can be looked at
during nexthop resolution. I believe this is ok because A is
a route replace operation, which could result in this:
-route install failed, in which case B should be nht'ing and
will receive the nht failure and the upper level protocol should
remove B.
-route install succeeded, no nexthop changes. In this case
allowing the resolution for B is ok, NHT will not notify the upper
level protocol so no action is needed.
-route install succeeded, nexthops changes. In this case
allowing the resolution for B is ok, NHT will notify the upper
level protocol and it can decide to reinstall B or not based
upon it's own algorithm.
This set of events was found by the bgp_distance_change topotest(s).
Effectively the tests were looking for the bug ( A, B order in the metaQ )
as the `correct` state. When under very heavy load, the A, B ordering
caused A to just be installed and fully resolved in the dataplane before
B is gotten to( which is entirely possible ).
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Rather than running selected source files through the preprocessor and a
bunch of perl regex'ing to get the list of all DEFUNs, use the data
collected in frr.xref.
This not only eliminates issues we've been having with preprocessor
failures due to nonexistent header files, but is also much faster.
Where extract.pl would take 5s, this now finishes in 0.2s. And since
this is a non-parallelizable build step towards the end of the build
(dependent on a lot of other things being done already), the speedup is
actually noticeable.
Also files containing CLI no longer need to be listed in `vtysh_scan`
since the .xref data covers everything. `#ifndef VTYSH_EXTRACT_PL`
checks are equally obsolete.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The debug for notification about a filtered prefix was
just printing the nexthop ifindex and vrf id. Not all
nexthops have this data. Just print out the actual nexthop
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Currently, the SID transposition algorithm implemented in bgpd handles
incorrectly the SRv6 locators with function length greater than 20 bits.
To prevent issues, we currently limit the function length to 20 bits.
This limit will be removed when the bgpd SID transposition is fixed.
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
According to RFC 8986, the SRv6 SID length cannot exceed 128 bits. This
commit ensures that the condition
`block_len + node_len + function_len + arg_len <= 128` is satisfied when
a new SRv6 locator is created.
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
This commit adds SRv6 locator's block length, node length and argument
length to the output of the command
"show segment-routing srv6 locator NAME detail [json]".
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
This commit adds the SRv6 locator's block length, node length and
argument length to the SRv6 configuration.
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
In the current implementation, an SRv6 locator only supports the
following structure:
* node-len = 24
* block-len = prefix-len - 24
* function-len = <configurable>
* argument-len = 0
This commit adds two optional arguments to the locator_prefix CLI
command: "node-len" and "block-len". These arguments allows an user to
configure the block length and node length of a SRv6 locator according
to the following logic:
* the node-len + block-len = prefix-len constraint must always be
satisfied;
* if node-len and block-len are both omitted, they are calculated as in
the current implementation (for backward compatibility reasons)
* if node-len is omitted, its value is computed as
prefix-len - block-len
* if block-len is omitted, its value is computed as
prefix-len - node-len
Signed-off-by: Carmine Scarpitta <carmine.scarpitta@uniroma2.it>
Daemons like isisd continue to use the previous link-params after they
are removed from zebra.
For example,
>r0# sh run zebra
> (...)
> interface eth-rt1
> link-params
> enable
> metric 100
> exit-link-params
> r0# conf
> r0(config)# interface eth-rt1
> r0(config-if)# link-params
> r0(config-link-params)# no enable
After "no enable", "sh run zebra" displays no more link-params context.
The "no enable" causes the release of the "link_params" pointer within
the "interface" structure. The zebra function to update daemons with
a ZEBRA_INTERFACE_LINK_PARAMS zapi message is called but the function
returns without doing anything because the "link_params" pointer is
NULL. Therefore, the "link_params" pointers are kept in daemons.
When the zebra "link_params" pointer is NULL:
- Send a zapi link param message that contains no link parameters
instead of sending no message.
- At reception in daemons, the absence of link parameters causes the
release of the "link_params" pointer.
Fixes: 16f1b9e ("Update Traffic Engineering Support for OSPFD")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
A given interface has no enabled link-params context. If a link-params
configuration command fails, the link-params is wrongly enabled:
> r4(config-link-params)# no enable
> r4(config-link-params)# delay
> (0-16777215) Average delay in micro-second as decimal (0...16777215)
> r4(config-link-params)# delay 50 min 300 max 500
> Average delay should be comprise between Min (300) and Max (500) delay
> r4(config-link-params)# do sh run zebra
> (...)
> interface eth-rt1
> link-params
> enable
> exit-link-params
link-params are enabled if and only if the interface structure has a
valid link_params pointer. Before checking the command validity,
if_link_params_get() is called to retrieve the link-params pointer.
However, this function initializes the pointer if it is NULL.
Only use if_link_params_get() to retrieve the pointer to avoid
confusion. In command setting functions, initialize the link_params
pointer if needed only after the validation of the command.
Fixes: 16f1b9e ("Update Traffic Engineering Support for OSPFD")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Add some neigh deletion debugs for when the neigh isn't
found or there is a MAC mismatch on what was sent and found.
Signed-off-by: Stephen Worley <sworley@nvidia.com>
Re-work the bgp vni table to use separately keyed tables for type2
routes.
So, with type2 routes, we have the main table keyed off of the IP and a
new MAC table keyed off of MACs.
By separating out the two, we are able to run path selection separately
for the neigh and mac. Keeping the two separate is also more in-line
with what happens in zebra (they are managed comptletely seperate).
With this change type2 routes go into each table like so:
```
Remote MAC-IP -> IP Table & MAC Table
Remote MAC -> MAC Table
Local MAC-IP -> IP Table
Local MAC -> MAC Table
```
The difference for local is necessary because we should not ever allow
multiple paths for a local MAC.
Also cleaned up the commands for querying the vni tables:
```
show bgp vni all type ...
show bgp vni VNI type ...
```
Old commands will be deprecated in a separate commit.
Signed-off-by: Stephen Worley <sworley@nvidia.com>
Add some special handling to accept lower seq routes for local
known routes when not ready. This aligns the code back a bit more
to where it was before to fix seen issues with sync routes.
Signed-off-by: Stephen Worley <sworley@nvidia.com>
Add a knob to accept lower seq number in evpn updates
from BGP in Zebra.
Note: Knob is enabled by default
Signed-off-by: Stephen Worley <sworley@nvidia.com>
There are lib debugs being set but never show up in
`show debug` commands because there was no way to show
that they were being used. Add a bit of infrastructure
to allow this and then use it for `debug route-map`
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
In order to set function length of SID freely, this PR relieves the
lower limitation of `func-bits`.
Signed-off-by: Ryoga Saito <ryoga.saito@linecorp.com>
Add default SRv6 Function Length for usecases like SRv6 L3VPN. The
default value (16) comes from the default Function length for SRv6
L3VPN in BGPd.
Signed-off-by: Ryoga Saito <ryoga.saito@linecorp.com>
