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

PAN: support for BGP multihop #6370

Merged
merged 1 commit into from Oct 29, 2020
Merged

Conversation

progwriter
Copy link
Contributor

Parsing, extraction, conversion.
The semantics are a bit unusual compared to other vendors: https://knowledgebase.paloaltonetworks.com/KCSArticleDetail?id=kA10g000000ClKkCAK

@batfish-bot
Copy link

This change is Reviewable

@codecov
Copy link

codecov bot commented Oct 28, 2020

Codecov Report

Merging #6370 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #6370      +/-   ##
============================================
- Coverage     72.98%   72.97%   -0.01%     
+ Complexity    35170    35163       -7     
============================================
  Files          2829     2829              
  Lines        142789   142795       +6     
  Branches      17160    17160              
============================================
- Hits         104216   104208       -8     
- Misses        30343    30349       +6     
- Partials       8230     8238       +8     
Impacted Files Coverage Δ Complexity Δ
...rammar/palo_alto/PaloAltoConfigurationBuilder.java 87.61% <100.00%> (+0.01%) 470.00 <1.00> (+1.00)
.../org/batfish/representation/palo_alto/BgpPeer.java 100.00% <100.00%> (ø) 17.00 <2.00> (+2.00)
...epresentation/palo_alto/PaloAltoConfiguration.java 88.97% <100.00%> (+<0.01%) 397.00 <0.00> (ø)
.../org/batfish/dataplane/rib/RouteAdvertisement.java 83.67% <0.00%> (-4.09%) 18.00% <0.00%> (-1.00%)
...col/src/main/java/org/batfish/role/InferRoles.java 89.09% <0.00%> (-1.82%) 49.00% <0.00%> (-2.00%)
...main/java/org/batfish/datamodel/acl/AclTracer.java 60.37% <0.00%> (-1.26%) 43.00% <0.00%> (-1.00%)
...src/main/java/org/batfish/coordinator/PoolMgr.java 59.52% <0.00%> (-1.20%) 15.00% <0.00%> (-1.00%)
...java/org/batfish/dataplane/ibdp/VirtualRouter.java 90.54% <0.00%> (-0.34%) 208.00% <0.00%> (-1.00%)
...a/org/batfish/representation/aws/LoadBalancer.java 82.18% <0.00%> (-0.32%) 70.00% <0.00%> (-1.00%)
...src/main/java/org/batfish/coordinator/WorkMgr.java 75.91% <0.00%> (-0.16%) 246.00% <0.00%> (-1.00%)

Copy link
Contributor

@corinaminer corinaminer 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 7 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 2089 at r1 (raw file):

            // Multihop (as batfish VI model understands it) is always on for PAN because of
            // "number + 2" computation
            // See https://knowledgebase.paloaltonetworks.com/KCSArticleDetail?id=kA10g000000ClKkCAK

....smh.

So it doesn't even matter to our VI model whether or not a peer has multihop explicitly set?

Copy link
Contributor Author

@progwriter progwriter 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: :shipit: complete! all files reviewed, all discussions resolved


projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 2089 at r1 (raw file):

Previously, corinaminer (Corina Miner) wrote…

....smh.

So it doesn't even matter to our VI model whether or not a peer has multihop explicitly set?

in this case, yes

@progwriter progwriter merged commit a1f4be8 into batfish:master Oct 29, 2020
@progwriter progwriter deleted the pan-multihop branch October 29, 2020 16:53
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.

None yet

3 participants