BGP redistribution: set MED to IGP metric

In the fullness of time this would be probably be done by routing policy because vendors may have knobs to control this behavior, but for now this is a default that seems to apply to big vendors
batfish · Dec 4, 2020 · 54a39f6 · 54a39f6
1 parent ff66fba
commit 54a39f6
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 18 deletions.
diff --git a/projects/batfish/src/main/java/org/batfish/dataplane/protocols/BgpProtocolHelper.java b/projects/batfish/src/main/java/org/batfish/dataplane/protocols/BgpProtocolHelper.java
@@ -306,6 +306,7 @@ public static Bgpv4Route.Builder convertNonBgpRouteToBgpRoute(
         .setLocalPreference(BgpRoute.DEFAULT_LOCAL_PREFERENCE)
         .setReceivedFromIp(protocol == RoutingProtocol.BGP ? nextHopIp : Ip.ZERO)
         .setNextHopIp(nextHopIp)
+        .setMetric(route.getMetric())
         .setTag(routeDecorator.getAbstractRoute().getTag());
     // Let everything else default to unset/empty/etc.
   }

diff --git a/.../java/org/batfish/dataplane/protocols/BgpProtocolHelperTransformBgpRouteOnExportTest.java b/.../java/org/batfish/dataplane/protocols/BgpProtocolHelperTransformBgpRouteOnExportTest.java
@@ -431,6 +431,26 @@ public void testNonBgpToBgpKeepTag() {
         equalTo(tag));
   }
 
