-
Notifications
You must be signed in to change notification settings - Fork 228
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
VXLAN topology taking reachability into account #3823
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @arifogel, @corinaminer, and @progwriter)
projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/vxlan/VxlanTopologyUtils.java, line 176 at r2 (raw file):
return false; } VniSettings vniSettingsV = nc.getVniSettings(hostV, vni).get();
do we not need to check the BumTransport method for vniSettingsV
?
projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/vxlan/VxlanTopologyUtils.java, line 182 at r2 (raw file):
Ip srcIpV = vniSettingsV.getSourceAddress(); int udpPort = vniSettingsU.getUdpPort(); return vxlanFlowDelivered(hostU, vrfU, srcIpU, hostV, srcIpV, udpPort, tracerouteEngine)
why don't we use bidirectional traceroute here as we do in BGP sessions reachability check ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @corinaminer, @haverma, and @progwriter)
projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/vxlan/VxlanTopologyUtils.java, line 176 at r2 (raw file):
Previously, haverma (Harsh Verma) wrote…
do we not need to check the BumTransport method for
vniSettingsV
?
No, because as stated above,: nodeU
and nodeV
must be compatible coming into this function. This would imply they use the same transport method.
projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/vxlan/VxlanTopologyUtils.java, line 182 at r2 (raw file):
Previously, haverma (Harsh Verma) wrote…
why don't we use bidirectional traceroute here as we do in BGP sessions reachability check ?
This is UDP, not TCP. There is no reverse/reply traffic.
Codecov Report
@@ Coverage Diff @@
## master #3823 +/- ##
===========================================
+ Coverage 73.99% 74% +<.01%
- Complexity 24160 24170 +10
===========================================
Files 2039 2039
Lines 98087 98135 +48
Branches 11755 11762 +7
===========================================
+ Hits 72579 72620 +41
Misses 20230 20230
- Partials 5278 5285 +7
|
There was a problem hiding this 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: complete! all files reviewed, all discussions resolved (waiting on @corinaminer and @progwriter)
No description provided.