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

evpn: support for masterless VTEPs #8070

Closed
wants to merge 2 commits into from

Conversation

rcgoodfellow
Copy link

Currently zebra only implements advertise-all-vni functionality for VTEPs that are attached to a bridge. VTEPs that do not belong to a bridge are ignored. However, there are cases where it is useful for such VTEPs to be included in advertise-all-vni functionality, such as provisioning host services directly through VTEP devices that have IP addresses.

This PR does 3 things

  1. Notifies EVPN clients of a new VNI when a masterless VTEP is added.
  2. Notifies EVPN clients of the MAC address of a masterless VTEPs.
  3. Removes logic that precludes BUM entries from being added for masterless VTEPs.

A bit more discussion is on the FRR slack

I've done some light testing on this patch in a virtual environment with this code running in the CentOS8 container that peers with a VM running CumulusVX 4.2 (unmodified FRR). Below are the configs and some relevant output from each of these hosts after establishing a BGP/EVPN session and placing a vtep3 on each host.

With the setup shown below, VXLAN packets successfully flow from a VM hanging off swp2 on the virtual Cumulus switch up to vtep3 on the patched FRR running on the CentOS8 VM.

Patched CentOS8

[root@ifr ops]# ip -d link show vtep3
11: vtep3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1450 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
    link/ether 1e:33:a5:d1:7d:15 brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535 
    vxlan id 3 local 10.99.0.2 dev ens2 srcport 0 0 dstport 4789 nolearning ttl auto ageing 300 udpcsum noudp6zerocsumtx noudp6zerocsumrx addrgenmode eui64 numtxqueues 1 numrxqueues
 1 gso_max_size 65536 gso_max_segs 65535 
[root@ifr ops]# ip -br addr show dev vtep3
vtep3            UNKNOWN        172.30.0.1/24 fe80::1c33:a5ff:fed1:7d15/64
[root@ifr ops]# bridge fdb
01:00:5e:00:00:01 dev ens1 self permanent
33:33:00:00:00:01 dev ens1 self permanent
[snip]
33:33:ff:00:00:00 dev ens2 self permanent
00:00:00:00:00:00 dev vtep3 dst 10.99.0.1 self permanent
[root@ifr ops]# ip neigh 
10.0.0.13 dev ens1  FAILED
10.0.0.1 dev ens1 lladdr 04:70:00:00:00:01 REACHABLE
10.0.0.12 dev ens1 lladdr 04:70:00:00:00:03 DELAY
169.254.0.1 dev ens2 lladdr 52:54:00:f1:40:56 PERMANENT proto zebra 
172.30.0.10 dev vtep3 lladdr 04:71:00:00:00:10 REACHABLE
[snip]
ifr# show running-config                     
Building configuration...                    
                                             
Current configuration:                       
!                                            
frr version 7.7-dev_git955129244045          
frr defaults datacenter                      
hostname ebbecefffd01                        
log file /var/log/frr/frr.conf               
hostname ifr                                 
!                                            
debug zebra vxlan                            
!                                            
router bgp 4200000004                        
 bgp router-id 10.99.0.2                     
 neighbor bobsled peer-group                 
 neighbor bobsled remote-as external         
 neighbor bobsled capability extended-nexthop
 neighbor ens2 interface peer-group bobsled  
 !                                           
 address-family ipv4 unicast                 
  network 10.99.0.2/32                       
 exit-address-family                         
 !                                           
 address-family ipv6 unicast                 
  neighbor bobsled activate                  
 exit-address-family                         
 !                                           
 address-family l2vpn evpn                   
  neighbor bobsled activate                  
  advertise-all-vni                          
  advertise-default-gw                       
 exit-address-family                         
