Page MenuHomePhabricator
Create Task
Maniphest T211930

Add eqsin routing special cases to jnt
Closed, ResolvedPublic
Actions

Description

We don't have a proper review platform for jnt, so going with a task for now.

eqsin has special routing needs to avoid routes that include a round trip through the US or (less often) the EU.

those changes have not been reflected into JNT yet.

Here are the changes that needs to be done but reviewed first.

  • Move the logic of policy-statement BGP_avoid_long_RTT_in to policy-statement BGP_community_actions with a conditional if site == "eqsin"
  • Move the logic of policy-statement BGP_avoid_long_RTT_out to policy-statement BGP(6)_outfilter with a conditional if site == "eqsin"

Those seems cleaner to implement that way rather than adding an import/export policy-statement. But also increase the complexity of the critical policy-statement BGP(6)_outfilter.

diff --git a/templates/cr/policy-options.conf b/templates/cr/policy-options.conf
index 7f025b3..8d31e20 100644
--- a/templates/cr/policy-options.conf
+++ b/templates/cr/policy-options.conf
@@ -152,7 +152,16 @@ policy-statement BGP6_outfilter {
             protocol aggregate;
             prefix-list bgp6-out;
         }
+        {% if site == "eqsin" %}
+        then {
+            community add PREPEND_3_NA;
+            community add PREPEND_3_EU;
+            accept;
+        }
+        {% else %}
         then accept;
+        {% endif %}
+
     }
     term customers {
         from community PEER_CUSTOMER;
@@ -410,6 +419,16 @@ policy-statement BGP_community_actions {
             next policy;
         }
     }
