Skip to content

bgpd: fix to show exist/non-exist-map in 'show run' properly #18828

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

Merged
merged 2 commits into from
May 20, 2025

Conversation

krishna-samy
Copy link
Contributor

Currently, peergroup_filter_check() does not check whether exist-map or non-exist-map is configured along with advertise-map. This check is missing only when the peer is part of peergroup and having the exist/non-exist-map. So the 'show run' does not show the configured exist/non-exist-map as expected.

This new check is needed because, unlike other filter type the adv-map can have exist/non-exist-map additionally and we don't store this in the filter_override but store only the adv-map. So, a specific check is required to account the exist/non-exist-map while printing the adv-map config. Fixing the same by adding a check.

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a topotest verifying this.

@ton31337
Copy link
Member

Can you also show this how it looks in show running?

@krishna-samy
Copy link
Contributor Author

Before fix:

r1(config-router-af)# neighbor 192.168.252.2 advertise-map v6_GLOBAL non-exist-map Temp
r1(config-router-af)# do sh run
Building configuration...
…

 neighbor 192.168.252.2 peer-group PG2
 !
 address-family ipv4 unicast
  neighbor 192.168.252.2 advertise-map v6_GLOBAL exist-map Temp
 exit-address-family
exit

UT after fix:

r1(config-router-af)# neighbor 192.168.252.2 advertise-map v6_GLOBAL non-exist-map Temp
r1(config-router-af)# do sh run
Building configuration...
…

 neighbor 192.168.252.2 peer-group PG2
 !
 address-family ipv4 unicast
  neighbor 192.168.252.2 advertise-map v6_GLOBAL non-exist-map Temp
 exit-address-family
exit


 
r1(config-router-af)# neighbor 192.168.252.2 advertise-map v6_GLOBAL exist-map Temp
r1(config-router-af)# do sh run
Building configuration...
…

 neighbor 192.168.252.2 peer-group PG2
 !
 address-family ipv4 unicast
  neighbor 192.168.252.2 advertise-map v6_GLOBAL exist-map Temp
 exit-address-family
exit
!
end

@ton31337
Copy link
Member

@Mergifyio backport stable/10.3 stable/10.2 stable/10.1 stable/10.0

Copy link

mergify bot commented May 17, 2025

backport stable/10.3 stable/10.2 stable/10.1 stable/10.0

✅ Backports have been created

@ton31337
Copy link
Member

@Mergifyio rebase

Currently, peergroup_filter_check() does not check whether exist-map or
non-exist-map is configured along with advertise-map. This check is
missing only when the peer is part of peergroup and having the
exist/non-exist-map. So the 'show run' does not show the
configured exist/non-exist-map as expected.

This new check is needed because, unlike other filter type the adv-map can have
exist/non-exist-map additionally and we don't store this in the
filter_override but store only the adv-map. So, a specific check is
required to account the exist/non-exist-map while printing the adv-map config.
Fixing the same by adding a check.

Signed-off-by: Krishnasamy <[email protected]>
Copy link

mergify bot commented May 19, 2025

rebase

✅ Branch has been successfully rebased

@ton31337 ton31337 force-pushed the krishna-samy/exist-map branch from aff1639 to aa012d8 Compare May 19, 2025 07:42
@frrbot frrbot bot added the tests Topotests, make check, etc label May 19, 2025
@github-actions github-actions bot added size/M and removed size/S labels May 19, 2025
@krishna-samy
Copy link
Contributor Author

Please create a topotest verifying this.

done

@krishna-samy
Copy link
Contributor Author

Can you also show this how it looks in show running?

done

@ton31337
Copy link
Member

Please fix frrbot (python styling) and good to merge.

@krishna-samy krishna-samy force-pushed the krishna-samy/exist-map branch from 25a2c43 to 5b7a0a1 Compare May 20, 2025 08:03
@krishna-samy krishna-samy force-pushed the krishna-samy/exist-map branch from 5b7a0a1 to f15cb73 Compare May 20, 2025 08:04
@krishna-samy
Copy link
Contributor Author

Please fix frrbot (python styling) and good to merge.

done.

Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@riw777 riw777 requested a review from ton31337 May 20, 2025 12:47
@ton31337 ton31337 merged commit 5e5cf4d into FRRouting:master May 20, 2025
13 checks passed
donaldsharp added a commit that referenced this pull request May 22, 2025
bgpd: fix to show exist/non-exist-map in 'show run' properly (backport #18828)
donaldsharp added a commit that referenced this pull request May 22, 2025
bgpd: fix to show exist/non-exist-map in 'show run' properly (backport #18828)
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.

3 participants