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

When filter removal does not empty access-list, restore its name #6

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

Conversation

vdavidoff
Copy link

Preface:

  1. I find it odd this bug would exist for over 4 years, so I wonder if I'm missing something
  2. I'm not a C developer
  3. The diff here seems to work properly but there may be stuff I'm missing (see (2))

Details:

On Ubuntu 20.04 with quagga 1.2.4-4build1 I observed that removing a single filter from an access list with a command in the form of no access-list $list_name permit $ip/32 resulted in the remaining filters in the list being migrated to a (null) access-list and the access-list being operated on vanished. This was confirmed with quagga 1.2.4 on FreeBSD 12 as well, just to try to rule out anything Ubuntu or Linux.

I believe this bug was introduced in commit cfbdd869687dc076256bce455d03b2ccf04b4a77. It seems that basically what's happening is that inside access_list_filter_delete the name of the access list being modified is set to NULL so that it won't be found during lookups while it's being manipulated, but in the case where the list is not fully empty after manipulation, the name is not set back which results in the list effectively being renamed to NULL.

# show running-config 
Building configuration...                
                                                                                  
Current configuration:                   
!                                        
log file /var/log/quagga/bgpd.log        
!                                        
router bgp 64681                         
 bgp router-id 10.7.141.40               
 redistribute connected                  
 neighbor UPSTREAM peer-group            
 neighbor UPSTREAM remote-as 65101       
 neighbor UPSTREAM next-hop-self                   
 neighbor UPSTREAM route-map NOTHING in  
 neighbor UPSTREAM route-map VIPS out    
 neighbor 10.7.141.2 peer-group UPSTREAM 
 neighbor 10.7.141.3 peer-group UPSTREAM 
!                                        
 address-family ipv6                     
 exit-address-family                     
 exit                                    
!                                        
access-list VIPS_0 permit 10.7.240.194/32
access-list VIPS_0 permit 10.7.240.197/32
access-list VIPS_0 permit 10.7.240.200/32
access-list VIPS_0 permit 10.7.240.204/32
access-list VIPS_0 permit 10.7.240.206/32
access-list VIPS_0 permit 10.7.240.209/32
access-list VIPS_0 permit 10.7.240.211/32
access-list VIPS_0 permit 10.7.240.220/32
access-list VIPS_0 permit 10.7.240.223/32
access-list VIPS_0 permit 10.7.240.226/32
access-list VIPS_0 permit 10.7.240.229/32
access-list VIPS_0 permit 10.7.240.232/32
access-list VIPS_0 permit 10.7.240.235/32
access-list VIPS_0 permit 10.7.240.238/32
access-list VIPS_0 permit 10.7.240.241/32
access-list VIPS_0 permit 10.7.240.244/32                                                                                                                            
access-list VIPS_0 permit 10.7.240.247/32  
access-list VIPS_0 permit 10.7.240.251/32          
access-list VIPS_0 permit 10.7.240.254/32
access-list VIPS_1 permit 10.7.240.193/32                                         
access-list VIPS_1 permit 10.7.240.196/32
access-list VIPS_1 permit 10.7.240.199/32
access-list VIPS_1 permit 10.7.240.202/32
access-list VIPS_1 permit 10.7.240.203/32
access-list VIPS_1 permit 10.7.240.208/32
access-list VIPS_1 permit 10.7.240.210/32
access-list VIPS_1 permit 10.7.240.219/32
access-list VIPS_1 permit 10.7.240.222/32
access-list VIPS_1 permit 10.7.240.225/32
access-list VIPS_1 permit 10.7.240.228/32
access-list VIPS_1 permit 10.7.240.231/32
access-list VIPS_1 permit 10.7.240.234/32
access-list VIPS_1 permit 10.7.240.237/32
access-list VIPS_1 permit 10.7.240.240/32
access-list VIPS_1 permit 10.7.240.243/32
access-list VIPS_1 permit 10.7.240.246/32
access-list VIPS_1 permit 10.7.240.249/32
access-list VIPS_1 permit 10.7.240.253/32
access-list VIPS_2 permit 10.7.240.195/32
access-list VIPS_2 permit 10.7.240.198/32
access-list VIPS_2 permit 10.7.240.201/32
access-list VIPS_2 permit 10.7.240.205/32
access-list VIPS_2 permit 10.7.240.207/32
access-list VIPS_2 permit 10.7.240.212/32
access-list VIPS_2 permit 10.7.240.218/32
access-list VIPS_2 permit 10.7.240.221/32
access-list VIPS_2 permit 10.7.240.224/32
access-list VIPS_2 permit 10.7.240.227/32
access-list VIPS_2 permit 10.7.240.230/32
access-list VIPS_2 permit 10.7.240.233/32
access-list VIPS_2 permit 10.7.240.236/32
access-list VIPS_2 permit 10.7.240.242/32
access-list VIPS_2 permit 10.7.240.245/32
access-list VIPS_2 permit 10.7.240.248/32
access-list VIPS_2 permit 10.7.240.252/32
!
route-map NOTHING deny 10
!
route-map VIPS permit 10
 match ip address VIPS_0
 set community 65101:100
 set metric 1
