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

EIGRP: add metric version and plumb throughout #6532

Merged
merged 5 commits into from Dec 30, 2020
Merged

EIGRP: add metric version and plumb throughout #6532

merged 5 commits into from Dec 30, 2020

Conversation

dhalperi
Copy link
Member

Different implementations of EIGRP compute the cost with greater and lesser
degrees of precision. Add a setting to each EIGRP process to indicate which
cost implementation it uses, and add the classic metric computations for NX-OS.

Different implementations of EIGRP compute the cost with greater and lesser
degrees of precision. Add a setting to each EIGRP process to indicate which
cost implementation it uses, and add the classic metric computations for NX-OS.
@batfish-bot
Copy link

This change is Reviewable

@dhalperi
Copy link
Member Author


projects/batfish/src/main/java/org/batfish/dataplane/ibdp/EigrpRoutingProcess.java, line 540 at r4 (raw file):

builder, _process, Direction.OUT);

this seems like a bug, but I'm not sure why it didn't manifest ever.

Copy link
Member Author

@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.

Reviewable status: 0 of 25 files reviewed, 2 unresolved discussions (waiting on @progwriter)


projects/batfish/src/main/java/org/batfish/dataplane/ibdp/EigrpRoutingProcess.java, line 315 at r2 (raw file):

ernalRouteFromNeighbor(

FYI all places we import routes from neighbors we construct our own route rather than using route.toBuilder, so this was easy to verify.


projects/batfish/src/main/java/org/batfish/dataplane/ibdp/EigrpRoutingProcess.java, line 525 at r3 (raw file):

be sent out to a given neighbor */

please double-check this change.

@codecov
Copy link

codecov bot commented Dec 30, 2020

Codecov Report

Merging #6532 (c5d013f) into master (66efc18) will decrease coverage by 0.00%.
The diff coverage is 71.66%.

@@             Coverage Diff              @@
##             master    #6532      +/-   ##
============================================
- Coverage     73.36%   73.35%   -0.01%     
- Complexity    35616    35619       +3     
============================================
  Files          2830     2831       +1     
  Lines        143817   143860      +43     
  Branches      17377    17383       +6     
============================================
+ Hits         105506   105531      +25     
- Misses        29954    29960       +6     
- Partials       8357     8369      +12     
Impacted Files Coverage Δ Complexity Δ
...sh/representation/cisco_xr/CiscoXrConversions.java 30.16% <0.00%> (-0.04%) 49.00 <0.00> (ø)
...n/java/org/batfish/datamodel/eigrp/WideMetric.java 74.22% <40.00%> (-2.12%) 25.00 <2.00> (+1.00) ⬇️
...java/org/batfish/datamodel/EigrpInternalRoute.java 62.06% <57.14%> (-0.90%) 10.00 <2.00> (ø)
...ava/org/batfish/datamodel/eigrp/ClassicMetric.java 74.39% <64.28%> (-2.43%) 25.00 <4.00> (+3.00) ⬇️
...java/org/batfish/datamodel/eigrp/EigrpProcess.java 64.28% <66.66%> (+0.18%) 12.00 <1.00> (+1.00)
...java/org/batfish/datamodel/EigrpExternalRoute.java 64.17% <75.00%> (+1.27%) 10.00 <1.00> (+1.00)
...rc/main/java/org/batfish/datamodel/EigrpRoute.java 100.00% <100.00%> (+5.00%) 8.00 <3.00> (+1.00)
...rg/batfish/datamodel/eigrp/EigrpMetricVersion.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...rg/batfish/dataplane/ibdp/EigrpRoutingProcess.java 89.92% <100.00%> (+0.10%) 71.00 <0.00> (ø)
...batfish/representation/cisco/CiscoConversions.java 88.25% <100.00%> (ø) 179.00 <0.00> (ø)
... and 6 more

Copy link
Contributor

@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.

:lgtm:

Reviewed 24 of 26 files at r1, 1 of 1 files at r4.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


projects/batfish/src/main/java/org/batfish/dataplane/ibdp/EigrpRoutingProcess.java, line 540 at r4 (raw file):

Previously, dhalperi (Dan Halperin) wrote…
builder, _process, Direction.OUT);

this seems like a bug, but I'm not sure why it didn't manifest ever.

we didn't use any sets in per-interface filters, but definitely a bug

@dhalperi dhalperi merged commit 2c14018 into master Dec 30, 2020
@dhalperi dhalperi deleted the eigrp branch December 30, 2020 23:22
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