+  @Test
+  public void testNonBgpToBgpKeepMetric() {
+    setUpPeers(false);
+    long metric = 333;
+    assertThat(
+        convertNonBgpRouteToBgpRoute(
+                StaticRoute.builder()
+                    .setNetwork(Prefix.ZERO)
+                    .setNextHopInterface("foo")
+                    .setAdministrativeCost(1)
+                    .setMetric(metric)
+                    .build(),
+                _fromBgpProcess.getRouterId(),
+                _sessionProperties.getTailIp(),
+                170,
+                RoutingProtocol.BGP)
+            .getMetric(),
+        equalTo(metric));
+  }
+
   @Test
   public void testAggregateProtocolIsCleared() {
     Builder routeBuilder = Bgpv4Route.builder().setProtocol(RoutingProtocol.AGGREGATE);

diff --git a/projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java b/projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java
@@ -3997,6 +3997,7 @@ public void testIosXeEigrpToBgpRedistConversion() throws IOException {
                   .setProtocol(RoutingProtocol.BGP)
                   .setAdmin(ebgpAdmin)
                   .setLocalPreference(DEFAULT_LOCAL_PREFERENCE)
+                  .setMetric(matchEigrp.getMetric())
                   .setNextHopIp(nextHopIp)
                   .setReceivedFromIp(nextHopIp)
                   .setOriginatorIp(bgpRouterId)
@@ -4026,6 +4027,7 @@ public void testIosXeEigrpToBgpRedistConversion() throws IOException {
                   .setProtocol(RoutingProtocol.IBGP)
                   .setAdmin(ibgpAdmin)
                   .setLocalPreference(DEFAULT_LOCAL_PREFERENCE)
+                  .setMetric(matchEigrp.getMetric())
                   .setNextHopIp(nextHopIp)
                   .setReceivedFromIp(Ip.ZERO) // for ibgp
                   .setOriginatorIp(bgpRouterId)
@@ -4066,6 +4068,7 @@ public void testIosXeEigrpToBgpRedistConversion() throws IOException {
                   .setProtocol(RoutingProtocol.BGP)
                   .setAdmin(ebgpAdmin)
                   .setLocalPreference(DEFAULT_LOCAL_PREFERENCE)
+                  .setMetric(matchEigrpEx.getMetric())
                   .setNextHopIp(nextHopIp)
                   .setReceivedFromIp(nextHopIp)
                   .setOriginatorIp(bgpRouterId)

diff --git a/projects/batfish/src/test/java/org/batfish/grammar/cisco_nxos/CiscoNxosGrammarTest.java b/projects/batfish/src/test/java/org/batfish/grammar/cisco_nxos/CiscoNxosGrammarTest.java
@@ -791,7 +791,6 @@ public void testBgpRedistFromEigrpConversion() throws IOException {
     EigrpRoute matchEigrp = internalRb.setNetwork(matchRm).build();
     EigrpRoute noMatchEigrp = internalRb.setNetwork(noMatchRm).build();
 
-    // TODO BGP metric should match original route's metric
     {
       // Redistribute matching EIGRP route into EBGP
       Bgpv4Route.Builder rb =
@@ -806,6 +805,7 @@ public void testBgpRedistFromEigrpConversion() throws IOException {
                   .setProtocol(RoutingProtocol.BGP)
                   .setAdmin(ebgpAdmin)
                   .setLocalPreference(DEFAULT_LOCAL_PREFERENCE)
+                  .setMetric(matchEigrp.getMetric())
                   .setNextHopIp(nextHopIp)
                   .setReceivedFromIp(nextHopIp)
                   .setOriginatorIp(bgpRouterId)
@@ -835,6 +835,7 @@ public void testBgpRedistFromEigrpConversion() throws IOException {
                   .setProtocol(RoutingProtocol.IBGP)
                   .setAdmin(ibgpAdmin)
                   .setLocalPreference(DEFAULT_LOCAL_PREFERENCE)
+                  .setMetric(matchEigrp.getMetric())
                   .setNextHopIp(nextHopIp)
                   .setReceivedFromIp(Ip.ZERO) // for ibgp
                   .setOriginatorIp(bgpRouterId)
@@ -875,6 +876,7 @@ public void testBgpRedistFromEigrpConversion() throws IOException {
                   .setProtocol(RoutingProtocol.BGP)
                   .setAdmin(ebgpAdmin)
                   .setLocalPreference(DEFAULT_LOCAL_PREFERENCE)
+                  .setMetric(matchEigrpEx.getMetric())
                   .setNextHopIp(nextHopIp)
                   .setReceivedFromIp(nextHopIp)
                   .setOriginatorIp(bgpRouterId)

diff --git a/projects/batfish/src/test/java/org/batfish/grammar/cumulus_frr/CumulusFrrGrammarTest.java b/projects/batfish/src/test/java/org/batfish/grammar/cumulus_frr/CumulusFrrGrammarTest.java
@@ -2008,6 +2008,7 @@ public void testBgpRedistribution_behavior() throws IOException {
                     .setAsPath(AsPath.ofSingletonAsSets(1L))
                     .setAdmin(20)
                     .setLocalPreference(100)
+                    .setMetric(22)
                     .build())));
   }
 }
diff --git a/tests/basic/genDp.ref b/tests/basic/genDp.ref
@@ -4,25 +4,25 @@
       "class" : "org.batfish.datamodel.answers.IncrementalBdpAnswerElement",
       "bgpBestPathRibRoutesByIteration" : {
         "1" : 20,
-        "2" : 40,
-        "3" : 60,
-        "4" : 66,
-        "5" : 70,
-        "6" : 74,
-        "7" : 76,
-        "8" : 76,
-        "9" : 76
+        "2" : 38,
+        "3" : 58,
+        "4" : 64,
+        "5" : 68,
+        "6" : 72,
+        "7" : 74,
+        "8" : 74,
+        "9" : 74
       },
       "bgpMultipathRibRoutesByIteration" : {
-        "1" : 24,
-        "2" : 48,
-        "3" : 92,
-        "4" : 102,
-        "5" : 108,
-        "6" : 118,
-        "7" : 120,
-        "8" : 120,
-        "9" : 120
+        "1" : 20,
+        "2" : 42,
+        "3" : 86,
+        "4" : 96,
+        "5" : 102,
+        "6" : 112,
+        "7" : 114,
+        "8" : 114,
+        "9" : 114
       },
       "dependentRoutesIterations" : 9,
       "mainRibRoutesByIteration" : {