!
route-map VIPS permit 20
 match ip address VIPS_1
 set community 65101:125
 set metric 1
!
route-map VIPS permit 30
 match ip address VIPS_2
 set community 65101:150
 set metric 1
!
route-map VIPS deny 40
!
line vty
!
end


# show ip access-list 
BGP:
Zebra IP access list VIPS_0
    permit 10.7.240.194/32
    permit 10.7.240.197/32
    permit 10.7.240.200/32
    permit 10.7.240.204/32
    permit 10.7.240.206/32
    permit 10.7.240.209/32
    permit 10.7.240.211/32
    permit 10.7.240.220/32
    permit 10.7.240.223/32
    permit 10.7.240.226/32
    permit 10.7.240.229/32
    permit 10.7.240.232/32
    permit 10.7.240.235/32
    permit 10.7.240.238/32
    permit 10.7.240.241/32
    permit 10.7.240.244/32
    permit 10.7.240.247/32
    permit 10.7.240.251/32
    permit 10.7.240.254/32
Zebra IP access list VIPS_1
    permit 10.7.240.193/32
    permit 10.7.240.196/32
    permit 10.7.240.199/32
    permit 10.7.240.202/32
    permit 10.7.240.203/32
    permit 10.7.240.208/32
    permit 10.7.240.210/32
    permit 10.7.240.219/32
    permit 10.7.240.222/32
    permit 10.7.240.225/32
    permit 10.7.240.228/32
    permit 10.7.240.231/32
    permit 10.7.240.234/32
    permit 10.7.240.237/32
    permit 10.7.240.240/32
    permit 10.7.240.243/32
    permit 10.7.240.246/32
    permit 10.7.240.249/32
    permit 10.7.240.253/32
Zebra IP access list VIPS_2
    permit 10.7.240.195/32
    permit 10.7.240.198/32
    permit 10.7.240.201/32
    permit 10.7.240.205/32
    permit 10.7.240.207/32
    permit 10.7.240.212/32
    permit 10.7.240.218/32
    permit 10.7.240.221/32
    permit 10.7.240.224/32
    permit 10.7.240.227/32
    permit 10.7.240.230/32
    permit 10.7.240.233/32
    permit 10.7.240.236/32
    permit 10.7.240.242/32
    permit 10.7.240.245/32
    permit 10.7.240.248/32
    permit 10.7.240.252/32


# conf t
# no access-list VIPS_2 permit 10.7.240.252/32
# exit


# show running-config                                                                                                                                                                                                                                                                               [77/97669]
Building configuration...

Current configuration:
!
log file /var/log/quagga/bgpd.log
!
router bgp 64681
 bgp router-id 10.7.141.40
 redistribute connected
 neighbor UPSTREAM peer-group
 neighbor UPSTREAM remote-as 65101
 neighbor UPSTREAM next-hop-self
 neighbor UPSTREAM route-map NOTHING in
 neighbor UPSTREAM route-map VIPS out
 neighbor 10.7.141.2 peer-group UPSTREAM
 neighbor 10.7.141.3 peer-group UPSTREAM
!
 address-family ipv6
 exit-address-family
 exit
