Skip to content

Commit

Permalink
Juniper: handle full range of local-pref values
Browse files Browse the repository at this point in the history
1. Use uint32 to accurately parse the exact range.
2. They are longs, not ints.
3. Fix VI representation and other parsers.
  • Loading branch information
dhalperi committed Feb 19, 2021
1 parent b51b78a commit c5d09bb
Show file tree
Hide file tree
Showing 14 changed files with 81 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ public final class MatchLocalPreference extends BooleanExpr {
private static final String PROP_METRIC = "metric";

@Nonnull private final IntComparator _comparator;
@Nonnull private final IntExpr _metric;
@Nonnull private final LongExpr _metric;

@JsonCreator
private static MatchLocalPreference jsonCreator(
@Nullable @JsonProperty(PROP_COMPARATOR) IntComparator comparator,
@Nullable @JsonProperty(PROP_METRIC) IntExpr metric) {
@Nullable @JsonProperty(PROP_METRIC) LongExpr metric) {
checkArgument(comparator != null, "%s must be provided", PROP_COMPARATOR);
checkArgument(metric != null, "%s must be provided", PROP_METRIC);
return new MatchLocalPreference(comparator, metric);
}

public MatchLocalPreference(IntComparator comparator, IntExpr metric) {
public MatchLocalPreference(IntComparator comparator, LongExpr metric) {
_comparator = comparator;
_metric = metric;
}
Expand All @@ -52,7 +52,7 @@ public IntComparator getComparator() {

@JsonProperty(PROP_METRIC)
@Nonnull
public IntExpr getMetric() {
public LongExpr getMetric() {
return _metric;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public void testBooleanExprVerifierUnrelated() {
assertNull(MatchIpv4.instance().accept(BOOLEAN_EXPR_VERIFIER, ctx));
assertNull(MatchIpv6.instance().accept(BOOLEAN_EXPR_VERIFIER, ctx));
assertNull(
new MatchLocalPreference(IntComparator.EQ, new LiteralInt(1))
new MatchLocalPreference(IntComparator.EQ, new LiteralLong(1))
.accept(BOOLEAN_EXPR_VERIFIER, ctx));
assertNull(
new MatchLocalRouteSourcePrefixLength(SubRange.singleton(1))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ popsf_local_preference
:
LOCAL_PREFERENCE
(
localpref = dec
localpref = uint32
| apply_groups
)
;
Expand Down Expand Up @@ -526,7 +526,7 @@ popst_local_preference
:
LOCAL_PREFERENCE
(
(ADD | SUBTRACT)? localpref = dec
(ADD | SUBTRACT)? localpref = uint32
| apply_groups
)
;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8726,7 +8726,7 @@ private RoutePolicyBoolean toRoutePolicyBoolean(Boolean_destination_rp_stanzaCon

private RoutePolicyBoolean toRoutePolicyBoolean(Boolean_local_preference_rp_stanzaContext ctx) {
IntComparator cmp = toIntComparator(ctx.int_comp());
IntExpr rhs = toCommonIntExpr(ctx.rhs);
LongExpr rhs = toCommonLongExpr(ctx.rhs);
return new RoutePolicyBooleanLocalPreference(cmp, rhs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Tcp_flags_alternativeContext;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Tcp_flags_atomContext;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Tcp_flags_literalContext;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Uint32Context;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.VariableContext;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Vlt_interfaceContext;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Vlt_l3_interfaceContext;
Expand Down Expand Up @@ -1769,6 +1770,10 @@ private static long toLong(DecContext ctx) {
return Long.parseLong(ctx.getText());
}

private static long toLong(Uint32Context ctx) {
return Long.parseLong(ctx.getText());
}

private static IpProtocol toIpProtocol(Ip_protocolContext ctx) {
if (ctx.dec() != null) {
int protocolNum = toInt(ctx.dec());
Expand Down Expand Up @@ -4856,8 +4861,9 @@ public void exitPopsf_interface(Popsf_interfaceContext ctx) {

@Override
public void exitPopsf_local_preference(Popsf_local_preferenceContext ctx) {
int localPreference = toInt(ctx.localpref);
_currentPsTerm.getFroms().setFromLocalPreference(new PsFromLocalPreference(localPreference));
_currentPsTerm
.getFroms()
.setFromLocalPreference(new PsFromLocalPreference(toLong(ctx.localpref)));
}

@Override
Expand Down Expand Up @@ -5022,15 +5028,13 @@ public void exitPopst_external(Popst_externalContext ctx) {

@Override
public void exitPopst_local_preference(Popst_local_preferenceContext ctx) {
int localPreference = toInt(ctx.localpref);
PsThenLocalPreference.Operator op =
ctx.ADD() != null
? PsThenLocalPreference.Operator.ADD
: (ctx.SUBTRACT() != null
? PsThenLocalPreference.Operator.SUBTRACT
: PsThenLocalPreference.Operator.SET);
PsThenLocalPreference then = new PsThenLocalPreference(localPreference, op);
_currentPsThens.add(then);
_currentPsThens.add(new PsThenLocalPreference(toLong(ctx.localpref), op));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@
import org.batfish.datamodel.Configuration;
import org.batfish.datamodel.routing_policy.expr.BooleanExpr;
import org.batfish.datamodel.routing_policy.expr.IntComparator;
import org.batfish.datamodel.routing_policy.expr.IntExpr;
import org.batfish.datamodel.routing_policy.expr.LongExpr;
import org.batfish.datamodel.routing_policy.expr.MatchLocalPreference;

public class RoutePolicyBooleanLocalPreference extends RoutePolicyBoolean {

private IntComparator _cmp;

private final IntExpr _expr;
private final LongExpr _expr;

public RoutePolicyBooleanLocalPreference(IntComparator cmp, IntExpr expr) {
public RoutePolicyBooleanLocalPreference(IntComparator cmp, LongExpr expr) {
_cmp = cmp;
_expr = expr;
}

public IntExpr getValue() {
public LongExpr getValue() {
return _expr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,40 @@
import org.batfish.datamodel.Configuration;
import org.batfish.datamodel.routing_policy.expr.BooleanExpr;
import org.batfish.datamodel.routing_policy.expr.IntComparator;
import org.batfish.datamodel.routing_policy.expr.LiteralInt;
import org.batfish.datamodel.routing_policy.expr.LiteralLong;
import org.batfish.datamodel.routing_policy.expr.MatchLocalPreference;

/** Represents a "from local-preference" line in a {@link PsTerm} */
public final class PsFromLocalPreference extends PsFrom {

private final int _localPreference;
private final long _localPreference;

public PsFromLocalPreference(int localPreference) {
public PsFromLocalPreference(long localPreference) {
_localPreference = localPreference;
}

public int getLocalPreference() {
public long getLocalPreference() {
return _localPreference;
}

@Override
public BooleanExpr toBooleanExpr(JuniperConfiguration jc, Configuration c, Warnings warnings) {
return new MatchLocalPreference(IntComparator.EQ, new LiteralInt(_localPreference));
return new MatchLocalPreference(IntComparator.EQ, new LiteralLong(_localPreference));
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
} else if (!(o instanceof PsFromLocalPreference)) {
return false;
}
PsFromLocalPreference that = (PsFromLocalPreference) o;
return _localPreference == that._localPreference;
}

@Override
public int hashCode() {
return Long.hashCode(_localPreference);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.batfish.representation.juniper;

import com.google.common.annotations.VisibleForTesting;
import java.io.Serializable;
import java.util.LinkedHashSet;
import java.util.Set;
Expand Down Expand Up @@ -130,8 +131,8 @@ Set<PsFromInterface> getFromInterfaces() {
return _fromInterfaces;
}

@Nullable
PsFromLocalPreference getFromLocalPreference() {
@VisibleForTesting
public @Nullable PsFromLocalPreference getFromLocalPreference() {
return _fromLocalPreference;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ junit_tests(
"//projects/batfish",
"//projects/batfish:batfish_testlib",
"//projects/batfish-common-protocol:common",
"//projects/batfish-common-protocol/src/test/java/org/batfish/common/matchers",
"//projects/batfish-common-protocol/src/test/java/org/batfish/datamodel/matchers",
"//projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper",
"//projects/batfish/src/main/java/org/batfish/grammar/flatjuniper",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.batfish.datamodel.AuthenticationMethod.GROUP_RADIUS;
import static org.batfish.datamodel.AuthenticationMethod.GROUP_TACACS;
import static org.batfish.datamodel.AuthenticationMethod.PASSWORD;
import static org.batfish.datamodel.BgpRoute.MAX_LOCAL_PREFERENCE;
import static org.batfish.datamodel.Configuration.DEFAULT_VRF_NAME;
import static org.batfish.datamodel.Flow.builder;
import static org.batfish.datamodel.Ip.ZERO;
Expand Down Expand Up @@ -426,13 +427,17 @@ private JuniperConfiguration parseJuniperConfig(String hostname) {
BatfishTestUtils.configureBatfishTestSettings(settings);
FlatJuniperCombinedParser flatJuniperParser =
new FlatJuniperCombinedParser(src, settings, null);
Warnings w = new Warnings();
FlatJuniperControlPlaneExtractor extractor =
new FlatJuniperControlPlaneExtractor(src, flatJuniperParser, new Warnings());
new FlatJuniperControlPlaneExtractor(src, flatJuniperParser, w);
ParserRuleContext tree =
Batfish.parse(
flatJuniperParser, new BatfishLogger(BatfishLogger.LEVELSTR_FATAL, false), settings);
extractor.processParseTree(TEST_SNAPSHOT, tree);
return SerializationUtils.clone((JuniperConfiguration) extractor.getVendorConfiguration());
JuniperConfiguration ret =
SerializationUtils.clone((JuniperConfiguration) extractor.getVendorConfiguration());
ret.setWarnings(w);
return ret;
}

private Map<String, Configuration> parseTextConfigs(String... configurationNames)
Expand Down Expand Up @@ -3535,15 +3540,15 @@ public void testJuniperPolicyStatementTermThenExtraction() {
{
PolicyStatement policy =
c.getMasterLogicalSystem().getPolicyStatements().get("LOCAL_PREFERENCE_POLICY");
assertThat(policy.getTerms(), hasKeys("T1", "T2", "T3"));
assertThat(policy.getTerms(), hasKeys("TSETMIN", "TADDMAX", "TSUB3"));
assertThat(
policy.getTerms().get("T1").getThens(),
contains(new PsThenLocalPreference(1, Operator.SET)));
policy.getTerms().get("TSETMIN").getThens(),
contains(new PsThenLocalPreference(0, Operator.SET)));
assertThat(
policy.getTerms().get("T2").getThens(),
contains(new PsThenLocalPreference(2, Operator.ADD)));
policy.getTerms().get("TADDMAX").getThens(),
contains(new PsThenLocalPreference(MAX_LOCAL_PREFERENCE, Operator.ADD)));
assertThat(
policy.getTerms().get("T3").getThens(),
policy.getTerms().get("TSUB3").getThens(),
contains(new PsThenLocalPreference(3, Operator.SUBTRACT)));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.batfish.representation.juniper;

import com.google.common.testing.EqualsTester;
import org.junit.Test;

public class PsFromLocalPreferenceTest {
@Test
public void testEquals() {
new EqualsTester()
.addEqualityGroup(5)
.addEqualityGroup(new PsFromLocalPreference(5), new PsFromLocalPreference(5))
.addEqualityGroup(new PsFromLocalPreference(6))
.testEquals();
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#
set system host-name juniper-policy-statement-then
#
set policy-options policy-statement LOCAL_PREFERENCE_POLICY term T1 then local-preference 1
set policy-options policy-statement LOCAL_PREFERENCE_POLICY term T2 then local-preference add 2
set policy-options policy-statement LOCAL_PREFERENCE_POLICY term T3 then local-preference subtract 3
set policy-options policy-statement LOCAL_PREFERENCE_POLICY term TSETMIN then local-preference 0
set policy-options policy-statement LOCAL_PREFERENCE_POLICY term TADDMAX then local-preference add 4294967295
set policy-options policy-statement LOCAL_PREFERENCE_POLICY term TSUB3 then local-preference subtract 3
#
6 changes: 3 additions & 3 deletions tests/parsing-tests/example-juniper.ref
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@
" (popst_common",
" (popst_local_preference",
" LOCAL_PREFERENCE:'local-preference'",
" localpref = (dec",
" localpref = (uint32",
" UINT16:'350')))))))))))",
" NEWLINE:'\\n')",
" (set_line",
Expand Down Expand Up @@ -1936,7 +1936,7 @@
" (popst_common",
" (popst_local_preference",
" LOCAL_PREFERENCE:'local-preference'",
" localpref = (dec",
" localpref = (uint32",
" UINT16:'350')))))))))))",
" NEWLINE:'\\n')",
" (set_line",
Expand Down Expand Up @@ -2195,7 +2195,7 @@
" (popst_common",
" (popst_local_preference",
" LOCAL_PREFERENCE:'local-preference'",
" localpref = (dec",
" localpref = (uint32",
" UINT16:'350')))))))))))",
" NEWLINE:'\\n')",
" (set_line",
Expand Down
2 changes: 1 addition & 1 deletion tests/parsing-tests/unit-tests-vimodel.ref
Original file line number Diff line number Diff line change
Expand Up @@ -81843,7 +81843,7 @@
"class" : "org.batfish.datamodel.routing_policy.expr.MatchLocalPreference",
"comparator" : "EQ",
"metric" : {
"class" : "org.batfish.datamodel.routing_policy.expr.LiteralInt",
"class" : "org.batfish.datamodel.routing_policy.expr.LiteralLong",
"value" : 123
}
}
Expand Down

0 comments on commit c5d09bb

Please sign in to comment.