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

BGP: handle implicit confederation membership #4990

Merged
merged 4 commits into from
Oct 15, 2019
Merged

BGP: handle implicit confederation membership #4990

merged 4 commits into from
Oct 15, 2019

Conversation

sfraint
Copy link
Member

@sfraint sfraint commented Oct 14, 2019

Handle case where one BGP peer is implicitly in a confederation with its peer.

@batfish-bot
Copy link

This change is Reviewable

@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #4990 into master will increase coverage by <.01%.
The diff coverage is 91.66%.

@@             Coverage Diff              @@
##             master    #4990      +/-   ##
============================================
+ Coverage     76.63%   76.63%   +<.01%     
- Complexity    29971    29972       +1     
============================================
  Files          2393     2393              
  Lines        115500   115510      +10     
  Branches      13645    13651       +6     
============================================
+ Hits          88508    88517       +9     
- Misses        20287    20288       +1     
  Partials       6705     6705
Impacted Files Coverage Δ Complexity Δ
...va/org/batfish/datamodel/bgp/BgpTopologyUtils.java 79.16% <91.66%> (+0.9%) 75 <0> (+6) ⬆️
.../src/main/java/org/batfish/datamodel/flow/Hop.java 53.33% <0%> (-6.67%) 5% <0%> (-1%)
...org/batfish/datamodel/flow/BidirectionalTrace.java 79.54% <0%> (-4.55%) 13% <0%> (-2%)
...java/org/batfish/symbolic/state/EdgeStateExpr.java 73.91% <0%> (-4.35%) 10% <0%> (-1%)
...tfish/representation/cisco/CiscoConfiguration.java 84.11% <0%> (+0.12%) 510% <0%> (+1%) ⬆️

@progwriter progwriter requested review from corinaminer and removed request for progwriter October 14, 2019 23:24
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.

+reviewer:@corinaminer -reviewer:@progwriter

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @corinaminer)

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.

:lgtm:

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sfraint)


projects/batfish-common-protocol/src/test/java/org/batfish/datamodel/bgp/BgpTopologyUtilsTest.java, line 407 at r2 (raw file):

    // Initiator implicitly matches listener confed, but remote ASNs doesn't overlap local AS
    assertPair(1L, null, LongSpace.of(2L), 1L, 3L, LongSpace.of(1L), null);
    assertPair(1L, null, LongSpace.of(1L), 1L, 3L, LongSpace.of(2L), null);

Only need half of these tests because assertPair tests both ways (swaps initiator and listener).

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 1 of 1 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@dhalperi dhalperi merged commit d9245fb into master Oct 15, 2019
@dhalperi dhalperi deleted the ibgp-confed branch October 15, 2019 03:24
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

5 participants