8.0 backports #8827
Merged
8.0 backports #8827
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Otherwise `ospf6_lookup_by_vrf_id` returns stale pointer. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
CID 1502790 Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
There are two possible use-cases for the `vrf_bind` function: - bind socket to an interface in a vrf - bind socket to a vrf device For the former case, there's one problem - success is returned when the interface is not found. In that case, the socket is left unbound without throwing an error. For the latter case, there are multiple possible problems: - If the name is not set, then the socket is left unbound (zebra, vrrp). - If the name is "default" and there's an interface with that name in the default VRF, then the socket is bound to that interface. - In most daemons, if the router is configured before the VRF is actually created, we're trying to open and bind the socket right after the daemon receives a VRF registration from zebra. We may not receive the VRF-interface registration from zebra yet at that point. Therefore, `if_lookup_by_name` fails, and the socket is left unbound. This commit fixes all the issues and updates the function description. Suggested-by: Pat Ruddy <pat@voltanet.io> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This is not necessary anymore with fixed `vrf_bind`. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
the config for dynamic candidate paths with bandwidth preferences was using a different order of keywords (required bandwidth X) than the corresponding command (bandwidth X required). This confuses frr-reload, and possibly users too. Make both use the same order. Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>
Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
…: passive-interface Signed-off-by: anlancs <anlan_cs@tom.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
There's a padding byte between "mhop" and "peer" fields in this structure. This structure is sometimes passed by value to functions and used in assignments. The standard doesn't guarantee that the padding bytes are copied on assignments. As this structure is used as a hash key, having this padding byte with unspecified value can lead to unwanted behavior. Fix the possible issue by making the "mhop" field to be 2 bytes. Also make the struct packed as a precaution for future changes. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Remove redundant prefix. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
"ip nht resolve-via-default" is currently placed in "Link Parameters Commands" section. Add a separate section and missing IPv6 counterpart. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
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>
When enabling bfd debug from the enable mode, library debugging is not enabled. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Never send an interface name/index for multihop sessions. It breaks "neighbor A.B.C.D update-source" config in BGP. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
The TTL field is actually the number of hops, not a TTL. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
It breaks "neighbor A.B.C.D update-source" config in BGP. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
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>
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>
Currently, passive interface flag is configured from the router node using "passive-interface IFNAME". There are multiple problems with this command: - it is not in line with all other interface-related commands - other parameters are configured from the interface node using "ip ospf" prefix - it is not in line with OSPFv3 - passive flag is configured from the interface node using "ipv6 ospf6 passive" command - most importantly, it doesn't work correctly when the interface is in a different VRF - when using VRF-lite, it incorrectly changes the vrf_id of the interface and it becomes desynced with the actual state; when using netns, it creates a new fake interface and configures it instead of configuring the necessary interface To fix all the problems, this commit adds a new command to the interface configuration node - "ip ospf passive". The purpose of the command is completely the same, but it works correctly in a multi-VRF environment. The old command is preserved for the backward compatibility, but the warning is added that it is deprecated because it doesn't work correctly with VRFs. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Since a single ospfd process can have multiple OSPF interfaces configured, we need to separate the global GR initialization and termination from per-instance initialization and termination. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When exiting from the GR helper mode, recalculate the DR only for interfaces of the appropriate types (broadcast and NMBA). This fixes a problem where the state of a neighbor reachable over a p2p interface was changing from Full/DROther to Full/Backup across a graceful restart. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Change the "show_ospf_grace_lsa_info" callback to account for the fact that the "vty" parameter can be null. This fixes a crash that happens when "debug ospf packet ls-update detail" is configured and a Grace-LSA is sent or received. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
During shutdown, the ospf->maxage_lsa table is iterated over to clean up all existing entries. While doing that, route_unlock_node() should be called only for the nodes that have an associated entry, otherwise the table will get corrupted and ospfd will crash. As a side note, using a routing table to store MaxAge LSAs was a very poor choice of a data structure, considering that a simple rb-tree or hash table would get the job done with a much simpler (and less error-prone) API. Something to cleanup in the future... Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Fix usage of NSSA debug guards in code paths that have nothing to do with NSSA areas. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
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>
Add a null check to protect against the case where the neighbor inactive timer is disabled. That can happen when the router is acting as a helper for another router that is attempting to restart gracefully. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When processing bulk messages we need more space to handle more mroutes. In this case we are doubling the stream size from 16k -> 32k, which should roughly double the number of mroutes we can handle in one go. Additionally. If we cannot parse the passed message into the stream to pass up to pimd then gracefully stop processing Signed-off-by: Donald Sharp <sharpd@nvidia.com>
To reproduce the issue: 1. Create summary-address: `summary-address 1.1.1.0/24`. 2. Try to delete it with the wrong tag: `no summary-address 1.1.1.0/24 tag 1`. Each time this command is executed, route_node_lookup is called which locks route node one more time. As the tag is wrong, the function return immediately without unlock. 3. Finally delete the summary-address: `no summary-address 1.1.1.0/24`. The route node won't be deleted. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
ospf6d (and all other daemons except zebra) doesn't correctly process `interface X vrf Y`, because it doesn't know existing VRFs at the time of configuration file reading. Therefore it doesn't apply configuration provided in the interface node. Fix the problem by removing `vrf Y` part, having just an interface name is enough. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently the interface area is configured from the router node using "interface IFNAME area ID" command. There are multiple problems with this command: - it is not in line with all other interface-related commands - other parameters are configured from the interface node using "ipv6 ospf6" prefix - it is not in line with OSPFv2 - area is configured from the interface node using "ip ospf area" command - most importantly, it doesn't work correctly when the interface is in a different VRF - instead of configuring the interface, it creates a new fake interface and configuring it instead To fix all the problems, this commit adds a new command to the interface configuration node - "ipv6 ospf6 area ID". The purpose of the command is completely the same, but it works correctly in a multi-VRF environment. The old command is preserved for the backward compatibility, but the warning is added that it is deprecated because it doesn't work correctly with VRFs. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Only one test is modified yet, to have both new and deprecated commands tested by the CI. Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
polychaeta
added
bfd
bgp
bugfix
documentation
isis
libfrr
ospf
ospfv3
pim
tests
polychaeta
reviewed
Jun 10, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Pylint found errors in source files changed by this PR:
Pylint report for my_frr/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py:
************* Module check_linux_vrf
my_frr/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/check_linux_vrf.py:8:12: E0602: Undefined variable 'luLast' (undefined-variable)
-----------------------------------
Your code has been rated at 8.08/10
Pylint report for my_frr/tests/topotests/lib/common_config.py:
************* Module lib.common_config
my_frr/tests/topotests/lib/common_config.py:1639:64: E0602: Undefined variable 'unicode' (undefined-variable)
-----------------------------------
Your code has been rated at 9.97/10
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-19541/ This is a comment from an automated CI system. |
Closed
mjstapp
approved these changes
Jun 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.