!                                            
line vty                                     
!                                            
end                                          
ifr# show evpn vni                                                                
VNI        Type VxLAN IF              # MACs   # ARPs   # Remote VTEPs  Tenant VRF
3          L2   vtep3                 1        0        1               default   
ifr# show evpn mac vni 3                                                   
Number of MACs (local and remote) known for this VNI: 1                    
Flags: N=sync-neighs, I=local-inactive, P=peer-active, X=peer-proxy        
MAC               Type   Flags Intf/Remote ES/VTEP            VLAN  Seq #'s
ifr# show bgp l2vpn evpn route vni 3                                          
BGP table version is 28, local router ID is 10.99.0.2                         
Status codes: s suppressed, d damped, h history, * valid, > best, i - internal
Origin codes: i - IGP, e - EGP, ? - incomplete                                
EVPN type-1 prefix: [1]:[ESI]:[EthTag]:[IPlen]:[VTEP-IP]                      
EVPN type-2 prefix: [2]:[EthTag]:[MAClen]:[MAC]:[IPlen]:[IP]                  
EVPN type-3 prefix: [3]:[EthTag]:[IPlen]:[OrigIP]                             
EVPN type-4 prefix: [4]:[ESI]:[IPlen]:[OrigIP]                                
EVPN type-5 prefix: [5]:[EthTag]:[IPlen]:[IP]                                 
                                                                              
   Network          Next Hop            Metric LocPrf Weight Path             
*> [2]:[0]:[48]:[04:71:00:00:00:10]                                           
                    10.99.0.1(infra)                                          
                                                           0 4200000003 i     
                    RT:59907:3 ET:8                                           
*> [2]:[0]:[48]:[1e:33:a5:d1:7d:15]                                           
                    10.99.0.2(ifr)                     32768 i                
                    ET:8 RT:59908:3                                           
*> [3]:[0]:[32]:[10.99.0.1]                                                   
                    10.99.0.1(infra)                                          
                                                           0 4200000003 i     
                    RT:59907:3 ET:8                                           
*> [3]:[0]:[32]:[10.99.0.2]                                                   
                    10.99.0.2(ifr)                     32768 i                
                    ET:8 RT:59908:3                                           
                                                                              
Displayed 4 prefixes (4 paths)                                                

Unmodified CumulusVX 4.2

infra# show running-config                   
Building configuration...                    
                                             
Current configuration:                       
!                                            
frr version 7.4+cl4u1                        
frr defaults datacenter                      
hostname infra                               
log syslog                                   
zebra nexthop proto only                     
service integrated-vtysh-config              
!                                            
router bgp 4200000003                        
 bgp router-id 10.99.0.1                     
 neighbor bobsled peer-group                 
 neighbor bobsled remote-as external         
 neighbor bobsled capability extended-nexthop
 neighbor swp1 interface peer-group bobsled  
 !                                           
 address-family ipv4 unicast                 
  network 10.99.0.1/32                       
 exit-address-family                         
 !                                           
 address-family ipv6 unicast                 
  neighbor bobsled activate                  
 exit-address-family                         
 !                                           
 address-family l2vpn evpn                   
  neighbor bobsled activate                  
  advertise-all-vni                          
 exit-address-family                         
!                                            
line vty                                     
!                                            
end                                          
infra# show evpn vni                                                              
VNI        Type VxLAN IF              # MACs   # ARPs   # Remote VTEPs  Tenant VRF
                                                                                  
3          L2   vtep3                 2        0        1               default   
infra# show evpn mac vni 3                                                 
Number of MACs (local and remote) known for this VNI: 2                    
Flags: N=sync-neighs, I=local-inactive, P=peer-active, X=peer-proxy        
MAC               Type   Flags Intf/Remote ES/VTEP            VLAN  Seq #'s
04:71:00:00:00:10 local        swp2                           3     0/0    
1e:33:a5:d1:7d:15 remote       10.99.0.2                            0/0    
infra# show bgp l2vpn evpn route vni 3                                        
BGP table version is 16, local router ID is 10.99.0.1                         
Status codes: s suppressed, d damped, h history, * valid, > best, i - internal
Origin codes: i - IGP, e - EGP, ? - incomplete                                
EVPN type-1 prefix: [4]:[ESI]:[EthTag]:[IPlen]:[VTEP-IP]                      
EVPN type-2 prefix: [2]:[EthTag]:[MAClen]:[MAC]:[IPlen]:[IP]                  
EVPN type-3 prefix: [3]:[EthTag]:[IPlen]:[OrigIP]                             
EVPN type-4 prefix: [4]:[ESI]:[IPlen]:[OrigIP]                                
EVPN type-5 prefix: [5]:[EthTag]:[IPlen]:[IP]                                 
                                                                              
   Network          Next Hop            Metric LocPrf Weight Path             
*> [2]:[0]:[48]:[04:71:00:00:00:10]                                           
                    10.99.0.1                          32768 i                
                    ET:8 RT:59907:3                                           
