If we have the following configuration:
```
vrf red
smth
exit-vrf
!
interface red vrf red
smth
```
And we delete the VRF using "no vrf red" command, we end up with:
```
interface red
smth
```
Interface config is preserved but moved to the default VRF.
This is not an expected behavior. We should remove the interface config
when the VRF is deleted.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Almost all functions currently marked with pure attribute acquire a
route_node lock. By marking them pure we allow compiler to optimize the
code and not call them when it already knows the return value. This is
completely incorrect.
Only two of eleven functions can be marked as pure. And they still won't
be optimized because they are never called from the same function twice.
Let's remove the ext_pure macro completely to reduce the chance of
repeating this mistake in the future.
Fixes#8866, #8809, #8595, #6992.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This commit fixes the following problem:
- enter the interface node
- move the interface to another VRF
- try to continue configuring the interface
It is not possible to continue configuration because the XPath stored in
the vty doesn't correspond with the actual state of the system anymore.
For example:
```
nfware# conf
nfware(config)# interface enp2s0
<-- move the enp2s0 to a different VRF -->
nfware(config-if)# ip router isis 1
% Failed to get iface dnode in candidate DB
```
To fix the issue, go through all connected vty shells and update the
stored XPath.
Suggested-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Always terminate default VRF last during FRR shutdown.
On shutdown we were simply looping over the RB tree and terminating
VRFs from the ROOT. This is not guaranteed to be the default last ever.
Instead switch to RB_SAFE and skip the default VRF till the very end.
Signed-off-by: Stephen Worley <sworley@nvidia.com>
The %p printf format specifier does already print the pointer address
with a leading "0x" prefix (indicating a hexadecimal number). There's
no need to add that prefix manually.
While here, replace explicit function names in log messages by
__func__.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Fix the following address sanitizer crash when running the command `find`:
ERROR: AddressSanitizer: dynamic-stack-buffer-overflow
WRITE of size 1 at 0x7fff4840fc1d thread T0
0 in print_cmd ../lib/command.c:1541
1 in cmd_find_cmds ../lib/command.c:2364
2 in find ../vtysh/vtysh.c:3732
3 in cmd_execute_command_real ../lib/command.c:995
4 in cmd_execute_command ../lib/command.c:1055
5 in cmd_execute ../lib/command.c:1219
6 in vtysh_execute_func ../vtysh/vtysh.c:486
7 in vtysh_execute ../vtysh/vtysh.c:671
8 in main ../vtysh/vtysh_main.c:721
9 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
10 in _start (/usr/bin/vtysh+0x21f64d)
Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>
Test uses staticd which required some C++ header protections.
Additionally, the test also runs in the ubuntu20 docker container as
grpc is supported there by the packaging system.
Signed-off-by: Christian Hopps <chopps@labn.net>
When the VRF node is exited using "exit" or "quit", there's still a VRF
pointer stored in the vty context. If you try to configure some router
related command, it will be applied to the previous VRF instead of the
default VRF. For example:
```
(config)# vrf test
(config-vrf)# ip router-id 1.1.1.1
(config-vrf)# do show run
...
!
vrf test
ip router-id 1.1.1.1
exit-vrf
!
...
(config-vrf)# exit
(config)# ip router-id 2.2.2.2
(config)# do show run
...
!
vrf test
ip router-id 2.2.2.2
exit-vrf
!
...
```
`vrf-exit` works correctly, because it stores a pointer to the default
VRF into the vty context (but weirdly keeping the VRF_NODE instead of
changing it to CONFIG_NODE).
Instead of relying on the behavior of exit function, always use the
default VRF when in CONFIG_NODE.
Another problem is missing `VTY_CHECK_CONTEXT`. If someone deletes the
VRF in which node the user enters the command, then zebra applies the
command to the default VRF instead of throwing an error.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently, we output the command exactly how it is defined in DEFUN.
We shouldn't output varnames and excessive whitespace.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
https://github.com/FRRouting/frr/pull/5865#discussion_r597670225
As this comment says. ZEBRA_FLAG_XXX should not have been used.
To communicate SRv6 Route Information. A simple Nexthop Flag would
have been sufficient for SRv6 information. And I fixed the whole
thing that way.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
I agree with the PR review from mstap and merged
the following two functions into one.
- zapi_nexthop_seg6_cmp
- zapi_nexthop_seg6local_cmp
[note]
If both of them have more complex extensions in the future,
I think it will be less confusing to remove the integration
of these functions and make them as separate functions as
before.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
This commit add usuful function to configure SRv6 localsid
which is represented with seg6local lwt route.
Now, it can support only NEXTHOP_TYPE_IFINDEX route.
Actual configurationof SRv6 localsid is performed with
ZEBRA_ROUTE_ADD. So this is just a wrapper function
for route-install.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
This commit add new nexthop's addional object for SRv6
routing about seg6 route. Before this commit,
we can add MPLS info as additional object on nexthop.
This commit make it add more support about seg6 routes.
seg6 routes are ones of the LWT routing mechanism,
so configuration of seg6local routes is performed by
ZEBRA_ROUTE_SEND, it's same as MPLS configuration.
Real configuration implementation isn't implemented at
this commit. later commit add that. This commit add
only nexthop additional object and some misc functions.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
This commit is a part of #5853 works that add new structures for
SRv6-locator. This structure will be used by zebra and another
routing daemon and its ZAPI messaging to manage SRv6-locator.
Encoder/decoder for ZAPI stream is also added by this commit.
Real configuration mechanism isn't implemented at this commit.
later commit add real configure implementation. This commit add only
SRv6-locator's structures and misc functions.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
This commit is a part of #5853 that add new cmd-node for SRv6 configuration.
This commit just add cmd-node and moving node cli only, acutual SRv6 config
command isn't added. (that is added later commit. of this branch)
new cli nodes:
* SRv6
* SRv6-locators
* SRv6-locator
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
This commit is a part of #5853 works that add new nexthop's addional
object for SRv6 routing about seg6local route. Before this commit,
we can add MPLS info as additional object on nexthop.
This commit make it add more support about seg6local routes.
seg6local routes are ones of the LWT routing mechanism,
so configuration of seg6local routes is performed by
ZEBRA_ROUTE_SEND, it's same as MPLS configuration.
Real configuration implementation isn't implemented at this commit.
later commit add that. This commit add only nexthop additional object
and some misc functions.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
Make seg6local_context2str function's prototype better.
This function is added on commit e496b4203, and function
interface's considering wasn't enough.
Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>
The backoff code assumed that yang operations always completed quickly.
It checked for > 100 YANG modeled commands happening in under 1 second
to enable batching. If 100 yang modeled commands always take longer than
1 second batching is never enabled. This is the exact opposite of what
we want to happen since batching speeds the operations up.
Here are the results for libyang2 code without and with batching.
| action | 1K rts | 2K rts | 1K rts | 2K rts | 20k rts |
| | nobatch | nobatch | batch | batch | batch |
| Add IPv4 | .881 | 1.28 | .703 | 1.04 | 8.16 |
| Add Same IPv4 | 28.7 | 113 | .590 | .860 | 6.09 |
| Rem 1/2 IPv4 | .376 | .442 | .379 | .435 | 1.44 |
| Add Same IPv4 | 28.7 | 113 | .576 | .841 | 6.02 |
| Rem All IPv4 | 17.4 | 71.8 | .559 | .813 | 5.57 |
(IPv6 numbers are basically the same as iPv4, a couple percent slower)
Clearly we need this. Please note the growth (1K to 2K) w/o batching is
non-linear and 100 times slower than batched.
Notes on code: The use of the new `nb_cli_apply_changes_clear_pending`
is to commit any pending changes (including the current one). This is
done when the code would not correctly handle a single diff that
included the current changes with possible following changes. For
example, a "no" command followed by a new value to replace it would be
merged into a change, and the code would not deal well with that. A good
example of this is BGP neighbor peer-group changing. The other use is
after entering a router level (e.g., "router bgp") where the follow-on
command handlers expect that router object to now exists. The code
eventually needs to be cleaned up to not fail in these cases, but that
is for future NB cleanup.
Signed-off-by: Christian Hopps <chopps@labn.net>
