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

bgpd: fix table-map option #17802

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

askorichenko
Copy link
Contributor

Schedule zebra to withdraw routes filtered out by a table-map.

Schedule zebra to withdraw routes filtered out by a table-map.

Signed-off-by: Alexander Skorichenko <[email protected]>
@donaldsharp
Copy link
Member

can you show me the broken behavior before this change?

@askorichenko
Copy link
Contributor Author

If a neighbor shares two routes 10.0.0.1/32, 10.10.10.1/32 :

nei# show run
!
router bgp 65002
 bgp router-id 10.255.0.20
 no bgp ebgp-requires-policy
 no bgp network import-check
 neighbor 10.255.0.10 remote-as 65001
 !
 address-family ipv4 unicast
  network 10.0.0.1/32
  network 10.10.10.1/32
 exit-address-family
exit
!

And a peer has a table map to permit only 10.0.0.1/32 route

peer# show run
!
router bgp 65001
 bgp router-id 10.255.0.10
 no bgp ebgp-requires-policy
 neighbor 10.255.0.20 remote-as 65002
 !
 address-family ipv4 unicast
  table-map TableMap
 exit-address-family
exit
!
access-list al seq 5 permit 10.0.0.1/32
!
route-map TableMap permit 10
 match ip address al
exit
!
  1. Then the peer STARTS (!) correctly against the above config, and drops (ignores) the filtered out route 10.10.10.1/32
# show ip fib
...
B>* 10.0.0.1/32 [20/0] via 10.255.0.20, ...
C>* 10.255.0.0/24 is directly connected, ...
L>* 10.255.0.10/32 is directly connected, ...
  1. Let's remove and then add the table-map option back.
    A removal immediately yields:
peer# no table-map TableMap
...
peer# show ip fib
...
B>* 10.0.0.1/32 [20/0] via 10.255.0.20, ...
B>* 10.10.10.1/32 [20/0] via 10.255.0.20, ... <- a previously missing route was correctly installed
C>* 10.255.0.0/24 is directly connected, ...
L>* 10.255.0.10/32 is directly connected, ...
  1. Now set the table-map back and see that this would have NO EFFECT on the fib until we clear the session
peer# table-map TableMap
...
peer# show ip fib
...
B>* 10.0.0.1/32 [20/0] via 10.255.0.20, ...
B>* 10.10.10.1/32 [20/0] via 10.255.0.20, ... <- the route wasn't withdrawn 
C>* 10.255.0.0/24 is directly connected, ...
L>* 10.255.0.10/32 is directly connected, ...

peer# clear bgp *
peer# show ip fib
...
B>* 10.0.0.1/32 [20/0] via 10.255.0.20, ...
C>* 10.255.0.0/24 is directly connected, ...
L>* 10.255.0.10/32 is directly connected, ...

@askorichenko askorichenko marked this pull request as ready for review January 9, 2025 17:38
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 add a topotest for this case because it's seems easy to replicate.

bool is_add = true;

if (bgp->table_map[afi][safi].name) {
struct attr local_attr = *(pi->attr);
Copy link
Member

Choose a reason for hiding this comment

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

nit: Can you drop ()?

struct attr local_attr = *pi->attr;

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.

code looks okay ... waiting on topo test

@askorichenko askorichenko marked this pull request as draft January 16, 2025 11:20
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.

4 participants