*> [2]:[0]:[48]:[1e:33:a5:d1:7d:15]                                           
                    10.99.0.2                              0 4200000004 i     
                    RT:59908:3 ET:8                                           
*> [3]:[0]:[32]:[10.99.0.1]                                                   
                    10.99.0.1                          32768 i                
                    ET:8 RT:59907:3                                           
*> [3]:[0]:[32]:[10.99.0.2]                                                   
                    10.99.0.2                              0 4200000004 i     
                    RT:59908:3 ET:8                                           
                                                                              
Displayed 4 prefixes (4 paths)                                                

Copy link

@polychaeta polychaeta left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution to FRR!

  • One of your commits has an improperly formatted commit message
Click for style suggestions

To apply these suggestions:

curl -s https://gist.githubusercontent.com/polychaeta/fdf2e4108560fa13963bf6a5508b247c/raw/9cf6d659e424f0bb03c1fc4e0795863b5d2dd37f/cr_8070_1613098966.diff | git apply

diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c
index e0362511a..120fcdd1d 100644
--- a/zebra/zebra_vxlan.c
+++ b/zebra/zebra_vxlan.c
@@ -4813,21 +4813,21 @@ int zebra_vxlan_if_up(struct interface *ifp)
 
 		/* Also, read and populate local MACs and neighbors. */
 		if (zif->brslave_info.br_if) {
-			/* If part of a bridge read the macfdb for the bridge. */
+			/* If part of a bridge read the macfdb for the bridge.
+			 */
 			zebra_evpn_read_mac_neigh(zevpn, ifp);
-		}
-		else {
+		} else {
 			/* Otherwise advertise the vtep itself. */
 			struct ethaddr macaddr;
 			char buf[ETHER_ADDR_STRLEN];
 
 			memcpy(&macaddr.octet, ifp->hw_addr, ETH_ALEN);
 			zlog_debug("Adv MAC %s",
-					prefix_mac2str(&macaddr, buf, sizeof(buf)));
+				   prefix_mac2str(&macaddr, buf, sizeof(buf)));
 			zmac = zebra_evpn_mac_add(zevpn, &macaddr);
-			zebra_evpn_mac_send_add_to_client(vni, &macaddr,
-					zmac->flags, zmac->loc_seq, zmac->es);
-
+			zebra_evpn_mac_send_add_to_client(
+				vni, &macaddr, zmac->flags, zmac->loc_seq,
+				zmac->es);
 		}
 	}
 

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

@LabN-CI
Copy link
Collaborator

LabN-CI commented Feb 12, 2021

Outdated results 💚

Basic BGPD CI results: SUCCESS, 0 tests failed

_ _
Result SUCCESS git merge/8070 8e1e4a6
Date 02/11/2021
Start 22:05:45
Finish 22:45:09
Run-Time 39:24
Total 1815
Pass 1815
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2021-02-11-22:05:45.txt
Log autoscript-2021-02-11-22:06:47.log.bz2
Memory 482 490 428

For details, please contact louberger

@LabN-CI
Copy link
Collaborator

LabN-CI commented Feb 12, 2021

Outdated results 💚

Basic BGPD CI results: SUCCESS, 0 tests failed

_ _
Result SUCCESS git merge/8070 7ed7329
Date 02/11/2021
Start 22:50:46
Finish 23:30:15
Run-Time 39:29
Total 1815
Pass 1815
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2021-02-11-22:50:46.txt
Log autoscript-2021-02-11-22:51:50.log.bz2
Memory 497 500 427

For details, please contact louberger

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Feb 12, 2021

Continuous Integration Result: SUCCESSFUL

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17097/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings
Report for zebra_vxlan.c | 6 issues
===============================================
< WARNING: line over 80 characters
< #4816: FILE: /tmp/f1-11893/zebra_vxlan.c:4816:
< ERROR: else should follow close brace '}'
< #4819: FILE: /tmp/f1-11893/zebra_vxlan.c:4819:
< WARNING: line over 80 characters
< #4826: FILE: /tmp/f1-11893/zebra_vxlan.c:4826:

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17097/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g8e1e4a613-0 (missing) -> 7.7-dev-20210212-00-g8e1e4a613-0~deb10u1
W: frr-pythontools: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g8e1e4a613-0 (missing) -> 7.7-dev-20210212-00-g8e1e4a613-0~deb10u1
W: frr-snmp: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g8e1e4a613-0 (missing) -> 7.7-dev-20210212-00-g8e1e4a613-0~deb10u1
W: frr-doc: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g8e1e4a613-0 (missing) -> 7.7-dev-20210212-00-g8e1e4a613-0~deb10u1
W: frr-rpki-rtrlib: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g8e1e4a613-0 (missing) -> 7.7-dev-20210212-00-g8e1e4a613-0~deb10u1

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Feb 12, 2021

