evpn: support for masterless VTEPs #8070
Conversation
There was a problem hiding this comment.
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.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, 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. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, 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. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, 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. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
There was a problem hiding this comment.
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.
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>
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, 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. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base5 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, 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. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
Fixed warnings:
New warnings:
Static Analysis warning summary compared to base:
5 Static Analyzer issues remaining.See details at |
Continuous Integration Result: SUCCESSFULCongratulations, 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. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base1 Static Analyzer issues remaining.See details at |
There was a problem hiding this comment.
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 -
- struct zebra_mac_vrf - This is an EVI indexed by name. zebra_mac_vrf will maintain a list of BDs.
- 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
|
Thanks for the review.
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.
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.
Does this mean adding something like or am I completely off on what a config-type is referring to here? For L3-only, this would be along the lines of 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
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 -
This is the proposed config for the new model - 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? |
|
So if the key is internally represented as 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 |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
This PR hasn't been touched in well over a year, closing it |
Currently zebra only implements
advertise-all-vnifunctionality 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 inadvertise-all-vnifunctionality, such as provisioning host services directly through VTEP devices that have IP addresses.This PR does 3 things
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
vtep3on each host.With the setup shown below, VXLAN packets successfully flow from a VM hanging off
swp2on the virtual Cumulus switch up tovtep3on the patched FRR running on the CentOS8 VM.Patched CentOS8
Unmodified CumulusVX 4.2