!
access-list VIPS_0 permit 10.7.240.194/32
access-list VIPS_0 permit 10.7.240.197/32
access-list VIPS_0 permit 10.7.240.200/32
access-list VIPS_0 permit 10.7.240.204/32
access-list VIPS_0 permit 10.7.240.206/32
access-list VIPS_0 permit 10.7.240.209/32
access-list VIPS_0 permit 10.7.240.211/32
access-list VIPS_0 permit 10.7.240.220/32
access-list VIPS_0 permit 10.7.240.223/32
access-list VIPS_0 permit 10.7.240.226/32
access-list VIPS_0 permit 10.7.240.229/32
access-list VIPS_0 permit 10.7.240.232/32
access-list VIPS_0 permit 10.7.240.235/32
access-list VIPS_0 permit 10.7.240.238/32
access-list VIPS_0 permit 10.7.240.241/32
access-list VIPS_0 permit 10.7.240.244/32
access-list VIPS_0 permit 10.7.240.247/32
access-list VIPS_0 permit 10.7.240.251/32
access-list VIPS_0 permit 10.7.240.254/32
access-list VIPS_1 permit 10.7.240.193/32
access-list VIPS_1 permit 10.7.240.196/32
access-list VIPS_1 permit 10.7.240.199/32
access-list VIPS_1 permit 10.7.240.202/32
access-list VIPS_1 permit 10.7.240.203/32
access-list VIPS_1 permit 10.7.240.208/32
access-list VIPS_1 permit 10.7.240.210/32
access-list VIPS_1 permit 10.7.240.219/32
access-list VIPS_1 permit 10.7.240.222/32
access-list VIPS_1 permit 10.7.240.225/32
access-list VIPS_1 permit 10.7.240.228/32
access-list VIPS_1 permit 10.7.240.231/32
access-list VIPS_1 permit 10.7.240.234/32
access-list VIPS_1 permit 10.7.240.237/32
access-list VIPS_1 permit 10.7.240.240/32
access-list VIPS_1 permit 10.7.240.243/32
access-list VIPS_1 permit 10.7.240.246/32
access-list VIPS_1 permit 10.7.240.249/32
access-list VIPS_1 permit 10.7.240.253/32
access-list (null) permit 10.7.240.195/32
access-list (null) permit 10.7.240.198/32
access-list (null) permit 10.7.240.201/32
access-list (null) permit 10.7.240.205/32
access-list (null) permit 10.7.240.207/32
access-list (null) permit 10.7.240.212/32
access-list (null) permit 10.7.240.218/32
access-list (null) permit 10.7.240.221/32
access-list (null) permit 10.7.240.224/32
access-list (null) permit 10.7.240.227/32
access-list (null) permit 10.7.240.230/32
access-list (null) permit 10.7.240.233/32
access-list (null) permit 10.7.240.236/32
access-list (null) permit 10.7.240.242/32
access-list (null) permit 10.7.240.245/32
access-list (null) permit 10.7.240.248/32
!
route-map NOTHING deny 10
!
route-map VIPS permit 10
 match ip address VIPS_0
 set community 65101:100
 set metric 1
!
route-map VIPS permit 20
 match ip address VIPS_1
 set community 65101:125
 set metric 1
!
route-map VIPS permit 30
 match ip address VIPS_2
 set community 65101:150
 set metric 1
!
route-map VIPS deny 40
!
line vty
!
end


# show ip access-list 
BGP:
Zebra IP access list VIPS_0
    permit 10.7.240.194/32
    permit 10.7.240.197/32
    permit 10.7.240.200/32
    permit 10.7.240.204/32
    permit 10.7.240.206/32
    permit 10.7.240.209/32
    permit 10.7.240.211/32
    permit 10.7.240.220/32
    permit 10.7.240.223/32
    permit 10.7.240.226/32
    permit 10.7.240.229/32
    permit 10.7.240.232/32
    permit 10.7.240.235/32
    permit 10.7.240.238/32
    permit 10.7.240.241/32
    permit 10.7.240.244/32
    permit 10.7.240.247/32
    permit 10.7.240.251/32
    permit 10.7.240.254/32
Zebra IP access list VIPS_1
    permit 10.7.240.193/32
    permit 10.7.240.196/32
    permit 10.7.240.199/32
    permit 10.7.240.202/32
    permit 10.7.240.203/32
    permit 10.7.240.208/32
    permit 10.7.240.210/32
    permit 10.7.240.219/32
    permit 10.7.240.222/32
    permit 10.7.240.225/32
    permit 10.7.240.228/32
    permit 10.7.240.231/32
    permit 10.7.240.234/32
    permit 10.7.240.237/32
    permit 10.7.240.240/32
    permit 10.7.240.243/32
    permit 10.7.240.246/32
    permit 10.7.240.249/32
    permit 10.7.240.253/32

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

Successfully merging this pull request may close these issues.

1 participant