Skip to content

Commit

Permalink
OSPF topology compute unnumbered sessions (#7042)
Browse files Browse the repository at this point in the history 
- point-to-point links need not be on the same prefix
- use ospfAddresses to determine possible OspfConfigId
  source addresses over getAllConcreteAddresses when
  ospfAddresses is present, e.g. for unnumbered interfaces.
  • Loading branch information
@arifogel
arifogel committed Jun 2, 2021
1 parent e30ff8c commit c9c89d8
Show file tree
Hide file tree
Showing 2 changed files with 221 additions and 29 deletions.
41 changes: 30 additions & 11 deletions ...s/batfish-common-protocol/src/main/java/org/batfish/datamodel/ospf/OspfTopologyUtils.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.batfish.datamodel.ospf;

import static com.google.common.base.MoreObjects.firstNonNull;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand All @@ -9,6 +11,7 @@
import com.google.common.graph.ValueGraphBuilder;
import java.util.Map.Entry;
import java.util.Optional;
import javax.annotation.Nullable;
import org.batfish.datamodel.Configuration;
import org.batfish.datamodel.Interface;
import org.batfish.datamodel.Ip;
Expand Down Expand Up @@ -108,8 +111,11 @@ public static void initNeighborConfigs(NetworkConfigurations configurations) {
}
String hostname = config.getHostname();
String vrfName = vrf.getName();
iface
.getAllConcreteAddresses()
firstNonNull(
Optional.ofNullable(iface.getOspfSettings().getOspfAddresses())
.map(OspfAddresses::getAddresses)
.orElse(null),
iface.getAllConcreteAddresses())
.forEach(
concreteAddress -> {
OspfNeighborConfigId id =
Expand Down Expand Up @@ -201,12 +207,6 @@ private static void establishCandidateLinks(
if (configId.getAddress().getIp().equals(remoteConfigId.getAddress().getIp())) {
return;
}
if (!configId
.getAddress()
.getPrefix()
.equals(remoteConfigId.getAddress().getPrefix())) {
return;
}
OspfSessionStatus status =
getSessionStatus(configId, remoteConfigId, networkConfigurations);
if (status != OspfSessionStatus.NO_SESSION) {
Expand Down Expand Up @@ -275,9 +275,28 @@ static OspfSessionStatus getSessionStatus(
// Optimistically assume unspecified network types match and therefore are compatible
OspfNetworkType localNetworkType = localIface.getOspfNetworkType();
OspfNetworkType remoteNetworkType = remoteIface.getOspfNetworkType();
if ((localNetworkType != null && remoteNetworkType != null)
&& (localNetworkType != remoteNetworkType)) {
return OspfSessionStatus.NETWORK_TYPE_MISMATCH;
@Nullable OspfNetworkType assumedNetworkType = null;
if (localNetworkType != null && remoteNetworkType != null) {
if (localNetworkType != remoteNetworkType) {
return OspfSessionStatus.NETWORK_TYPE_MISMATCH;
}
assumedNetworkType = localNetworkType;
} else if (localNetworkType != null) {
assumedNetworkType = localNetworkType;
} else if (remoteNetworkType != null) {
assumedNetworkType = remoteNetworkType;
}
// Skip prefix check for P2P sessions
if (assumedNetworkType != null
&& assumedNetworkType != OspfNetworkType.POINT_TO_POINT
&& !localConfigId
.getAddress()
.getPrefix()
.equals(remoteConfigId.getAddress().getPrefix())) {
// If not P2P and prefixes do not match, the session should not come up. This can commonly
// occur even when things are properly configured, so just silently throw out this edge with
// NO_SESSION.
return OspfSessionStatus.NO_SESSION;
}

OspfInterfaceSettings localOspf = localIface.getOspfSettings();
Expand Down
209 changes: 191 additions & 18 deletions ...tfish-common-protocol/src/test/java/org/batfish/datamodel/ospf/OspfTopologyUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

import static com.google.common.base.MoreObjects.firstNonNull;
import static org.batfish.datamodel.ospf.OspfTopologyUtils.getSessionStatus;
import static org.batfish.datamodel.ospf.OspfTopologyUtils.initNeighborConfigs;
import static org.batfish.datamodel.ospf.OspfTopologyUtils.trimLinks;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Iterables;
import com.google.common.graph.EndpointPair;
import com.google.common.graph.MutableValueGraph;
import com.google.common.graph.ValueGraphBuilder;
Expand All @@ -19,6 +23,7 @@
import org.batfish.datamodel.Interface.Builder;
import org.batfish.datamodel.Ip;
import org.batfish.datamodel.IpLink;
import org.batfish.datamodel.LinkLocalAddress;
import org.batfish.datamodel.NetworkConfigurations;
import org.batfish.datamodel.Vrf;
import org.junit.Ignore;
Expand All @@ -34,6 +39,13 @@ public class OspfTopologyUtilsTest {
new OspfNeighborConfigId(
"r2", "vrf2", "proc2", "iface2", ConcreteInterfaceAddress.parse("192.0.2.1/31"));

private static final OspfNeighborConfigId LOCAL_CONFIG_ID_UNNUMBERED =
new OspfNeighborConfigId(
"r1", "vrf1", "proc1", "iface1", ConcreteInterfaceAddress.parse("192.0.2.0/32"));
private static final OspfNeighborConfigId REMOTE_CONFIG_ID_UNNUMBERED =
new OspfNeighborConfigId(
"r2", "vrf2", "proc2", "iface2", ConcreteInterfaceAddress.parse("192.0.2.1/32"));

private static NetworkConfigurations buildNetworkConfigurations(Ip localIp, Ip remoteIp) {
return buildNetworkConfigurations(
localIp,
Expand Down Expand Up @@ -152,6 +164,16 @@ private static NetworkConfigurations buildNetworkConfigurations(
OspfInterfaceSettings localOspfSettings,
Ip remoteIp,
OspfInterfaceSettings remoteOspfSettings) {
return buildNetworkConfigurations(
localIp, localOspfSettings, remoteIp, remoteOspfSettings, true);
}

private static NetworkConfigurations buildNetworkConfigurations(
Ip localIp,
OspfInterfaceSettings localOspfSettings,
Ip remoteIp,
OspfInterfaceSettings remoteOspfSettings,
boolean initNeighborConfigs) {
return buildNetworkConfigurations(
localIp,
false,
Expand All @@ -166,7 +188,8 @@ private static NetworkConfigurations buildNetworkConfigurations(
0L,
1500,
StubType.NONE,
remoteOspfSettings);
remoteOspfSettings,
initNeighborConfigs);
}

private static NetworkConfigurations buildNetworkConfigurations(
Expand All @@ -184,6 +207,40 @@ private static NetworkConfigurations buildNetworkConfigurations(
int remoteMtu,
StubType remoteAreaType,
OspfInterfaceSettings remoteOspfSettings) {
return buildNetworkConfigurations(
localIp,
localPassive,
localRouterId,
localArea,
localMtu,
localAreaType,
localOspfSettings,
remoteIp,
remotePassive,
remoteRouterId,
remoteArea,
remoteMtu,
remoteAreaType,
remoteOspfSettings,
true);
}

private static NetworkConfigurations buildNetworkConfigurations(
Ip localIp,
boolean localPassive,
Ip localRouterId,
long localArea,
int localMtu,
StubType localAreaType,
OspfInterfaceSettings localOspfSettings,
Ip remoteIp,
boolean remotePassive,
Ip remoteRouterId,
long remoteArea,
int remoteMtu,
StubType remoteAreaType,
OspfInterfaceSettings remoteOspfSettings,
boolean initNeighborConfigs) {
return NetworkConfigurations.of(
ImmutableMap.of(
LOCAL_CONFIG_ID.getHostname(),
Expand All @@ -195,7 +252,8 @@ private static NetworkConfigurations buildNetworkConfigurations(
localArea,
localMtu,
localAreaType,
localOspfSettings),
localOspfSettings,
initNeighborConfigs),
REMOTE_CONFIG_ID.getHostname(),
buildConfiguration(
REMOTE_CONFIG_ID,
Expand All @@ -205,7 +263,8 @@ private static NetworkConfigurations buildNetworkConfigurations(
remoteArea,
remoteMtu,
remoteAreaType,
remoteOspfSettings)));
remoteOspfSettings,
initNeighborConfigs)));
}

private static Configuration buildConfiguration(
Expand All @@ -217,6 +276,20 @@ private static Configuration buildConfiguration(
int mtu,
StubType areaType,
OspfInterfaceSettings ospfSettings) {
return buildConfiguration(
configId, ospfNeighborIp, passive, routerId, area, mtu, areaType, ospfSettings, true);
}

private static Configuration buildConfiguration(
OspfNeighborConfigId configId,
Ip ospfNeighborIp,
boolean passive,
Ip routerId,
long area,
int mtu,
StubType areaType,
OspfInterfaceSettings ospfSettings,
boolean initNeighborConfigs) {
String hostname = configId.getHostname();
String vrfName = configId.getVrfName();
String procName = configId.getProcName();
Expand All @@ -238,21 +311,23 @@ private static Configuration buildConfiguration(
.setProcessId(procName)
.setReferenceBandwidth(7.0)
.setNeighborConfigs(
ImmutableMap.of(
new OspfNeighborConfigId(
hostname,
vrfName,
procName,
ifaceName,
ConcreteInterfaceAddress.create(ospfNeighborIp, 31)),
OspfNeighborConfig.builder()
.setVrfName(vrfName)
.setInterfaceName(ifaceName)
.setHostname(hostname)
.setArea(area)
.setPassive(passive)
.setIp(ospfNeighborIp)
.build()))
initNeighborConfigs
? ImmutableMap.of(
new OspfNeighborConfigId(
hostname,
vrfName,
procName,
ifaceName,
ConcreteInterfaceAddress.create(ospfNeighborIp, 31)),
OspfNeighborConfig.builder()
.setVrfName(vrfName)
.setInterfaceName(ifaceName)
.setHostname(hostname)
.setArea(area)
.setPassive(passive)
.setIp(ospfNeighborIp)
.build())
: ImmutableMap.of())
.setRouterId(routerId)
.build()));
Builder iface = Interface.builder().setName(ifaceName).setMtu(mtu);
Expand Down Expand Up @@ -522,4 +597,102 @@ public void testGetSessionStatusNbma() {
assertThat(val, equalTo(OspfSessionStatus.NO_SESSION));
}
}

@Test
public void testInitNeighborConfigsOspfAddresses() {
NetworkConfigurations configs =
buildNetworkConfigurations(
LOCAL_CONFIG_ID_UNNUMBERED.getAddress().getIp(),
OspfInterfaceSettings.defaultSettingsBuilder()
.setOspfAddresses(
OspfAddresses.of(ImmutableList.of(LOCAL_CONFIG_ID_UNNUMBERED.getAddress())))
.setProcess(LOCAL_CONFIG_ID_UNNUMBERED.getProcName())
.setNetworkType(OspfNetworkType.POINT_TO_POINT)
.build(),
REMOTE_CONFIG_ID_UNNUMBERED.getAddress().getIp(),
OspfInterfaceSettings.defaultSettingsBuilder()
.setOspfAddresses(
OspfAddresses.of(ImmutableList.of(REMOTE_CONFIG_ID_UNNUMBERED.getAddress())))
.setProcess(REMOTE_CONFIG_ID_UNNUMBERED.getProcName())
.setNetworkType(OspfNetworkType.POINT_TO_POINT)
.build(),
false);
updateConfigsForUnnumbered(configs);

// Confirm that unnumbered interfaces are candidates for sessions using ospfAddresses
initNeighborConfigs(configs);
assertThat(
Iterables.getOnlyElement(configs.getVrf("r1", "vrf1").get().getOspfProcesses().values())
.getOspfNeighborConfigs()
.keySet(),
containsInAnyOrder(LOCAL_CONFIG_ID_UNNUMBERED));
assertThat(
Iterables.getOnlyElement(configs.getVrf("r2", "vrf2").get().getOspfProcesses().values())
.getOspfNeighborConfigs()
.keySet(),
containsInAnyOrder(REMOTE_CONFIG_ID_UNNUMBERED));
}

private static void updateConfigsForUnnumbered(NetworkConfigurations configs) {
LinkLocalAddress lla = LinkLocalAddress.of(Ip.parse("169.254.0.1"));
Interface iface1 = configs.getInterface("r1", "iface1").get();
iface1.setAddress(lla);
iface1.setAllAddresses(ImmutableList.of(lla));
Interface iface2 = configs.getInterface("r2", "iface2").get();
iface2.setAddress(lla);
iface2.setAllAddresses(ImmutableList.of(lla));
}

@Test
public void testGetSessionStatusP2PDifferentPrefix() {
NetworkConfigurations configs =
buildNetworkConfigurations(
LOCAL_CONFIG_ID_UNNUMBERED.getAddress().getIp(),
OspfInterfaceSettings.defaultSettingsBuilder()
.setOspfAddresses(
OspfAddresses.of(ImmutableList.of(LOCAL_CONFIG_ID_UNNUMBERED.getAddress())))
.setProcess(LOCAL_CONFIG_ID_UNNUMBERED.getProcName())
.setNetworkType(OspfNetworkType.POINT_TO_POINT)
.build(),
REMOTE_CONFIG_ID_UNNUMBERED.getAddress().getIp(),
OspfInterfaceSettings.defaultSettingsBuilder()
.setOspfAddresses(
OspfAddresses.of(ImmutableList.of(REMOTE_CONFIG_ID_UNNUMBERED.getAddress())))
.setProcess(REMOTE_CONFIG_ID_UNNUMBERED.getProcName())
.setNetworkType(OspfNetworkType.POINT_TO_POINT)
.build());
updateConfigsForUnnumbered(configs);
initNeighborConfigs(configs);

assertThat(
getSessionStatus(LOCAL_CONFIG_ID_UNNUMBERED, REMOTE_CONFIG_ID_UNNUMBERED, configs),
equalTo(OspfSessionStatus.ESTABLISHED));
}

@Test
public void testGetSessionStatusNonP2PDifferentPrefix() {
NetworkConfigurations configs =
buildNetworkConfigurations(
LOCAL_CONFIG_ID_UNNUMBERED.getAddress().getIp(),
OspfInterfaceSettings.defaultSettingsBuilder()
.setOspfAddresses(
OspfAddresses.of(ImmutableList.of(LOCAL_CONFIG_ID_UNNUMBERED.getAddress())))
.setProcess(LOCAL_CONFIG_ID_UNNUMBERED.getProcName())
.setNetworkType(OspfNetworkType.BROADCAST)
.build(),
REMOTE_CONFIG_ID_UNNUMBERED.getAddress().getIp(),
OspfInterfaceSettings.defaultSettingsBuilder()
.setOspfAddresses(
OspfAddresses.of(ImmutableList.of(REMOTE_CONFIG_ID_UNNUMBERED.getAddress())))
.setProcess(REMOTE_CONFIG_ID_UNNUMBERED.getProcName())
.setNetworkType(OspfNetworkType.BROADCAST)
.build(),
false);
updateConfigsForUnnumbered(configs);
initNeighborConfigs(configs);

assertThat(
getSessionStatus(LOCAL_CONFIG_ID_UNNUMBERED, REMOTE_CONFIG_ID_UNNUMBERED, configs),
equalTo(OspfSessionStatus.NO_SESSION));
}
}

0 comments on commit c9c89d8

Please sign in to comment.