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

Add support for "force" with next-hop-self in Cumulus configs. #5800

Merged
merged 5 commits into from May 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -65,6 +65,11 @@ ALERTS
'alerts'
;

ALL
:
'all'
;

ALLOWAS_IN
:
'allowas-in'
Expand Down Expand Up @@ -291,6 +296,11 @@ FILE
'file' -> pushMode(M_Remark)
;

FORCE
:
'force'
;

FORWARDING
:
'forwarding'
Expand Down
Expand Up @@ -303,7 +303,7 @@ sbafin_default_originate

sbafin_next_hop_self
:
NEXT_HOP_SELF
NEXT_HOP_SELF (FORCE | ALL)?
;

sbafin_route_reflector_client
Expand Down
Expand Up @@ -691,6 +691,12 @@ public void exitSbafin_next_hop_self(Sbafin_next_hop_selfContext ctx) {
return;
}
_currentBgpNeighborIpv4UnicastAddressFamily.setNextHopSelf(true);
// "force" and "all" are aliases. See https://github.com/FRRouting/frr/pull/4200.
// The functionality is described at
// http://docs.frrouting.org/en/latest/bgp.html#clicmd-[no]neighborPEERnext-hop-self[all].
if (ctx.FORCE() != null || ctx.ALL() != null) {
_currentBgpNeighborIpv4UnicastAddressFamily.setNextHopSelfAll(true);
}
}

@Override
Expand Down
Expand Up @@ -11,6 +11,7 @@ public class BgpNeighborIpv4UnicastAddressFamily implements Serializable {
@Nullable private Boolean _defaultOriginate;
@Nullable private Boolean _routeReflectorClient;
@Nullable private Boolean _nextHopSelf;
@Nullable private Boolean _nextHopSelfAll;
@Nullable private String _routeMapIn;
@Nullable private String _routeMapOut;

Expand Down Expand Up @@ -44,6 +45,15 @@ public void setNextHopSelf(boolean nextHopSelf) {
_nextHopSelf = nextHopSelf;
}

@Nullable
public Boolean getNextHopSelfAll() {
return _nextHopSelfAll;
}

public void setNextHopSelfAll(boolean nextHopSelfAll) {
_nextHopSelfAll = nextHopSelfAll;
}

void inheritFrom(@Nonnull BgpNeighborIpv4UnicastAddressFamily other) {
if (_activated == null) {
_activated = other.getActivated();
Expand Down
Expand Up @@ -771,17 +771,29 @@ private static List<Statement> getAcceptStatements(BgpNeighbor neighbor, BgpVrf

@VisibleForTesting
static @Nullable SetNextHop getSetNextHop(BgpNeighbor neighbor, BgpVrf bgpVrf) {
if (neighbor.getRemoteAs() == null
|| bgpVrf.getAutonomousSystem() == null
|| !neighbor.getRemoteAs().equals(bgpVrf.getAutonomousSystem())) {
if (neighbor.getRemoteAs() == null || bgpVrf.getAutonomousSystem() == null) {
return null;
}

boolean isIBgp = neighbor.getRemoteAs().equals(bgpVrf.getAutonomousSystem());
// TODO: Need to handle dynamic neighbors.
boolean nextHopSelf =
Optional.ofNullable(neighbor.getIpv4UnicastAddressFamily())
.map(BgpNeighborIpv4UnicastAddressFamily::getNextHopSelf)
.orElse(false);

if (isIBgp) {
// Check for "force".
// TODO: Handle v6 AFI.
BgpNeighborIpv4UnicastAddressFamily ipv4af = neighbor.getIpv4UnicastAddressFamily();
if (ipv4af != null) {
Boolean nextHopSelfAll = ipv4af.getNextHopSelfAll();
if (nextHopSelfAll == null || !nextHopSelfAll) {
nextHopSelf = false;
}
}
}

return nextHopSelf ? new SetNextHop(SelfNextHop.getInstance()) : null;
}

Expand Down
Expand Up @@ -384,16 +384,14 @@ public void testGetAcceptStatements() {
}

{
// if is ibgp and neighbor configuration set next-hop-self set, then set
// next-hop-self
// if is ibgp and neighbor configuration has next-hop-self set, then getSetNextHop should
// return null.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(10000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelf(true);

assertThat(
getSetNextHop(bgpNeighbor, bgpVrf), equalTo(new SetNextHop(SelfNextHop.getInstance())));
assertNull(getSetNextHop(bgpNeighbor, bgpVrf));
}
}

Expand Down Expand Up @@ -1696,4 +1694,88 @@ public void testConvertVxlan_localIpPrecedence() {
c, vsConfig, ImmutableMap.of(1001, vrf.getName()), null, loopbackTunnelIp, new Warnings());
assertThat(vrf.getLayer3Vnis().get(1001).getSourceAddress(), equalTo(loopbackTunnelIp));
}

@Test
public void testNextHopSelfAll() {
BgpNeighbor bgpNeighbor = new BgpIpNeighbor("10.0.0.1");
BgpVrf bgpVrf = new BgpVrf("bgpVrf");
{
// If eBGP and next-hop-self is NOT set, then next-hop should not be self.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(20000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelf(false);
ipv4af.setNextHopSelfAll(false);
assertThat(getSetNextHop(bgpNeighbor, bgpVrf), equalTo(null));
}

{
// If eBGP and next-hop-self is set WITHOUT force, then next-hop should be self.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(20000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelf(true);
ipv4af.setNextHopSelfAll(false);
assertThat(
getSetNextHop(bgpNeighbor, bgpVrf), equalTo(new SetNextHop(SelfNextHop.getInstance())));
}

{
// If eBGP and next-hop-self is set WITH force, then next-hop should be self.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(20000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelf(true);
ipv4af.setNextHopSelfAll(true);
assertThat(
getSetNextHop(bgpNeighbor, bgpVrf), equalTo(new SetNextHop(SelfNextHop.getInstance())));
}

{
// If iBGP and next-hop-self NOT set, then next-hop should be null.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(10000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelf(false);
ipv4af.setNextHopSelfAll(false);
assertThat(getSetNextHop(bgpNeighbor, bgpVrf), equalTo(null));
}

{
// If iBGP and next-hop-self is set WITHOUT force, then next-hop should be null.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(10000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelf(true);
ipv4af.setNextHopSelfAll(false);
assertThat(getSetNextHop(bgpNeighbor, bgpVrf), equalTo(null));
}

{
// If iBGP and next-hop-self is set WITH force, then next-hop should be self.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(10000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelf(true);
ipv4af.setNextHopSelfAll(true);
assertThat(
getSetNextHop(bgpNeighbor, bgpVrf), equalTo(new SetNextHop(SelfNextHop.getInstance())));
}

{
// If iBGP and force is null, then next-hop should be null.
bgpNeighbor.setRemoteAs(10000L);
bgpVrf.setAutonomousSystem(10000L);
BgpNeighborIpv4UnicastAddressFamily ipv4af = new BgpNeighborIpv4UnicastAddressFamily();
bgpNeighbor.setIpv4UnicastAddressFamily(ipv4af);
ipv4af.setNextHopSelfAll(true);
assertThat(getSetNextHop(bgpNeighbor, bgpVrf), equalTo(null));
}
}
}