-
Notifications
You must be signed in to change notification settings - Fork 4
bgp simplification #991
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
base: master
Are you sure you want to change the base?
bgp simplification #991
Conversation
specifically disableConnectedCheck and updateSource, needed for the new simplified config Signed-off-by: Emanuele Di Pascale <[email protected]>
Signed-off-by: Emanuele Di Pascale <[email protected]>
🚀 Temp artifacts published: |
@@ -405,6 +409,64 @@ func planFabricConnections(agent *agentapi.Agent, spec *dozer.Spec) error { | |||
}, | |||
} | |||
|
|||
// for spines, we want to advertise every /32 in the service IP range | |||
// for leaves, we want to advertise only the switch's own VTEP IP | |||
var servicePrefix string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it different between spines and leafs? shouldn't we always advertise protocol IP regardless of the role and on top of it VTEP IP for leafs?
} | ||
ipStr := ip.String() | ||
allowasIn := agent.Spec.Switch.Redundancy.Type == meta.RedundancyTypeMCLAG | ||
if _, exists := spec.VRFs[VRFDefault].BGP.Neighbors[ipStr]; !exists { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can already exist only if we're facing a switching with the same protocol IP which shouldn't be possible but even if it is the neighbor config is exactly the same so no need to do a check
@@ -1108,10 +1205,6 @@ func planDefaultVRFWithBGP(agent *agentapi.Agent, spec *dozer.Spec) error { | |||
Enabled: true, | |||
MaxPaths: pointer.To(getMaxPaths(agent)), | |||
}, | |||
L2VPNEVPN: dozer.SpecVRFBGPL2VPNEVPN{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really don't need to enable EVPN & adrvertise all vni anymore?
if value.UpdateSource != nil && *value.UpdateSource != "" { | ||
bgpNeigh.Neighbor[name].Transport = &oc.OpenconfigNetworkInstance_NetworkInstances_NetworkInstance_Protocols_Protocol_Bgp_Neighbors_Neighbor_Transport{ | ||
Config: &oc.OpenconfigNetworkInstance_NetworkInstances_NetworkInstance_Protocols_Protocol_Bgp_Neighbors_Neighbor_Transport_Config{ | ||
LocalAddress: pointer.To(*value.UpdateSource), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to deref and take a ref to it again here, just pass value.UpdateSource
No description provided.