Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zebra: Do not flush an existing vni configuration trying to remove wrong vni #18108

Merged

Conversation

ton31337
Copy link
Member

Closes #17952

@piotrsuchy
Copy link
Contributor

piotrsuchy commented Feb 12, 2025

Hey @ton31337, thanks for this PR this does indeed fix the issue I described, however there is one edge case:

root@bd6e65241d7a:/# vtysh

Hello, this is FRRouting (version 10.4-dev).
Copyright 1996-2005 Kunihiro Ishiguro, et al.

bd6e65241d7a# configure
bd6e65241d7a(config)# vni 1
bd6e65241d7a(config)# sh run
% Unknown command: sh run
bd6e65241d7a(config)# do sh run
Building configuration...

Current configuration:
!
frr version 10.4-dev
frr defaults traditional
hostname bd6e65241d7a
log syslog informational
vni 1
no ipv6 forwarding
service integrated-vtysh-config
!
end
bd6e65241d7a(config)# no vni 2
bd6e65241d7a(config)# do sh run
Building configuration...

Current configuration:
!
frr version 10.4-dev
frr defaults traditional
hostname bd6e65241d7a
log syslog informational
vni 1
no ipv6 forwarding
service integrated-vtysh-config
!
end
#### EVERYTHING CORRECT, THIS PATCH FIXES THE ABOVE SCENARIO, HOWEVER:

bd6e65241d7a(config)# vrf vrf-test
bd6e65241d7a(config-vrf)# vni 2
bd6e65241d7a(config-vrf)# exit
bd6e65241d7a(config)# do show run
Building configuration...

Current configuration:
!
frr version 10.4-dev
frr defaults traditional
hostname bd6e65241d7a
log syslog informational
vni 1
no ipv6 forwarding
service integrated-vtysh-config
!
vrf vrf-test
 vni 2
exit-vrf
!
end
bd6e65241d7a(config)# no vni 2
bd6e65241d7a(config)# do show run
Building configuration...

Current configuration:
!
frr version 10.4-dev
frr defaults traditional
hostname bd6e65241d7a
log syslog informational
no ipv6 forwarding
service integrated-vtysh-config
!
vrf vrf-test
 vni 2
exit-vrf
!
end

If there is a vrf with that VNI it will still delete the default one, without checking if that's the one specified, so it's like a false positive. We say 'delete 2', it deletes the default one, so '1' in this case, because there is a '2' in the vrf. (the value has to match the value in the VRF). Do you think there is a way to guard against that as well? Because that seems to be our case (in an environment where there are a lot of vrfs).

…ong vni

Before:

```
pc.donatas.net(config)# do sh run | include vni
vni 1
pc.donatas.net(config)# no vni 2
pc.donatas.net(config)# do sh run | include vni
pc.donatas.net(config)#
```

Signed-off-by: Donatas Abraitis <[email protected]>
@ton31337 ton31337 force-pushed the fix/zebra_no_vni_validation branch from d8161fd to 44fe398 Compare February 12, 2025 21:37
@ton31337
Copy link
Member Author

@piotrsuchy could you retest with the latest changes?

@piotrsuchy
Copy link
Contributor

It does work exactly how I thought it should now! Thanks!

@donaldsharp donaldsharp merged commit 66434fc into FRRouting:master Feb 19, 2025
13 checks passed
@donaldsharp
Copy link
Member

@mergifiyio backport dev/10.3 stable/10.0 stable/10.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deleting the default VNI instead of saying a VNI like this doesn't exist for 'no vni <VNI>' vtysh command
3 participants