+    {% if site == "eqsin" %}
+    /* De-pref transit routes learned in NA & EU - T190559 */
+    term BGP_depref_NA_EU {
+        from community [ AS1299:CUSTS_US AS1299:PEERS_US AS2914:PEERS_US AS6453:PEERS_US AS3491:CUSTS_US AS3491:PEERS_US AS1299:CUSTS_EU AS1299:PEERS_EU AS6453:PEERS_EU AS2914:PEERS_EU AS2914:CUSTS_EU AS2914:CUSTS_US ];
+        then {
+            local-preference 80;
+            next policy;
+        }
+    }
+    {% endif %}
     term peer-internal {
         from community PEER_INTERNAL;
         then {
@@ -467,7 +486,15 @@ policy-statement BGP_outfilter {
             protocol aggregate;
             prefix-list bgp-out;
         }
+        {% if site == "eqsin" %}
+        then {
+            community add PREPEND_3_NA;
+            community add PREPEND_3_EU;
+            accept;
+        }
+        {% else %}
         then accept;
+        {% endif %}
     }
     term customers {
         from community PEER_CUSTOMER;
@@ -475,6 +502,7 @@ policy-statement BGP_outfilter {
     }
     then reject;
 }
 policy-statement BGP_prepend1_out {
     term prepend {
         from {
@@ -662,7 +690,19 @@ policy-statement ospf_export {
     }
     then reject;
 }
+community AS1299:CUSTS_EU members 1299:30000;
+community AS1299:CUSTS_US members 1299:35000;
+community AS1299:PEERS_EU members 1299:20000;
+community AS1299:PEERS_US members 1299:25000;
 community AS{{ asn }}:ALL members "^{{ asn }}:[0-9]+$";
+community AS2914:CUSTS_EU members 2914:3275;
+community AS2914:CUSTS_US members 2914:3075;
+community AS2914:PEERS_EU members 2914:3200;
+community AS2914:PEERS_US members 2914:3000;
+community AS3491:CUSTS_US members 3491:200;
+community AS3491:PEERS_US members 3491:2000;
+community AS6453:PEERS_EU members 6453:2000;
+community AS6453:PEERS_US members 6453:1000;
 community AVOIDED_PATH members {{ asn }}:0;
 community GRACEFUL_SHUTDOWN members 65535:0;
 community PARTIAL_TRANSIT_ROUTE members {{ asn }}:5;
@@ -672,6 +712,8 @@ community PEER_INTERNAL members {{ asn }}:6;
 community PEER_PRIVATE_PEER members {{ asn }}:8;
 community PEER_PUBLIC_PEER members {{ asn }}:9;
 community PREFERRED_TRANSIT members {{ asn }}:10;
+community PREPEND_3_EU members [ 1299:2003 65103:2000 2914:4223 2914:4213 ];
+community PREPEND_3_NA members [ 2914:4023 65103:1000 1299:5003 2914:4013 ];
 community SELECTED_PATH members {{ asn }}:11;
 community TRANSIT_ROUTE members {{ asn }}:4;
 as-path too-many-hops ".{100,}";

In addition we need to do the following on parts of the config that are not managed by jnt:

delete protocols bgp group Transit6 import BGP_avoid_long_RTT_in
delete protocols bgp group Transit4 import BGP_avoid_long_RTT_in
delete protocols bgp group Private-Peer4 import BGP_avoid_long_RTT_in
delete protocols bgp group Private-Peer6 import BGP_avoid_long_RTT_in
delete protocols bgp group Transit6 export BGP_avoid_long_RTT_out
delete protocols bgp group Transit4 export BGP_avoid_long_RTT_out

Then, until T204281 is tested, either:

  • Move PCCW from private-peer to transit (preferred option)
  • keep term peer-private-peer removed from BGP_community_actions
  • Accept to give a higher priority to PCCW (least preferred option)

Related Objects

Event Timeline

ayounsi triaged this task as Medium priority.Dec 13 2018, 8:12 PM
ayounsi created this task.
Restricted Application added a project: SRE. · View Herald TranscriptDec 13 2018, 8:12 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
faidon added a comment.Dec 21 2018, 11:41 AM
  1. On received routes: I don't think we should be making these kind of community-matching in BGP_community_actions. Rather, I think we should have ASnnnn_in policy-statements, that map our upstream's communities into our own communities (e.g. UPSTREAM_CUST_US), and then have BGP_community_actions act on that. That would make reading this match more straightfoward. Note that this follows what we've done with our other communities (e.g. see AS13030_in and the likes).
  1. On advertised routes: we shouldn't add the communities of all of our upstreams, in announcements to each of them. Besides this being leaky and spammy, it's also not our intended behavior! Imagine a route that is us -> 1299 -> 2914 -> dst AS; this policy would mean that we're asking 1299 to prepend 3 and then also asking 2914 to prepend 3. (1299 would probably filter the non-1299 communities and this wouldn't happen, but let's not rely on that!). Instead, and very similar to above, I propose to have our internal communities about that (e.g. a community that means "keep this route local to APAC"), and then ASnnnn_out policies that map our own policy to our upstream's.
  1. I'd prefer if we didn't do this with a bunch of if site == "eqsin". I haven't looked at this deeply so I don't know how we should do this right now. Maybe we need region-wide policy-statements that we can then reference either in templates/includes/policies (like we do already for other sites), or in a config.yaml option?
ayounsi reassigned this task from ayounsi to faidon.Jan 9 2019, 11:20 PM

Thanks for the feedback! If the back and forth in diffs via a task gets old, we can find a different solution.

  1. 2. 3. Indeed using the ASXXX_in and _out as well as templates/includes/policies make sens, especially as the scaffolding is already there.

Note that looking at existing _in, I noticed some unused policy-statement that should be audited after that.

  1. I don't think though there is currently any gain in using an outbound internal community that maps to an upstream policy.

Using directly the upstream community makes the configuration more clear (see bellow).

In addition to the diff below, the policy statements need to be applied to the matching peers export/import policies.

I used the existing AVOIDED_PATH as so far we never had to make the distinction between AVOIDED_PATH and "keep traffic in the region" (more fine grained traffic engineering). We could introduce a 2nd local_pref now, or later when/if the need arises and keep the config simpler for now.

diff --git a/templates/includes/policies/eqsin-transits.conf b/templates/includes/policies/eqsin-transits.conf
index e69de29..2907c3f 100644
--- a/templates/includes/policies/eqsin-transits.conf
+++ b/templates/includes/policies/eqsin-transits.conf
@@ -0,0 +1,80 @@
+community AS1299:CUSTS_EU members 1299:30000;
+community AS1299:CUSTS_US members 1299:35000;
+community AS1299:PEERS_EU members 1299:20000;
+community AS1299:PEERS_US members 1299:25000;
+community AS1299:PREPEND_3_EU members 1299:2003;
+community AS1299:PREPEND_3_NA members 1299:5003;
+community AS2914:CUSTS_EU members 2914:3275;
+community AS2914:CUSTS_US members 2914:3075;
+community AS2914:PEERS_EU members 2914:3200;
+community AS2914:PEERS_US members 2914:3000;
+community AS2914:PREPEND_3_EU_PEERS members 2914:4223;
+community AS2914:PREPEND_3_EU_CUSTS members 2914:4213;
+community AS2914:PREPEND_3_NA_PEERS members 2914:4023;
+community AS2914:PREPEND_3_NA_CUSTS members 2914:4013;
+community AS3491:CUSTS_US members 3491:200;
+community AS3491:PEERS_US members 3491:2000;
+community AS6453:PEERS_EU members 6453:2000;
+community AS6453:PEERS_US members 6453:1000;
+community AS6453:PREPEND_3_EU members 65103:2000;
+community AS6453:PREPEND_3_NA members 65103:1000;
+
+policy-statement AS1299_in {
+   /* De-pref routes learned in NA & EU - T190559 */
+    term depref_NA_EU {
+        from community [ AS1299:CUSTS_US AS1299:PEERS_US AS1299:CUSTS_EU AS1299:PEERS_EU ];
+        then {
+            community add AVOIDED_PATH;
+        }
+    }
+}
+policy-statement AS1299_out {
+    /* Prepend AS when readvertized in NA & EU - T190559 */
+    term depref_NA_EU {
+        then {
+            community add AS1299:PREPEND_3_NA;
+            community add AS1299:PREPEND_3_EU;
+            next policy;
+        }
+    }
+}
+policy-statement AS2914_in {
+   /* De-pref routes learned in NA & EU - T190559 */
+    term depref_NA_EU {
+        from community [ AS2914:PEERS_US AS2914:PEERS_EU AS2914:CUSTS_EU AS2914:CUSTS_US ];
+        then {
+            community add AVOIDED_PATH;
+        }
+    }
+}
+policy-statement AS2914_out {
+    /* Prepend AS when readvertized in NA & EU - T190559 */
+    term depref_NA_EU {
+        then {
+            community add AS2914:PREPEND_3_EU_PEERS;
+            community add AS2914:PREPEND_3_EU_CUSTS;
+            community add AS2914:PREPEND_3_NA_PEERS;
+            community add AS2914:PREPEND_3_NA_CUSTS;
+            next policy;
+        }
+    }
+}
+policy-statement AS6453_in {
+   /* De-pref routes learned in NA & EU - T190559 */
+    term depref_NA_EU {
+        from community [ AS6453:PEERS_US AS6453:PEERS_EU ];
+        then {
+            community add AVOIDED_PATH;
+        }
+    }
+}
+policy-statement AS6453_out {
+    /* Prepend AS when readvertized in NA & EU - T190559 */
+    term depref_NA_EU {
+        then {
+            community add AS6453:PREPEND_3_EU;
+            community add AS6453:PREPEND_3_NA;
+            next policy;
+        }
+    }
+}
• Phabricator_maintenance moved this task from Backlog to Acknowledged on the SRE board.Jan 26 2019, 10:53 PM
Stashbot added a comment.Mar 28 2019, 1:45 AM

Mentioned in SAL (#wikimedia-operations) [2019-03-28T01:45:51Z] <XioNoX> add AS specific policy-statements to cr2-eqsin (but don't apply them yet) - T211930

Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptMar 28 2019, 1:45 AM
ayounsi claimed this task.Mar 28 2019, 4:49 PM

Moving forward on that as the latest plan (taking the feedback into consideration) is anyway better than what we currently have deployed in Singapore.

Stashbot added a comment.Mar 28 2019, 9:16 PM

Mentioned in SAL (#wikimedia-operations) [2019-03-28T21:16:30Z] <XioNoX> add AS specific policy-statements to cr2-eqsin v6 transits - T211930

Stashbot added a comment.Mar 28 2019, 10:11 PM

Mentioned in SAL (#wikimedia-operations) [2019-03-28T22:11:10Z] <XioNoX> add AS specific policy-statements to cr1-eqsin v6 transits - T211930

ayounsi added a comment.Mar 28 2019, 11:01 PM

Done for IPv6. Confirmed that we apply the proper local-pref, and we advertise the proper communities to the proper peers.
I'll do v4 after the weekend.

ayounsi added a comment.EditedApr 1 2019, 9:27 PM
cr1-eqsin
[edit protocols bgp group Transit4]
-    import [ BGP_sanitize_in BGP_transit_in BGP_avoid_long_RTT_in BGP_community_actions ];
+    import [ BGP_sanitize_in BGP_transit_in BGP_community_actions ];
-    export [ BGP_avoid_long_RTT_out BGP_outfilter ];
+    export BGP_outfilter;
[edit protocols bgp group Transit4 neighbor 62.115.148.76]
+     import [ BGP_sanitize_in BGP_transit_in AS1299_in BGP_community_actions ];
+     export [ AS1299_out BGP_outfilter ];
Stashbot added a comment.Apr 1 2019, 9:28 PM

Mentioned in SAL (#wikimedia-operations) [2019-04-01T21:28:40Z] <XioNoX> Push AS specific policy-statements to cr1/2-eqsin v4 peers - T211930

ayounsi added a comment.Apr 1 2019, 10:21 PM
cr2-eqsin
[edit protocols bgp group Transit4]
-    import [ BGP_sanitize_in BGP_transit_in BGP_avoid_long_RTT_in BGP_community_actions ];
+    import [ BGP_sanitize_in BGP_transit_in BGP_community_actions ];
-    export [ BGP_avoid_long_RTT_out BGP_outfilter ];
+    export BGP_outfilter;
[edit protocols bgp group Transit4 neighbor 180.87.164.61]
+     import [ BGP_sanitize_in BGP_transit_in AS6453_in BGP_community_actions ];
+     export [ AS6453_out BGP_outfilter ];
[edit protocols bgp group Transit4 neighbor 116.51.26.209]
+     import [ BGP_sanitize_in BGP_transit_in AS2914_in BGP_community_actions ];
+     export [ AS2914_out BGP_outfilter ];
[edit protocols bgp group Private-Peer4 neighbor 103.102.166.135]
+     import [ BGP_sanitize_in BGP_Private_Peer_in AS3491_in BGP_community_actions ];
ayounsi closed this task as Resolved.Apr 1 2019, 11:01 PM

Pushed progressively and confirmed with the looking glasses that only the proper communities were received on the other side.
As well as the proper local_pref was applied.
Cleaned up the old policies and pushed everything to jnt.

faidon mentioned this in rOHPU9f98cc87f1f0: Add eqsin routing specifics.Oct 3 2019, 11:11 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits