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

OSPF: not-advertised summaries do not install a discard route #6724

Merged
merged 1 commit into from
Mar 13, 2021
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.batfish.datamodel.ospf;

import static com.google.common.base.Preconditions.checkArgument;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.io.Serializable;
import java.util.Objects;
Expand All @@ -10,28 +13,59 @@
/** Represents the information about a route summary at an OSPF Area Border Router. */
@ParametersAreNonnullByDefault
public final class OspfAreaSummary implements Serializable {
private static final String PROP_ADVERTISE = "advertise";
/** Describes which routes should be advertised and/or installed locally when summarizing. */
public enum SummaryRouteBehavior {
/**
* Advertise the inter-area summary and install a discard route to prevent loops. This is the
* Cisco-like and Juniper-like default.
*/
ADVERTISE_AND_INSTALL_DISCARD,
/**
* Do not advertise the inter-area summary, but still install a discard route. This is like
* Juniper "restrict" mode.
*/
NOT_ADVERTISE_AND_INSTALL_DISCARD,
/**
* Do not advertise the inter-area summary and do not install a discard route. This is like
* Cisco "no-advertise" mode.
*/
NOT_ADVERTISE_AND_NO_DISCARD,
}

private static final String PROP_METRIC = "metric";
private static final String PROP_BEHAVIOR = "behavior";

private final boolean _advertised;
private final SummaryRouteBehavior _behavior;
@Nullable private final Long _metric;

@JsonCreator
private static OspfAreaSummary create(
@JsonProperty(PROP_ADVERTISE) boolean advertised,
@JsonProperty(PROP_BEHAVIOR) @Nullable SummaryRouteBehavior behavior,
@JsonProperty(PROP_METRIC) @Nullable Long metric) {
return new OspfAreaSummary(advertised, metric);
checkArgument(behavior != null, "Missing %s", PROP_BEHAVIOR);
return new OspfAreaSummary(behavior, metric);
}

@JsonIgnore
public boolean isAdvertised() {
return _behavior == SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD;
}

@JsonIgnore
public boolean installsDiscard() {
return _behavior == SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
|| _behavior == SummaryRouteBehavior.NOT_ADVERTISE_AND_INSTALL_DISCARD;
}

public OspfAreaSummary(boolean advertised, @Nullable Long metric) {
_advertised = advertised;
public OspfAreaSummary(SummaryRouteBehavior behavior, @Nullable Long metric) {
_behavior = behavior;
_metric = metric;
}

/** Returns true if the summarized route should be advertised externally. */
@JsonProperty(PROP_ADVERTISE)
public boolean getAdvertised() {
return _advertised;
/** Returns the {@link SummaryRouteBehavior} for this prefix. */
@JsonProperty(PROP_BEHAVIOR)
public SummaryRouteBehavior getBehavior() {
return _behavior;
}

/**
Expand All @@ -53,11 +87,11 @@ public boolean equals(@Nullable Object o) {
return false;
}
OspfAreaSummary that = (OspfAreaSummary) o;
return getAdvertised() == that.getAdvertised() && Objects.equals(getMetric(), that.getMetric());
return getBehavior() == that.getBehavior() && Objects.equals(getMetric(), that.getMetric());
}

@Override
public int hashCode() {
return Objects.hash(_advertised, _metric);
return Objects.hash(_behavior.ordinal(), _metric);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.hamcrest.Matchers.equalTo;

import org.batfish.datamodel.matchers.OspfAreaSummaryMatchersImpl.HasMetric;
import org.batfish.datamodel.matchers.OspfAreaSummaryMatchersImpl.InstallsDiscard;
import org.batfish.datamodel.matchers.OspfAreaSummaryMatchersImpl.IsAdvertised;
import org.hamcrest.Matcher;

Expand Down Expand Up @@ -32,5 +33,18 @@ public static IsAdvertised isAdvertised(Matcher<? super Boolean> subMatcher) {
return new IsAdvertised(subMatcher);
}

/** Returns a matcher asserting that the OSPF summary route will be advertised. */
public static InstallsDiscard installsDiscard() {
return new InstallsDiscard(equalTo(true));
}

/**
* Provides a matcher that matches if the provided {@code subMatcher} matches the OSPF area
* summary's advertised flag.
*/
public static InstallsDiscard installsDiscard(Matcher<? super Boolean> subMatcher) {
return new InstallsDiscard(subMatcher);
}

private OspfAreaSummaryMatchers() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,18 @@ static final class IsAdvertised extends FeatureMatcher<OspfAreaSummary, Boolean>

@Override
protected Boolean featureValueOf(OspfAreaSummary arg0) {
return arg0.getAdvertised();
return arg0.isAdvertised();
}
}

static final class InstallsDiscard extends FeatureMatcher<OspfAreaSummary, Boolean> {
InstallsDiscard(Matcher<? super Boolean> subMatcher) {
super(subMatcher, "An OspfAreaSummary that installs discard:", "installs discard");
}

@Override
protected Boolean featureValueOf(OspfAreaSummary arg0) {
return arg0.installsDiscard();
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.batfish.datamodel.ospf;

import static org.batfish.datamodel.ospf.OspfAreaSummary.SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD;
import static org.batfish.datamodel.ospf.OspfAreaSummary.SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;

Expand All @@ -12,24 +14,24 @@
public class OspfAreaSummaryTest {
@Test
public void testEquals() {
OspfAreaSummary summary = new OspfAreaSummary(true, 100L);
new EqualsTester()
.addEqualityGroup(summary, summary, new OspfAreaSummary(true, 100L))
.addEqualityGroup(new OspfAreaSummary(false, 100L))
.addEqualityGroup(new OspfAreaSummary(true, 200L))
.addEqualityGroup(new Object())
.addEqualityGroup(
new OspfAreaSummary(ADVERTISE_AND_INSTALL_DISCARD, 100L),
new OspfAreaSummary(ADVERTISE_AND_INSTALL_DISCARD, 100L))
.addEqualityGroup(new OspfAreaSummary(NOT_ADVERTISE_AND_NO_DISCARD, 100L))
.addEqualityGroup(new OspfAreaSummary(NOT_ADVERTISE_AND_NO_DISCARD, 200L))
.testEquals();
}

@Test
public void testJavaSerialization() {
OspfAreaSummary summary = new OspfAreaSummary(true, 100L);
OspfAreaSummary summary = new OspfAreaSummary(ADVERTISE_AND_INSTALL_DISCARD, 100L);
assertThat(SerializationUtils.clone(summary), equalTo(summary));
}

@Test
public void testJsonSerialization() {
OspfAreaSummary summary = new OspfAreaSummary(true, 100L);
OspfAreaSummary summary = new OspfAreaSummary(ADVERTISE_AND_INSTALL_DISCARD, 100L);
assertThat(BatfishObjectMapper.clone(summary, OspfAreaSummary.class), equalTo(summary));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -735,17 +735,19 @@ private RibDelta<OspfInterAreaRoute> computeInterAreaSummariesForArea(OspfArea a
return deltaBuilder.build();
}

/** Compute internal summaries for a single area. */
/** Compute the discard routes corresponding to area summaries. */
@Nonnull
private RibDelta<OspfInternalSummaryRoute> computeInternalSummariesForArea(OspfArea area) {
RibDelta.Builder<OspfInternalSummaryRoute> deltaBuilder = RibDelta.builder();
area.getSummaries()
.keySet()
.forEach(
prefix ->
(prefix, summary) -> {
if (summary.installsDiscard()) {
computeInternalSummaryRoute(prefix, area.getAreaNumber())
// TODO: support withdrawals
.ifPresent(r -> deltaBuilder.from(_internalSummaryRib.mergeRouteGetDelta(r))));
.ifPresent(r -> deltaBuilder.from(_internalSummaryRib.mergeRouteGetDelta(r)));
}
});
return deltaBuilder.build();
}

Expand All @@ -764,7 +766,7 @@ private RibDelta<OspfInternalSummaryRoute> computeInternalSummariesForArea(OspfA
@Nonnull
private Optional<OspfInterAreaRoute> computeInterAreaSummaryRoute(
Prefix prefix, OspfAreaSummary summary, long areaNumber) {
if (!summary.getAdvertised()) {
if (!summary.isAdvertised()) {
return Optional.empty();
}
/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@
import org.batfish.datamodel.isis.IsisInterfaceMode;
import org.batfish.datamodel.isis.IsisLevel;
import org.batfish.datamodel.ospf.OspfAreaSummary;
import org.batfish.datamodel.ospf.OspfAreaSummary.SummaryRouteBehavior;
import org.batfish.datamodel.ospf.OspfMetricType;
import org.batfish.datamodel.routing_policy.expr.AsExpr;
import org.batfish.datamodel.routing_policy.expr.AutoAs;
Expand Down Expand Up @@ -6713,7 +6714,13 @@ public void exitRo_area_range(Ro_area_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(areaNum, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down Expand Up @@ -7008,7 +7015,13 @@ public void exitRoa_range(Roa_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(_currentOspfArea, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@
import org.batfish.datamodel.isis.IsisInterfaceMode;
import org.batfish.datamodel.isis.IsisLevel;
import org.batfish.datamodel.ospf.OspfAreaSummary;
import org.batfish.datamodel.ospf.OspfAreaSummary.SummaryRouteBehavior;
import org.batfish.datamodel.ospf.OspfMetricType;
import org.batfish.datamodel.routing_policy.expr.AsExpr;
import org.batfish.datamodel.routing_policy.expr.AutoAs;
Expand Down Expand Up @@ -7615,7 +7616,13 @@ public void exitRo_area_range(Ro_area_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(areaNum, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down Expand Up @@ -8107,7 +8114,13 @@ public void exitRoa_range(Roa_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(_currentOspfArea, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@
import org.batfish.datamodel.isis.IsisInterfaceMode;
import org.batfish.datamodel.isis.IsisLevel;
import org.batfish.datamodel.ospf.OspfAreaSummary;
import org.batfish.datamodel.ospf.OspfAreaSummary.SummaryRouteBehavior;
import org.batfish.datamodel.ospf.OspfMetricType;
import org.batfish.datamodel.routing_policy.expr.AsExpr;
import org.batfish.datamodel.routing_policy.expr.AutoAs;
Expand Down Expand Up @@ -7540,7 +7541,13 @@ public void exitRo_area_range(Ro_area_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(areaNum, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down Expand Up @@ -8018,7 +8025,13 @@ public void exitRoa_range(Roa_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(_currentOspfArea, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@
import org.batfish.datamodel.isis.IsisLevel;
import org.batfish.datamodel.isis.IsisMetricType;
import org.batfish.datamodel.ospf.OspfAreaSummary;
import org.batfish.datamodel.ospf.OspfAreaSummary.SummaryRouteBehavior;
import org.batfish.datamodel.ospf.OspfMetricType;
import org.batfish.datamodel.routing_policy.expr.AsExpr;
import org.batfish.datamodel.routing_policy.expr.AsPathSetElem;
Expand Down Expand Up @@ -6384,7 +6385,13 @@ public void exitRo_area_range(CiscoXrParser.Ro_area_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(areaNum, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down Expand Up @@ -6682,7 +6689,13 @@ public void exitRoa_range(Roa_rangeContext ctx) {

Map<Prefix, OspfAreaSummary> area =
_currentOspfProcess.getSummaries().computeIfAbsent(_currentOspfArea, k -> new TreeMap<>());
area.put(prefix, new OspfAreaSummary(advertise, cost));
area.put(
prefix,
new OspfAreaSummary(
advertise
? SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.NOT_ADVERTISE_AND_NO_DISCARD,
cost));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@
import org.batfish.datamodel.isis.IsisHelloAuthenticationType;
import org.batfish.datamodel.isis.IsisOption;
import org.batfish.datamodel.ospf.OspfAreaSummary;
import org.batfish.datamodel.ospf.OspfAreaSummary.SummaryRouteBehavior;
import org.batfish.datamodel.ospf.OspfDefaultOriginateType;
import org.batfish.datamodel.ospf.OspfMetricType;
import org.batfish.datamodel.ospf.StubType;
Expand Down Expand Up @@ -4676,7 +4677,11 @@ public void exitO_reference_bandwidth(O_reference_bandwidthContext ctx) {
public void exitOa_area_range(FlatJuniperParser.Oa_area_rangeContext ctx) {
if (_currentAreaRangePrefix != null) {
OspfAreaSummary summary =
new OspfAreaSummary(!_currentAreaRangeRestrict, _currentAreaRangeMetric);
new OspfAreaSummary(
_currentAreaRangeRestrict
? SummaryRouteBehavior.NOT_ADVERTISE_AND_INSTALL_DISCARD
: SummaryRouteBehavior.ADVERTISE_AND_INSTALL_DISCARD,
_currentAreaRangeMetric);
_currentArea.getSummaries().put(_currentAreaRangePrefix, summary);
} else {
todo(ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,7 @@ private org.batfish.datamodel.ospf.OspfProcess toOspfProcess(
OspfAreaSummary summary = e2.getValue();
int prefixLength = prefix.getPrefixLength();
int filterMinPrefixLength =
summary.getAdvertised()
summary.isAdvertised()
? Math.min(Prefix.MAX_PREFIX_LENGTH, prefixLength + 1)
: prefixLength;
summaryFilter.addLine(
Expand Down