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

Cisco IOS implement route-map set as-path replace #8081

Merged
merged 4 commits into from
Feb 25, 2022

Conversation

arifogel
Copy link
Member

Fixes #8059

  • parse and convert route-map statement set as-path replace
  • add new VI Statement for replacing elements of an as-path
  • for now, just support replacing with local-as

Follow-on work:

  • On IOS, should use confed insetad of local-as if in confed and neighbor is not in the confed

- parse and convert route-map statement `set as-path replace`
- add new VI Statement for replacing elements of an as-path
- for now, just support replacing with local-as

Follow-on work:
- On IOS, should use confed insetad of local-as if in confed and neighbor is not in the confed
@batfish-bot
Copy link

This change is Reviewable

Copy link
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 6 of 15 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 7 of 15 files reviewed, 3 unresolved discussions (waiting on @arifogel and @corinaminer)


projects/batfish/src/main/java/org/batfish/representation/cisco/RouteMapSetAsPathReplaceAnyLine.java, line 30 at r2 (raw file):

  public void applyTo(
      List<Statement> statements, CiscoConfiguration cc, Configuration c, Warnings w) {
    // TODO: confirm prepend occurs after replace

Docs say: https://content.cisco.com/chapter.sjs?uri=/searchable/chapter/content/en/us/td/docs/ios-xml/ios/iproute_bgp/configuration/xe-17/irg-xe-17-book/bgp-replace-ASNs.html.xml

  • loop detection
  • replace
  • prepend

in that order


projects/batfish/src/main/java/org/batfish/representation/cisco/RouteMapSetAsPathReplaceAnyLine.java, line 30 at r2 (raw file):

  public void applyTo(
      List<Statement> statements, CiscoConfiguration cc, Configuration c, Warnings w) {
    // TODO: confirm prepend occurs after replace

Where is this implemented?


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/routing_policy/statement/ReplaceAsesInAsSequence.java, line 58 at r2 (raw file):

  }

  private static final class AnyAs implements AsSequenceExpr {

This all needs to be public, right? Otherwise, how will searchRoutePolicies do its thing?

cc: @millstein

Copy link
Member Author

@arifogel arifogel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 7 of 15 files reviewed, 3 unresolved discussions (waiting on @corinaminer, @dhalperi, and @millstein)


projects/batfish/src/main/java/org/batfish/representation/cisco/RouteMapSetAsPathReplaceAnyLine.java, line 30 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Docs say: https://content.cisco.com/chapter.sjs?uri=/searchable/chapter/content/en/us/td/docs/ios-xml/ios/iproute_bgp/configuration/xe-17/irg-xe-17-book/bgp-replace-ASNs.html.xml

  • loop detection
  • replace
  • prepend

in that order

Loop detection should happen before incoming transformation, and is irrelevant for outgoing transformation, so that much is fine.
Updated conversion to ensure prepend is after all replaces.


projects/batfish/src/main/java/org/batfish/representation/cisco/RouteMapSetAsPathReplaceAnyLine.java, line 30 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Where is this implemented?

Where is what implemented?


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/routing_policy/statement/ReplaceAsesInAsSequence.java, line 58 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

This all needs to be public, right? Otherwise, how will searchRoutePolicies do its thing?

cc: @millstein

Fixed.

@codecov
Copy link

codecov bot commented Feb 25, 2022

Codecov Report

Merging #8081 (af99a7b) into master (56602d1) will decrease coverage by 0.00%.
The diff coverage is 85.83%.

@@             Coverage Diff              @@
##             master    #8081      +/-   ##
============================================
- Coverage     74.42%   74.41%   -0.01%     
- Complexity    43091    43110      +19     
============================================
  Files          3359     3362       +3     
  Lines        168752   168869     +117     
  Branches      20203    20218      +15     
============================================
+ Hits         125586   125659      +73     
- Misses        33520    33551      +31     
- Partials       9646     9659      +13     
Impacted Files Coverage Δ
...er/aspath/RoutePolicyStatementAsPathCollector.java 63.63% <0.00%> (-1.99%) ⬇️
.../communities/RoutePolicyStatementVarCollector.java 63.63% <0.00%> (-1.99%) ⬇️
...ava/org/batfish/question/TracingHintsStripper.java 54.05% <0.00%> (-1.51%) ⬇️
...ting_policy/statement/ReplaceAsesInAsSequence.java 85.05% <85.05%> (ø)
...fish/grammar/cisco/CiscoControlPlaneExtractor.java 62.49% <90.00%> (+0.04%) ⬆️
...del/bgp/community/CommunityStructuresVerifier.java 96.73% <100.00%> (+0.01%) ⬆️
...uting_policy/as_path/AsPathStructuresVerifier.java 96.25% <100.00%> (+0.02%) ⬆️
...tfish/representation/cisco/CiscoConfiguration.java 84.53% <100.00%> (-0.14%) ⬇️
...ntation/cisco/RouteMapSetAsPathReplaceAnyLine.java 100.00% <100.00%> (ø)
...on/cisco/RouteMapSetAsPathReplaceSequenceLine.java 100.00% <100.00%> (ø)
... and 9 more

Copy link
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 15 files at r1, 7 of 7 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @corinaminer)

Copy link
Member Author

@arifogel arifogel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r4.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @corinaminer)

Copy link
Member Author

@arifogel arifogel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @corinaminer)

@arifogel arifogel merged commit 60c419a into batfish:master Feb 25, 2022
@arifogel arifogel deleted the ari-ios-as-path-replace branch February 25, 2022 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Cisco IOS BGP as-path replace command support
3 participants