Continuous Integration Result: SUCCESSFUL

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17098/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17098/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr-doc: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-03-g88ace2623-0 (missing) -> 7.7-dev-20210212-03-g88ace2623-0~deb10u1
W: frr-snmp: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-03-g88ace2623-0 (missing) -> 7.7-dev-20210212-03-g88ace2623-0~deb10u1
W: frr: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-03-g88ace2623-0 (missing) -> 7.7-dev-20210212-03-g88ace2623-0~deb10u1
W: frr-pythontools: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-03-g88ace2623-0 (missing) -> 7.7-dev-20210212-03-g88ace2623-0~deb10u1
W: frr-rpki-rtrlib: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-03-g88ace2623-0 (missing) -> 7.7-dev-20210212-03-g88ace2623-0~deb10u1

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Feb 12, 2021

Continuous Integration Result: SUCCESSFUL

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17099/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17099/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr-snmp: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g7ed73294c-0 (missing) -> 7.7-dev-20210212-00-g7ed73294c-0~deb10u1
W: frr-rpki-rtrlib: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g7ed73294c-0 (missing) -> 7.7-dev-20210212-00-g7ed73294c-0~deb10u1
W: frr-doc: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g7ed73294c-0 (missing) -> 7.7-dev-20210212-00-g7ed73294c-0~deb10u1
W: frr-pythontools: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g7ed73294c-0 (missing) -> 7.7-dev-20210212-00-g7ed73294c-0~deb10u1
W: frr: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210212-00-g7ed73294c-0 (missing) -> 7.7-dev-20210212-00-g7ed73294c-0~deb10u1

Copy link

@polychaeta polychaeta left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution to FRR!

Click for style suggestions

To apply these suggestions:

curl -s https://gist.githubusercontent.com/polychaeta/6751452e8cc9f77126a4d36003f8acd3/raw/2f46d3fe8f0ce60ca2878a80809f77fcd7bfb968/cr_8070_1613851409.diff | git apply

diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c
index 7f52121d4..38a3e8f6b 100644
--- a/zebra/zebra_vxlan.c
+++ b/zebra/zebra_vxlan.c
@@ -1020,13 +1020,16 @@ static int zevpn_build_hash_table_zns(struct ns *ns,
 					struct ethaddr macaddr;
 					char buf[ETHER_ADDR_STRLEN];
 
-					memcpy(&macaddr.octet, ifp->hw_addr, ETH_ALEN);
+					memcpy(&macaddr.octet, ifp->hw_addr,
+					       ETH_ALEN);
 					zlog_debug("Adv MAC %s",
-							prefix_mac2str(&macaddr, buf, sizeof(buf)));
-					zmac = zebra_evpn_mac_add(zevpn, &macaddr);
+						   prefix_mac2str(&macaddr, buf,
+								  sizeof(buf)));
+					zmac = zebra_evpn_mac_add(zevpn,
+								  &macaddr);
 					zebra_evpn_mac_send_add_to_client(
-							vni, &macaddr, zmac->flags, zmac->loc_seq,
-							zmac->es);
+						vni, &macaddr, zmac->flags,
+						zmac->loc_seq, zmac->es);
 				}
 			}
 		}

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

Ryan Goodfellow added 2 commits February 20, 2021 12:12
Currently zebra only implements `advertise-all-vni` functionality for
VTEPs that are attached to a bridge. Vteps that do not belong to a
bridge are ignored. However, there are cases where it is useful for such
VTEPs to be included in `advertise-all-vni` functionality, such as
provisioning host services directly through VTEP devices that have IP
addresses.

This commit does 3 things
    1. Notifies EVPN clients of a new VNI when a masterless VTEP is
       added.
    2. Notifies EVPN clients of the MAC address of a masterless VTEPs.
    3. Removes logic that precludes BUM entries from being added for
       masterless VTEPs.

Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
This commit ensures that masterless VTEPS are advertised on startup and
not just in response to netdev up/down events.

Signed-off-by: Ryan Goodfellow <rgoodfel@isi.edu>
@LabN-CI
Copy link
Collaborator

LabN-CI commented Feb 20, 2021

Outdated results 💚

Basic BGPD CI results: SUCCESS, 0 tests failed

_ _
Result SUCCESS git merge/8070 ec965b7
Date 02/20/2021
Start 15:05:43
Finish 15:45:00
Run-Time 39:17
Total 1815
Pass 1815
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2021-02-20-15:05:43.txt
Log autoscript-2021-02-20-15:06:48.log.bz2
Memory 504 492 427

For details, please contact louberger

@LabN-CI
Copy link
Collaborator

LabN-CI commented Feb 20, 2021

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/8070 00e25fc
Date 02/20/2021
Start 15:50:42
Finish 16:30:11
Run-Time 39:29
Total 1815
Pass 1815
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2021-02-20-15:50:42.txt
Log autoscript-2021-02-20-15:51:46.log.bz2
Memory 503 502 427

For details, please contact louberger

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Feb 20, 2021

Continuous Integration Result: SUCCESSFUL

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17230/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings
Report for zebra_vxlan.c | 8 issues
===============================================
< WARNING: line over 80 characters
< #1023: FILE: /tmp/f1-19118/zebra_vxlan.c:1023:
< WARNING: line over 80 characters
< #1025: FILE: /tmp/f1-19118/zebra_vxlan.c:1025:
< WARNING: line over 80 characters
< #1026: FILE: /tmp/f1-19118/zebra_vxlan.c:1026:
< WARNING: line over 80 characters
< #1028: FILE: /tmp/f1-19118/zebra_vxlan.c:1028:

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17230/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr-rpki-rtrlib: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-00-gec965b778-0 (missing) -> 7.7-dev-20210220-00-gec965b778-0~deb10u1
W: frr-doc: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-00-gec965b778-0 (missing) -> 7.7-dev-20210220-00-gec965b778-0~deb10u1
W: frr-snmp: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-00-gec965b778-0 (missing) -> 7.7-dev-20210220-00-gec965b778-0~deb10u1
W: frr-pythontools: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-00-gec965b778-0 (missing) -> 7.7-dev-20210220-00-gec965b778-0~deb10u1
W: frr: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-00-gec965b778-0 (missing) -> 7.7-dev-20210220-00-gec965b778-0~deb10u1

CLANG Static Analyzer Summary

  • Github Pull Request 8070, comparing to Git base SHA 497bb82

No Changes in Static Analysis warnings compared to base

5 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17230/artifact/shared/static_analysis/index.html

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Feb 20, 2021

Continuous Integration Result: SUCCESSFUL

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17231/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17231/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-ge77f218d8-0 (missing) -> 7.7-dev-20210220-02-ge77f218d8-0~deb10u1
W: frr-doc: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-ge77f218d8-0 (missing) -> 7.7-dev-20210220-02-ge77f218d8-0~deb10u1
W: frr-pythontools: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-ge77f218d8-0 (missing) -> 7.7-dev-20210220-02-ge77f218d8-0~deb10u1
W: frr-rpki-rtrlib: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-ge77f218d8-0 (missing) -> 7.7-dev-20210220-02-ge77f218d8-0~deb10u1
W: frr-snmp: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-ge77f218d8-0 (missing) -> 7.7-dev-20210220-02-ge77f218d8-0~deb10u1

CLANG Static Analyzer Summary

  • Github Pull Request 8070, comparing to Git base SHA 1c0f3e7

Fixed warnings:

  • Memory error: Memory leak in clippy.c, function main, line 87

New warnings:

Static Analysis warning summary compared to base:

  • Fixed warnings: 1
  • New warnings: 5

5 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17231/artifact/shared/static_analysis/index.html

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17232/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Debian 10 amd64 build: Successful with additional warnings

Debian Package lintian failed for Debian 10 amd64 build:
(see full package build log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17232/artifact/DEB10BUILD/ErrorLog/log_lintian.txt)

