Merged
Conversation
ghost
force-pushed
the
fix_remove_ip_rif
branch
from
d1798b3 to
20b760d
Compare
ghost
marked this pull request as ready for review
Author
ghost
force-pushed
the
fix_remove_ip_rif
branch
2 times, most recently
from
27e4149 to
3b445c9
Compare
Author
|
@prsunny, I have fixed the unit tests. Could you review the PR? |
prsunny
reviewed
Apr 14, 2021
config/main.py
Outdated
Contributor
There was a problem hiding this comment.
why all? why not just show ip route static. Lets optimize this as best as possible since this could potentially introduce further delays in this command.
Author
There was a problem hiding this comment.
vrf all is required to check scopes of all the VRFs present in the system, because command show ip route shows routes only in scope of default VRF.
Good point regarding static prefix, I have updated and retested the changes. Thank you.
Please, take a look on the difference between outputs of commands show ip route static and show ip route vrf all static. That's why I have added vrf all:
admin@sonic:~$ show ip route vrf all static
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
VRF Vrf11:
S 20.0.0.144/32 [1/0] is directly connected, Ethernet68 inactive, weight 1, 00:00:18
VRF default:
S>* 0.0.0.0/0 [200/0] via 192.168.111.3, eth0, weight 1, 23:09:50
S 20.0.0.1/32 [1/0] is directly connected, Ethernet64 inactive, weight 1, 00:01:14
is directly connected, Ethernet68 (vrf Vrf11) inactive, weight 1, 00:01:14
admin@sonic:~$ show ip route static
Codes: K - kernel route, C - connected, S - static, R - RIP,
O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
F - PBR, f - OpenFabric,
> - selected route, * - FIB route, q - queued, r - rejected, b - backup
S>* 0.0.0.0/0 [200/0] via 192.168.111.3, eth0, weight 1, 23:10:01
S 20.0.0.1/32 [1/0] is directly connected, Ethernet64 inactive, weight 1, 00:01:25
is directly connected, Ethernet68 (vrf Vrf11) inactive, weight 1, 00:01:25
added 2 commits
April 15, 2021 12:47
* Check whether a static route present, which is related to the RIF of the interface for which we want to delete the last IP entry. This is required to prevent deleting of RIFs, if there are static routes in the system. Signed-off-by: Maksym Belei <[email protected]>
* Update unit tests according to the changes. Signed-off-by: Maksym Belei <[email protected]>
ghost
force-pushed
the
fix_remove_ip_rif
branch
from
3b445c9 to
37a2e87
Compare
prsunny
approved these changes
Apr 23, 2021
Contributor
|
@maksymbelei95, @prsunny. Should we add a check to disallow configuring of static route with nexthop as ifname if the interface has no ip address? |
Contributor
Author
|
@prsunny, if there is no active questions regarding this PR, could you merge it? |
gitsabari
pushed a commit
to gitsabari/sonic-utilities
that referenced
this pull request
Jun 15, 2021
*Added checking of static routes, related to the interface, before deleting of the last IP entry to prevent deleting the RIF if a static route is present in the system. Signed-off-by: Maksym Belei <[email protected]>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What I did
Resolves sonic-net/sonic-buildimage#7028
Added checking of static routes, related to the interface, before deleting of the last IP entry to prevent deleting the RIF if a static route is present in the system.
The related unit tests has added.
How I did it
By checking IPv4 and IPv6 routes, received from Zebra.
How to verify it
sudo config int ip add Ethernet2 10.0.0.18/31sudo config route add prefix 20.0.0.1/32 nexthop Ethernet2sudo config int ip rem Ethernet2 10.0.0.18/31If
10.0.0.18/31is the last IP entry, you will receive the next error:Error: Cannot remove the last IP entry of interface Ethernet2. A static ip route is still bound to the RIF.The similar situation is for IPv6:
Error: Cannot remove the last IP entry of interface Ethernet2. A static ipv6 route is still bound to the RIF.