Skip to content

Commit

Permalink
Arista: fixup batfish#6465 with support for naked continue
Browse files Browse the repository at this point in the history
And add another missing test.
  • Loading branch information
dhalperi committed Dec 2, 2020
1 parent 2653a3e commit cbd7ca5
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 16 deletions.
Expand Up @@ -47,6 +47,7 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
Expand Down Expand Up @@ -1803,19 +1804,33 @@ private void convertRouteMap(Configuration c, RouteMap routeMap) {
}
lastEntry = currentEntry;
}
Map<Integer, Integer> noMatchNextBySeq = noMatchNextBySeqBuilder.build();

// sequences that are valid targets of a continue statement
Set<Integer> continueTargets =
// sequence -> continue sequence if match, or null if sequence does not have a continue
ImmutableMap<Integer, Integer> continues =
routeMap.getClauses().values().stream()
.map(
clause ->
clause.getContinueLine() == null ? null : clause.getContinueLine().getTarget())
clause -> {
if (clause.getContinueLine() == null) {
return null; // no continue
}
RouteMapContinue cont = clause.getContinueLine();
Integer effectiveTarget = cont.getTarget();
if (effectiveTarget == null) {
// Naked continue, continue to next line.
effectiveTarget = noMatchNextBySeq.get(clause.getSeqNum());
}
if (effectiveTarget == null) {
// Naked continue on last line.
return null;
}
return new SimpleEntry<>(clause.getSeqNum(), effectiveTarget);
})
.filter(Objects::nonNull)
.filter(routeMap.getClauses().keySet()::contains)
.collect(ImmutableSet.toImmutableSet());

// sequence -> next sequence if no match, or null if last sequence
Map<Integer, Integer> noMatchNextBySeq = noMatchNextBySeqBuilder.build();
.filter(e -> routeMap.getClauses().containsKey(e.getValue()))
.collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue));
// sequences that are valid targets of a continue statement
Set<Integer> continueTargets = ImmutableSet.copyOf(continues.values());

// Build the top-level RoutingPolicy that corresponds to the route-map. All it does is call
// the first interval and return its result in a context-appropriate way.
Expand Down Expand Up @@ -1860,7 +1875,7 @@ private void convertRouteMap(Configuration c, RouteMap routeMap) {
currentRoutingPolicyName = computeRoutingPolicyName(routeMapName, currentSequence);
} // or else undefined reference
currentRoutingPolicyStatements.add(
toStatement(c, routeMapName, currentEntry, noMatchNextBySeq, continueTargets));
toStatement(c, routeMapName, currentEntry, noMatchNextBySeq, continues, continueTargets));
}

// finalize last routing policy
Expand All @@ -1877,6 +1892,7 @@ private void convertRouteMap(Configuration c, RouteMap routeMap) {
String routeMapName,
RouteMapClause entry,
Map<Integer, Integer> noMatchNextBySeq,
Map<Integer, Integer> continues,
Set<Integer> continueTargets) {
BooleanExpr guard = toMatchBooleanExpr(c, entry);

Expand All @@ -1886,9 +1902,9 @@ private void convertRouteMap(Configuration c, RouteMap routeMap) {
rmSet.applyTo(trueStatements, this, c, _w);
}

RouteMapContinue cont = entry.getContinueLine();
LineAction action = entry.getAction();

RouteMapContinue cont = entry.getContinueLine();
if (cont == null) {
// No continue: on match, return the action.
if (action == LineAction.PERMIT) {
Expand All @@ -1905,11 +1921,8 @@ private void convertRouteMap(Configuration c, RouteMap routeMap) {
assert action == LineAction.DENY;
trueStatements.add(Statements.SetLocalDefaultActionReject.toStaticStatement());
}
int target = cont.getTarget();
if (continueTargets.contains(target)) {
// TODO: verify correct semantics: possibly, should add two statements in this case; first
// should set default action to permit/deny if this is a permit/deny entry, and second
// should call policy for next entry.
Integer target = continues.get(entry.getSeqNum());
if (target != null) {
trueStatements.add(call(computeRoutingPolicyName(routeMapName, target)));
} else {
String targetName = String.format("clause: '%s' in route-map: '%s'", target, routeMapName);
Expand Down
Expand Up @@ -2501,6 +2501,28 @@ public void testRouteMapExhaustive() {
}
}

@Test
public void testRouteMapNakedContinue() {
Configuration c = parseConfig("arista_route_map_naked_continue");
assertThat(c.getRoutingPolicies(), hasKey("RM"));
RoutingPolicy rm = c.getRoutingPolicies().get("RM");
Bgpv4Route base =
Bgpv4Route.builder()
.setTag(0L)
.setSrcProtocol(RoutingProtocol.BGP)
.setMetric(0L)
.setAsPath(AsPath.ofSingletonAsSets(2L))
.setOriginatorIp(Ip.ZERO)
.setOriginType(OriginType.INCOMPLETE)
.setProtocol(RoutingProtocol.BGP)
.setNextHopIp(Ip.parse("192.0.2.254"))
.setNetwork(Prefix.ZERO)
.build();
Bgpv4Route after = processRouteIn(rm, base);
assertThat(after.getMetric(), equalTo(3L));
assertThat(after.getCommunities(), equalTo(CommunitySet.of(StandardCommunity.of(1))));
}

@Test
public void testSnmpExtraction() {
Configuration config = parseConfig("arista_snmp");
Expand Down
@@ -0,0 +1,11 @@
!RANCID-CONTENT-TYPE: arista
!
hostname arista_route_map_naked_continue
!
route-map RM permit 5
continue
set community 0:1 additive
!
route-map RM permit 9999
set metric 3
!

0 comments on commit cbd7ca5

Please sign in to comment.