W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr source: pkg-js-tools-test-is-missing
W: frr source: newer-standards-version 4.5.0.3 (current is 4.3.0)
W: frr-doc: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-g00e25fc4a-0 (missing) -> 7.7-dev-20210220-02-g00e25fc4a-0~deb10u1
W: frr-pythontools: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-g00e25fc4a-0 (missing) -> 7.7-dev-20210220-02-g00e25fc4a-0~deb10u1
W: frr-rpki-rtrlib: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-g00e25fc4a-0 (missing) -> 7.7-dev-20210220-02-g00e25fc4a-0~deb10u1
W: frr-snmp: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-g00e25fc4a-0 (missing) -> 7.7-dev-20210220-02-g00e25fc4a-0~deb10u1
W: frr: changelog-file-missing-explicit-entry 7.5-0 -> 7.7-dev-20210220-02-g00e25fc4a-0 (missing) -> 7.7-dev-20210220-02-g00e25fc4a-0~deb10u1

CLANG Static Analyzer Summary

  • Github Pull Request 8070, comparing to Git base SHA 1c0f3e7

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-17232/artifact/shared/static_analysis/index.html

Copy link
Contributor

@AnuradhaKaruppiah AnuradhaKaruppiah left a comment

Choose a reason for hiding this comment

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

The EVPN datastructures in zebra are being re-worked to allow support for vlan-based and vlan-aware-bundle models. And with tunnel types MPLS or VxLAN.

The DSs will be changed to -

  1. struct zebra_mac_vrf - This is an EVI indexed by name. zebra_mac_vrf will maintain a list of BDs.
  2. struct zebra_evpn - This will now represent a single BD (not an EVI). Plan is to use {bridge-ifIndex, VID} as the key for the BD (to make it independent of the tunnel-type) i.e. addition of a bridge/VLAN will instantiate an EVPN BD in zebra. The tunnel type will decide the label (l2-VNI or MPLS) for the BD.

In the context of the new datastructures, I am having trouble fitting this bridgeless mode in. What is the BD in this mode? Also what is the intent behind dropping the bridge -
A. You only need to route post-decaps?
B. You need to bridge the overlay traffic post-decaps but don't want the linux bridge?

If it is A we can introduce a new config-type (L3-only) which will change the BD's key to something else (or drop the BD altogether). With B, I am unclear how to represent the BD in the new model.
For either case the criteria for instantiating the BD will need be based on config to maintain backward compatibility for existing models.


/* If down or not mapped to a bridge, we're done. */
if (!if_is_operative(ifp) || !zif->brslave_info.br_if)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't unconditionally drop this dependency. For cases where the the BD is represented by a {bridge, vlan} this may create problems including invalid memory access based on the sequence of events.

Copy link
Author

Choose a reason for hiding this comment

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

Would the correct thing to do here be to check the interface type, and if it is a type that should depend on bridge membership (let's say all interface types except for VTEPs to be conservative) then retain this condition, otherwise drop the dependency?

@rcgoodfellow
Copy link
Author

Thanks for the review.

What is the BD in this mode?

The BD on the local machine is just the VTEP itself, beyond the local machine it extends to include whatever other endpoints are within the EVI.

Also what is the intent behind dropping the bridge

Our use case is services that communicate to clients within an EVI by listening to an address on the VTEP directly. So in this case there is no need for a bridge, but there is a need for EVPN advertisements to go out for the local tunnel-IP of the node hosting the VTEP and for the MAC of the VTEP itself. So I guess we are closest to alternative A, but we're not really routing post-decap, more like serving requests post-decap and sending responses back through the VTEP for encap and transit.

If it is A we can introduce a new config-type (L3-only) which will change the BD's key to something else (or drop the BD altogether).

Does this mean adding something like

address-family l2vpn evpn
  advertise-bridgeless-vni
exit-address-family

or am I completely off on what a config-type is referring to here? For L3-only, this would be along the lines of zl3vni_send_add_to_client?

I hope this context is helpful in understanding where we are trying to go with this.

As an aside I'm having a bit of trouble wrapping my head around the following

  1. struct zebra_evpn - This will now represent a single BD (not an EVI). Plan is to use {bridge-ifIndex, VID} as the key for the BD (to make it independent of the tunnel-type) i.e. addition of a bridge/VLAN will instantiate an EVPN BD in zebra. The tunnel type will decide the label (l2-VNI or MPLS) for the BD.

My understanding of the relationship between an EVIs and BDs is that the implementation of an EVI (indexed by a VNI) results in an isolated BD over a shared underlay. I get the idea of using {bridge-ifindex, VID} as a key in a model where the bridge always playing a role is an assumption and the goal is to index EVPN BDs independent of tunnel type - but then the notion that adding a bridge/VLAN will instantiate an EVPN BD is also confusing given that the a bridge/VLAN has uses that do not imply the use of EVPN. I think I'm missing a few concepts for the new model.

@AnuradhaKaruppiah
Copy link
Contributor

AnuradhaKaruppiah commented Mar 1, 2021

Thanks for the review.

What is the BD in this mode?

The BD on the local machine is just the VTEP itself, beyond the local machine it extends to include whatever other endpoints are within the EVI.

Also what is the intent behind dropping the bridge

Our use case is services that communicate to clients within an EVI by listening to an address on the VTEP directly. So in this case there is no need for a bridge, but there is a need for EVPN advertisements to go out for the local tunnel-IP of the node hosting the VTEP and for the MAC of the VTEP itself. So I guess we are closest to alternative A, but we're not really routing post-decap, more like serving requests post-decap and sending responses back through the VTEP for encap and transit.

If it is A we can introduce a new config-type (L3-only) which will change the BD's key to something else (or drop the BD altogether).

Does this mean adding something like

address-family l2vpn evpn
  advertise-bridgeless-vni
exit-address-family

or am I completely off on what a config-type is referring to here? For L3-only, this would be along the lines of zl3vni_send_add_to_client?

I hope this context is helpful in understanding where we are trying to go with this.

As an aside I'm having a bit of trouble wrapping my head around the following

  1. struct zebra_evpn - This will now represent a single BD (not an EVI). Plan is to use {bridge-ifIndex, VID} as the key for the BD (to make it independent of the tunnel-type) i.e. addition of a bridge/VLAN will instantiate an EVPN BD in zebra. The tunnel type will decide the label (l2-VNI or MPLS) for the BD.

My understanding of the relationship between an EVIs and BDs is that the implementation of an EVI (indexed by a VNI) results in an isolated BD over a shared underlay. I get the idea of using {bridge-ifindex, VID} as a key in a model where the bridge always playing a role is an assumption and the goal is to index EVPN BDs independent of tunnel type - but then the notion that adding a bridge/VLAN will instantiate an EVPN BD is also confusing given that the a bridge/VLAN has uses that do not imply the use of EVPN. I think I'm missing a few concepts for the new model.

@rcgoodfellow Thanks for the clarifications. The reason for the changes (change to the BD key) are two fold -

  1. To allow other (than VxLAN) tunnel types, primarily MPLS. In which case there is no VNI i.e. VNI cannot be the BD's key.
  2. To allow vlan aware bundles were a MAC-VRF is a list of BDs. In the current FRR code (vlan based model) BD and MAC-VRF are 1:1 and essentially the same datastructure. That will not work for the vlan-aware-bundle model

This is the proposed config for the new model -
image

While I understand your use case I am unable to think of a BD-key which will be independent of the tunnel-type (i.e. not the VNI). What do you think would be a suitable key?

@rcgoodfellow
Copy link
Author

So if the key is internally represented as {bridge-ifindex, VID}, i suppose bridgeless VTEPs (or interfaces with MPLS enabled that are not on a bridge, but I know very little about MPLS so this is pure speculation) could use that same general structure, but it would be an interface index (not necessarily a bridge ifindex) for the first part of the key and the VID would simply be zero. The underlying mechanisms could inspect the netlink ifinfomsg associated with the ifindex to see if the first part of the key is a bridge or not and act accordingly. In terms of the syntax you provided I suppose the following seems reasonable.

evpn instance Tenant-Blue
  rd 27.0.0.15:1000
  bridge-1:100-200, bridge-2:100, vtep1701
  route-target import 10100:10200
  route-target export 10100:10200

And then assuming the bridges have indices 5 and 6 and the VTEP is ifindex 7 with bridge-1 having exactly the VIDs 100 and 200, you'd have the index set

{5, 100}
{5, 200}
{6, 100}
{7, 0}

@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@donaldsharp
Copy link
Member

This PR hasn't been touched in well over a year, closing it

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.

6 participants